Fix And Reverify

---
name: fix-and-reverify
description: >
  Fix what a review found, then review the fix. Runs review and fix in rounds: triage findings
  by severity and confidence, fix the ones that clear the gate, confirm the tree still builds
  and its tests still pass, then attack the fix itself rather than the whole change again.
  Fixes automatically by default and reports anything with a real trade-off; `--interactive`
  asks instead.
when_to_use: >
  After implementing a change, after a review has produced findings, or on "fix the findings",
  "address what the reviewer raised", "close out the review loop".
license: MIT
compatibility: Claude Code, Codex, OpenCode, Pi
allowed-tools: Bash Read Write Edit Glob Grep Task Skill AskUserQuestion
argument-hint: "[--interactive] [--mode <simple|standard|deep>] [--rounds <N>] [--base <branch>] [--no-snapshot] [instructions]"
metadata:
  author: edloidas
---

# Fix and Reverify

## The premise

**A fix is unreviewed code.** The reviewer that found the bug never saw the patch, and the agent
that wrote the patch is the one that wrote the bug. So the loop is: review finds, you fix, the next
round reviews the fix. Not the whole change again — the fix.

That is what makes rounds affordable rather than exponential. Round 1 attacks a branch; round 3
attacks eleven lines. The scope shrinks every round, and so does the cost.

This skill **fixes and verifies**. Finding is `/changes-review`'s job and this skill calls it; the
only other skill it invokes is `live-probe`, to re-observe a behavioral finding after a fix. Both
call sites say what to do when the skill is missing. Everything downstream of the loop (committing,
squashing, pushing, publishing) belongs to whoever called it.

Two failure modes it exists to prevent:

- **Fixing everything a reviewer said.** Reviewers over-report and over-rate. A low-confidence
  minor finding costs more to fix — the patch, the risk the patch carries, the round of re-review it
  earns — than to carry as one line in a report.
- **Assuming a fix worked.** "Fixed" is a claim about code nobody has read. Re-review is what turns
  it into a fact.

## Arguments

| Argument | Description |
| -------- | ----------- |
| _(none)_ | Fix automatically, report what needs a decision. Uses findings already in context; reviews first when there are none |
| `--interactive` | Ask before anything with a real trade-off, and before dismissing a finding |
| `--mode <simple\|standard\|deep>` | Review depth for round 1 only. Later rounds follow the ladder |
| `--rounds <N>` | Lower the round cap (default and maximum 5) |
| `--base <branch>` | Round-1 scope, passed straight through to the review |
| `--no-snapshot` | Create no snapshot commits. Later rounds then re-review the whole change |
| `$ARGUMENTS` | Focusing instructions — `only criticals`, `skip the intent findings`, `ignore the test files` |

Default is unattended. Nothing waits for a human unless `--interactive` says so.

## Asking the User

Every question in this skill is written as `AskUserQuestion` options. Use that tool where
the host offers it, or the host's nearest structured-choice equivalent. Where the host has
neither, ask the same question in normal chat as a numbered list of 2–5 options —
recommended first, one short line of description each — and wait for the user to reply
with a number.

Done means: a **Phase 6** stop condition holds and the report is printed, or a gate is waiting on
the user.

## Phase 0: Where to start

A bare invocation looks at what just happened before it spends a review.

| What is in context | Start at |
| ------------------ | -------- |
| A review already produced findings — a change review, triaged PR feedback, an audit report | Phase 2, with those findings. Round 1's review is already spent |
| No findings | Phase 1 |
| Findings, but the tree changed after they were produced | Phase 1. Findings older than the code are addressed to a diff that no longer exists |

**Never re-run a review whose findings are already sitting in context.** It is the most expensive
thing this skill can do and it buys nothing.

Findings from a source other than a change review arrive on someone else's scale, or on none. Put
them on this one before the gate — a rating from another scale is not this gate's rating:

- **Confidence.** A finding that named the failing input and what it produces is high; one that read
  the code and found the story plausible is medium; a comment asking whether something might be a
  problem is low.
- **Severity.** Collapse whatever bands the source used onto critical, moderate, and minor by what
  the failure costs and who can reach it. An audit's `informational` band is minor; a PR comment
  carries no severity at all until you assign one.

## Phase 1: Review

