skinnyandbald/pr-resolution

Resolve PR Comments in Parallel (v3)

DEFAULT WORKFLOW for resolving PR comments with parallel execution.

MANDATORY: Background Execution

The ENTIRE workflow (Phases 0-7) MUST run as a background agent. This prevents the workflow from being sidetracked by user questions, CI failures, or context switches.

Foreground steps (do these FIRST, before launching the agent):

1. Detect PR number from args, current branch, or ask user
2. Resolve the PR branch name — run gh pr view $PR_NUM --json headRefName -q .headRefName to get the exact branch. Store as $PR_BRANCH.
3. Build session context (see checklist below). Store as $SESSION_CONTEXT.
4. Print: "Launching PR resolution for #$PR_NUM (branch: $PR_BRANCH) in background. You'll be notified when it completes."
5. Launch background agent with the full workflow, passing the branch name and session context:

Agent(
  run_in_background: true,
  prompt: "You are resolving PR comments for PR #$PR_NUM.
Branch: $PR_BRANCH

Session context (from parent conversation — advisory, not authoritative; prefer repo evidence when they conflict):
$SESSION_CONTEXT

Read the pr-resolution skill at ~/.claude/skills/pr-resolution/SKILL.md and execute Phases 0-7.

IMPORTANT:
- FIRST: create an isolated detached worktree at the PR branch tip (Phase 0). Do NOT `git checkout` or `git stash` in the tree you inherited — the parent session may be sitting in it.
- Run every subsequent phase (edits, commit, push, verification) from inside that worktree
- For questions classified as [question] that need human input, skip them and note them in your final output
- For comments classified as [unverified], reply to the thread explaining what couldn't be verified, leave the thread OPEN, and note it in your final output (exclude these from the Phase 5f zero-unresolved-threads check)
- For CI failures, fix them as part of the workflow — do NOT stop or ask for help
- Complete ALL phases including the CI gate (Phase 6) and shepherd launch (Phase 7)
- Your final output should summarize: comments resolved, comments skipped (with reasons), comments flagged for human review, CI status"
)

Session Context Checklist

Before launching, answer these questions from the current conversation. Include only factual answers — skip any that don't apply. Format as markdown bullet points.

1. Were any names, identifiers, or terms discussed or corrected in this conversation? (e.g., "The skill is called write, not content:write")
2. Were any architectural decisions made that affect how comments should be interpreted? (e.g., "We intentionally removed the retry logic in this PR")
3. Is there terminology the PR uses that differs from what a reviewer might assume? (e.g., "The workflow field here means the n8n workflow, not a GitHub Actions workflow")
4. Are there constraints from the conversation the background agent needs? (e.g., "Don't rename any exported functions — downstream consumers depend on them")

If no answers apply, include: - No additional session context. (explicit omission, not silent)

That's it for the foreground. Everything below is executed by the background agent.

---

Quick Reference

| Action | Command | |--------|---------| | Get comments | ~/.claude/skills/pr-resolution/bin/get-pr-comments PR_NUM | | Parse CodeRabbit | ~/.claude/skills/pr-resolution/bin/parse-coderabbit-review PR_NUM | | Check CI | gh pr checks | | Resolve thread | ~/.claude/skills/pr-resolution/bin/resolve-pr-thread NODE_ID | | Push detached HEAD to PR branch | ~/.claude/skills/pr-resolution/bin/push-to-pr-branch PR_BRANCH |

Workflow Overview

Phase 0: Pre-Flight     → Detached worktree at PR tip + mergeability check + GoodToGo (if installed)
Phase 1: Discovery      → Gather comments, parse bot formats, enumerate
Phase 2: Classification → Categorize by priority, group by file
Phase 3: Resolution     → Launch parallel agents by file group
Phase 4: Verification   → Local checks + GoodToGo gate (if installed)
Phase 5: Completion     → Commit, push, resolve threads
Phase 6: CI Gate        → Monitor CI + mergeability, fix actionable failures, re-push
Phase 7: Shepherd       → Inline polling loop for new bot comments + CI

---

Phase 0: Pre-Flight

Detached worktree at the PR tip (MANDATORY — do this FIRST)

The background agent inherits whatever working tree (and branch) the user was on. Do NOT git checkout or git stash in that tree — the parent session may be sitting in it, and switching its branch or stashing its WIP corrupts the user's state. Instead, operate in a private worktree on a detached HEAD at the PR branch tip. A detached worktree commits and pushes to the branch ref without ever claiming the branch name, so it coexists with the interactive session. (Claude Code's isolation: worktree flag does NOT solve this — it branches from origin/HEAD onto a fresh branch, not the PR branch — so the skill manages its own worktree explicitly.)

