Implementation Review Workflow

Implementation 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="${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 1: Identify Task and Diff Base

BRANCH="$(git branch --show-current)"

# Use BASE_COMMIT from arguments if provided (task-scoped review)
# Otherwise fall back to main/master (full branch review)
if [[ -z "$BASE_COMMIT" ]]; then
  DIFF_BASE="main"
  git rev-parse main >/dev/null 2>&1 || DIFF_BASE="master"
else
  DIFF_BASE="$BASE_COMMIT"
fi

git log ${DIFF_BASE}..HEAD --oneline

Step 2: Execute Review

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

$FLOWCTL codex impl-review "$TASK_ID" --base "$DIFF_BASE" --receipt "$RECEIPT_PATH"

Output includes VERDICT=SHIP|NEEDS_WORK|MAJOR_RETHINK.

Step 3: Handle Verdict

If VERDICT=NEEDS_WORK:

1. Parse issues from output
2. Fix code and run tests
3. Commit fixes
4. Re-run step 2 (receipt enables session continuity)
5. Repeat until SHIP

Step 4: Receipt

Receipt is written automatically by flowctl codex impl-review when --receipt provided. Format: {"mode":"codex","task":"<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 implementation of <summary> on current branch")"

# 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: Identify Changes (RP)

BRANCH="$(git branch --show-current)"

# Use BASE_COMMIT from arguments if provided (task-scoped review)
# Otherwise fall back to main/master (full branch review)
if [[ -z "$BASE_COMMIT" ]]; then
  DIFF_BASE="main"
  git rev-parse main >/dev/null 2>&1 || DIFF_BASE="master"
else
  DIFF_BASE="$BASE_COMMIT"
fi

git log ${DIFF_BASE}..HEAD --oneline
CHANGED_FILES="$(git diff ${DIFF_BASE}..HEAD --name-only)"
git diff ${DIFF_BASE}..HEAD --stat

Save:

• Branch name
• Changed files list
• Commit summary
• DIFF_BASE (for reference in review prompt)

Compose a 1-2 sentence summary for the setup-review command.

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"

# Add ALL changed files
for f in $CHANGED_FILES; do
  $FLOWCTL rp select-add --window "$W" --tab "$T" "$f"
done

# Add task spec if known
$FLOWCTL rp select-add --window "$W" --tab "$T" .flow/specs/<task-id>.md

Why this matters: Chat only sees selected files.

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.

## Changes Under Review
Branch: [BRANCH_NAME]
Files: [LIST CHANGED FILES]
Commits: [COMMIT SUMMARY]

## Original Spec
[PASTE flowctl show OUTPUT if known]

## Review Focus
[USER'S FOCUS AREAS]

## Review Criteria

Conduct a John Carmack-level review:

1. **Correctness** - Matches spec? Logic errors?
2. **Simplicity** - Simplest solution? Over-engineering?
3. **DRY** - Duplicated logic? Existing patterns?
4. **Architecture** - Data flow? Clear boundaries?
5. **Edge Cases** - Failure modes? Race conditions?
6. **Tests** - Adequate coverage? Testing behavior?
7. **Security** - Injection? Auth gaps?

## Scenario Exploration (for changed code only)

Walk through these scenarios mentally for any new/modified code paths:

- [ ] Happy path - Normal operation with valid inputs
- [ ] Invalid inputs - Null, empty, malformed data
- [ ] Boundary conditions - Min/max values, empty collections
- [ ] Concurrent access - Race conditions, deadlocks
- [ ] Network issues - Timeouts, partial failures
- [ ] Resource exhaustion - Memory, disk, connections
- [ ] Security attacks - Injection, overflow, DoS vectors
- [ ] Data corruption - Partial writes, inconsistency
- [ ] Cascading failures - Downstream service issues

Only flag issues that apply to the **changed code** - not pre-existing patterns.

## Output Format

For each issue:
- **Severity**: Critical / Major / Minor / Nitpick
- **File:Line**: Exact location
- **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 "Impl Review: $BRANCH"

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":"impl_review","id":"<TASK_ID>","mode":"rp","timestamp":"$ts"}
EOF
  echo "REVIEW_RECEIPT_WRITTEN: $REVIEW_RECEIPT_PATH"
fi

If no verdict tag in response, 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 code BEFORE re-reviewing. Never re-review without making changes.

MAX ITERATIONS: Limit fix+re-review cycles to ${MAX_REVIEW_ITERATIONS:-3} iterations (default 3, configurable in Ralph's config.env). If still NEEDS_WORK after max rounds, output <promise>RETRY</promise> and stop — let the next Ralph iteration start fresh.

If verdict is NEEDS_WORK:

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

2. Fix the code - Address each issue in order

3. Run tests/lints - Verify fixes don't break anything

4. Commit fixes (MANDATORY before re-review):

   git add -A
   git commit -m "fix: address review feedback"
   

   If you skip this and re-review without committing changes, reviewer will return NEEDS_WORK again.

5. Request re-review (only AFTER step 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-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
• Missing changed files - Add ALL changed files to selection

Codex backend only:

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