Plan Review Workflow

Plan Review Workflow

Philosophy

The reviewer model only sees selected files. RepoPrompt's Builder discovers context you'd miss (rp backend). Codex uses context hints from flowctl (codex backend).

Phase 0: Backend Detection

Run this first. Do not skip.

CRITICAL: flowctl is BUNDLED — NOT installed globally. which flowctl will fail (expected). Always use:

set -e
FLOWCTL="${DROID_PLUGIN_ROOT:-${CLAUDE_PLUGIN_ROOT}}/scripts/flowctl"
REPO_ROOT="$(git rev-parse --show-toplevel 2>/dev/null || pwd)"

# Priority: --review flag > env > config (flag parsed in SKILL.md)
BACKEND=$($FLOWCTL review-backend)

if [[ "$BACKEND" == "ASK" ]]; then
  echo "Error: No review backend configured."
  echo "Run /flow-next:setup to configure, or pass --review=rp|codex|none"
  exit 1
fi

echo "Review backend: $BACKEND (override: --review=rp|codex|none)"

If backend is "none": Skip review, inform user, and exit cleanly (no error).

Then branch to backend-specific workflow below.

Codex Backend Workflow

Use when BACKEND="codex".

Step 0: Save Checkpoint

Before review (protects against context compaction):

EPIC_ID="${1:-}"
$FLOWCTL checkpoint save --epic "$EPIC_ID" --json

Step 1: Execute Review

RECEIPT_PATH="${REVIEW_RECEIPT_PATH:-/tmp/plan-review-receipt.json}"

# --files: comma-separated CODE files for reviewer context
# Epic/task specs are auto-included; pass files the plan will CREATE or MODIFY
# Read epic spec to identify affected paths, then list key files
CODE_FILES="src/main.py,src/config.py"  # Customize per epic

$FLOWCTL codex plan-review "$EPIC_ID" --files "$CODE_FILES" --receipt "$RECEIPT_PATH"

Output includes VERDICT=SHIP|NEEDS_WORK|MAJOR_RETHINK.

Step 2: Update Status

# Based on verdict
$FLOWCTL epic set-plan-review-status "$EPIC_ID" --status ship --json
# OR
$FLOWCTL epic set-plan-review-status "$EPIC_ID" --status needs_work --json

Step 3: Handle Verdict

If VERDICT=NEEDS_WORK:

1. Parse issues from output
2. Fix plan via $FLOWCTL epic set-plan
3. Re-run step 1 (receipt enables session continuity)
4. Repeat until SHIP

Step 4: Receipt

Receipt is written automatically by flowctl codex plan-review when --receipt provided. Format: {"mode":"codex","epic":"<id>","verdict":"<verdict>","session_id":"<thread_id>","timestamp":"..."}

RepoPrompt Backend Workflow

Use when BACKEND="rp".

Atomic Setup Block

# Atomic: pick-window + builder
eval "$($FLOWCTL rp setup-review --repo-root "$REPO_ROOT" --summary "Review plan for <EPIC_ID>: <summary>")"

# Verify we have W and T
if [[ -z "${W:-}" || -z "${T:-}" ]]; then
  echo "<promise>RETRY</promise>"
  exit 0
fi

echo "Setup complete: W=$W T=$T"

If this block fails, output <promise>RETRY</promise> and stop. Do not improvise.

Phase 1: Read the Plan (RP)

If Flow issue:

$FLOWCTL show <id> --json
$FLOWCTL cat <id>

Save output for inclusion in review prompt. Compose a 1-2 sentence summary for the setup-review command.

Save checkpoint (protects against context compaction during review):

$FLOWCTL checkpoint save --epic <id> --json

This creates .flow/.checkpoint-<id>.json with full state. If compaction occurs during review-fix cycles, restore with $FLOWCTL checkpoint restore --epic <id>.

Phase 2: Augment Selection (RP)

Builder selects context automatically. Review and add must-haves:

# See what builder selected
$FLOWCTL rp select-get --window "$W" --tab "$T"

