flow-next-impl-review

# Implementation Review Mode **Workflow is backend-split. Read [workflow-common.md](workflow-common.md) for Phase 0 (backend detection + philosophy + trivial-diff triage), then read ONLY the file matching your active backend. The opt-in `--deep`/`--validate`/`--interactive` phase detail (including the phase-ordering matrix) lives in [optional-phases.md](optional-phases.md), loaded only when a flag fires:** - `BACKEND=codex` → [workflow-codex.md](workflow-codex.md) - `BACKEND=copilot` → [workflow-copilot.md](workflow-copilot.md) - `BACKEND=cursor` → [workflow-cursor.md](workflow-cursor.md) - `BACKEND=claude` → [workflow-claude.md](workflow-claude.md) - `BACKEND=host` → [workflow-host.md](workflow-host.md) - `BACKEND=rp` → [workflow-rp.md](workflow-rp.md) Do not load the others — only the active backend's file is needed. Each backend file carries its own Critical Rules and anti-patterns. Conduct a John Carmack-level review of implementation changes on the current branch. **Role**: Code Review Coordinator (NOT the reviewer) **Backends** (branch on the Phase 0 `RP_ELIGIBLE` probe): - When `RP_ELIGIBLE=1`: RepoPrompt (rp), Codex CLI (codex), GitHub Copilot CLI (copilot), Cursor CLI (cursor), Claude Code CLI (claude), or host-native (`host`) - When `RP_ELIGIBLE=0`: Codex CLI (codex), GitHub Copilot CLI (copilot), Cursor CLI (cursor), Claude Code CLI (claude), or host-native (`host`) — rp is macOS-only; never list it in guidance you surface (`--review=rp` stays accepted) ## Preamble — execute Phase 0 exactly once **The executable Phase 0 lives in [workflow-common.md](workflow-common.md) §"Phase 0: Backend Detection" — Read it and execute it ONCE, before any other bash in this skill.** It defines `$FLOWCTL` (bundled — NOT installed globally; `which flowctl` fails, expected), probes `RP_ELIGIBLE`, resolves `$BACKEND` via the single `flowctl review-backend` call, and handles the ASK / `none` cases. Every later bash block here (triage, deep-pass selection) uses the `$FLOWCTL` it defines. Never invoke `flowctl review-backend` a second time in the same run. Exception: a `--review=<backend>` argument (see Backend Selection below) wins — when present, set `BACKEND` from the flag and skip Phase 0's `review-backend` call + ASK handling (still run its `$FLOWCTL` / `RP_ELIGIBLE` setup lines). When `RP_ELIGIBLE=0` (not macOS, no supported RepoPrompt CLI), never *steer* the user toward rp: every backend summary, recommendation, or override hint you surface presents only the runnable configured backends `codex`, `copilot`, `cursor`, `claude`, `host` (plus `none`). `export` is not an impl-review mode at all — a manual export review lives in `/flow-next:plan-review --review=export`; never present it here. Suppression is not a ban: an explicit `--review=rp`, `FLOW_REVIEW_BACKEND=rp`, or `review.backend=rp` still resolves to rp and errors at runtime via `require_rp_cli()`. ## Backend Selection **Priority** (first match wins): 1. `--review=rp|codex|copilot|cursor|claude|host|none` argument 2. `FLOW_REVIEW_BACKEND` env var — bare backend (`rp`, `codex`, `copilot`, `cursor`, `claude`, `host`, `none`) OR spec form (`codex:<model>:xhigh`, `copilot:<model>`, `cursor:<model>`, `claude:<model>:<effort>`); `host` is bare-only (`host:<model>` is rejected) 3. `.flow/config.json` → `review.backend` (same bare / spec forms) 4. **Error** - no auto-detection ### Parse from arguments first Check $ARGUMENTS for: - `--review=rp` or `--review rp` → use rp - `--review=codex` or `--review codex` → use codex - `--review=copilot` or `--review copilot` → use copilot - `--review=cursor` or `--review cursor` → use cursor - `--review=claude` or `--review claude` → use claude - `--review=host` or `--review host` → use host - `--review=export` or `--review export` → fail closed: report that `export` is not an impl-review backend and stop before any dispatch; the manual path is `/flow-next:plan-review --review=export` - `--review=none` or `--review none` → skip review If found, use that backend and skip all other detection. ### Otherwise: Phase 0 resolves it No `--review` flag → `$BACKEND` comes from [workflow-common.md](workflow-common.md) Phase 0 (executed once per the Preamble): the single `flowctl review-backend "$REVIEW_ID"` call with ASK handling included. Do not re-resolve here. ### Backend detail (model / effort / spec grammar) — on demand The per-backend "at a glance" descriptions, the `backend[:model[:effort]]` spec grammar, and the `FLOW_REVIEW_BACKEND` spec-form examples live in [references/backend-specs.md](references/backend-specs.md). Read it only when you must surface backend guidance to the user or resolve a model/effort spec — a normal review already has `$BACKEND` and needs nothing from it. When `RP_ELIGIBLE=0`, omit the **rp** line from any guidance you surface (explicit `--review=rp` still honored). ## Critical Rules **Per-backend rules** for `rp`, `codex`, `copilot`, `cursor`, and `claude` live at the top of each `workflow-<backend>.md` — read the active backend's file (routing table above) and follow its Critical Rules section. **For host backend:** `host` is bare-only. After selection, read [workflow-host.md](workflow-host.md). The review must use a fresh, tool-enforced read-only reviewer from a different model family and fail closed when no cross-family pin is available. **For all backends:** - If `REVIEW_RECEIPT_PATH` set: write receipt after review (any verdict) - Any failure → output `<promise>RETRY</promise>` and stop **Hard invariants:** - **The coordinator never authors a verdict.** A SHIP with no backend response behind it has broken this. - **One backend per review.** A transcript that dispatches a second backend after the first answered has broken this. - **Review is never skipped without consent.** A `none` backend that ends the run without the user's consent has broken this. ## Input Arguments: $ARGUMENTS Format: `[task ID] [--base <commit>] [--validate] [--deep[=passes]] [--interactive] [focus areas]` - `--base <commit>` - Compare against this commit instead of main/master (for task-scoped reviews) - `--validate` - After NEEDS_WORK verdict, run a validator pass that drops false-positive findings (opt-in) - `--deep` / `--deep=<passes>` - Run additional specialized passes (adversarial / security / performance) after primary review (opt-in) - `--interactive` - On NEEDS_WORK, walk through each finding with the user (Apply/Defer/Skip/Acknowledge) (opt-in, Ralph-incompatible) - Task ID - Optional, for context and receipt tracking - Focus areas - Optional, specific areas to examine **Scope behavior:** - With `--base`: Reviews only changes since that commit (task-scoped) - Without `--base`: Reviews entire branch vs main/master (full branch review) **Opt-in flags:** - `--validate` — adds a validator pass on NEEDS_WORK that re-checks each finding for false positives. All findings dropping upgrades verdict to SHIP. - `FLOW_VALIDATE_REVIEW=1` env var — enables `--validate` session-wide (works in Ralph). - `--deep` — adds adversarial pass always + security/performance auto-enabled per diff paths. `--deep=adversarial,security` restricts to listed passes. - `FLOW_REVIEW_DEEP=1` env var — enables `--deep` session-wide (works in Ralph). - `--interactive` — per-finding walkthrough on NEEDS_WORK. **No env var form** — per-invocation only, always hard-errors in Ralph mode (`REVIEW_RECEIPT_PATH` or `FLOW_RALPH=1`) to prevent accidental autonomous engagement. - Default review behavior (no flags) is unchanged. ## Workflow ```bash REPO_ROOT="$(git rev-parse --show-toplevel 2>/dev/null || pwd)" ``` ### Step 0: Parse Arguments Parse $ARGUMENTS for: - `--base <commit>` → `BASE_COMMIT` (if provided, use for scoped diff) - `--no-triage` → set `TRIAGE_DISABLED=1` (skip trivial-diff pre-check) - `--validate` → set `VALIDATE=true` (validator pass on NEEDS_WORK) - `--deep` / `--deep=<passes>` → set `DEEP=true` + optional `DEEP_PASSES` CSV - `--interactive` → set `INTERACTIVE=true` (per-finding walkthrough on NEEDS_WORK; Ralph-blocked) - First positional arg matching `fn-*` → `TASK_ID` - Remaining args → focus areas If `--base` not provided, `BASE_COMMIT` stays empty (will fall back to main/master). **Opt-in flags + env vars — ONE parse fence for `--validate` / `--deep` / `--interactive`:** ```bash VALIDATE=false DEEP=false DEEP_PASSES="" # optional CSV: "adversarial,security" INTERACTIVE=false for arg in $(printf '%s\n' "$ARGUMENTS"); do # command substitution word-splits under bash AND zsh; an unquoted $ARGUMENTS does not split under zsh (dogfood E1: --validate silently dropped) case "$arg" in --validate) VALIDATE=true ;; --deep) DEEP=true ;; --deep=*) DEEP=true; DEEP_PASSES="${arg#--deep=}" ;; --interactive) INTERACTIVE=true ;; esac done # Env opt-ins (Ralph-friendly). --interactive has NO env var form — per-invocation only. if [[ "${FLOW_VALIDATE_REVIEW:-}" == "1" ]]; then VALIDATE=true fi if [[ "${FLOW_REVIEW_DEEP:-}" == "1" ]]; then DEEP=true fi # Optional-phase COUNT (PR #392): sizes the scope-ownership lease the backend # workflows hold through the post-finalize phases (one exec allowance per # pass). --deep counts one per selected pass (3 when unrestricted: adversarial # + the auto-gated security/performance passes), --validate one, # --interactive one. Carry this number into the finalize / host record blocks # as a LITERAL - shell state does not survive across prompt turns. OPTIONAL_PHASES_COUNT=0 if [[ "$DEEP" == "true" ]]; then if [[ -n "$DEEP_PASSES" ]]; then OPTIONAL_PHASES_COUNT=$((OPTIONAL_PHASES_COUNT + $(printf '%s' "$DEEP_PASSES" | tr ',' '\n' | grep -c .))) else OPTIONAL_PHASES_COUNT=$((OPTIONAL_PHASES_COUNT + 3)) fi fi [[ "$VALIDATE" == "true" ]] && OPTIONAL_PHASES_COUNT=$((OPTIONAL_PHASES_COUNT + 1)) [[ "$INTERACTIVE" == "true" ]] && OPTIONAL_PHASES_COUNT=$((OPTIONAL_PHASES_COUNT + 1)) echo "OPTIONAL_PHASES_COUNT=$OPTIONAL_PHASES_COUNT" # 1 when a held phase resumes the primary reviewer session (--deep / # --validate); the interactive walkthrough alone never needs one. PHASES_RESUME_SESSION=0 [[ "$DEEP" == "true" || "$VALIDATE" == "true" ]] && PHASES_RESUME_SESSION=1 echo "PHASES_RESUME_SESSION=$PHASES_RESUME_SESSION" # Ralph-block: Ralph must never engage interactive. if [[ "$INTERACTIVE" == "true" ]]; then if [[ -n "${REVIEW_RECEIPT_PATH:-}" || "${FLOW_RALPH:-}" == "1" ]]; then echo "Error: --interactive requires a user at the terminal; not compatible with Ralph mode (REVIEW_RECEIPT_PATH or FLOW_RALPH detected)." >&2 exit 2 fi fi if [[ "$DEEP" == "true" || "$VALIDATE" == "true" || "$INTERACTIVE" == "true" ]]; then echo "OPTIONAL PHASES ACTIVE — STOP. Read optional-phases.md (deep=$DEEP validate=$VALIDATE interactive=$INTERACTIVE) before continuing." fi ``` When that sentinel prints, STOP and Read [optional-phases.md](optional-phases.md) before any further step — it owns the phase-ordering + flag-combination matrix, the deep-pass selection bash, the validator dispatch, and the walkthrough steps (per-finding loop detail in [walkthrough.md](walkthrough.md), pass prompt templates in [deep-passes.md](deep-passes.md)). All three phases are default-OFF: when no flag fires, run the primary review only and write no `validator` / `deep_passes` / `walkthrough` receipt keys. ### Step 0.5: Trivial-diff triage Before invoking the configured backend, run a fast pre-check that short-circuits lockfile-only, docs-only, release-chore, and generated-file diffs. On SKIP, the receipt is written with `mode: "triage_skip"` / `verdict: "SHIP"` and the expensive backend call is skipped entirely. Opt-out: `--no-triage` argument or `FLOW_RALPH_NO_TRIAGE=1` env var. ```bash if [[ -z "${TRIAGE_DISABLED:-}" && -z "${FLOW_RALPH_NO_TRIAGE:-}" ]]; then ROUTE="$($FLOWCTL review-route ${TASK_ID:+"$TASK_ID"} --json)" # pure: canonical TASK_ID + receipt path (no rotation, no state change) TASK_ID="$(jq -r '.task_id // empty' <<<"$ROUTE")" RECEIPT_PATH="$(jq -r '.receipt_path' <<<"$ROUTE")" # Subcommand + one literal flag stay on the command line (the Ralph guard # blocks a variable in either of the two tokens after the launcher). TRIAGE_ARGS=(--receipt "$RECEIPT_PATH") [[ -n "$BASE_COMMIT" ]] && TRIAGE_ARGS+=(--base "$BASE_COMMIT")