# PR branch comes from the prompt context ($PR_BRANCH); resolve if absent:
PR_BRANCH=${PR_BRANCH:-$(gh pr view $PR_NUM --json headRefName -q '.headRefName')}

# This workflow resolves SAME-REPO PRs — it fetches and pushes origin/$PR_BRANCH. A fork PR's
# head lives on the contributor's fork, not origin, so fail clearly instead of mis-fetching:
if [ "$(gh pr view "$PR_NUM" --json isCrossRepository -q '.isCrossRepository' 2>/dev/null)" = "true" ]; then
  echo "FATAL: PR #$PR_NUM is from a fork; this workflow handles same-repo PRs only." >&2
  exit 1
fi

REPO_ROOT=$(git rev-parse --show-toplevel)
HOOKS_PATH_BEFORE=$(git config --get core.hooksPath || true)   # guard against husky drift (below)

git fetch origin "$PR_BRANCH"
WT=$(mktemp -d)
rmdir "$WT"   # some Git versions refuse to add a worktree into an existing dir; let Git create it cleanly
git worktree add --detach "$WT" "origin/$PR_BRANCH"   # detached at the remote tip; claims no branch
cd "$WT"

Make dependencies available in the worktree (the verification and CI phases need them) WITHOUT running a fresh install's post-install hooks:

# Fast path: reuse the parent's installed deps. No install runs, so husky's `prepare`
# never fires and core.hooksPath cannot drift. Covers the common case (deps unchanged).
if [ -d "$REPO_ROOT/node_modules" ] && [ ! -e node_modules ]; then
  ln -sf "$REPO_ROOT/node_modules" node_modules   # -f so a stale/broken symlink (which `-e` misses) doesn't abort the link
fi
if [ -e "$REPO_ROOT/.env" ] && [ ! -e .env ]; then ln -s "$REPO_ROOT/.env" .env; fi   # only when the worktree has no .env — never replace a tracked .env (the commit would capture the symlink)

# Only if the PR changed a dependency manifest/lockfile do you need a real install.
# Run it with HUSKY=0 so the `prepare` script cannot rewrite core.hooksPath in the shared .git:
#   HUSKY=0 npm ci        # or the repo's package manager (pnpm i --frozen-lockfile, etc.)

