name: pr-review description: Address review comments and CI failures for the current branch's PR
Constraints
- Key commands:
gh pr list --head <BRANCH>·gh pr view <PR> --comments·gh pr checks <PR>·gh run view <RUN_ID> --log-failed - Workflow fixes cannot be verified locally — the fix is only confirmed once the remote CI re-runs and passes
Resolving Review Threads
After fixing a P1/P2 comment, mark its thread as resolved on GitHub using the GraphQL mutation:
GH_PAGER= gh api graphql -f query='mutation { resolveReviewThread(input: { threadId: "PRRT_xxxx" }) { thread { isResolved } } }'
To get thread IDs with their status and a brief description:
GH_PAGER= gh api graphql -f query='
{
repository(owner: "OWNER", name: "REPO") {
pullRequest(number: PR_NUMBER) {
reviewThreads(first: 50) {
nodes {
id
isResolved
isOutdated
path
line
comments(first: 1) { nodes { body } }
}
}
}
}
}' | python3 -c "
import json,sys
data = json.load(sys.stdin)
for t in data['data']['repository']['pullRequest']['reviewThreads']['nodes']:
body = t['comments']['nodes'][0]['body'][:100].replace('\n',' ') if t['comments']['nodes'] else ''
print(f\"ID={t['id']}, resolved={t['isResolved']}, outdated={t['isOutdated']}, {t['path']}:{t['line']}\")
print(f\" >> {body}\")
"
Resolve threads that are outdated after re-checking whether the concern is already addressed or clearly superseded by the new code. GitHub can mark a thread outdated because the diff moved, not only because the issue was fixed.
Review Restraint Policy
Before acting on any review comment or suggestion, classify it by importance:
| Level | Criteria | Action |
|---|---|---|
| P1 — Must Fix | Bug, security issue, broken behavior, API contract violation, CI failure | Fix immediately |
| P2 — Should Fix | Correctness risk, meaningful maintainability improvement, clear code smell with real impact | Fix with brief justification |
| P3 — Conditional | Style preference, minor naming, "could be cleaner" | Fix if it aligns with best practices and the change is safe + trivial; defer if complex or has any potential functional impact |
| P4 — Reject | Contradicts project conventions, introduces unnecessary complexity, or is factually wrong | Reject with explanation |
Rules:
- For P3, apply this two-question test before touching the code:
- Best practice? — Does the change follow language/framework conventions (e.g. prefer imports over FQNs, use existing imports, standard patterns)?
- Safe & trivial? — Is the diff mechanical with zero risk of behavioral change and low effort?
- Both yes → fix it silently, mark thread resolved.
- Either no → do NOT modify code; record in the summary table and let the user decide.
- Do not add comments to code unless the comment explains non-obvious logic that is truly necessary. Never add comments just to acknowledge a review suggestion was applied.
- When the two-question test is ambiguous, prefer deferring rather than guessing.
Procedure
- Locate PR — get PR number for current branch
- Fix CI failures — for each failing check:
- Fetch logs:
gh run view <RUN_ID> --log-failed - Workflow issue (wrong config, missing step, bad path):
- Fix
.github/workflows/directly - Commit & push the workflow change
- Wait for the re-triggered run to complete: poll with
GH_PAGER= gh pr checks <PR>(orGH_PAGER= gh run watch <RUN_ID>) until the affected check finishes - If it passes → continue to the next failing check
- If it still fails → fetch new logs (
gh run view <NEW_RUN_ID> --log-failed) and repeat from step i
- Fix
- Code issue, non-breaking: fix source code directly
- Code issue, breaking change required: stop and report to developer for a decision
- Fetch logs:
- Address review comments — apply the Review Restraint Policy to each comment:
- P1/P2: implement the fix, then immediately resolve the thread using the GraphQL mutation above
- Outdated threads: re-check whether the concern is already addressed or clearly superseded; only then resolve the thread
- P3: apply the two-question test — fix + resolve if both answers are yes; otherwise record in summary and defer to user
- P4: record rejection reason in summary
- Commit & push — single commit covering all non-workflow fixes (workflow fixes are pushed incrementally during step 2)
- Final verification — once all fixes are applied, confirm every check is green:
GH_PAGER= gh pr checks <PR>
Output
- Per CI failure: root cause and resolution (or escalation reason)
- After all fixes: summary of check statuses confirming all green
- Review summary table — produced at the end of every review session:
| # | Source (comment / CI) | Issue description | Priority | Action taken | Reason if not fixed |
|---|-----------------------|-------------------|----------|--------------|---------------------|
| 1 | Reviewer @xxx | use import instead of FQN | P3 | Fixed | Best practice + trivial mechanical change |
| 2 | Reviewer @xxx | rename internal var foo→bar | P3 | Not fixed | Non-standard opinion, no best-practice backing — deferred to user |
| 3 | CI: lint | null-check missing | P1 | Fixed | — |
All P3/P4 items that were not fixed must appear in this table with a clear reason, so the user can make an informed decision.