pr-review-loop

What this skill does

# PR Review Loop

## ⛔ STOP - READ THIS FIRST ⛔

### 1. Locate the scripts directory

**FIRST**, use the Glob tool to find the scripts:
```
Glob pattern: **/pr-review-loop/*/scripts/commit-and-push.sh
Path: ~/.claude/plugins/cache
```
This gives you the full absolute path to the scripts directory.

**Then use the full literal path for every script call.** For example:
```bash
/home/user/.claude/plugins/cache/devon-claude-skills/pr-review-loop/1.0.0/skills/pr-review-loop/scripts/commit-and-push.sh "msg"
```

**NEVER use variables** like `$SCRIPTS/commit-and-push.sh` — this breaks permission matching.
**NEVER use compound commands** like `export PATH=... && commit-and-push.sh` — this also breaks permissions.
**Always inline the full absolute path** in every Bash call.

### 2. No raw git commands

| ❌ FORBIDDEN | ✅ USE INSTEAD |
|--------------|----------------|
| `git commit` | `scripts/commit-and-push.sh "msg"` |
| `git commit -m "..."` | `scripts/commit-and-push.sh "msg"` |
| `git push` | `scripts/commit-and-push.sh "msg"` |
| `git push origin` | `scripts/commit-and-push.sh "msg"` |

**If you use `git commit` or `git push` directly, it will be BLOCKED.**

---

## ⚠️ Do NOT Run as a Background Task Agent

**The PR review loop MUST run in the main conversation, NOT as a background Task.**

Tasks cannot spawn sub-Tasks. If the review loop runs as a Task, agent reviewers (from
AGENT-REVIEWERS.md) will silently fail to spawn — they require the Task tool, which is
only available in the main conversation.

**What to do instead:** Execute the review loop steps directly in the main conversation.
This allows you to spawn agent reviewer Tasks in parallel (C3 of each round) while
keeping Gemini/bot comment handling inline.