Guard core.hooksPath (defensive). A dependency install can run package lifecycle scripts (e.g. husky's prepare) that rewrite core.hooksPath in the shared .git/config; the exact value they set depends on the husky version, so stay version-agnostic — capture it before setup and restore it after. (git worktree add itself runs no lifecycle script, so this only matters if you ran an install above.)

NOW=$(git config --get core.hooksPath || true)
if [ "$NOW" != "$HOOKS_PATH_BEFORE" ]; then
  if [ -n "$HOOKS_PATH_BEFORE" ]; then git config core.hooksPath "$HOOKS_PATH_BEFORE"; else git config --unset core.hooksPath || true; fi
fi

Teardown (MANDATORY on EVERY exit — success or error). Remove the worktree before the agent ends, from every phase's exit path (see references/exit-states.md). Exception: if a push failed and the worktree still holds commits not yet on origin/$PR_BRANCH, it is the only ref to them — report it instead of removing, so they stay recoverable:

cd "$REPO_ROOT"
if git -C "$WT" rev-parse HEAD >/dev/null 2>&1 \
   && ! git merge-base --is-ancestor "$(git -C "$WT" rev-parse HEAD)" "origin/$PR_BRANCH" 2>/dev/null; then
  echo "NOTE: unpushed commits remain in $WT (HEAD $(git -C "$WT" rev-parse --short HEAD)) — NOT removing it; re-push or cherry-pick them, then 'git worktree remove $WT'." >&2
else
  git worktree remove "$WT" --force 2>/dev/null || true
fi

There is no "wrong branch" to guard against anymore — the detached worktree is always at the PR tip — so the old FATAL: Expected branch check is gone.

Mergeability check (MANDATORY — do this BEFORE Discovery)

CI green ≠ mergeable. GitHub tracks merge state separately from check runs, so a PR can have all-passing CI but still be blocked by a merge conflict with the base branch. The skill must detect this explicitly — gh pr checks alone will not.

RESULT=$(~/.claude/skills/pr-resolution/bin/check-mergeability "$PR_NUM")
STATUS=$(echo "$RESULT" | jq -r '.status')
BASE_REF=$(gh pr view "$PR_NUM" --json baseRefName -q '.baseRefName')
git fetch origin "$BASE_REF"

bin/check-mergeability polls gh pr view --json mergeable,mergeStateStatus up to 60s (GitHub computes asynchronously after pushes) and returns one of these statuses:

| Status | Meaning | Action | |--------|---------|--------| | CLEAN | MERGEABLE and base is current (or only failing checks) | Continue to Discovery | | BEHIND | MERGEABLE but PR is behind base | Auto-sync: git merge "origin/$BASE_REF" (no conflicts expected since GitHub said MERGEABLE), then ~/.claude/skills/pr-resolution/bin/push-to-pr-branch "$PR_BRANCH". Re-check. Continue. | | CONFLICT | CONFLICTING / DIRTY | STOP. Exit PRE_FLIGHT_CONFLICT. Report conflicting files (git merge --no-commit "origin/$BASE_REF"; git diff --name-only --diff-filter=U; git merge --abort). Do NOT auto-resolve — conflicts often carry semantic meaning a code review missed. | | UNKNOWN | GitHub couldn't compute after polling | Exit PRE_FLIGHT_UNKNOWN_MERGE_STATE — humans should investigate. | | ERROR | API or usage failure | Exit PRE_FLIGHT_ERROR and surface the error message. |

See references/exit-states.md for the complete exit state taxonomy across all phases.

Rationale: Resolving conflicts is a code decision, not a process step. The skill's job is to surface them, not paper over them. Auto-merging clean updates (BEHIND without conflict) is safe because git already verified no overlap.

Tests: bin/tests/check-mergeability.test.sh covers all five status paths plus argument validation and --repo forwarding.

GoodToGo check (optional)

If gtg is installed, run the GoodToGo pre-flight check (see references/goodtogo.md):

if command -v gtg &> /dev/null; then
  # gtg auto-detects repo from git remote
  GTG_RESULT=$(gtg $PR_NUM --format json 2>/dev/null)
  GTG_STATUS=$(echo "$GTG_RESULT" | jq -r '.status')
fi

Route based on status (or skip straight to Phase 1 if gtg is not installed):

• READY → Quick verify and commit (fast path — skip Phases 1-3)
• CI_FAILING → Fix CI first
• ACTION_REQUIRED → Continue with full workflow
• UNRESOLVED_THREADS → Continue with full workflow

---

Phase 1: Discovery

1. Gather comments using scripts from references/discovery.md
2. Parse bot formats using rules from references/bot-formats.md
3. Print enumeration - counts MUST match before proceeding

Zero comments found? Bots (CodeRabbit, Gemini, Cubic, CodeScene) take 1-5 minutes to post reviews. If discovery finds zero comments, skip Phases 2-5 and jump directly to Phase 6 (CI Gate), then Phase 7 (Shepherd). The shepherd will catch late-arriving bot comments. Do NOT exit early — the shepherd is the whole point when there are no initial comments.

---

Phase 2: Classification, Validation & Grouping

1. Classify each comment using references/classification.md
2. Validate each comment — read the actual code at the referenced lines and verify the bot's finding is correct. See the Validation section in references/classification.md for the full checklist. Mark invalid findings with resolution type invalid.
3. Reply to invalid findings — for each comment you determine is invalid, post a direct reply to that comment thread explaining why the finding doesn't apply BEFORE resolving the thread. Use gh api to reply (see references/completion.md for comment reply patterns), then resolve using ~/.claude/skills/pr-resolution/bin/resolve-pr-thread NODE_ID.
4. CodeScene handling: CodeScene comments flag measurable code health regressions (complexity, duplication, nesting). Treat them as high-confidence, but still validate each finding against the referenced code/context before applying changes.
5. Group valid comments by file for parallel execution:

## Parallel Execution Plan

### Group A: src/api/route.ts (3 comments → 1 agent)
- #1 [blocking] Line 45 - Add error handling
- #3 [suggestion] Line 67 - Improve validation

### Group B: src/components/Button.tsx (1 comment → 1 agent)
- #2 [suggestion] Line 23 - Add prop types

### Group C: CI Failures (if any → 1 agent)
- Fix lint/type errors

Total: 3 parallel agents

---

Phase 3: PARALLEL EXECUTION

MANDATORY: Launch agents simultaneously using the Task tool:

Agent 1: "Fix comments on src/api/route.ts"
- Comment #1: Add error handling at line 45
- Comment #3: Improve validation at line 67

Agent 2: "Fix comments on src/components/Button.tsx"
- Comment #2: Add prop types at line 23

Agent 3: "Fix CI failures"
- Lint errors
- Type errors

Parallel execution rules:

| Condition | Execution | |-----------|-----------| | Same file | → Same agent (avoid conflicts) | | Different files | → Parallel agents | | CI failures | → Dedicated agent | | Questions | → Ask human first |

Pre-existing failures discovered during Phase 3:

| Type | Action | |------|--------| | Required checks failing (branch protection blocks merge) | Fix in this PR. After editing the out-of-scope file, perform a local merge check (e.g., git fetch origin $BASE_REF && git merge --no-commit origin/$BASE_REF) to catch any new conflict before proceeding. Flag the out-of-scope change prominently in the Phase 5 resolution summary. | | Non-required checks failing (pre-existing on main, not required for merge) | Do NOT fix in this PR. Create a GitHub issue with gh issue create --title "[title]" --body "[details]" documenting the failure so it isn't lost. Reference the issue in the Phase 5 summary. |

Out-of-scope edits can introduce merge conflicts that Phase 0 couldn't anticipate (the conflict didn't exist yet). The immediate post-edit local merge check is the safeguard.