Invoke `/changes-review`. Pass the scope, the issue when one resolved, and the mode from the ladder.

| Round | Mode | Scope |
| ----- | ---- | ----- |
| 1 | `--mode` if the caller passed one, otherwise the review's own default | The change |
| 2 | `standard` | The round-1 fix |
| 3–5 | `simple` | The previous round's fix |

When the review returns, print one line: `Round <N> <mode>: <count> findings`.

Round 2 still has enough new surface to be worth several reviewers. By round 3 the diff is a
handful of lines and a full run spends most of its budget re-deriving a spec that settled two rounds
ago — so late rounds go `simple`, and reachability is the lens that still changes the outcome.
Round 1 is the only one a caller should override: a workflow that wants a whole branch attacked hard
asks for `deep` there.

**Pass no reasoning.** `changes-review` owns that rule — it dispatches every reviewer **cold to
the implementer's reasoning** — and this skill is subject to it twice over: it *is* the implementer
by round 2, and a reviewer cannot detect that reasoning leaked into its prompt.

### Snapshots

Scoping a round to the fix needs a boundary. Before applying a round's fixes, commit the tree as a
snapshot:

```bash
git add -A && git commit -m "wip: fix-and-reverify round <N> snapshot"
```

Fixes then land on top, uncommitted, and the next round reviews the uncommitted diff — exactly the
fix and nothing else.

Snapshots are scaffolding, not history. Report their shas and leave squashing to the caller; do not
squash, amend, or push them yourself. Two constraints come with them:

- A snapshot commits the **entire** working tree. Before the first one, name any file it would
  include that is outside the review scope — unrelated work gets committed with it, and the caller
  needs to know that before it happens.
- `--no-snapshot` gives up scoped rounds. Later rounds re-review the whole change, which means every
  finding already accepted comes back and every round costs the same as the first. Say so in the
  report.

## Phase 2: Triage

The gate. Both dials have to clear it before a single file is edited. When the gate has been
applied to every finding, print one line: `Triage: <F> to fix, <R> reported, <H> held, <D> dropped`.

| | High confidence | Medium | Low |
| --- | --- | --- | --- |
| **Critical** | Fix | Fix | Hold for re-check |
| **Moderate** | Fix | Fix | Hold for re-check |
| **Minor** | Report — fix only if purely stylistic | Report | Drop, named once |

Why a gate at all: below it, carrying beats fixing. A fix costs the patch, the risk the patch
carries, and the round of re-review it earns. A carried finding costs one line. That trade only
inverts for a finding both consequential and credible.

- **Purely stylistic** is the one minor exception — a stray import, a misleading name, a formatting
  slip. Free to fix, and carrying it costs more than the patch. It does not extend to any minor
  finding with behavior attached.
- **Decisions** — findings a review marked as a question rather than a defect — are never fixed at
  any severity. They need an answer. Report them.
- **Held** findings go to Phase 5, not to the bin. Low confidence means unsettled, not wrong.

### Routine or escalated

Clearing the gate says the finding is worth fixing. It does not say the fix is yours to choose.

| Routine — fix it | Escalated — report it |
| ---------------- | --------------------- |
| Bug with one clear correct behavior | Multiple valid approaches with real trade-offs |
| Missing error handling, obvious approach | Breaking or public API change |
| Code not matching a documented pattern | Security-sensitive change |
| Test fix, assertion update | Ambiguous requirement, unclear right fix |
| Single-file, localized change | Cross-cutting change across systems |
| Removing dead code or an unused import | Architectural decision |

**Escalated findings are reported, never fixed** in the default mode. An unattended run must not
land an architectural decision nobody chose. Under `--interactive` they become questions instead.

When the classification is genuinely unclear, escalate. Getting it wrong that way costs a line in a
report; getting it wrong the other way costs a decision the caller never made.

## Phase 3: Fix

Fixes run in subagents. The orchestrator holds the ledger; it does not hold file contents. That is
what keeps a five-round loop inside one context.

Dispatch one fixer per file-cluster of findings that cleared the gate, giving it those findings
**in the reviewer's own wording**, the files named in them, and nothing else. It returns what it changed in a line or two, plus anything it could
not fix and why. The prompt is `references/fix-agent-prompt.md`.

- **Two fixers must never share a file.** That is what the clustering above is for — concurrent
  edits to one file lose work silently.