Individual agent reviewers (leaf-level Tasks that don't need to spawn further Tasks)
should still be spawned via the Task tool — that works because they're called from the
main conversation, not from within another Task.

---

Streamline the push-review-fix cycle for PRs with automated reviewers.

## Supported Review Bots

| Bot | Trigger | Priority Format |
|-----|---------|-----------------|
| **Gemini Code Assist** | `/gemini review` comment | `![critical]`, `![high]`, `![medium]`, `![low]` |
| **Cursor Bugbot** | Auto on push | `<!-- **High Severity** -->`, `### Bug:` |
| **Claude** | Manual via script | `🚨`, `**Critical**`, `### Critical Issues`, `⚠️` |

Priority detection automatically parses all formats when summarizing and fetching comments.

### Priority to Exit-Condition Mapping

The quality-weighted exit condition (see ONE MORE LOOP Rule) depends on classifying findings as P1/P2 vs P3/nitpick. Use this table:

| Source label | P-level | Blocks quality-weighted exit? |
|---|---|---|
| Gemini `![critical]`, `![high]` | P1/P2 | Yes — must be resolved or merged with explicit user sign-off |
| Gemini `![medium]` | P2 if correctness/security/breaking; else P3 | Yes if correctness/security/breaking; no if style/prose |
| Gemini `![low]` | P3/nitpick | No |
| Cursor `**High Severity**`, `### Bug:` | P1/P2 | Yes |
| Claude `🚨`, `### Critical Issues`, `**Critical**` | P1/P2 | Yes |
| Claude `⚠️` | P2 if correctness/security/breaking; else P3 | Yes if correctness/security/breaking; no if style/prose |
| Agent comment, no explicit label | Infer from content: correctness/security/breaking → P1/P2; else P3 | Per inferred level |

**"Won't fix" on a P1/P2 finding does NOT resolve it on its own** — it still blocks quality-weighted exit unless (i) the finding is reclassified to P3 with explicit justification (drop one P-level with reasoning), (ii) the finding is actually fixed in a later round, or (iii) the user explicitly signs off on the carry-forward, in which case the finding is recorded as an acknowledged unresolved item in the merge-readiness summary.

The (iii) escape valve exists so the model isn't forced to relabel a genuine "won't fix" as a reclassification: surface the finding to the user, they sign off, it's recorded in the merge-readiness summary.

**Classifying `![medium]` (Gemini's most-used label) — use these heuristics:**

- P2 (blocking): the comment describes a correctness bug, security concern, breaking change, data loss risk, or incorrect error handling. Example: *"this will return nil for empty input"*, *"missing null check could crash on production data"*, *"env var not validated before use"*.
- P3 (non-blocking): the comment describes naming, phrasing, formatting, alternative implementations of equivalent behavior, documentation polish, or preference-level style. Example: *"variable name is ambiguous"*, *"consider using map over for-loop"*, *"comment could be clearer"*.
- When ambiguous: default to P2 on the first round; downgrade to P3 only if the fix is a judgment call, not a correctness improvement.

## Comment Formats: Line Comments vs PR Comments

Different bots use different comment formats:

### Line Comments (Gemini, Cursor)
- Posted on specific lines of the diff
- Each issue is a separate comment thread
- Reply using `reply-to-comment.sh <PR> <comment-id> "Fixed"`
- Thread auto-resolves when replied to

### PR Comments (Claude)
- Single large comment on the PR (not on specific lines)
- Contains multiple issues organized by sections (e.g., "Issues & Concerns")
- Issues reference file:line locations in prose (e.g., "**Location:** `role_summaries_controller.rb:60`")
- Reply to the comment addressing multiple issues at once

**Handling Claude's PR comments:**

1. **Fetch the comment** to get the comment ID:
   ```bash
   gh pr view <PR> --json comments --jq '.comments[] | select(.author.login == "claude") | {id: .id, body: .body}'
   ```

2. **Parse issues** from the structured markdown:
   - Look for numbered headings like `### **1. BREAKING CHANGE: ...**`
   - Extract file:line references from `**Location:**` lines
   - Note priority indicators: 🚨 (critical), ⚠️ (warning), etc.

3. **Reply with a consolidated response** addressing each issue:
   ```bash
   gh pr comment <PR> --body "## Response to Claude Review

   **Issue 1 (Breaking API change):** Fixed - added deprecation support for old parameter name

   **Issue 2 (Missing validation):** Fixed - added ALLOWED_VIEW_ROLES validation

   **Issue 3 (Logic change):** Won't fix - this is intentional, documented in PR description

   **Issue 4 (Missing tests):** Fixed - added RSpec tests for SystemRoles methods
   "
   ```

4. **Individual issues** can also be addressed via separate replies if preferred:
   ```bash
   gh pr comment <PR> --body "**Re: Issue #2 (Missing Input Validation):** Fixed in commit abc123 - added validation for view_role parameter"
   ```

The key difference: Claude comments don't have "threads" to resolve - you reply to the main PR comment referencing which issues you're addressing.

## Critical: Be Skeptical of Reviews

**Not all suggestions are good.** Evaluate each review comment critically:

- Does this actually improve the code, or is it pedantic?
- Is this suggestion appropriate for the project's context?
- Would implementing this introduce unnecessary complexity?

**Skip suggestions that are:**
- Platform-specific when not applicable (Windows comments for Linux-only code)
- Overly defensive (excessive null checks, unlikely edge cases)
- Stylistic preferences that don't match project conventions
- Adding documentation for self-explanatory code

When in doubt, ask the user rather than blindly applying changes.

### Self-Contradiction Detection

Track changes across rounds. When a fix in round N reverses or conflicts with a fix from a previous round, this signals that the review loop may be degrading the code rather than improving it.

**When a contradiction is detected:**

1. **Identify all involved changes**: List the original code, the round N-K change, and the round N change that contradicts it.
2. **Analyze both positions**: Each round may have had valid reasoning. Assess whether the later round caught a genuine mistake in the earl