git-pr-feedback

## Context - Repo: !`git remote -v` - Current branch: !`git branch --show-current` - Git status: !`git status --porcelain=v2 --branch` ## Parameters Parse these parameters from the command (all optional): | Parameter | Description | |-----------|-------------| | `$1` | PR number (if omitted, use PR of current branch; if no such PR, list actionable PRs). Mutually exclusive with `--all`. | | `--commit` | Create commit(s) after addressing feedback. | | `--push` | Push changes after committing (implies `--commit`). | | `--all` | Address feedback on every actionable open PR. Dispatches one subagent per PR in an isolated worktree; the orchestrator pushes, replies, and resolves. Implies `--commit --push` unless `--dry-run` is set. Mutually exclusive with `$1`. | | `--dry-run` | With `--all`, print the dispatch plan and stop — no subagents spawned, no commits, no pushes. Ignored without `--all`. | | `--limit N` | Maximum concurrent subagents under `--all` (default `3`). Use a small number to stay under GitHub rate limits and avoid API rate-limit cascades from many concurrent subagents — the hazard bites 1M-context sessions, which on Fable is every session, since 1M is its default window (see [`skill-fork-context.md`](../../../.claude/rules/skill-fork-context.md)). | | `--include-automation` | With `--all`, also surface automation-authored PRs (release-please, dependabot, renovate, `*[bot]`, `*-bot`). Excluded by default because they carry no human review feedback and their CI failures are resolved by automation re-running, not hand edits. | **Mode selection**: | Mode | Triggered when | Flow | |------|----------------|------| | Single-PR | No `--all` | Steps 1–7 below operate on one PR. | | Multi-PR | `--all` is passed | **Step 1A** dispatches subagents; the orchestrator finalises (push, reply, resolve, re-request) and writes a combined summary. Skip Steps 1–6. | If both `$1` and `--all` are given, error and stop with: `--all is mutually exclusive with a PR number argument.` ## When to Use This Skill | Use this skill when... | Use another skill instead when... | |------------------------|----------------------------------| | A PR has reviewer comments to address | CI checks are failing with no review comments -> use `git-fix-pr` | | You need to systematically work through review feedback | You're creating a new PR -> use `git-commit-push-pr` | | A reviewer has requested changes | You want to understand PR workflow patterns -> use `git-branch-pr-workflow` | ## Your Task Review PR workflow results and reviewer comments, then address substantive feedback. For feedback categorization, decision trees, commit format, and report templates, see [REFERENCE.md](REFERENCE.md). --- ### Step 1: Determine PR and Gather All Data > If `--all` is set, **skip this step** and jump to **Step 1A: Multi-PR Mode** below. 1. **Parse owner/repo** from the git remote URL. 2. **Resolve the PR number** in this order: 1. If `$1` was provided, use it. 2. Otherwise, try the PR for the current branch: ```bash gh pr view --json number -q '.number' ``` 3. If step 2 fails (no PR for the branch) **or** the command is on a detached/default branch, fall back to listing actionable PRs: ```bash bash ${CLAUDE_SKILL_DIR}/scripts/list-actionable-prs.sh <owner> <repo> ``` The script emits a JSON array of open, non-draft PRs that have unresolved review threads, failing/errored CI, `CHANGES_REQUESTED`, or top-level feedback (a non-empty conversation comment or `COMMENTED` review body newer than the latest commit, counted in `topLevelFeedback`). Handle the result as follows: | Result | Action | |--------|--------| | Empty array | Report "No PRs need attention." and stop. | | One entry | Use that PR number and continue. | | Multiple entries | Print a compact table (number, author, CI, unresolved, topLevelFeedback, reviewDecision, title) ordered as returned, then stop and instruct the user to re-run `/git:pr-feedback <number>`. Do **not** guess which PR they meant. | 3. **Switch to PR branch** if not already on it: ```bash gh pr view $PR --json headRefName -q '.headRefName' git switch <branch-name> git pull origin <branch-name> ``` 4. **Fetch ALL PR data** using the bundled script (single GraphQL query): ```bash bash ${CLAUDE_SKILL_DIR}/scripts/fetch-pr-data.sh <owner> <repo> <pr-number> ``` 5. **For failed checks only**, fetch detailed logs: ```bash gh run view $RUN_ID --log-failed ``` | Check Status | Action | |--------------|--------| | All passing | Skip to Step 2 | | Failed CI | Get logs with `gh run view`, may need fixes | | Pending | Note status, focus on comments | If the GraphQL query fails with a rate limit error, wait 60 seconds and retry once. --- ### Step 1A: Multi-PR Mode (--all) Reached only when `--all` is passed. Follow [references/multi-pr-mode.md](references/multi-pr-mode.md): list actionable PRs, honour `--dry-run`, dispatch one worktree subagent per PR capped at `--limit N` (subagents commit but never push), then the orchestrator alone pushes, replies, resolves and re-requests review, and handles blocked subagents per its failure table. Then skip Steps 2–6 and go to Step 7 with a combined summary. --- ### Step 2: Analyze Feedback Categorize all comments from the GraphQL response (see [REFERENCE.md](REFERENCE.md) for category definitions): 1. Skip any thread where `isResolved: true` or `isOutdated: true` — already handled. 2. Categorize each remaining comment as Blocking, Substantive, Suggestion, Question, or Nitpick. 3. For each actionable comment, capture: thread `id`, top-level comment `databaseId`, file, line, scope, and whether the body contains a ` ```suggestion ` block. 4. Track one item per actionable thread, including the thread `id` and `databaseId` so Steps 3–5 can reply and resolve — via `TodoWrite` when the session has the task tools (see `.claude/rules/agentic-permissions.md` § Task-tool availability), otherwise as a checklist you keep in the response. --- ### Step 3: Address Feedback **Verify before accepting — especially claims from an automated reviewer.** Before accepting or acting on any suggestion, independently verify the claim against the actual source, the live system, or upstream documentation. This matters most for automated reviewers (Gemini Code Assist, Copilot, and other bot authors), whose suggestions are frequently confidently-wrong: read the code/config the claim is about, check the rendered/live behaviour, and confirm against docs rather than applying a bot suggestion on trust. A claim that fails verification is **refuted** — reply with the refutation and the supporting evidence, and do **not** change the code. A written refutation with evidence is a legitimate way to resolve a thread (see Step 6). Work through actionable items systematically. For each thread, decide using the table below — see [REFERENCE.md](REFERENCE.md) for the full decision tree. | Comment shape | Action | |---------------|--------| | Contains a ` ```suggestion ` block, fix is correct | **Accept the suggestion**: apply the suggestion's exact replacement to the file (see [REFERENCE.md](REFERENCE.md) "Accepting Suggestions"). Record the comment author's `login` and `name`/`email` for co-author attribution in Step 4. | | Contains a ` ```suggestion ` block, fix needs adjustment | Implement an improved variant; explain the deviation in the reply. Record the suggester for co-author attribution. | | Inline code comment without suggestion | Read context, implement fix, verify no regressions | | Claim that verification refutes (wrong on the facts) | **Refuted**: do not change the code. Reply with the refutation and the evidence that disproves it (the source line, rendered config, live behaviour, or doc that contradicts the claim), then resolve. Apply this whenever a bot/automated-reviewer suggestion does not survive verification. | | Question / clarification | Skip code change; draft an inline reply for Step 4 | | Blocking review (`REQUEST_CHANGES`) | Address every concern before resolving any thread | | Failed CI check | Identify failure type (lint/type/test/build), fix locally, run to verify | | Out-of-scope feedback | Do not implement in this PR. Open a follow-up issue (see Step 3a) and reference its number in the reply. | Mark each item in progress while working it and done once the file change (if any) lands locally. Do **not** resolve threads yet — replies and resolution happen after the commit so reviewers see the linked SHA. ### Step 3a: File follow-up issues for out-of-scope feedback For any thread categorised as out-of-scope (or where the user opts to defer rather than implement now): 1. Draft a one-line title and short body that quotes the reviewer comment and links the PR thread URL. 2. Use `mcp__github__issue_write` (action `create`) or `gh issue create -R <owner>/<repo> --title "<title>" --body "<body>"` to file the issue. 3. Capture the returned issue number — Step 6's reply uses it (`Deferred to #<n> — <reason>.`). Skip this step if the user has explicitly said not to file follow-ups. When ambiguous, ask via `AskUserQuestion` before creating an issue. --- ### Step 4: Commit Changes (if --commit or --push) Group related fixes into logical commits — one commit per logical group of accepted suggestions, not one per suggestion. See [REFERENCE.md](REFERENCE.md) for commit message format. For any commit that contains an **accepted (or adapted) suggestion**, append a `Co-authored-by:` trailer for each unique suggester. This mirrors GitHub's "Commit suggestion" / "Add suggestion to batch" behaviour, which credits the suggester as co-author. See [REFERENCE.md](REFERENCE.md) "Co-author Attribution" for how to construct the trailer line and resolve the suggester's email. Run pre-commit hooks if configured, then stage any formatter changes. ### Step 5: Push Changes (if --push) ```bash git push origin HEAD ``` ### Step 5a: Re-request Review (if --push) After a successful push that addresses substantive feedback, re-request review from any reviewer whose threads were resolved or who left a `CHANGES_REQUESTED` review. Skip this step when only nitpicks or questions were addressed. Determine reviewers to re-request from the GraphQL response captured in Step 1: - `latestReviews` entries with `state == "CHANGES_REQUESTED"` - Authors of any review thread you resolved in Step 6 Then call: ```bash gh api -X POST \ /repos/<owner>/<repo>/pulls/<pr>/requested_reviewers \ -f 'reviewers[]=<login1>' \ -f 'reviewers[]=<login2>' ``` If `gh api` returns 422 ("Reviews may only be requested from collaborators"), the reviewer cannot be re-requested via the API — note it in the Step 7 summary and continue. ### Step 6: Reply and Resolve Threads For every actionable thread tracked in Step 2, post a reply and then **resolve the thread by default**. Owner/repo/PR are the same values used in Step 1. Resolving is the default action after replying — leaving threads open is the exception, reserved for the explicit cases listed in step 3. A reply alone does **not** end the conversation in GitHub's UI: the thread stays in the reviewer's "unresolved" queue until someone clicks **Resolve conversation**. Without this step the PR will continue to show unresolved feedback even after every concern has been addressed. 1. **Reply** with `mcp__github__add_reply_to_pull_request_comment` using the top-level comment's `databaseId` (a number, not the GraphQL node ID). Keep replies short — see [REFERENCE.md](REFERENCE.md) "Reply Templates". - Code change made → reference the commit SHA: `Fixed in <sha> by <one-line summary>.` - Suggestion accepted verbatim → `Accepted suggestion in <sha>.` - Suggestion adapted → explain the deviation: `Applied a variant in <sha>: <reason>.` - Deferred / out of scope → reference the follow-up issue filed in Step 3a: `Deferred to #<issue> — <reason>.` - Question → answer it directly. - Refuting a suggestion (claim verification disproved) → state the reasoning **with evidence**: `Leaving as-is: <reason> — <evidence that disproves the claim>.` 2. **Always resolve** with the GraphQL `resolveReviewThread` mutation using the thread `id` (a `PRRT_…` GraphQL node ID). Resolution is the default after a reply — only skip when one of step 3's "leave open" exceptions applies. Call: ```bash gh api graphql -f query='mutation($id:ID!){resolveReviewThread(input:{threadId:$id}){thread{isResolved}}}' -F id="$THREAD_ID" ``` Resolve when **any** of these completion conditions hold and none of step 3's "leave open" exceptions apply: - You pushed a commit that addresses the concern (`Fixed in <sha>`, `Accepted in <sha>`, `Applied a variant in <sha>`). - You answered the reviewer's question directly. - You refuted or declined the suggestion with explicit reasoning in the reply (a written refutation — not silence — completes the thread). - You deferred to a follow-up issue filed in Step 3a (the deferral and issue link are the resolution). Resolving must happen in the **same turn** as the reply. Treat "reply posted but thread unresolved" as an incomplete step — re-run the resolve call before reporting Step 7. 3. **Leave the thread open** only when one of these holds: - Your reply asks the reviewer a follow-up question (you are waiting on them). - The fix is partial — some of the concern still applies to this PR. - The reviewer explicitly asked to keep the thread open. - You haven't pushed yet (no resolving SHA exists) — finish Step 5, then resolve. - You're acting on a PR you don't own and the user has not approved resolving on third-party PRs. If `--commit`/`--push` was not passed, still post replies for questions and refutations, but defer resolution until a future invocation with `--push` lands the SHA. Note these pending resolutions in the Step 7 summary so they're not forgotten. ### Step 7: Summary Report Provide a summary table of feedback addressed, replies posted, threads resolved, and next steps. See [REFERENCE.md](REFERENCE.md) for the report template. --- ## Workflow harness (template) `workflows/pr-feedback-wave.workflow.js` ships beside this skill and covers the `--all` path only. **It is a TEMPLATE to adapt, not a script to run verbatim.** Read it, then rewrite it for the work in front of you. **Adapt freely:** the per-PR and synthesis agent prompts, the default wave width, the `--include-automation` and `--dry-run` handling, the ordering heuristic inside the finalise plan, and any forge-specific commands. **Preserve across any adaptation:** (a) the loop bound comes from [`scripts/list-actionable-prs.sh`](scripts/list-actionable-prs.sh)'s JSON array, passed in as `args.prs`, never from a prose "for each open PR" — and the concurrency ceiling stays the caller's `--limit N`, because a harness that picks its own width silently overrides the flag that exists to dodge the burst rate limit; (b) `PR_RESULT_SCHEMA` is Step 1A.6's return contract made **enforced** rather than requested — the closed `fix|accept|adapt|defer|answer|decline` action enum and the boolean `resolve` make a vague "handled it" structurally impossible, and a null agent becomes an explicit `PARSE_ERROR` row instead of a silent pass; (c) the Finalise plan is a barrier — push, reply, resolve and re-request all draw on one GitHub rate-limit pool against one remote, so the ORDER is a cross-PR fact no single per-PR agent could know, and Step 7's rollup has to see every PR at once. Two consequences of (c) that are equally non-negotiable: the fanned-out agents **commit but never push**, and each result's status is a pure function of the returned contract (null / `blockers[]` / `commits[]`), never a judgement an agent re-derives. **Agent budget:** 1 + PRs — one agent per actionable PR plus one finalise-plan agent. The wave width bounds concurrency, not the total. The scale guard asks before every run, because the list comes from the caller at runtime. **Skip the harness when:** exactly one PR is actionable — the modal case, which is the single-PR path in Steps 1–7 — or `--dry-run` is set; that is a linear pass and the harness is pure overhead (the template aborts below two). The steps above remain the authoritative description of *what* each stage must produce; the harness only fixes *how* the work is split. Two clauses this template carries. Both are unconditional here — every fanned-out unit runs in its own worktree, and this skill's entire output is a forge mutation: > Never `Workflow({resumeFromRunId})` to retry a few failed worktree agents — a > resume re-runs agents that already succeeded and opens duplicate PRs (#1868).