---
name: ureview
description: Use after verify passes for the future maintainer's audit — sit in the chair of the person who'll touch this code in 3-6 months and ask "what will bite us later?" at the decision level. Surfaces wrong abstractions, load-bearing-but-unobvious shapes, next-change traps, drift from surrounding code; raises a Scope flag if the whole change looks like the wrong call. Dispatches up:reviewer (critical, high-confidence filter), processes findings fairly, fills the task file's `## Conclusion`.
---
# Review
Review's stance: the future maintainer's audit. Sit in the chair of the person who'll touch this code in 3-6 months and ask the headline question — what will bite us later?
The job is to spot the design or structural choice that will force a nasty rewrite when someone next has to extend, migrate, or refactor. Catch the shape you'll regret in 6 months while it's still cheap to change.
Four angles the audit looks for, all under the same headline:
- Wrong abstraction / premature commit — a shape that fits today's case but will break under the N+1 case, forcing the whole thing to be ripped out.
- Load-bearing but unobvious — an implicit invariant, default, or ordering that the next maintainer will violate by accident, then spend days debugging.
- Bites the next change — a check in the wrong layer, a mutable default, an enum that'll silently accept new values; fine now, painful at the next touch.
- Inconsistent with surrounding code — duplicates an existing helper, leaks an abstraction, drifts naming, or couples to untouched code, compounding the next refactor's cost.
Review has license to question scope, surfacing it as a flag for the user. If review notices "this whole change may have been the wrong call" or "the design rests on a premise that looks wrong now that the code exists," it goes into the Conclusion as a `Scope flag` for the user to act on. Review surfaces; redesign belongs to udesign.
Review is a process, not just a section. Its end product is the `## Conclusion` section of the task file, filled in based on an independent code review and the work that was done.
## When to invoke
- After `up:uverify` passes
- Before merge to main
- Before opening a PR
- Never skipped, regardless of task size
## Brevity
Before writing the `## Conclusion`, read `plugins/up/skills/_brevity.md`. Apply its five principles. `Outcome:` is ≤1 sentence + the commit SHA — never re-narrate the diff. Omit subsections whose content would be "none" / "clean" / "no deviations" / "no findings" / the default: `Plan adherence`, `Review findings`, `Scope flag`, `Future work`, `Verified by`, `Deviations from plan`, `Known risks`. `Invariants:`, `Assumptions check:`, and `Unknowns outcome:` stay when the task had any IV / AS / UK — they carry audit value even on pass. The Exception clause still holds: findings, deviations, risks, violated assumptions, and deferrals always carry evidence and "why".
`## Code smells` is shared across stages, not a Conclusion subsection: at task end delete the header if it stayed empty (brevity 1). Leave recorded smells in their own section; promote one to `Future work` only if this task decides to schedule its fix — don't duplicate.
## Two roles, two attitudes
The `up:reviewer` subagent is **critical**. It is dispatched with a diff, a plan, and invariants — but not the rationale behind the changes. It looks for violated invariants, plan misalignment, bugs, and risks. Confidence-filtered (≥80). Severity-tiered.
You (the dispatcher) are **fair**. Fair means: take every finding seriously, but verify it against the codebase before acting. Fair is neither reflexive agreement nor reflexive pushback. Fair is: restate → verify → evaluate → decide.
The asymmetry is deliberate. A tough reviewer catches more real issues; a fair dispatcher avoids overcorrecting on mistaken calls.
## Process
### 1. Dispatch `up:reviewer`
Get git SHAs:
```bash
BASE_SHA=$(git merge-base HEAD main) # or the branch point for this task
HEAD_SHA=$(git rev-parse HEAD)
```
Dispatch the `up:reviewer` agent with:
- Task file path (`docs/tasks/.md`)
- `BASE_SHA` and `HEAD_SHA`
- Working directory (explicitly — the agent does not inherit `cwd` reliably)
Do **not** pass session history to the reviewer. The reviewer must not see the rationale behind changes — only the Plan, Invariants, and diff. Independence is the point.
**Dispatch prompt skeleton** (guidance):
```
Task file: .md>
BASE_SHA:
HEAD_SHA:
Working directory:
```
### 2. Read feedback without reacting
Receive the reviewer's output. Do not immediately reply with fixes or pushback. Classify first:
- Critical: fix before proceeding
- Important: fix before merge
- Plan finding: the plan itself may be wrong
### 3. Evaluate each item fairly
For every finding:
1. Restate in your own words. If you can't restate it, ask the reviewer to clarify — don't guess.
2. Verify against the codebase. Does the issue actually exist as described? Open the file, read the lines.
3. Evaluate technically: is the suggested fix right for *this* codebase and the Design?
4. Decide: implement, push back with technical reasoning, or escalate to the user.
### 4. Announce the plan before editing
Before any fix goes in, tell the user what you decided for each finding. One line per finding:
- what the reviewer said,
- your verdict (fix / push back / defer),
- if fixing: the exact change you are about to make.
In interactive mode, this is a short summary — the user can interject, then you apply the fixes.
In hands-off mode, see `up:handsoff` for the contract. Stage-specific delta: announce and apply in the same step — no pause for interjection. The restate → verify → evaluate → decide process from step 3 is still required. Each applied fix is logged as `- ureview: fixed — ` under `### Hands-off decisions`. Low-confidence / ambiguous findings go to `### Deferred (needs user input)` and are not auto-fixed. Fixes must honor the safety principles (no destructive edits, no force-push, additive over subtractive).
"Evaluating reviewer findings fairly." *(then a flurry of edits with no explanation)*
"Reviewer findings:
- Important #1: reviewer tool list missing `Bash`. Verdict: fix. Editing Design section of task file.
- Important #2: try.md and step-back.md missing frontmatter. Verdict: fix. Adding frontmatter to both.
Applying now."
### 5. Apply fixes
Fix Critical and Important issues. Commit each as its own logical unit.
For every fix, run the consistency pass (same rule as `up:uexecute`): if you're tightening a rule or changing a pattern, grep the diff and the wider repo for the same pattern and apply the change everywhere in the same commit. Do not leave siblings in a mixed state — that's how the reviewer's next round finds the same class of issue four more times.
If fixes are substantial, re-dispatch the reviewer on the new diff.
### 6. Write the `## Conclusion`
```markdown
## Conclusion
Outcome: <≤1 sentence on whether the Goal is achieved or what real-world validation remains, + commit SHA. Don't re-narrate the diff.>
Invariants:
- IV1 —
- IV2 — <...>
### Assumptions check (omit entire subsection if the task had no AS)
- AS1 — held | violated | unverifiable —
- AS2 — ...
### Unknowns outcome (omit entire subsection if the task had no UK)
- UK1 — resolved | still-open —
- UK2 — ...
Plan adherence: (omit entire subsection if no deviations)
Review findings: (omit entire subsection if no Critical or Important)
- Critical:
- Important:
Scope flag: (omit unless reviewer raised one — never auto-act; surface verbatim for the user)
-
Future work: (omit entire subsection if none — do not write "none")
- - — Justification: OR
Verified by: (omit if only the routine reviewer+verify ran)
```
A violated AS is always material — it means the design rested on a premise that turned out false. Record evidence and, if it invalidates the outcome, either redo the affected phase or surface it to the user.
## Receiving feedback — rules
Never:
- "You're absolutely right" / "Great catch" / "Thanks for catching that"
- Implement blindly without verifying against the codebase
- Batch fixes without checking each independently
- Respond partial when multiple findings may be linked — clarify all first
Do:
- Verify against codebase reality before acting
- Push back with technical reasoning when the reviewer is wrong
- Ask for clarification when a finding is unclear
- Show the fix in a diff — actions over words
Pushback is legitimate when:
- The suggestion breaks existing behavior
- The reviewer lacks context only the Design has (e.g. intentional tradeoff)
- The suggestion violates YAGNI (over-engineering an unused path)
- The suggestion conflicts with explicit Design / Invariants decisions
## Never
- Accept "ready to merge" without evidence
- Merge with open Critical or Important findings
- Skip the Conclusion write-up
- Run review on yourself (always use the subagent — preserve independence)
## Hands-off mode
See `up:handsoff` for the full contract. Stage-specific delta is embedded in step 4 above: announce-and-apply without the user interjection pause; high-confidence actionable findings are fixed in-line and logged under `### Hands-off decisions`; low-confidence / ambiguous findings go to `### Deferred (needs user input)`. All applied fixes must honor the safety principles (no destructive edits, no force-push, additive over subtractive).
## Terminal state
Conclusion written, all Critical/Important resolved or explicitly deferred with justification → Status → `validating`. Review does not mark `done`: control returns to `/up:make` to validate the Goal (step 11) before any finish action. The user chooses the finish action; you don't auto-merge.