"Phase 5: Review — systematic code review against the plan, identified risks, and framework anti-patterns. Fixes issues found, runs bin/flow ci after any fix, then transitions to Security."
Install
npx skillscat add benkruger/flow/flow-review Install via the SkillsCat registry.
FLOW Review — Phase 5: Review
Usage
/flow:flow-review
/flow:flow-review --auto
/flow:flow-review --manual/flow:flow-review— uses configured mode from.flow.json(default: manual)/flow:flow-review --auto— significant findings auto-fixed here (no user routing choice), auto-advance to Security/flow:flow-review --manual— significant findings prompt user for fix/go-back choice
- Find the project root: run
git worktree list --porcelainand note the
path on the firstworktreeline. - Get the current branch: run
git branch --show-current. - Use the Read tool to read
<project_root>/.flow-states/<branch>.json.- If the file does not exist: STOP. "BLOCKED: No FLOW feature in progress.
Run /flow:flow-start first."
- If the file does not exist: STOP. "BLOCKED: No FLOW feature in progress.
- Check
phases.flow-simplify.statusin the JSON.- If not
"complete": STOP. "BLOCKED: Phase 4: Simplify must be
complete. Run /flow:flow-simplify first."
- If not
Mode Resolution
- If
--autowas passed → commit=auto, continue=auto - If
--manualwas passed → commit=manual, continue=manual - Otherwise, read
.flow.jsonfrom the project root. Useskills.flow-review.commitandskills.flow-review.continue. - If
.flow.jsonhas noskillskey → use built-in defaults: commit=manual, continue=manual
Announce
At the very start, output the following banner in your response (not via Bash) inside a fenced code block:
```text
============================================
FLOW v0.19.0 — Phase 5: Review — STARTING
============================================
```Update State
Update state for phase entry:
bin/flow phase-transition --phase flow-review --action enterParse the JSON output to confirm "status": "ok".
If "status": "error", report the error and stop.
Logging
After every Bash command completes, log it to .flow-states/<branch>.log.
Run the command directly — do not append any suffix:
COMMANDThen Read .flow-states/<branch>.log (empty string if it does not
exist yet) and Write it back with this line appended:
YYYY-MM-DDTHH:MM:SSZ [Phase 5] Step X — desc (exit EC)Get <branch> from the state file.
Framework Instructions
Read the framework field from the state file and follow only the matching
section below for the diff analysis sub-agent prompt and framework-specific
hard rules. Do not announce the framework — just follow the matching
section silently.
If Rails
Diff Analysis Sub-Agent Prompt
Provide these instructions to the Step 1 sub-agent (fill in the details):
You are analyzing a feature diff for the FLOW review phase.
Feature:Tool rules: Use Glob and Read tools for all file and directory
operations. Use Grep for searching code. Only use Bash for git commands
(git diff, git log, git blame, git show). Never pipe git output through
sed, grep, awk, or any other command — read the full output and extract
what you need in your response. Never usecd <path> && git— run git
commands from the current directory. Never use Bash for any other
purpose — no find, ls, cat, wc, test -f, stat, or running project tooling.Plan file:
<paste the plan file contents — includes context, approach, risks, and tasks>First, get the full diff:
git diff origin/main...HEADRead every changed file completely. Then check:
Plan alignment:
- Does the implementation match the approach described in the plan?
- Do schema changes match what was planned?
- Do model, controller, worker, and route changes match the plan?
- Are all planned tasks reflected in the diff?
- Flag any deviation — minor drift or major mismatch.
Risk coverage:
- For each risk identified in the plan, confirm it was handled in the diff.
- Flag any risk not addressed.
Rails anti-pattern check:
- Associations: every belongs_to/has_many has inverse_of:, dependent:,
class_name: explicit- Queries: no N+1, no DB queries in views, no .first/.last for defaults
- Callbacks: Current attribute usage correct, no update_column
- Models: self.table_name in namespaced Base, no STI
- Soft deletes: .unscoped usage correct
- Workers: halt! in pre_perform!, queue matches sidekiq.yml
- Tests: create_*! helpers used, both branches tested, assertions present
- RuboCop: scan diff for rubocop:disable comments, check .rubocop.yml changes
- Code clarity: descriptive names, no inline comments, no over-engineering
Return structured findings in three categories:
- Plan alignment issues (with file:line references)
- Uncovered risks (with which risk and why)
- Anti-pattern violations (with file:line and what to fix)
If a category has no findings, say so explicitly.
Rails-Specific Hard Rules
- Any
# rubocop:disablecomment in the diff is an automatic finding — remove it and fix the code - Any modification to
.rubocop.ymlin the diff is an automatic finding — revert it and fix the code
If Python
Diff Analysis Sub-Agent Prompt
Provide these instructions to the Step 1 sub-agent (fill in the details):
You are analyzing a feature diff for the FLOW review phase.
Feature:Tool rules: Use Glob and Read tools for all file and directory
operations. Use Grep for searching code. Only use Bash for git commands
(git diff, git log, git blame, git show). Never pipe git output through
sed, grep, awk, or any other command — read the full output and extract
what you need in your response. Never usecd <path> && git— run git
commands from the current directory. Never use Bash for any other
purpose — no find, ls, cat, wc, test -f, stat, or running project tooling.Plan file:
<paste the plan file contents — includes context, approach, risks, and tasks>First, get the full diff:
git diff origin/main...HEADRead every changed file completely. Then check:
Plan alignment:
- Does the implementation match the approach described in the plan?
- Do module and script changes match what was planned?
- Are all planned tasks reflected in the diff?
- Flag any deviation — minor drift or major mismatch.
Risk coverage:
- For each risk identified in the plan, confirm it was handled in the diff.
- Flag any risk not addressed.
Python anti-pattern check:
Imports: no circular imports, no wildcard imports (
from x import *).
Mutable defaults: no mutable default arguments (def f(x=[])).
Error handling: no bareexcept:, no broadexcept Exception
without re-raise.
Type safety: consistent use of type hints if the project uses them.
Tests: fixtures used where appropriate, both branches tested,
assertions present.
Lint: scan diff for noqa/type:ignore comments.
Code clarity: descriptive names, no inline comments, no over-engineering.Return structured findings in three categories:
- Plan alignment issues (with file:line references)
- Uncovered risks (with which risk and why)
- Anti-pattern violations (with file:line and what to fix)
If a category has no findings, say so explicitly.
Python-Specific Hard Rules
- Any
# noqaor# type: ignorecomment in the diff is a finding — remove it and fix the code - Any modification to lint configuration in the diff is a finding — revert it and fix the code
Step 1 — Launch diff analyzer sub-agent
Read plan_file from the state file to get the plan file path. Use the
Read tool to read the plan file — it contains the approach, risks, and
planned tasks.
Then launch a mandatory sub-agent to analyze the full diff. Use the Task tool:
subagent_type:"general-purpose"description:"Review diff analysis"
Provide the sub-agent with the Diff Analysis Sub-Agent Prompt from the
framework section above (fill in the feature name and plan file contents).
Wait for the sub-agent to return before proceeding.
Step 2 — Review sub-agent findings
Read the sub-agent's structured findings. For each category:
Plan alignment issues — Confirm each finding against the plan file.
Minor drift is a note. Major drift means go back to Code.
Uncovered risks — Confirm each finding. An unaddressed risk
is a bug waiting to happen.
Anti-pattern violations — Confirm each finding against the actual code.
The sub-agent may have false positives — verify before flagging.
Compile the confirmed findings list for Step 3.
Step 3 — Fixing Findings
For each finding:
Minor finding (style, missing option, small oversight):
- Fix it directly
- Describe what was fixed and why
Significant finding (logic error, missing risk coverage, plan mismatch):
If commit=auto, fix it directly here in Review without asking.
If commit=manual, use AskUserQuestion:
"Found a significant issue: . How would you like to proceed?"
- Fix it here in Review
- Go back to Code
- Go back to Plan
After fixing any findings, if commit=auto use /flow:flow-commit --auto, otherwise use /flow:flow-commit for the Review fixes.
Then run bin/flow ci --if-dirty — required before any state transition.
Step 4 — Present review summary
Show a summary of what was found and fixed inside a fenced code block:
```text
============================================
FLOW — Phase 5: Review — SUMMARY
============================================
Plan alignment : ✓ matches approved plan
Risk coverage : ✓ all risks accounted for
Findings fixed
--------------
- <description of fix and why>
- <description of fix and why>
- <description of fix and why>
bin/flow ci : ✓ green
============================================
```Back Navigation
Use AskUserQuestion if a finding is too significant to fix in Review:
- Go back to Code — implementation issue
- Go back to Plan — plan was missing something
Go back to Code: update Phases 5 and 4 to pending, Phase 3 toin_progress, then invoke flow:flow-code.
Go back to Plan: update Phases 5, 4, and 3 to pending, Phase 2 toin_progress, then invoke flow:flow-plan.
Done — Update state and complete phase
Complete the phase:
bin/flow phase-transition --phase flow-review --action completeParse the JSON output. If "status": "error", report the error and stop.
Use the formatted_time field in the COMPLETE banner below. Do not print
the timing calculation.
Output in your response (not via Bash) inside a fenced code block:
```text
============================================
FLOW v0.19.0 — Phase 5: Review — COMPLETE (<formatted_time>)
============================================
```Invoke flow:flow-status.
If continue=auto, skip the transition question and invoke flow:flow-security directly.
If continue=manual, use AskUserQuestion:
"Phase 5: Review is complete. Ready to begin Phase 6: Security?"
- Yes, start Phase 6 now — invoke
flow:flow-security- Not yet — print paused banner
- I have a correction or learning to capture
If "I have a correction or learning to capture":
- Ask the user what they want to capture
- Invoke
/flow:flow-notewith their message - Re-ask with only "Yes, start Phase 6 now" and "Not yet"
If Yes — invoke flow:flow-security using the Skill tool.
If Not yet, output in your response (not via Bash) inside a fenced code block:
```text
============================================
FLOW — Paused
Run /flow:flow-continue when ready to continue.
============================================
```Hard Rules
- Always run
bin/flow ciafter any fix made during Review - Never transition to Security unless
bin/flow ciis green - Never skip the plan alignment check
- Never skip the risk coverage check
- Read the full diff before starting — no partial reviews
- Plus the Framework-Specific Hard Rules from the framework section above
- Never use Bash to print banners — output them as text in your response
- Never use Bash for file reads — use Glob, Read, and Grep tools instead of ls, cat, head, tail, find, or grep
- Never use
cd <path> && git— usegit -C <path>for git commands in other directories - Never cd before running
bin/flow— it detects the project root internally