- A finding a review reported as one root cause with several symptoms is **one** fixer. Splitting it
  produces three patches that each treat a symptom.
- **No scope expansion.** A fixer that spots a second bug reports it; it does not fix it. That
  report enters the ledger as a finding and faces the gate in the next round like anything else.
- Where the host has no subagent facility, apply the same prompts inline, one at a time, in the same
  clustering order, and say so in the report.

Before dispatching, print one line: `Fixing <F> findings in <C> fixers across <N> files`.

## Phase 4: Verify the fix

Before the next review round, the tree has to build and its tests have to pass — or at minimum,
everything the change touches. A review round spent on code that does not compile is a wasted round.

1. **Find the project's own checks** — package manifest scripts, a makefile, a task runner, the CI
   workflow. Never invent a command the repo does not define. When they have run, print one line:
   `Checks: <command>, <command> — green` (or `red: <the failing check>`).
2. **Establish the baseline first.** Checks already red before any fix are not the fix's fault.
   Record that and judge the fixes only against *new* failures.
3. **Prefer the narrow checks** — a type check plus the tests covering the touched files. Run the
   full suite when it is fast enough to be free.
4. **Green → next round.**
5. **Red → the fix is the suspect.** Find the cause and correct it, then re-run only the check that
   failed.
6. **Red twice on the same check → revert that fix.** The snapshot is the revert point. Record it as
   reported-not-fixed with the failing output and move on. **Never fix around a failing check** —
   that trades one known defect for an unknown one.

**Green checks are not evidence the symptom is gone.** They prove the fix did not break anything the
project already tests, which is a different claim. Where a finding was about observable output —
layout, rendering, wire format, exit code, timing, log content, a golden result — re-observe it the
way it was demonstrated: invoke `live-probe` with the original claim, same rung, and compare the
artifact against the one that established the finding. Where the host cannot chain skills, follow the
same method inline. Three outcomes:

- **The symptom is gone** → `fixed`.
- **The symptom reproduces** → treat it as a red check: correct the cause and re-observe, and on a
  second reproduction revert the fix and record it as `reverted` with the artifact. Never widen a
  test to cover a symptom the fix failed to remove.
- **Nothing could be observed** → `held`, not `fixed`. The absence is named in the report.

A repo with no checks at all: say so in the report and do not manufacture a test suite to fill the
gap. That is scope expansion wearing a safety vest. The same holds when nothing in the project can
be run and observed — the finding stays `held` with the absence named.

## Phase 5: Carry the ledger

Reviewers in the next round are blind to this one, by design. Every finding you consciously did not
fix will be found again, and recognizing it is your job, not theirs.

Every finding ends each round in exactly one state:

| State | Meaning | Next round |
| ----- | ------- | ---------- |
| `fixed` | Patched, checks green | Expect it gone. If it comes back, the fix did not work — that is a live finding, not a duplicate |
| `held` | Unsettled — a low-confidence finding, or a fix whose symptom could not be re-observed | Re-checked (below) |
| `reported` | Cleared the gate but escalated, or minor and carried | Not re-fixed, not re-reported. Recognized and skipped |
| `dismissed` | Re-checked and still could not be demonstrated | Never raised again |
| `reverted` | The fix broke a check twice, or failed to remove the symptom twice | Reported with the output |

### Re-checking held findings

Whenever a round leaves findings in the `held` state, dispatch one subagent per held finding
alongside the next round's review, whose only job is to settle it — demonstrate the failure or
refute it. Prompt: `references/recheck-prompt.md`. These run
in parallel with the review and see only their own finding.

| Verdict | Action |
| ------- | ------ |
| High confidence | Fix it this round, subject to the routine/escalated split |
| Medium | Report it |
| Still low | Dismiss it, named once in the report |

**One re-check per finding.** Two rounds of "maybe" is itself an answer, and a third agent asking
the same question is not going to get a different one.

## Phase 6: Stop

Stop as soon as any of these holds:

- Nothing clears the gate.
- **The round fixed nothing.** There is no new code, so another review round asks a question that
  has already been answered. This is the condition that ends most runs.
- The round cap is reached — 5, or `--rounds`.
- A check is red for a reason the fixes cannot resolve.

Never start a round after one that changed nothing.

