ywc-handle-pr-reviews

Handle PR Review Comments

Announce at start: "I'm using the ywc-handle-pr-reviews skill to address review comments and reply to each thread."

Review PR comments, fix issues where needed, and reply to each comment.

Rationalization Defense

When tempted to skip a step, check this table first:

Excuse	Reality
"This comment is just opinion, no fix needed"	Even disagreement requires a reply explaining why. Silent skip looks like neglect.
"I'll batch all replies after fixing everything"	Reply per thread as fixes land. Reviewers track which threads are resolved.
"Push without reply, the diff speaks for itself"	Reviewers cannot tell intent from diff alone. Always pair fix with reply.
"Resolving the thread without reply is fine"	Resolving without reply implies the reviewer's concern was wrong. Always state how it was addressed.
"Conflict in suggestions, pick one and move on"	Surface the conflict to the user. Do not silently choose between reviewer A and reviewer B.
"PR not found for this branch, scan recent PRs"	Stop and ask. Acting on a wrong PR overwrites unrelated reviewer threads.
"All review comments are addressed — CI is a separate concern"	Fixes to source code (refactors, new imports, logic changes) can break CI. Always re-verify CI after pushing review fixes. A PR with all comments addressed but failing CI is still blocked from merging.
"The comments only needed replies, no code changed — so CI is fine to skip"	CI can already be red for reasons unrelated to the comments (a flaky earlier push, a base-branch change, a dependency break). Handling a PR means leaving it mergeable. Always check current CI status when handling a PR, even when your replies pushed no code.
"CI is green and comments are addressed — the PR is mergeable"	While the review was in progress the base branch may have advanced and the PR may now be CONFLICTING. Handling a PR means leaving it mergeable, which requires checking gh pr view --json mergeable,mergeStateStatus, not just CI. A conflicting PR is blocked from merge regardless of how many comments were resolved.
"The PR conflicts with base — rebase the branch to clear it"	Rebasing rewrites SHAs and orphans the very review threads you are replying to. Merge the base into the feature branch (git merge --no-ff origin/<base>) instead. See references/pr-conflict-resolution.md.

Violating the letter of these rules is violating the spirit. Code review is a conversation, not a checklist.

Context

• Current branch: !git branch --show-current
• Recent commits: !git log --oneline -5

Task

Follow the steps below to handle PR review comments.

1. Identify PR Number

• If a PR number is specified in $ARGUMENTS, use it
• Otherwise, identify the current branch's PR with gh pr view --json number
• If no PR exists for the current branch, stop and notify the user rather than proceeding blindly

Resolve {owner}/{repo} dynamically at this stage — it's used throughout the workflow:

gh repo view --json nameWithOwner --jq .nameWithOwner

2. Retrieve and Filter Comments

Retrieve all PR review comments and filter out those that don't need action. This prevents duplicate work and keeps the process focused on genuinely unresolved feedback.

bash tools/claude-code/skills/ywc-handle-pr-reviews/scripts/fetch-unresolved-comments.sh \
  {owner}/{repo} {pr_number}
# exit 0 → JSON array of unresolved threads on stdout (may be [])
# exit 1 → gh CLI error (not authenticated or PR not found)

The script fetches paginated comments, groups them into threads, and applies the skip conditions automatically: threads containing <!-- <review_comment_addressed> --> are dropped, and threads where your reply is newer than the latest reviewer comment are dropped. The output JSON array contains only actionable threads — each element has id, body, path, line, user, created_at, and thread_comment_count. If the array is [], skip to Step 7 and report "no unresolved comments".

3. Group and Analyze Comments

Before fixing anything, group all remaining comments by file. Processing file-by-file is more efficient — you read each file once, apply all related fixes together, and create one coherent commit per file instead of jumping back and forth.

Processing strategy:

1. Collect all unresolved comments and group by target file path
2. For each file, read the file once and analyze all related comments together
3. Apply fixes for that file and commit as a unit
4. Move to the next file

4. Classify and Fix Comments

For each comment, classify it into one of four categories:

Category	Action
Approval / Acknowledgment (tagged review_comment_addressed)	Skip — already resolved
Clear code change request (the fix is straightforward and unambiguous)	Apply the fix
Controversial or ambiguous change request (disagreement, architectural concern, or trade-off involved)	Do NOT auto-fix. Present the comment to the user with context, explain the trade-off, and ask for a decision. Reviewer feedback deserves thoughtful consideration, not blind acceptance
Question only (no code change implied)	Reply with an explanation — no fix needed

Security guardrails (never auto-apply, always defer to user):

• Removal of authentication, authorization, or security middleware
• Addition of eval(), exec(), or dynamic code execution
• Disabling RLS, bypassing tenant isolation, or weakening input validation
• Hardcoded credentials or secrets

When applying fixes:

• Read the target file and identify the exact problem area referenced by the comment
• Apply the minimal fix that addresses the reviewer's concern
• If the comment references outdated code (the file has changed since the review), check whether the concern still applies to the current code. If it does, fix the current version. If not, reply explaining the code has already changed

Commit rules:

• One commit per file (group all fixes for the same file into a single commit)
• Use commit message format: fix: summary of the fix
• Push to remote after all commits are ready — pushing once at the end is cleaner than pushing after each commit

5. Reply to Comments

