| 1 | --- |
| 2 | name: "pr-review-response" |
| 3 | description: "Teaches agents to reply to PR review comment threads after fixing issues, making resolutions traceable" |
| 4 | domain: "pull-requests, code-review, traceability" |
| 5 | confidence: "low" |
| 6 | source: "observed (agents fix review feedback silently — reviewers can't tell which comments were addressed)" |
| 7 | tools: |
| 8 | - name: "github-mcp-server-pull_request_read" |
| 9 | description: "Read PR review threads and comments" |
| 10 | when: "Step 1 — fetching review comments to understand what needs fixing" |
| 11 | - name: "gh api (REST)" |
| 12 | description: "Reply to review comment threads and resolve threads via GraphQL" |
| 13 | when: "Step 3 — posting reply to each comment thread after fixing" |
| 14 | --- |
| 15 | |
| 16 | ## Context |
| 17 | |
| 18 | When an agent fixes code in response to PR review comments (from Copilot, a human reviewer, or any GitHub reviewer), the fix alone is not enough. The reviewer needs to see — on the PR thread itself — which comments were addressed and how. Without replies, comments stay visually unresolved, reviewers must re-read the entire diff to verify fixes, and there's no traceable link between feedback and resolution. |
| 19 | |
| 20 | Use this skill whenever: |
| 21 | - You are fixing code based on PR review feedback |
| 22 | - You are addressing Copilot review suggestions |
| 23 | - You are responding to reviewer-requested changes on a PR |
| 24 | - A squad member hands you review comments to resolve |
| 25 | |
| 26 | ## SCOPE |
| 27 | |
| 28 | ✅ THIS SKILL PRODUCES: |
| 29 | - Reply comments on each review thread explaining the fix |
| 30 | - Optionally resolved threads (via GraphQL when appropriate) |
| 31 | - Commit messages that reference the PR and review context |
| 32 | |
| 33 | ❌ THIS SKILL DOES NOT PRODUCE: |
| 34 | - The code fixes themselves (that's the agent's domain work) |
| 35 | - New review comments or reviews |
| 36 | - PR descriptions or summaries |
| 37 | |
| 38 | ## Patterns |
| 39 | |
| 40 | ### Step 1: Read the review comments |
| 41 | |
| 42 | **Using MCP tools (preferred when available):** |
| 43 | |
| 44 | ``` |
| 45 | github-mcp-server-pull_request_read |
| 46 | method: "get_review_comments" |
| 47 | owner: "{owner}" |
| 48 | repo: "{repo}" |
| 49 | pullNumber: {pr_number} |
| 50 | ``` |
| 51 | |
| 52 | This returns review threads with metadata: `isResolved`, `isOutdated`, `isCollapsed`, and their associated comments. Each comment has an `id` you'll need for replies. |
| 53 | |
| 54 | **Using gh CLI (fallback):** |
| 55 | |
| 56 | ```bash |
| 57 | gh api repos/{owner}/{repo}/pulls/{pr_number}/comments --paginate |
| 58 | ``` |
| 59 | |
| 60 | Each comment object contains `id`, `body`, `path`, `line`, and `in_reply_to_id`. Top-level comments have no `in_reply_to_id` — those are the ones you reply to. |
| 61 | |
| 62 | ### Step 2: Fix the code |
| 63 | |
| 64 | Make the actual code changes. This is your normal domain work — the skill doesn't prescribe how to fix, only how to communicate the fix. |
| 65 | |
| 66 | **Track what you changed.** For each review comment, note: |
| 67 | - The comment `id` (top-level, not a reply) |
| 68 | - The file and line referenced |
| 69 | - What you actually changed (brief description) |
| 70 | - The commit SHA after pushing (if available) |
| 71 | |
| 72 | ### Step 3: Reply to each review thread |
| 73 | |
| 74 | After fixing and committing, reply to **each** review comment thread individually. |
| 75 | |
| 76 | **REST API call (via gh CLI):** |
| 77 | |
| 78 | ```bash |
| 79 | gh api repos/{owner}/{repo}/pulls/{pr_number}/comments/{comment_id}/replies \ |
| 80 | -f body="Fixed in {sha_short} — {brief description of what was changed}" |
| 81 | ``` |
| 82 | |
| 83 | **Important:** `{comment_id}` must be the ID of the **top-level** comment in the thread. You cannot reply to a reply — only to the original review comment. |
| 84 | |
| 85 | **Example replies:** |
| 86 | |
| 87 | ```bash |
| 88 | # Specific and traceable |
| 89 | gh api repos/bradygaster/squad/pulls/42/comments/18234/replies \ |
| 90 | -f body="Fixed in a1b2c3d — switched to path.dirname(squadDirInfo.path) for worktree consistency" |
| 91 | |
| 92 | # When applying a suggested code change |
| 93 | gh api repos/bradygaster/squad/pulls/42/comments/18235/replies \ |
| 94 | -f body="Applied suggestion — updated error message to include the file path for debuggability" |
| 95 | |
| 96 | # When pushing back on a suggestion |
| 97 | gh api repos/bradygaster/squad/pulls/42/comments/18236/replies \ |
| 98 | -f body="Considered but not applied — this path needs to stay absolute because worktree resolution depends on it. See detectSquadDir() in detect-squad-dir.ts." |
| 99 | ``` |
| 100 | |
| 101 | ### Step 4: Resolve threads (optional, GraphQL only) |
| 102 | |
| 103 | Thread resolution is only available via the GitHub GraphQL API. Use this when your fix fully addresses the comment and no further discussion is needed. |
| 104 | |
| 105 | **First, get the thread IDs** (they're different from comment IDs): |
| 106 | |
| 107 | ```bash |
| 108 | gh api graphql -f query=' |
| 109 | query { |
| 110 | repository(owner: "{owner}", name: "{repo}") { |
| 111 | pullRequest(number: {pr_number}) { |
| 112 | reviewThreads(first: 100) { |
| 113 | nodes { |
| 114 | id |
| 115 | isResolved |
| 116 | comments(first: 1) { |
| 117 | nodes { body databaseId } |
| 118 | } |
| 119 | } |
| 120 | } |
| 121 | } |
| 122 | } |
| 123 | } |
| 124 | ' |
| 125 | ``` |
| 126 | |
| 127 | Match thread IDs to comment IDs using `databaseId`, then resolve: |
| 128 | |
| 129 | ```bash |
| 130 | gh api graphql -f query=' |
| 131 | mutation { |
| 132 | resolveReviewThread(input: {threadId: "{thread_node_id}"}) { |
| 133 | thread { id isResolved } |
| 134 | } |
| 135 | } |
| 136 | ' |
| 137 | ``` |
| 138 | |
| 139 | **When to resolve vs. leave open:** |
| 140 | - ✅ Resolve: You fixed exactly what was requested, no ambiguity |
| 141 | - ❌ Don't resolve: You pushed back, applied a different fix, or the comment needs further discussion |
| 142 | - ❌ Don't resolve: The reviewer is a human — let them confirm and resolve themselves |
| 143 | |
| 144 | **Rule of thumb:** Agent-to-agent threads (e.g., Copilot review → agent fix) can be resolved by the fixer. Human reviewer threads should be left for the human to resolve. |
| 145 | |
| 146 | ### Step 5: Commit message traceability |
| 147 | |
| 148 | Commit messages should reference the PR context: |
| 149 | |
| 150 | ``` |
| 151 | fix: address review feedback on PR #{pr_number} |
| 152 | |
| 153 | - Switched to path.dirname() for worktree path resolution (comment #18234) |
| 154 | - Updated error message to include file path (comment #18235) |
| 155 | |
| 156 | Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> |
| 157 | ``` |
| 158 | |
| 159 | For single-comment fixes, a shorter format works: |
| 160 | |
| 161 | ``` |
| 162 | fix: use path.dirname() for worktree consistency (PR #{pr_number} review) |
| 163 | |
| 164 | Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> |
| 165 | ``` |
| 166 | |
| 167 | ## AGENT WORKFLOW (Summary) |
| 168 | |
| 169 | 1. **READ** — Fetch review threads using MCP tool or `gh api` |
| 170 | 2. **FIX** — Make code changes, tracking comment ID → change mapping |
| 171 | 3. **COMMIT** — Push with traceable commit message referencing PR and comments |
| 172 | 4. **REPLY** — Post individual reply to each thread via `gh api .../replies` |
| 173 | 5. **RESOLVE** — (Optional) Resolve agent-to-agent threads via GraphQL |
| 174 | 6. **STOP** — Do not batch-reply, do not skip threads, do not resolve human threads |
| 175 | |
| 176 | ## Examples |
| 177 | |
| 178 | ### Example: Copilot flags a potential null dereference |
| 179 | |
| 180 | **Review comment (id: 55123):** |
| 181 | > `squadDir` could be undefined here. Consider adding a null check. |
| 182 | |
| 183 | **Agent workflow:** |
| 184 | 1. Read the comment via `get_review_comments` |
| 185 | 2. Add the null check in `src/cli/core/detect-squad-dir.ts` |
| 186 | 3. Commit: `fix: add null check for squadDir (PR #99 review)` |
| 187 | 4. Reply: |
| 188 | ```bash |
| 189 | gh api repos/bradygaster/squad/pulls/99/comments/55123/replies \ |
| 190 | -f body="Fixed in f4e5d6c — added early return when squadDir is undefined, matching the pattern in loadConfig()" |
| 191 | ``` |
| 192 | 5. Resolve the thread (Copilot → agent, safe to resolve) |
| 193 | |
| 194 | ### Example: Multiple review comments on one PR |
| 195 | |
| 196 | **Comments:** |
| 197 | - id: 55123 — "Null check needed" on `detect-squad-dir.ts:42` |
| 198 | - id: 55124 — "Consider using path.join()" on `detect-squad-dir.ts:58` |
| 199 | - id: 55125 — "This log message is too verbose" on `output.ts:15` |
| 200 | |
| 201 | **Agent handles each individually:** |
| 202 | ```bash |
| 203 | # Fix all three, commit |
| 204 | git add packages/squad-cli/src/cli/core/detect-squad-dir.ts packages/squad-cli/src/cli/core/output.ts |
| 205 | git commit -m "fix: address 3 review comments on PR #99 |
| 206 | |
| 207 | - Added null check for squadDir (comment #55123) |
| 208 | - Switched to path.join() for cross-platform paths (comment #55124) |
| 209 | - Reduced log verbosity to debug level (comment #55125)" |
| 210 | |
| 211 | git push |
| 212 | |
| 213 | # Reply to each thread individually |
| 214 | gh api repos/bradygaster/squad/pulls/99/comments/55123/replies \ |
| 215 | -f body="Fixed — added early return when squadDir is undefined" |
| 216 | |
| 217 | gh api repos/bradygaster/squad/pulls/99/comments/55124/replies \ |
| 218 | -f body="Fixed — switched to path.join(squadDir, 'config.json') for cross-platform consistency" |
| 219 | |
| 220 | gh api repos/bradygaster/squad/pulls/99/comments/55125/replies \ |
| 221 | -f body="Fixed — changed from console.log to debug() so it only shows with --verbose flag" |
| 222 | ``` |
| 223 | |
| 224 | ### Example: Handling Copilot suggestion blocks |
| 225 | |
| 226 | Copilot sometimes provides `suggestion` blocks with exact code to apply: |
| 227 | |
| 228 | **Review comment (id: 55130):** |
| 229 | ```` |
| 230 | Consider using optional chaining: |
| 231 | ```suggestion |
| 232 | const name = config?.agent?.name ?? 'default'; |
| 233 | ``` |
| 234 | ```` |
| 235 | |
| 236 | **Reply format when applying:** |
| 237 | ```bash |
| 238 | gh api repos/bradygaster/squad/pulls/99/comments/55130/replies \ |
| 239 | -f body="Applied suggestion — using optional chaining with nullish coalescing" |
| 240 | ``` |
| 241 | |
| 242 | **Reply format when not applying:** |
| 243 | ```bash |
| 244 | gh api repos/bradygaster/squad/pulls/99/comments/55130/replies \ |
| 245 | -f body="Not applied — config is guaranteed non-null at this point (validated on line 12). Optional chaining would mask errors." |
| 246 | ``` |
| 247 | |
| 248 | ### Example: Pushing back on a review comment |
| 249 | |
| 250 | Not every review comment should be accepted. When a suggestion is incorrect or doesn't apply: |
| 251 | |
| 252 | ```bash |
| 253 | gh api repos/bradygaster/squad/pulls/99/comments/55140/replies \ |
| 254 | -f body="Considered but not applied — this file is in the zero-dependency bootstrap set (see copilot-instructions.md § Protected Files). Adding path.join() would require importing from the SDK, which breaks the bootstrap constraint." |
| 255 | ``` |
| 256 | |
| 257 | Do NOT resolve the thread when pushing back. Leave it open for the reviewer to confirm. |
| 258 | |
| 259 | ## Anti-Patterns |
| 260 | |
| 261 | - ❌ **Fixing silently** — Making code changes without replying to the review thread. The reviewer has no way to know which comments were addressed. |
| 262 | - ❌ **Batch-replying "all fixed"** — A single comment saying "Addressed all review feedback" on the PR. Each thread needs its own reply so reviewers can verify individually. |
| 263 | - ❌ **Resolving without explaining** — Marking threads resolved without posting a reply first. The resolution gives no context on what was done. |
| 264 | - ❌ **Resolving human reviewer threads** — Only resolve threads from automated reviewers (Copilot, bots). Let human reviewers confirm and resolve their own threads. |
| 265 | - ❌ **Vague replies** — "Fixed" or "Done" without saying what was changed. The reply should be specific enough that the reviewer doesn't need to re-read the diff. |
| 266 | - ❌ **Replying before pushing** — Reply after your fix is committed and pushed, not before. The reply should reference actual committed code. |
| 267 | - ❌ **Ignoring comments you disagree with** — If you don't apply a suggestion, reply explaining why. Silence looks like you missed it. |
| 268 | - ❌ **Replying to replies** — The REST API only supports replying to top-level review comments. Attempting to reply to a reply will fail with a 404. |