# Always add the epic spec
$FLOWCTL rp select-add --window "$W" --tab "$T" .flow/specs/<epic-id>.md

# Always add ALL task specs for this epic
for task_spec in .flow/tasks/${EPIC_ID}.*.md; do
  [[ -f "$task_spec" ]] && $FLOWCTL rp select-add --window "$W" --tab "$T" "$task_spec"
done

# Add PRD/architecture docs if found
$FLOWCTL rp select-add --window "$W" --tab "$T" docs/prd.md

Why this matters: Chat only sees selected files. Reviewer needs both epic spec AND task specs to check for consistency.

Phase 3: Execute Review (RP)

Build combined prompt

Get builder's handoff:

HANDOFF="$($FLOWCTL rp prompt-get --window "$W" --tab "$T")"

Write combined prompt:

cat > /tmp/review-prompt.md << 'EOF'
[PASTE HANDOFF HERE]

---

## IMPORTANT: File Contents
RepoPrompt includes the actual source code of selected files in a `<file_contents>` XML section at the end of this message. You MUST:
1. Locate the `<file_contents>` section
2. Read and analyze the actual source code within it
3. Base your review on the code, not summaries or descriptions

If you cannot find `<file_contents>`, ask for the files to be re-attached before proceeding.

## Plan Under Review
[PASTE flowctl show OUTPUT]