Attitude layer (mandatory): every reply must go through the discipline defined in ywc-receive-review — verify each comment against the codebase before agreeing; replies state the fix or a technical pushback ending in a question; the forbidden vocabulary list ("You're absolutely right!", "Great point!", "Thanks!") applies to all body content surfaced via the reply API below.

Reply to every processed comment so the reviewer knows their feedback was addressed. Use the thread reply API to keep conversations organized:

gh api repos/{owner}/{repo}/pulls/{pull_number}/comments/{in_reply_to_id}/replies -F body=@- <<'REPLY_BODY'
...
REPLY_BODY

Reply language: Match the language of the original comment. If the reviewer wrote in Korean, reply in Korean. If in English, reply in English.

Reply format by category:

• Fixed:

  Fixed: [commit-hash](https://github.com/{owner}/{repo}/pull/{pr}/commits/{full_sha})
  Description of what was changed and why
  

• Deferred to user: This requires a decision — I've flagged it for the PR author.
• No fix needed: Reply explaining the reasoning (e.g., intentional design choice, already handled elsewhere)
• Outdated comment: This code has been updated since the review. The concern no longer applies because [reason].

6. Error Handling

Things can go wrong during the process. Handle these gracefully:

Error	How to handle
gh pr view fails (no PR for branch)	Stop and tell the user. Don't guess the PR number
Push fails (non-fast-forward, remote feature branch advanced)	git pull --rebase origin <feature-branch> (rebase on the feature branch you own is safe), then re-push. Don't force-push without approval. See references/pr-conflict-resolution.md
Comment reply API returns 403/404	Log the error, skip that reply, and report it in the final summary
Referenced file no longer exists	Reply to the comment explaining the file was removed, and skip the fix

6.5 CI Status Check & Fix

Always run this step — do not skip it, even when your replies pushed no code. A PR review thread and CI status are two independent blockers on the same PR. Addressing every comment but leaving CI red still leaves the PR unmergeable, so handling a PR means checking both. This step covers two cases at once: CI regressions introduced by the fixes in Step 4, and pre-existing CI failures that were already red before you touched the PR (a flaky earlier push, a base-branch change, a broken dependency).

PR_NUMBER=$(gh pr view --json number --jq .number)
gh pr checks $PR_NUMBER
# exit 0 = all checks passed; non-zero = one or more checks failed or are pending

• All checks pass: proceed to Step 7.
• Checks still pending: wait for completion with gh pr checks $PR_NUMBER --watch, then re-evaluate.
• Any check fails:
  1. Get failure logs:

     gh run list --branch $(git branch --show-current) \
       --json databaseId,conclusion \
       --jq '.[] | select(.conclusion == "failure") | .databaseId' | head -1
     gh run view <run-id> --log-failed
     

  2. Categorize and fix:

     Failure type	Fix action
     Lint / format	Run the project's auto-fix command, commit, push
     Type / test / build	Analyze output, fix implementation, commit, push
     Infra / flaky / clearly unrelated to this PR's scope	Do not blindly patch. Surface the failing check and logs to the user and ask how to proceed (e.g., re-run the job, or fix in a separate PR)

  3. Re-check CI after each push. Up to 2 fix attempts.
  4. If CI still fails after 2 attempts: record the failing check names in Step 7's summary and set the overall outcome to DONE_WITH_CONCERNS (not DONE).

6.6 Merge-Readiness (Conflict) Check

Always run this step. CI green and all comments addressed still does not guarantee the PR can merge — while the review was in progress the base branch may have advanced into a conflict. A PR review thread, CI status, and merge-readiness are three independent blockers on the same PR; handling a PR means leaving all three clear.

Action required: Read claude-code/skills/references/pr-conflict-resolution.md before proceeding — it defines the mergeable / mergeStateStatus semantics and the merge-not-rebase update procedure.

gh pr view $PR_NUMBER --json mergeable,mergeStateStatus --jq '{mergeable, mergeStateStatus}'

• MERGEABLE / CLEAN → proceed to Step 7.
• BEHIND → the branch is merely out of date (no textual conflict). Follow Update Branch From Base (merge the base into the feature branch — never rebase, since that orphans the review threads you just replied to), push, and re-verify CI (one additional cycle).
• CONFLICTING / DIRTY → follow Update Branch From Base in the reference. If it auto-resolves, push and re-verify CI (one additional cycle). If it reports real textual conflicts, surface the conflicting files + PR URL to the user, stop, and set the outcome to BLOCKED — do not auto-resolve or force-push.
• BLOCKED → a required check or review gate is missing — not a conflict. Do not run the base-merge procedure; surface which required check or review is outstanding and set the outcome to BLOCKED.
• UNKNOWN → poll briefly per the reference, then re-read.

7. Final Summary

Report the results so the user has a clear picture of what happened:

• Total comments processed vs skipped
• List of applied fixes with file paths and commit hashes
• Any comments deferred to the user (with links)
• Any errors encountered during the process

Notes

• Follow any additional instructions in $ARGUMENTS
• When in doubt about a reviewer's intent, ask the user rather than guessing — misinterpreting feedback wastes more time than a quick clarification

Integration

• upstream: ywc-create-pr (a PR must exist before review comments can be handled); ywc-sequential-executor / ywc-parallel-executor (executors that generate the PRs being reviewed)
• downstream: None — this skill closes the PR review loop; subsequent work is driven by the reviewer's approval or further comments