refactor(review): wire entire review on new architecture; delete legacy (3/3)

[peyton-alt]

https://entire.io/gh/entireio/cli/trails/286

Summary

Final of three stacked PRs rebuilding `entire review`'s internals. Stacked on PR-A2 (#1106). Wires the new architecture as `entire review`'s actual code path and deletes the legacy 1,513-line `cmd/entire/cli/review.go`. User-visible surface preserved — matches PR #993 + #1009.

Stack:

• A1 (refactor(review): replace pending-marker file; add reviewer interfaces (1/3) #1105): env-var handshake + types
• A2 (refactor(review): add per-agent reviewers and orchestrator (2/3) #1106): per-agent reviewers + orchestrator + scope/prompt
• A3 (this PR): wire `entire review` command + delete legacy

What's in this PR

CU6 — Wire `entire review` command (`dd1482b36`)

User-visible surface (matches #993):
```
entire review # main command
entire review --edit # re-open the picker
entire review --agent # override which configured agent runs
entire review attach # post-hoc attach (discoverability shortcut)
entire review attach --force # skip confirmation
entire review attach --agent # agent that created the session
entire review attach --skills <s,...> # declare which skills ran
```

`--track-only` is intentionally NOT included (dropped by #1009).

Architecture:

• `cmd/entire/cli/review/cmd.go`: `NewCommand(deps)` + `runReview` dispatching launchable vs non-launchable agents
• `cmd/entire/cli/review/picker.go`: `pickConfig` + first-run setup + agent selection helpers
• `cmd/entire/cli/review/marker_fallback.go`: shared file for non-launchable agents (cursor, opencode, factoryai-droid, copilot-cli) — single registry, no per-agent fallback files
• `cmd/entire/cli/review_bridge.go` + `review_helpers.go`: small bridge files in cli package for cycle-bound functions (`headHasReviewCheckpoint` needs checkpoint→codex→review; `launchableReviewerFor` dispatches agent name → `*ReviewerTemplate`)
• Scope detection happens BEFORE the launchability branch — non-launchable agents also get the scope banner and a scope-aware prompt persisted to the marker (regression caught in code review)

CU7 — Documentation (`76fd30d2b`)

• CLAUDE.md `entire review` section rewritten to reflect: env handshake, AgentReviewer/Sink/Run, ReviewerTemplate, Sink serial-dispatch contract, scope detection, launchable vs non-launchable split
• Anti-feature list explicitly callouts what NOT to recreate
• AGENTS.md is a symlink to CLAUDE.md — auto-synced

Files changed:

• 10 new in `cmd/entire/cli/review/` (cmd, picker, attach, marker_fallback, status_test, plus tests)
• 2 new in `cmd/entire/cli/` (review_bridge, review_helpers)
• Modified: root.go (registers `review.NewCommand`), .golangci.yaml (adds `AgentReviewer` to ireturn allow-list), CLAUDE.md
• Deleted: `cmd/entire/cli/review.go` (1,513 lines) + `review_test.go` (~600 lines) — all still-used functions migrated; legacy detour code that was already torn out by Simplified entire review a bit #998/one more pass on top of feat/entire-review #1009 stays gone

Why bridge files in `cli/` package

Two real import cycles required the DI bridge:

1. `review → checkpoint → codex → review` (checkpoint imports codex, which imports review for env constants)
2. `review → claudecode/codex/geminicli → review` (per-agent reviewers import review)

The fix: `Deps` struct injection. `buildReviewDeps` (in `review_bridge.go`) constructs a `Deps` with cli-package implementations of cycle-bound functions; `review.NewCommand` accepts the `Deps`. The `review/` subpackage stays pure (only depends on `review/types`, settings, etc.).

Adding a new launchable agent in the future = one case in `launchableReviewerFor`. No edits to the `review/` subpackage.

Test plan

• `mise run check` (fmt + lint + test:ci) green
• Migration completeness: `grep -rn 'PendingReviewMarker' cmd/entire/cli/` shows only marker_fallback.go (intentional, for non-launchable agents)
• No `--track-only` references: `grep -rn 'track-only\|TrackOnly\|trackOnly' cmd/` returns empty
• Cobra subcommand registration: `entire review --help` shows `--edit`, `--agent`, `attach`
• Test coverage in new files: cmd_test (279), picker_test (322), attach_test (47), marker_fallback_test (139)

Smoke test (full command flow)

End-to-end verification that `entire review` works against a real binary in a real repo. Procedure:

```bash
git checkout feat/entire-review-v2-a3
go build -o /tmp/entire-smoke-a3 ./cmd/entire

Run unit + package tests against this branch

go test ./cmd/entire/cli/review/... \
./cmd/entire/cli/agent/{claudecode,codex,geminicli}/ -count=1

Verify command surface

/tmp/entire-smoke-a3 review --help # should list --edit, --agent, attach (no --track-only)
/tmp/entire-smoke-a3 review attach --help # should list --force, --agent, --skills

Verify no --track-only or stale references

grep -rn 'track-only\|TrackOnly\|trackOnly' cmd/ # expect empty
ls cmd/entire/cli/review.go cmd/entire/cli/review_test.go 2>&1 # expect 'No such file'

Optional: full end-to-end run with a real agent (~$0.02)

• Configure a review agent via .entire/settings.local.json with skills,
• then: /tmp/entire-smoke-a3 review --agent claude-code
• Expect: scope banner → "─────── claude-code review ───────" → narrative → "1 agent(s) done — 1 succeeded, 0 failed, 0 cancelled"
  ```

Expected: all package tests green; `--help` shows the right flags; legacy file is gone.

Actual output (verified locally on `feat/entire-review-v2-a3` @ 76fd30d):
```
ok cmd/entire/cli/review 1.604s
ok cmd/entire/cli/review/types 0.181s
ok cmd/entire/cli/agent/claudecode 3.640s
ok cmd/entire/cli/agent/codex 1.466s
ok cmd/entire/cli/agent/geminicli 1.334s

$ entire review --help
Flags:
--agent string select a specific configured agent (default: alphabetically first)
--edit re-open the review config picker
```
✓ Surface matches #993 (no `--track-only`).

Real-agent end-to-end smoke (configured `entire review --agent claude-code` against a live claude-code) requires API access and is documented in the optional procedure above for reviewers who want to verify with a real agent.

Anti-features explicitly NOT recreated

• `PendingReviewMarker` adoption from the lifecycle hook (PR-A1 already deleted this; PR-A3 confirms by deleting the legacy review.go that wrote it)
• `--track-only` flag (dropped by one more pass on top of feat/entire-review #1009)
• `--postreview` / `--finalize` / empty review commits / `/entire-review:finish` skill installer
• Per-agent fallback files (single `marker_fallback.go` for all non-launchable agents)

Behavioural parity with PR #993

Surface	#993	This PR
`entire review`	✓	✓
`entire review --edit`	✓	✓
`entire review --agent `	✓	✓
`entire review attach `	✓	✓ (delegates to `runAttachSurfaceReviewErrors`)
`entire review attach --force/--agent/--skills`	✓	✓
`--track-only`	✗ (dropped by #1009)	✗

---

Note

Medium Risk
Moderate risk because it replaces the entire review execution path (agent selection, scope computation, and launch/non-launch dispatch) and moves logic across new packages/DI boundaries; regressions could break review runs or marker/attach behavior.

Overview
entire review is now wired to the new review architecture (AgentReviewer + Run + sinks) and the legacy monolithic implementation is deleted, with a new review subpackage handling command routing, config picking, and launchable-vs-non-launchable flows.

Non-launchable agents now use a shared marker-file fallback (RunMarkerFallback), while launchable agents rely on the env-var handshake; the CLI package adds small bridge/helpers to inject cycle-bound dependencies and to keep review attach discoverable under entire review.

Docs and lint config are updated accordingly (new review settings schema/prompt support, updated CLAUDE.md section, and golangci ireturn allow-list), and tests are reorganized/added to cover the new command surface and marker/picker behaviors.

^{Reviewed by Cursor Bugbot for commit 11df159. Configure here.}

[Copilot]

Pull request overview

This PR completes the entire review refactor by wiring the new cmd/entire/cli/review/ architecture into the actual Cobra command registration and removing the legacy monolithic implementation (cmd/entire/cli/review.go + its tests). It also updates docs and lint configuration to reflect the new reviewer/orchestrator design.

Changes:

• Registers review.NewCommand(...) on the root command and introduces cli-package “bridge” helpers to avoid import cycles.
• Adds the new review command implementation (config picker, non-launchable marker fallback, command/help tests) under cmd/entire/cli/review/.
• Updates documentation/lint config and deletes the legacy review.go implementation + tests.

Reviewed changes

Copilot reviewed 16 out of 16 changed files in this pull request and generated 7 comments.

Show a summary per file

File	Description
cmd/entire/cli/root.go	Switches root command registration to cli/review.NewCommand(buildReviewDeps(...)).
cmd/entire/cli/review/cmd.go	New entire review command entry point + dispatch between launchable reviewers and marker fallback.
cmd/entire/cli/review/cmd_test.go	Verifies help surface, hook gating, non-launchable marker behavior, and --agent override behavior.
cmd/entire/cli/review/picker.go	Implements interactive review config picker, config persistence helpers, and agent selection helpers.
cmd/entire/cli/review/picker_test.go	Unit tests for picker helpers (merge/split/selection/SaveReviewConfig).
cmd/entire/cli/review/marker_fallback.go	Reintroduces marker file helpers + “manual agent” fallback path for non-launchable agents.
cmd/entire/cli/review/marker_fallback_test.go	Tests marker round-trip and marker fallback behavior/output.
cmd/entire/cli/review/status_test.go	Comment-only stub documenting why status logic tests live outside the subpackage.
cmd/entire/cli/review/attach_test.go	Tests entire review attach help surface and no-args behavior.
cmd/entire/cli/review_bridge.go	Builds review.Deps and provides launchableReviewerFor to break import cycles.
cmd/entire/cli/review_helpers.go	Hosts cycle-bound functions (headHasReviewCheckpoint, newReviewAttachCmd) in cli package.
cmd/entire/cli/attach_test.go	Updates attach tests to call review.SaveReviewConfig from the new package.
CLAUDE.md	Updates entire review documentation to reflect env-var handshake + new architecture.
.golangci.yaml	Adds review/types.AgentReviewer to the ireturn allow-list.
cmd/entire/cli/review.go	Deletes the legacy entire review implementation.
cmd/entire/cli/review_test.go	Deletes legacy tests, replaced by tests under cmd/entire/cli/review/.

[peyton-alt]

@BugBot review

[cursor]

✅ Bugbot reviewed your changes and found no new issues!

Comment @cursor review or bugbot run to trigger another review on this PR

^{Reviewed by Cursor Bugbot for commit 11df159. Configure here.}