/fix-pr-review: Triage, Fix, and Resolve PR Review Comments
Takes a PR review from CodeRabbit, /review-pr, or pasted text. It triages each finding, applies validated fixes, runs /done, then replies to and resolves the GitHub conversations in one flow.
Use AskUserQuestion only when the run still needs a user decision: branch safety, stash confirmation, contested-item confirmation, --interactive per-fix confirmations, type-check failure triage, and post-completion next actions. Invoking /fix-pr-review or imperatively asking to fix review findings authorizes execution of the validated FIX plan. Do not ask for separate plan approval. Mark every required option as concrete and considered, strongest first with "(Recommended)".
Quick Reference
Phase Overview
| Phase | What | Key Output |
|---|---|---|
| 1 | Prereqs, input detection, branch safety, baseline type-check | BASE_SHA, repo_map, baseline_errors |
| 2 | Fetch review data from GitHub or local file | Unified Comment[] array |
| 3 | Triage subagent: classify each finding via R-rubric | Triage plan (FIX/DISMISS/DEFER/DISAGREE/NEEDS-INPUT) |
| 4 | Plan execution gate: validate + honor invocation intent | Executable plan |
| 5 | Execute fixes: sequential edits + per-file narrow type-check | Modified files, fix_status per item |
| 5.5 | Convergence subagent: class completeness, inverse risk, new siblings | Per-fix verdicts; missing sites applied |
| 6 | /done acceptance verification scoped to the fix diff, no commit or publish handoff |
Verified fix diff, done_verified_snapshot |
| 7 | Reply + resolve on GitHub (skipped for local files) | Threads resolved |
| 8 | Finalize: settle NEEDS-INPUT, restore stash, report, suppressions write, next actions | Final report |
R-Rubric Summary (Phase 3 STEP 4; first match wins)
| Rule | Classification | Requires |
|---|---|---|
| R1 | DISMISS: self-contradictory/wrong | no extra field |
| R2 | DISMISS: hallucinated file:line | Dead-link re-anchor failed |
| R4 | DISMISS: already fixed | prior_commit_sha |
| R5 | DISMISS: contradicts CLAUDE.md | claude_md_quote |
| R3 | DISMISS: pure style/naming | Not reusability-flagged |
| R10 | NEEDS-INPUT: fix would change base-branch observable behavior | why_unclear in the R10 shape |
| R6 | FIX: bug/security/perf/correctness/reusability | fix_plan ≥30 chars, change_class, test_scenario, inverse_risk, class_completeness (with verdict) |
| R7 | DEFER: valid but out of scope | Tracking reference |
| R8 | DISAGREE: legitimate technical disagreement | disagree_rationale |
| R9 | NEEDS-INPUT: ambiguous/needs user knowledge | why_unclear |
R4/R5 sit above R3 by design, not by accident, and R10 sits above R6 so a technically correct finding never reaches the fix loop on the strength of being correct. The full rubric and the reason for that order live in references/triage-rubric.md.
Key Cross-References
- R-rubric definition:
references/triage-rubric.md, loaded by the triage subagent at Phase 3 STEP 4, not by main - Plan validation rules: Phase 4 (validates fields required by R-rubric)
- Fix execution routing: Phase 5 (executes R6 FIX items)
- Convergence: Phase 5.5 (consumes
class_completeness+inverse_riskfrom the plan; feedsfix_statusand the per-fixconvergence:report line) - Reply validator:
references/github-reply-resolve.mdStep 7b (format rules for GitHub replies) - Report grouping: Phase 8 (groups by R-classification)
- Suppressions: Phase 3 "Load review suppressions" (main agent reads, before dispatch) +
references/triage-prompt.md(subagent applies them before classifying) + Phase 8 (write)
Reference files
Each carries its firing condition in the pointer at the point of use. Load it there, not up front.
references/fetch-review-data.md: per-input-type GraphQL/REST fetch, CodeRabbit review-body anatomy,Commentschema. Loaded by main in Phase 2, at the GitHub fetch step, and again at theComment-schema normalisation step if the local-file path meant it was not read there.references/triage-prompt.md: the whole Phase 3 subagent prompt (STEP 0 → STEP 6 + output format). Read by main in Phase 3, placeholder-substituted, passed verbatim.references/triage-rubric.md: R1-R10 detail, NEEDS-INPUT calibration,change_classworked examples, reply formats. Loaded by the triage SUBAGENT at STEP 4.references/github-reply-resolve.md: Steps 7a-7d posting/resolving mechanics. Loaded by main in Phase 7; never loaded for local-file input.references/final-report.md: the Phase 8 report template and its rendering rules. Loaded by main in Phase 8 before printing.references/branch-safety.md: repo plus branch landing with detached-HEAD and different-branch questions. Loaded by main in Phase 1 for GitHub inputs; skipped for local files.references/per-fix-loop.md: per-fix loop, interactive confirmations, narrow type-check plus baseline compare, retry/skip/abort. Loaded by main at the Phase 5 loop.references/needs-input-triage.md: NEEDS-INPUT settle, per-item triage, Phase 8 fix plus verify, preflight before GitHub replies. Loaded by main in Phase 8 step 1, only when the count is nonzero.
One reference is not bundled here: ${CLAUDE_SKILL_DIR}/../review-pr/references/repo-map.md holds the repo_map_files / repo_map_exports shell, the one copy this skill shares with /review-pr and /harden-plan. Loaded by main in Phase 1 when packages/ or apps/ exists.
Usage
/fix-pr-review https://github.com/owner/repo/pull/123
/fix-pr-review https://github.com/owner/repo/pull/123#pullrequestreview-4089716169
/fix-pr-review https://github.com/owner/repo/pull/123#discussion_r3064352825
/fix-pr-review ./review.md # local output from /review-pr
/fix-pr-review /tmp/review-pr-123-findings.md # local review export
/fix-pr-review # no arg → ask user to paste
Optional flags:
--dry-run: stop after plan display, don't execute--interactive: ask before applying each FIX item--all-nitpicks: full-triage nitpicks instead of default-dismiss
Who gets triaged
Every reviewer's comments go through the same triage: CodeRabbit, humans, and other bots alike. The "CodeRabbit" framing throughout reflects the most common use case.
Phase 1: Prereqs + input detection + branch safety (main)
Prereq checks
command -v gh >/dev/null 2>&1 || { echo "Install gh CLI: https://cli.github.com"; exit 1; }
git rev-parse --is-inside-work-tree >/dev/null 2>&1 || { echo "Not inside a git repo. cd into your clone first."; exit 1; }
gh auth status 2>&1 | grep -q "Logged in" || { echo "Run 'gh auth login' first"; exit 1; }
Detect input type from the argument
- Contains
#pullrequestreview-<id>→ review URL - Contains
#discussion_r<id>→ discussion URL (single comment + thread) - Starts with
.//// ends with.md→ local file - Matches
https://github.com/<owner>/<repo>/pull/<num>with no fragment → PR URL - Empty → ask user to paste content or provide URL
Record execution authorization
Set EXECUTION_AUTHORIZED=true when the original request explicitly invokes /fix-pr-review or imperatively asks to fix, address, apply, handle, or resolve review findings. A bare URL or a read-only request to inspect, explain, review, or triage sets it to false. Preserve the matching request text as execution_authorization_evidence; Phase 4 may replace it with an explicit execution choice, and Phase 7 passes the final evidence to preflight-mutations.
Ensure correct repo + branch (GitHub inputs only)
Parse owner, repo, and number from the URL and land on the PR branch per ${CLAUDE_SKILL_DIR}/references/branch-safety.md. Load it now. If the tree is mid-merge, mid-rebase, or mid-cherry-pick, stop and hand conflict resolution to resolving-merge-conflicts before touching anything.
Auto-stash uncommitted work (branch safety)
This step runs for every input type, GitHub or local file. Fixes never land on top of unstashed WIP.
git status --porcelain
If non-empty, use AskUserQuestion:
Question: header: "Stash" text: "Uncommitted changes detected. Auto-stash before applying fixes? Contents will be restored at the end." options: - label: "Auto-stash" description: "Stash changes now. They'll be restored when the run completes" - label: "Abort" description: "Stop. I'll commit or stash my work manually first"
On "Auto-stash", run git stash push -u -m "fix-pr-review auto-stash $(date +%s)". Only on success, record STASH_OID=$(git rev-parse -q --verify refs/stash) and set STASH_PUSHED=true, then confirm git status --porcelain is empty. A failed push records nothing and aborts the run instead of marking a stash that was never created. If the run aborts, the user can find the work in git stash list as fix-pr-review auto-stash <timestamp>.
On "Abort", print "Commit or stash your uncommitted work first." and exit.
Stash-restore guard. Every restore in this run, here and at every early exit, runs only when STASH_PUSHED=true: apply the recorded OID by OID only when the top of stack equals it (git stash apply "$STASH_OID", never a bare pop), drop only after a clean apply with the top still equal, and on any mismatch leave every entry untouched, record stash_restored: foreign-top, print stash_restored: foreign-top, stash left untouched, and continue without applying or dropping.
Compute the merge base (for already-fixed detection later)
BASE_SHA=$(git merge-base "origin/$(gh pr view <url> --json baseRefName -q .baseRefName)" HEAD)
Stash as BASE_SHA for use in Phase 3's already-fixed checks.
Pre-fix type-check baseline (what the narrow type-check compares against)
Run one baseline type-check before Phase 5, capture the set of files already failing:
bun turbo run check-types 2>&1 | tee /tmp/fix-pr-review-baseline-$$.log
Define one parser for the whole run. For each diagnostic, key its file and build the identity <diagnostic code> + <normalized message> after removing file position, line, and column data; retain duplicate identities as a multiset. Parse the Phase 1 output into baseline_errors[path] with this parser. Phase 5 and every Phase 8 baseline path must use the same parser and identity normalization. If type-check tooling is missing (no turbo, no tsc), skip the baseline and mark the narrow type-check as skipped for all fixes.
Compute shared-package repo map (for reusability-aware classification)
Inventory shared packages and apps so the Phase 3 classifier can cross-check comments about reuse and extraction against what already exists. Scan both packages/ and apps/. Cross-app helper duplication, for example apps/backend/src/modules/v1/feature-a/helpers.ts versus feature-b/helpers.ts, is common in NestJS-style monorepos and stays invisible to a packages-only scan.
Load ${CLAUDE_SKILL_DIR}/../review-pr/references/repo-map.md and run its Local mode block, the one copy of this shell, shared with /review-pr and /harden-plan. It carries the bash -c wrapping the globs need to survive zsh, and caps each half at 500 lines with the truncation marked. Load it when packages/ or apps/ exists; when neither does there is nothing to run and the fallback below applies.
Stash both outputs as repo_map_files and repo_map_exports for the Phase 3 subagent prompt. If neither packages/ nor apps/ exists (non-monorepo), set both to N/A (not a monorepo) and flag IS_MONOREPO=false. The classifier prompt uses this to reroute greps to src/ and the repo root.
Phase 2: Fetch review data (main)
Dual-path input for /review-pr findings
/review-pr now posts findings as individual inline comments, one per finding on a specific code line. These create standard PullRequestReviewThreads on GitHub, identical to CodeRabbit threads. The existing GraphQL fetch below handles them with zero special parsing.
Manually exported or legacy /review-pr findings files use the existing local-file path. Phase 7 skips GitHub operations for those inputs.
Fetch from GitHub (PR URL / review URL / discussion URL)
Phase 1 detected exactly one of these three input types. Load ${CLAUDE_SKILL_DIR}/references/fetch-review-data.md now and run only that type's section. It holds the paginated GraphQL reviewThreads query and its isResolved filter, the /pulls/<num>/reviews/<review_id> and /pulls/comments/<comment_id> REST endpoints, the CodeRabbit review-body anatomy, and the parse for the 🤖 Prompt for all review comments with AI agents block, the only place nitpicks appear, since they never get inline threads.
A fetch that errors surfaces the error and exits, so triage never runs on a partial comment set. That covers a GraphQL rate limit, a 404, a private repo, and a per-page failure inside the pagination loop. For 404, print Couldn't access PR. Check repo access and run 'gh auth refresh -s repo'.
For local files (./review.md, /tmp/review-pr-*-findings.md, etc.)
Parse the /review-pr output format. Extract findings from the ## Findings section, preserving Severity / File / Category / Issue / Why it matters / Suggested fix plus Rule-class / Enclosing-symbol / Inverse risk / Class-sites when present. /review-pr emits these per finding and dropping them forces this skill to re-derive work the reviewer already did.
Seed, don't re-derive: when Inverse risk: is present, seed STEP 5's inverse_risk from it and VERIFY it against the code (confirm the named failure mode is real and still applies) rather than deriving a new one from scratch. When Class-sites: <A>/<N> is present, seed STEP 1.5's class_completeness with those N sites and verify the count against your own search. Re-run the search only to catch sites the reviewer missed, not to rebuild the list. Rule-class and Enclosing-symbol seed the class sweep's signature. If a seeded value contradicts what you read in the code, the code wins. Record the discrepancy in the field.
Severity mapping: /review-pr uses Critical | Serious | Moderate | Minor while CodeRabbit uses Critical | Major | Minor | Refactor | Nitpick. Both are valid. Normalize to the internal Comment schema which accepts either convention. Map for triage priority: Critical = highest, Serious/Major = high, Moderate = medium, Minor/Refactor = low, Nitpick = default-dismiss.
Normalize to internal Comment list
Every input path ends here. The Comment schema, the exact field names Phases 3-8 read, is defined in references/fetch-review-data.md. Load that file now if you took the local-file path and have not read it yet.
Short-circuit cases
- Empty list (all threads resolved, local file has no findings): print
Nothing to triage. No unresolved comments found.→ restore the stash under the guard → exit 0. - Only nitpicks remain AND
--all-nitpicksnot set: printOnly nitpicks found (N). Pass --all-nitpicks to triage them, or ignore.→ restore the stash under the guard → exit 0.
Phase 3: Triage subagent (general-purpose)
Load review suppressions (main agent, before dispatch)
Before dispatching the subagent, load .claude/review-suppressions.yml at the base revision, never the worktree: a checked-out PR must not suppress its own triage. For GitHub inputs, read it at the pinned base OID: git show "$PINNED_BASE_OID:.claude/review-suppressions.yml" locally (fetch origin once when the objects are absent), or gh api repos/<owner>/<repo>/contents/.claude/review-suppressions.yml?ref=$PINNED_BASE_OID in cross-repo mode. For local-file inputs without a PR, HEAD itself may be the change under triage. Resolve the trusted base as the one candidate merge-base that contains every other candidate's: collect one base per mainline, preferring each remote alias over its local counterpart since a local mainline may itself be the change under triage, and take the base only when it is the unique one containing all the rest. Ties, incomparable pairs, and empty sets disable suppressions.
BASES=$(for remote in origin/main origin/master origin/develop; do
bare=${remote#origin/}
if git rev-parse --verify -q "$remote" >/dev/null 2>&1; then ref=$remote
elif git rev-parse --verify -q "$bare" >/dev/null 2>&1; then ref=$bare
else continue
fi
git merge-base HEAD "$ref" 2>/dev/null || continue
done | sort -u)
RESULT=""
if [ -n "$BASES" ]; then RESULT=$(printf '%s\n' "$BASES" | while IFS= read -r b; do
[ -n "$b" ] || continue
BAD=$(printf '%s\n' "$BASES" | grep -vx "$b" | while IFS= read -r o; do
git merge-base --is-ancestor "$o" "$b" 2>/dev/null || printf 'bad\n'
done)
[ -z "$BAD" ] && printf 'WINNER %s\n' "$b"
done); fi
Read the file at the winning commit and log the source. Count winners in a way that stays successful on empty input, since grep -c exits 1 when it counts zero:
WINNERS=$(printf '%s\n' "$RESULT" | grep -c WINNER || true)
Take it only when WINNERS equals 1 and the winner is not HEAD itself: a winner equal to HEAD means no older base exists to trust. Otherwise set SUPPRESSIONS = "": triaging without a policy adds noise, trusting the reviewed change hides findings. An empty candidate set emits no winner lines at all. When the base has no such file, set SUPPRESSIONS = "" and log that a PR-added file was ignored.
Pass loaded suppressions into the subagent prompt as a ## Review suppressions section (same approach as CLAUDE.md content, PR diff, and repo maps; main agent fetches, subagent receives as context).
Dispatch
Dispatch one general-purpose subagent with Read, Grep, and Bash tools. The triage plan comes from this subagent alone. If it fails outright, abort the run and say so. Classifying inline skips the grounding and class-sweep passes the whole plan is built on.
Important: The Bash allowlist (git log/diff/blame/show/merge-base/rev-parse, grep, rg) is a prompt-level instruction. Claude Code's Agent tool doesn't sandbox Bash per-command. The subagent is trusted not to run other commands, not mechanically prevented from doing so.
Prompt template
The whole prompt is references/triage-prompt.md. Read it now, substitute its <...> placeholders (<SKILL_DIR> = this skill's absolute directory, plus the Phase 1 context values, the git diff <BASE_SHA>...HEAD output, the repo maps, SUPPRESSIONS, and the Phase 2 Comment[] array), and pass the result to the subagent VERBATIM. It is the string the subagent runs, not instructions for you to follow, summarise, or restate inline.
Its STEP 4 sends the subagent to references/triage-rubric.md on its own. You do not read that file; the R-Rubric Summary table above is the main-agent view. What comes back is what Phase 4 validates: per FIX the prompt emits fix_plan, change_class, test_scenario, inverse_risk: and class_completeness:; reusability_context: rides on every item, not just FIX.
Phase 4: Plan execution gate (main)
Validates against the R-rubric: summary table above, full detail in references/triage-rubric.md. Required fields per classification are defined there.
Plan validation (before display)
Before anything is shown to the user, mechanically validate the classifier's output:
- Every DISMISS with
rubric: R5MUST have non-emptyclaude_md_quote. - Every DISMISS with
rubric: R4MUST have non-emptyprior_commit_sha. - Every DISAGREE MUST have non-empty
disagree_rationale(and it MUST NOT be a pure style preference; check for keywords like "prefer", "cleaner", "nicer" without a concrete counter-argument). - Every FIX MUST have
fix_planlength >= 30 characters. - Every FIX MUST have
change_classset to exactlyhardeningorlogic-change(the calibration the classifier applied is inreferences/triage-rubric.md; this check is purely the literal value). - Every FIX MUST have non-empty
test_scenario. Forchange_class: hardening, the value MUST be exactlysmoke test, happy path unchanged. Forchange_class: logic-change, the value MUST be a 1-sentence concrete repro (not just "verify it works"). - Every FIX MUST have non-empty
inverse_riskthat either names a specific failure mode or is exactlynone, pure addition. Hedges fail validation: an empty value, or anything of the shape "could have issues" / "minor risk" / "some risk" / "possible regression" / "none" on its own. A named failure mode says what breaks, where. Phase 5.5 consumes this field; an unnamed risk is unverifiable there. - Every FIX MUST have a
class_completenessblock with a non-emptyverdictstarting with eitherCOMPLETEorINCOMPLETE.INCOMPLETEMUST name the excluded sites and give a reason for each. AnINCOMPLETEverdict with no per-site reason fails validation. - Every NEEDS-INPUT with
rubric: R10MUST have awhy_unclearcarrying all three of itscurrent:,proposed:, andintent evidence:parts, each non-empty. The shape itself lives inreferences/triage-rubric.md.found nothingis a valid evidence value; an empty one is not. - Every item MUST have non-empty
grounding_aandgrounding_b. - Every item MUST carry a
reusability_contextfield, even when it is just{ flagged: false }. That holds for every classification, NEEDS-INPUT included. Phase 7's reply validator branches on it, so a missing field silently disables the reusability gate on that reply.
On validation failure: re-dispatch the classifier with the specific missing fields listed. Max 1 retry. Second failure → abort with the validation errors printed.
Plan display
Print the plan with a header:
# Fix Plan, PR #<num>: <title>
# <N> findings triaged: <F> fix, <D> dismiss, <E> defer, <G> disagree, <I> needs-input, <n> nitpicks
Highlight DISMISS-by-CLAUDE.md prominently
If any DISMISS has rubric: R5, print a separate highlighted section BEFORE the main plan:
## ⚠ Dismissed because they contradict CLAUDE.md: review and override if any are exceptions
[D<n>] <file:line>: <comment ask>
CLAUDE.md rule: "<verbatim quote>"
Reply will be: "<reply>"
If this rule has a legitimate exception in this case, select D<n>
in the contested-item confirmation to change it to FIX or NEEDS-INPUT.
Contested-item confirmation (multiSelect)
Contested items are the ones that will post a reply and resolve a thread WITHOUT any code change: every DISMISS, DEFER, and DISAGREE. A wrong classification here silently closes a reviewer's conversation. Confirm the triage before Phase 5/7 can act on it.
Skip this step if there are zero contested items. Otherwise, use AskUserQuestion:
Question: header: "Triage" text: " item(s) will get a reply + thread resolution with no code change. Select any to RECLASSIFY. Unselected items proceed as planned." options: [one option per DISMISS/DEFER/DISAGREE item: "[] file:line: (), <reason, first ~60 chars>"] multiSelect: true
If contested items exceed the option limit, split into multiple multiSelect questions. DISMISS first (the most costly to get wrong).
For each selected item, use a follow-up AskUserQuestion:
Question: header: "Item " text: "file:line, currently : . Reclassify as?" options: - label: "FIX (Recommended)" description: "Treat as a real issue and add it to the FIX list with a fix plan" - label: "NEEDS-INPUT" description: "Park it and post nothing; it surfaces in the final report" - label: "Keep as-is" description: "Keep the original classification, since this was selected by mistake"
On "FIX": re-dispatch the classifier scoped to just this item to produce the full FIX field set: fix_plan, change_class, test_scenario, inverse_risk, class_completeness (class sweep included; a reclassified item has never been swept), and reusability_context (carry the contested item's own value through unless the sweep changed it). Then re-run plan validation on the changed item. Producing a partial field set here fails validation and burns the single retry. On "NEEDS-INPUT": move to NEEDS-INPUT with why_unclear: "user contested the <classification> classification", carrying the item's reusability_context through unchanged. On "Keep as-is": no change. On "Other": treat the freeform text as the reclassification instruction.
Nothing is posted or resolved during this step. Phase 7 remains the only place GitHub is touched, and it acts only on items that survived this confirmation. Resolving a thread is irreversible noise in the reviewer's conversation: a DISMISS, DEFER, or DISAGREE that skipped this gate stays open and unanswered until it has been through it.
Execution
If --dry-run: print the plan, print dry run, not executing, restore the stash under the guard, exit 0.
If EXECUTION_AUTHORIZED=true, proceed directly to Phase 5. The invocation already authorizes execution of every validated FIX item. If --interactive was set, ask for per-item confirmation in Phase 5. It does not add a plan-level confirmation.
Otherwise, use AskUserQuestion:
Question: header: "Execute" text: "The request did not explicitly authorize edits. Execute the validated plan? fixes, dismissals, deferrals, disagrees, needs-input." options: - label: "Execute plan (Recommended)" description: "Apply all FIX items in dependency order" - label: "Cancel" description: "Leave the worktree unchanged and restore any stash"
On "Execute plan": set EXECUTION_AUTHORIZED=true, record the choice as execution_authorization_evidence, and proceed to Phase 5. On "Cancel": restore the stash under the guard if pushed, print cancelled, and exit 0.
Phase 5: Execute fixes (main, sequential)
Executes R6 FIX items from the validated plan (see the R-Rubric Summary table for R6 criteria and Phase 4 for validation).
Dependency resolution
Build an execution order from dependencies: fields with a simple topological sort. On a cycle (A→B→A): abort with Cyclic fix dependencies detected. Correct the dependency fields and rerun /fix-pr-review <original-input>. Restore the stash under the guard. Exit non-zero.
Pre-edit snapshots (revert mechanism; Edit tool has no undo)
Bind Phase 5's state source before the first edit:
| Context | active_snapshot |
active_baseline_errors |
Abort scope |
|---|---|---|---|
| Ordinary Phase 5 | perfix_snapshot[idx] |
Phase 1 baseline_errors |
Current fix |
Phase 8 item <idx> |
phase8_item_snapshot[idx] |
phase8_item_baseline_errors[idx] |
Current item only |
In ordinary Phase 5, before the first Edit touches a file anywhere in the run, cache its full contents or authoritative absence in the run-level abort snapshot:
preedit_snapshot[path] = <full file content from Read>
Immediately before each ordinary fix, capture every declared path's then-current content or authoritative absence in perfix_snapshot[idx]; it includes all earlier landed fixes and becomes active_snapshot for Retry and Skip. After that fix's final successful attempt and before any later edit, derive its exact binary-safe forward patch, including additions and deletions, from perfix_snapshot[idx] to the current declared paths. Freeze that patch in perfix_forward_patch[idx], freeze the exact current content or authoritative absence in perfix_postimage[idx], and initialize perfix_owned_components[idx] with that patch and postimage. Never derive fix ownership later from the aggregate working-tree diff. preedit_snapshot is used only by Abort-all. In Phase 8 context, active_snapshot must already contain every declared path and no nested branch may read from or write to either ordinary snapshot. Restoring from an active snapshot writes back recorded content and removes paths recorded as authoritatively absent. Abort-all alone restores every run-level preedit_snapshot entry; a Phase 8 item restores only phase8_item_files[idx], preserving earlier landed fixes.
Per-fix loop
For each FIX item in topological order, work the loop per ${CLAUDE_SKILL_DIR}/references/per-fix-loop.md. Load it now. It holds the interactive confirmations, the narrow type-check with baseline compare, and the retry, skip, and abort branches, including the symptom-patching rule that sends an item to systematic-debugging instead of spending another retry.
Fix execution tracking
fix_status[idx] = ok | retried_ok | inconclusive | skipped | aborted | type_check_skipped
| reverted_inverse_risk | inverse_risk_applied | partial | restored_failed
landed_fix_statuses = {ok, retried_ok, inconclusive, type_check_skipped}
landed_fix_statuses is the authoritative landed-fix set for later phases and the final report. Phase 5.5 writes reverted_inverse_risk only after a successful restore and inverse_risk_applied when safe removal is unproven; both statuses and partial are non-landed. restored_failed means Phase 8 restored the item's snapshot after a failure and is always non-landed.
Phase 5.5: Convergence (subagent)
Run after all fixes are applied, before the /done pipeline. A run converges when every
fix is class-complete, carries no inverse risk, and spawned no new siblings. Anything
short of that is what the next review round will find.
Dispatch ONE general-purpose subagent. It gets git diff HEAD plus, per fix, the
class_completeness site list and the inverse_risk string. For a Phase 8 remediation
of inverse_risk_applied, also pass phase8_remediation_kind, the complete prior
owned-component ledger, and each component's planned removal or replacement evidence.
It fetches whatever else it needs. Keep it in a subagent: it re-reads files and greps
the repo, and main only needs verdicts.
For each fix below, verify against the working tree, not against the fix plan's claims.
1. CLASS COMPLETENESS: every site the class sweep marked `affected` in
`class_completeness.sites` must actually be changed. A fix that landed on 3 of 4
sites is INCOMPLETE, not done.
(Sites the plan's `verdict` deliberately excluded are not counted as unfixed.)
2. INVERSE RISK: the named failure mode must NOT be present in the applied code.
3. NEW SIBLINGS: did the fix itself introduce a new instance of the pattern it fixes,
or a new branch (error state, empty state, early return) that its siblings have but
this one lacks?
Report per fix, nothing else:
fix: <idx>
class_complete: yes | no, <unfixed site if no>
inverse_risk_present: no | yes, <file:line + one sentence>
new_siblings: none | <file:line + one sentence>
Evidence rules differ per check. "I lack evidence" is not an answer for check 1:
- Check 1 is decided MECHANICALLY by `git diff HEAD`. Each affected site either
appears in the diff or it does not; there is no undecidable state. Report `no`
with the unfixed site whenever a site is absent from the diff.
- Checks 2 and 3 default to `no` / `none` unless you can cite a concrete file:line
in the applied code. Do not speculate, do not re-review the PR, do not report
style issues.
For a Phase 8 removal, replace check 1 with mechanical remediation completeness:
every prior owned component must appear exactly once in the remediation ledger, and
the resulting content must prove that component absent. Do not require the original
affected site to remain in git diff HEAD, and never reapply the rejected fix to make
that site appear. For a Phase 8 replacement, require every prior owned component to
be removed or superseded by the recorded replacement bytes. In both branches, missing
component evidence is class_complete: no; the inverse-risk check requires affirmative
evidence across the complete component ledger rather than its ordinary default.
Handling:
class_complete: no→ apply the missing sites now, then re-verify ONCE. If the second pass still reportsno, stop: markfix_status[idx] = partial, record the still-unfixed sites, and surface them in Phase 8. Do not loop a third time; like the narrow type-check retry (max 2) and the self-heal loop (max 2), this loop is capped. For a Phase 8 remediation, correct only the missing removal or replacement evidence; do not apply an original affected site merely because it is absent from the diff.- Treat each corrective class-completeness or new-sibling edit as part of the fix it completes.
Capture its preimages, then append its exact patch and postimages to that fix's ordered
perfix_owned_components[idx]; also register it as later-edit evidence for every other earlier fix. inverse_risk_present: yes→ the suggestion was wrong; applying it anyway ships a worse defect. In Phase 8 context, restore every declared path fromactive_snapshot, including authoritative absence, and abort only the current item. In ordinary context,preedit_snapshotremains reserved for Abort-all. Use onlyperfix_owned_components[idx]and later-edit evidence to prove ownership; never reconstruct ownership fromgit diff HEADor another aggregate diff. A candidate inverse removes every owned component in reverse order. Apply it only when the frozen evidence proves exclusive non-overlap and the temporary candidate preserves every other later edit. Restore the wholeperfix_snapshot[idx]only when no other later fix or convergence edit touched any declared path and every current declared path equals the final owned component's postimage. Otherwise leave the fix applied, setfix_status[idx] = inverse_risk_applied, recordrevert: not attempted, exclusive ownership unproven, route it to NEEDS-INPUT, and state in Phase 8 that the risky fix remains applied. On an ordinary inverse/snapshot revert, markfix_status[idx] = reverted_inverse_riskand route to NEEDS-INPUT. A Phase 8 active-snapshot restore uses its state-preserving restore rule with fallbackreverted_inverse_risk.new_siblings→ treat as part of the same fix and handle it now.
Record the outcome per fix as convergence[idx]; Phase 8 renders it, plus one converged /
not-converged verdict for the run. If the subagent fails, run the three checks inline and
note convergence checked inline.
Phase 6: /done verification (main)
After all fixes are applied, run /done on the pending fix diff (git diff HEAD). /done is a four-section acceptance workflow, not a fixed three-command pipeline: §1 binds the run and selects the acceptance lanes, §2 verifies each required lane at its boundary, §3 assigns a state to every request item, lane, and evidence facet, and §4 builds the readiness card. Bind the originating request to the validated fix plan and scope every check to the fix diff, not the entire PR.
The Code lane always applies here. It runs /fix-ts-errors to green (catching the cross-file errors a per-file narrow check cannot see, and running the full workspace check at least once), runs /parallel-review over the fix diff and applies its converge-reviews result, runs /simplify including its blocking added-comment scan, and accounts for each fix against the diff. Using the rule /done §1 states, select every other lane the fixes actually touched: UI, documentation, global configuration or skills, external metadata or data, publication or deployment. Record the rest as not-applicable with their exclusion reason.
Do not take /done §4's handoffs. This skill owns publication: git-commit and file-pr are unavailable in Phase 6, and Phase 8's post-completion prompt is the only place a commit or push may start.
Self-heal loop (explicit iteration tracking)
self_heal_iter = 0
done_remaining = []
while self_heal_iter < 2:
findings = run_parallel_review(diff="git diff HEAD")
blockers = [f for f in findings if f.severity in ("Critical", "Serious")]
if not blockers:
break
# Dispatch fresh subagent per blocker with the finding + the diff
for f in blockers:
apply_subagent_corrective_edits(f)
run_fix_ts_errors()
self_heal_iter += 1
# After loop: record whatever blockers remain (if any)
done_remaining = blockers
If done_remaining is non-empty after 2 iterations, record it for the final report and continue to Phase 7. The user sees the remaining issues in Phase 8 and decides there.
Moderate/Minor findings are recorded in done_remaining without self-heal. User decides at commit time.
When every required lane is verified, materialize the exact fix content as the done_verified_snapshot required by git-commit's Verified content snapshot contract. Build it with /done §4's path-scoped snapshot procedure (create_verified_snapshot): read HEAD into an isolated GIT_INDEX_FILE, hash only the declared paths with git hash-object -w, stage each with git update-index --add --cacheinfo <mode>,<blob>,<path>, --force-remove each declared deletion, then git write-tree. The declared-path set here is the union of every landed fix's declared paths; the auto-stashed WIP and any other dirty path stays out of it and out of the object database. Record the snapshot tree and included path manifest while the run-level stash is still untouched. A Phase 8 fix may update only its declared paths in this snapshot after that item's verification lands cleanly; rebuild the tree by reading the prior snapshot instead of HEAD into the isolated index and applying only those verified bytes through the same update-index --cacheinfo mechanism, never by recapturing the live worktree. If no valid done_verified_snapshot exists, Commit and Push are unavailable.
Phase 7: Reply + resolve on GitHub (main)
Reply format rules live in references/github-reply-resolve.md and are referenced by Phase 4 validation. This skill does NOT read or write /review-pr's cache (~/.claude/skills/review-pr/cache/). Thread resolution happens on GitHub. /review-pr's re-review picks up resolved threads via its GraphQL prior-review timeline fetch.
Replying and resolving threads is this phase's entire GitHub footprint. The review's own CHANGES_REQUESTED state stays exactly as CodeRabbit left it. CodeRabbit clears it itself on its next auto-re-review, once the user pushes.
Define has_github_surface[idx] = thread_id != null AND (can_reply OR can_resolve). Initialize every item without that surface to paired not-applicable states, regardless of input source. For ./review.md or any other local-file input, all items meet that condition; initialize them, then skip the rest of this phase. Phase 8 still requires deterministic per-item state for its final report.
Posting mechanics
Load ${CLAUDE_SKILL_DIR}/references/github-reply-resolve.md now. It holds Step 7a (regenerate every FIX reply from the actual post-fix diff, never the Phase 3 reply_placeholder, and which fix_status values are barred from replying at all), Step 7b (the mechanical reply validator: forbidden prefixes, 40-char floor, must-contain patterns, and the reusability_context-gated rule), Step 7c (the addPullRequestReviewThreadReply + resolveReviewThread GraphQL calls), and Step 7d (promoted nitpicks have no thread to close).
Per-item status tracking
gh_status[idx] = {
reply_state: landed | verified-existing | confirmed-absent | reconcile-required | skipped | not-applicable,
reply_err: <error message if any>,
resolve_state: resolved | already-resolved | confirmed-open | reconcile-require
…(truncated)