## Review Focus
[USER'S FOCUS AREAS]

## Review Scope

You are reviewing:
1. **Epic spec** - The high-level plan
2. **Task specs** - Individual task breakdowns

**CRITICAL**: Check for consistency between epic and tasks. Flag if:
- Task specs contradict or miss epic requirements
- Task acceptance criteria don't align with epic acceptance criteria
- Task approaches would need to change based on epic design decisions
- Epic mentions states/enums/types that tasks don't account for

## Review Criteria

Conduct a John Carmack-level review:

1. **Completeness** - All requirements covered? Missing edge cases?
2. **Feasibility** - Technically sound? Dependencies clear?
3. **Parallelizability** - Do independent tasks touch disjoint files? Flag overlapping file scopes that will cause merge conflicts.
4. **Clarity** - Specs unambiguous? Acceptance criteria testable?
5. **Architecture** - Right abstractions? Clean boundaries?
6. **Risks** - Blockers identified? Security gaps? Mitigation?
7. **Scope** - Right-sized? Over/under-engineering?
8. **Task sizing** - M tasks preferred. Flag over-splitting: 7+ tasks? Sequential S tasks that should be combined?
9. **Testability** - How will we verify this works?
10. **Consistency** - Do task specs align with epic spec?

## Output Format

For each issue:
- **Severity**: Critical / Major / Minor / Nitpick
- **Location**: Which task or section (e.g., "fn-1.3 Description" or "Epic Acceptance #2")
- **Problem**: What's wrong
- **Suggestion**: How to fix

**REQUIRED**: You MUST end your response with exactly one verdict tag. This is mandatory:
`<verdict>SHIP</verdict>` or `<verdict>NEEDS_WORK</verdict>` or `<verdict>MAJOR_RETHINK</verdict>`

Do NOT skip this tag. The automation depends on it.
EOF

Send to RepoPrompt

$FLOWCTL rp chat-send --window "$W" --tab "$T" --message-file /tmp/review-prompt.md --new-chat --chat-name "Plan Review: <EPIC_ID>"

WAIT for response. Takes 1-5+ minutes.

Phase 4: Receipt + Status (RP)

Write receipt (if REVIEW_RECEIPT_PATH set)

if [[ -n "${REVIEW_RECEIPT_PATH:-}" ]]; then
  ts="$(date -u +%Y-%m-%dT%H:%M:%SZ)"
  mkdir -p "$(dirname "$REVIEW_RECEIPT_PATH")"
  cat > "$REVIEW_RECEIPT_PATH" <<EOF
{"type":"plan_review","id":"<EPIC_ID>","mode":"rp","timestamp":"$ts"}
EOF
  echo "REVIEW_RECEIPT_WRITTEN: $REVIEW_RECEIPT_PATH"
fi

Update status

Extract verdict from response, then:

# If SHIP
$FLOWCTL epic set-plan-review-status <EPIC_ID> --status ship --json

# If NEEDS_WORK or MAJOR_RETHINK
$FLOWCTL epic set-plan-review-status <EPIC_ID> --status needs_work --json

If no verdict tag, output <promise>RETRY</promise> and stop.

Fix Loop (RP)

CRITICAL: Do NOT ask user for confirmation. Automatically fix ALL valid issues and re-review — our goal is production-grade world-class software and architecture. Never use AskUserQuestion in this loop.

CRITICAL: You MUST fix the plan BEFORE re-reviewing. Never re-review without making changes.

If verdict is NEEDS_WORK:

1. Parse issues - Extract ALL issues by severity (Critical → Major → Minor)

2. Fix the epic spec - Address each issue.

3. Update epic spec in flowctl (MANDATORY before re-review):

   # Option A: stdin heredoc (preferred, no temp file)
   $FLOWCTL epic set-plan <EPIC_ID> --file - --json <<'EOF'
   <updated epic spec content>
   EOF
   
   # Option B: temp file (if content has single quotes)
   $FLOWCTL epic set-plan <EPIC_ID> --file /tmp/updated-plan.md --json
   

   If you skip this step and re-review with same content, reviewer will return NEEDS_WORK again.

   Recovery: If context compaction occurred, restore from checkpoint first:

   $FLOWCTL checkpoint restore --epic <EPIC_ID> --json
   

4. Sync affected task specs - If epic changes affect task specs, update them:

   $FLOWCTL task set-spec <TASK_ID> --file - --json <<'EOF'
   <updated task spec content>
   EOF
   

   Task specs need updating when epic changes affect:

   • State/enum values referenced in tasks
   • Acceptance criteria that tasks implement
   • Approach/design decisions tasks depend on
   • Lock/retry/error handling semantics
   • API signatures or type definitions

5. Request re-review (only AFTER steps 3-4):

   IMPORTANT: Do NOT re-add files already in the selection. RepoPrompt auto-refreshes file contents on every message. Only use select-add for NEW files created during fixes:

   # Only if fixes created new files not in original selection
   if [[ -n "$NEW_FILES" ]]; then
     $FLOWCTL rp select-add --window "$W" --tab "$T" $NEW_FILES
   fi
   

   Then send re-review request (NO --new-chat, stay in same chat).

   CRITICAL: Do NOT summarize fixes. RP auto-refreshes file contents - reviewer sees your changes automatically. Just request re-review. Any summary wastes tokens and duplicates what reviewer already sees.

   cat > /tmp/re-review.md << 'EOF'
   Issues addressed. Please re-review.
   
   **REQUIRED**: End with `<verdict>SHIP</verdict>` or `<verdict>NEEDS_WORK</verdict>` or `<verdict>MAJOR_RETHINK</verdict>`
   EOF
   
   $FLOWCTL rp chat-send --window "$W" --tab "$T" --message-file /tmp/re-review.md
   

6. Repeat until Ship

Anti-pattern: Re-adding already-selected files before re-review. RP auto-refreshes; re-adding can cause issues.

Anti-pattern: Re-reviewing without calling epic set-plan first. This wastes reviewer time and loops forever.

Anti-pattern: Updating epic spec without syncing affected task specs. Causes reviewer to flag consistency issues again.

Anti-patterns

All backends:

• Reviewing yourself - You coordinate; the backend reviews
• No receipt - If REVIEW_RECEIPT_PATH is set, you MUST write receipt
• Ignoring verdict - Must extract and act on verdict tag
• Mixing backends - Stick to one backend for the entire review session

RP backend only:

• Calling builder directly - Must use setup-review which wraps it
• Skipping setup-review - Window selection MUST happen via this command
• Hard-coding window IDs - Never write --window 1

Codex backend only:

• Using --last flag - Conflicts with parallel usage; use --receipt instead
• Direct codex calls - Must use flowctl codex wrappers