A round ends only by meeting one of those conditions or by running the next one. A description of
the round that would come next — "round 3 would attack the eleven-line fix" — is not an ending;
run it. When the run does end, print the report below and nothing after it.

## Interactive mode

`--interactive` turns escalations into questions and asks before a dismissal. It does not ask about
routine fixes, it does not ask about the same finding twice, and it never asks permission to run a
review.

Ask per **Asking the User**, four options at most, every option carrying the reason it might be
right. An unanswered question is not consent — **wait**. The chat form looks like this:

```
**Fix the negative-offset truncation in `parseOffset`, or report it?**

1. **Switch to `Math.floor`** (recommended) — matches the documented behavior for negative
   offsets, one line, no caller affected.
2. **Clamp the input at zero** — sidesteps the negative case, but narrows what callers may pass.
3. **Report it, do not fix** — which behavior is correct is a product decision, not a code one.

**Reply with 1, 2, or 3.**
```

The bold question, the numbered options each with its reason, and the bold line asking for a reply.
A question that does not look like a question gets scrolled past.

## Report

```
## Fix and reverify: 4 fixed, 2 reported, 1 dismissed, 1 reverted · 3 rounds

Rounds: 1 standard (12 findings) → 2 standard (3) → 3 simple (0)
Checks: `pnpm typecheck`, `pnpm test -- src/offset` — green
Snapshots: abc1234, def5678 — squash before committing

### Fixed

- `src/parse/offset.ts:42` — `parseOffset` truncates instead of flooring, so `-1.5` parses as
  `-1` → switched to `Math.floor`
- `src/net/socket.ts:130` — the reconnect timer is never cleared on unmount → cleared in teardown
- `test/offset.test.ts:12` — the assertion pinned the truncating behavior → updated to the floored
  values

### Reported, not fixed

- "The offset parser belongs behind the schema validator, not in front of it" — escalated: two
  valid layerings, and moving it changes the public `parse()` signature. Validate-then-parse
  changes one call site; parse-then-validate keeps the signature and duplicates the range check
- "`offset` and `skip` read as synonyms in the public API" — minor, carried

### Dismissed

- "`clampOffset` can be reached with a NaN" — re-checked in round 2, could not be demonstrated:
  every call site coerces through `Number.parseInt` first

### Reverted

- The `AbortController` guard in `src/net/socket.ts:88` — broke `pnpm test -- src/net` twice:
  `expected 1 reconnect attempt, received 0`
```

Rules for the shape:

- **A finding you did not fix survives in the reviewer's wording**, not in your summary of it. You
  are reporting your own decision not to act; paraphrasing the claim is how a rejected finding
  quietly becomes a weaker one.
- Every ledger state with entries gets a section. Drop the empty ones from the heading counts and
  from the body — an empty `### Dismissed` reads as padding.
- Name the checks that ran. "Verified" without a command is a claim, not evidence.
- Nothing is silently swallowed. A finding that appears nowhere in this report is a bug in the run.

When the first review comes back clean:

```
## Fix and reverify: nothing to fix

Scope: <files, lines, base> · reviewed at <mode>
```

Then stop. Do not commit the fixes, do not squash the snapshots, do not push, and do not start a
round the stop rules already ended.

## Error handling

| Situation | Action |
| --------- | ------ |
| The review skill is unavailable | With findings in context, fix them and report that no round could be verified. With none, stop |
| No findings in context and the review returns none | Print `Nothing to fix.` and stop |
| `live-probe` unavailable, or nothing runnable | Re-observe inline per Phase 4; failing that, `held` with the absence named |
| Tree holds work unrelated to the review scope | Name those files before the first snapshot, then continue |
| Snapshot fails — no repository, detached head, hooks reject it | Continue with `--no-snapshot` semantics and say so |
| Checks red before any fix | Record the baseline, judge fixes against new failures only |
| The same check fails twice after a fix | Revert that fix, report it with the output |
| A fixer fails or returns nothing usable | Report the finding as not fixed, with the reason |
| A fix touches a file no finding named | Treat it as scope expansion: revert the extra edit, keep the fix |
| Host has no subagents | Run the prompts inline and sequentially, say so in the report |
| Host has no question tool under `--interactive` | Ask in chat in the shape above and wait |
| Round cap reached with findings still open | Report them. Do not start another round |