Wait for all agents to complete.

---

Phase 4: Verification Gate (MANDATORY)

1. Run local checks from references/verification.md
2. If gtg is installed, run final verification from references/goodtogo.md (deterministic READY/BLOCK signal)
3. Verify all resolutions - every comment needs explicit resolution

DO NOT commit until all checks pass. Phase 5 MUST NOT run if Phase 4 verification exits non-zero.

---

Phase 5: Completion (MANDATORY — DO NOT SKIP)

Follow steps from references/completion.md:

5a. Commit

1. Commit all fixes together

5b. Capture timestamp (for shepherd)

2. Capture the current UTC timestamp immediately before push:

LAST_TIMESTAMP=$(date -u +%Y-%m-%dT%H:%M:%SZ)

5c. Push

3. Push to remote
   • If push fails: Skip Phase 6-7. The resolution is incomplete.

5d. Post resolution summary

4. Post resolution summary comment to PR

5e. Resolve threads individually (MANDATORY)

Resolve each thread one-by-one, only after confirming the comment was addressed:

• Invalid findings: already resolved in Phase 2 (after posting reply)
• Code fixes: resolve now, after verifying the fix is in the pushed commit
• Won't fix / disagree: resolve now, after posting a reply explaining why
• Non-actionable: resolve now
• Unverified: do NOT resolve. Reply to the thread explaining what couldn't be verified and why it needs human review. Leave the thread open. Include in the completion summary under "Flagged for human review."

For each thread:

~/.claude/skills/pr-resolution/bin/resolve-pr-thread NODE_ID

Do NOT use resolve-all-threads to bulk-resolve. Each thread must be individually confirmed as addressed before resolving. Bulk resolution hides unaddressed comments.

5f. Final verification (MANDATORY)

5. Verify zero unresolved threads remain:

UNRESOLVED=$(gh api graphql -f query='
  query($owner: String!, $repo: String!, $pr: Int!) {
    repository(owner: $owner, name: $repo) {
      pullRequest(number: $pr) {
        reviewThreads(first: 1) { totalCount }
        unresolvedThreads: reviewThreads(first: 1, filterBy: {resolved: false}) { totalCount }
      }
    }
  }
' -F owner="$OWNER" -F repo="$REPO" -F pr=$PR_NUM \
  --jq '.data.repository.pullRequest.unresolvedThreads.totalCount')

echo "Unresolved threads: $UNRESOLVED"

If UNRESOLVED > 0: List each remaining thread, investigate whether it was addressed, and resolve individually with bin/resolve-pr-thread. Do NOT bulk-resolve to make the count go to zero — find out why it wasn't resolved and fix the gap.

Exception: Threads classified as unverified are intentionally left open for human review. Exclude these from the zero-unresolved check — they are tracked in the completion summary, not auto-resolved.

Workflow is NOT complete until every non-unverified thread is verified as addressed and resolved.

---

Phase 6: CI Gate (MANDATORY)

After pushing in Phase 5, monitor CI until green or exit condition. Follow the bounded CI retry loop from references/ci-gate.md.

1. Appearance wait — poll every 15s until at least one check exists for HEAD_SHA (2 min timeout)
2. Settle wait — poll every 60s until all checks reach terminal status (15 min timeout)
3. Classify failures using the decision matrix from references/ci-gate.md:
   • ACTIONS_FIXABLE: GitHub Actions run (app_slug == "github-actions") + failing job name matches fixable pattern + local npm command exists
   • THIRD_PARTY_FIXABLE: recognized third-party checks that post structured feedback as PR review comments. Only applies if the check actually appears in the repo's check runs — see Step 3b in references/ci-gate.md for detection rules and fix strategies per provider
   • EXTERNAL: everything else (unrecognized third-party apps, no local repro, transient infra failures)
4. For ACTIONS_FIXABLE or THIRD_PARTY_FIXABLE failures: a. Fetch failure details (truncated logs for Actions; PR review comments for third-party) b. Diagnose — prioritize PR-modified files, expand scope if needed c. Fix the code, verify locally where possible (timeout 120 npm run <command>) d. Commit with fix(ci): resolve <check-name> failure e. Push, wait 60s grace period, update HEAD_SHA and LAST_TIMESTAMP f. Return to step 1
5. Max 3 fix attempts per check name (persisted to /tmp/ci-gate-state-$PR_NUM, keyed by normalized name)
6. Total timeout: 30 minutes

Exit routing:

• CI_GREEN or CI_EXTERNAL_ONLY → proceed to Phase 7
• CI_MERGE_CONFLICT or CI_UNKNOWN_MERGE_STATE or CI_MERGE_CHECK_ERROR → report prominently (non-success), proceed to Phase 7
• CI_TIMEOUT or CI_ESCALATION or CI_NO_CHECKS → report status, proceed to Phase 7

Pre-existing failure policy: If a check fails on the branch, fix it. Do NOT classify failures as "pre-existing" to skip them.

---

Phase 7: Shepherd (MANDATORY — DO NOT SKIP)

After Phase 6 completes, continue polling for new bot comments in the same agent. Do NOT launch a separate background agent — you already have full context, and a new agent launch wastes tokens re-loading the system prompt.

This phase is MANDATORY — even when zero comments were found in Phase 1. Bots (CodeRabbit, Gemini, Cubic, CodeScene) take 1-5 minutes to post reviews after a PR is created or pushed. The shepherd catches these late-arriving comments. Do not rationalize skipping ("no comments found", "no push made", "unlikely", "docs-only", "no new comments expected") — run the shepherd every time. If no push was made in Phase 5, use the current UTC timestamp for LAST_TIMESTAMP.

1. Capture context:

OWNER_REPO=$(gh repo view --json nameWithOwner -q '.nameWithOwner')
BRANCH="$PR_BRANCH"   # HEAD is detached in the worktree — derive from PR_BRANCH, not `git branch --show-current`
RUN_ID=$(date +%s)

2. Read shepherd.md in this skill directory and execute the shepherd state machine inline. You already have PR_NUM, LAST_TIMESTAMP, OWNER_REPO, BRANCH, and RUN_ID. Use ~/.claude/skills/pr-resolution as the SKILL_DIR (do not use find to resolve it).

3. When the shepherd reaches POST_SUMMARY, include the summary in your final output and exit.

---

Example: PR with 6 Comments

## Discovery
1. [blocking] src/api/route.ts:45 - "Missing auth check"
2. [suggestion] src/api/route.ts:67 - "Add input validation"
3. [suggestion] src/components/Form.tsx:23 - "Restore removed guard"
4. [nitpick] src/utils/format.ts:12 - "Trailing whitespace"
5. [question] src/lib/auth.ts:89 - "Handle null?"
6. CI: Lint error

## Validation
1. ✓ VALID — auth middleware skipped for this route, real issue
2. ✓ VALID — user input passed unsanitized to query
3. ✗ INVALID — guard was intentionally removed in prior commit (git blame confirms)
   → Reply to thread: "This guard was removed in abc123 because [reason]. Not restoring."
4. ✗ INVALID — no trailing whitespace exists at line 12, false positive
   → Reply to thread: "Checked the file — no trailing whitespace at this line."
5. [question] → Ask human

## Parallel Plan (2 valid + 1 question + 1 CI)
- Agent 1: src/api/route.ts (#1, #2)
- Agent 2: src/lib/auth.ts (#5 - question)
- Agent 3: CI fix (#6)

## Execution
Reply to #3, #4 with reasons → Resolve those threads →
Launch fix agents in parallel → Wait → Verify → Commit → Push

---

Related

| Resource | Description | |----------|-------------| | detailed-reference.md | Single-threaded detailed reference | | /commit-commands:commit | Clean commit workflow |