I'll explore the repository to find review-related commands.

I found a CLI repo (Entire) written in Go. Let me find the review command definition.

Let me look at the review command definition to find all subcommands and flags.

Let me check the fix subcommand and attach to complete the picture.

Let me check the --fix flag reference and the attach subcommand details.

I now have a complete picture. Here are all the potential command combinations for entire review.

entire review — Command Combinations

Base command

Run-time overrides (combine with run)

Listing / discovery (no repo or profile needed)

Configure (--configure)

Other modes (mutually exclusive: --configure, --edit, --findings)

Subcommand: attach

Constraints / rules

• --configure, --edit, --findings are mutually exclusive
• --model requires --agent
• Profile can be passed positionally OR via --profile, not both
• Accepts at most one positional argument
• --set-* flags only meaningful with --configure
• Note: the old entire review --fix flow has been removed (referenced only in legacy comments)

The command itself is currently hidden from entire help (still maturing / labs feature), but works normally when invoked directly.

entire review attach is for retroactively tagging a review you already ran manually — i.e., a review that happened without going through entire review.

The problem it solves

Normally entire review launches agents for you, and the review session gets automatically captured and attached to the current commit's checkpoint as review metadata.

But sometimes you review code outside that flow — for example:

• You manually opened Claude Code / Codex / Gemini and asked it to review your branch
• A teammate ran a review session
• You used review skills ad-hoc in a normal agent session

In those cases the review happened, but Entire doesn't know it counts as a review — it's just a regular agent session. review attach lets you go back and label that existing session as an agent_review and link it to the current commit's checkpoint.

What it actually does

From the code (review_helpers.go):

• Takes an existing session ID
• Tags it as a review and attaches review metadata to HEAD's checkpoint
• Records the first user prompt in the transcript as the review prompt
• Optionally records which skills were run (--skills)

It's just a convenience alias

The docstring is explicit about this:

Equivalent to entire attach --review <session-id> — provided here for discoverability alongside the other review subcommands.

So entire review attach <id> and entire attach --review <id> do the same thing. The subcommand exists purely so people exploring entire review can discover it, rather than having to know about the --review flag on the separate attach command.

Bonus: pending review markers

There's a subtle extra behavior — it checks for a "pending review marker":

If a prior step left a marker (e.g. a review was started/expected), attach will auto-fill the agent name, skills, and prompt from that marker and then clear it. This smooths the case where the review flow was partially set up but the session needs to be linked manually after the fact.

Honestly, you've got a point — and it's worth checking whether the alias is truly just an alias or whether it's quietly diverged. Let me verify, because that changes the answer.

Good instinct — and digging in, it turns out the "it's just an alias" story is stale. The two commands have actually diverged, and that changes the picture.

They're not equivalent anymore

entire attach --review <id> (attach.go) is the plain version:

That's it — flag in, attach out.

entire review attach <id> (review_helpers.go) does extra work around pending review markers:

So the docstring's claim — "Equivalent to entire attach --review" — is no longer true. It's misleading and should probably be fixed.

The real reason it exists

It's the second half of the marker-fallback flow. Recall from review/cmd.go: when you run entire review against an agent that has no review-runner adapter, it can't drive the agent itself, so it calls RunMarkerFallback — it writes a "pending review" marker (agent, skills, prompt) and tells you to go run the review manually.

entire review attach is the designed completion step: you come back, run entire review attach <session-id>, and it picks up that marker to auto-fill everything and clear it. entire attach --review has none of that wiring.

So your critique still partly lands

• The discoverability justification in the docstring is weak — the parent review command is Hidden: true, so it's not surfaced in entire help anyway.
• The "it's an alias" framing is just wrong now and undersells the actual difference, which is the part that matters.

The honest version of the docstring would be something like: "Like entire attach --review, but also consumes a pending review marker left by entire review when the agent has no review runner." That's a real, non-redundant reason to exist — the current comment just describes it badly.

Want me to fix the docstring so it reflects what the command actually does?

Fair — let me actually trace the full flow instead of hand-waving, because the "why" only makes sense if I show you the journey end to end.

Now I've got the whole flow. The key thing nobody told you: review attach only exists for agents Entire can't drive itself. For the common agents you never touch it. Here's the actual split.

There are two kinds of review agents

1. Adapter-backed agents (claude-code, codex, gemini, pi)

entire review launches the agent itself. When it spawns the process, it sets ENTIRE_REVIEW_* env vars on it. The lifecycle hook sees those vars and tags the resulting session as a review automatically. You never run attach. This is the normal path and it does work automatically.

2. Adapterless agents (e.g. Cursor)

Entire has no adapter that knows how to spawn and drive these with the right environment. So entire review literally cannot start the review for you. Instead, RunMarkerFallback runs and:

• writes a marker file recording "you should review with agent X, these skills, this prompt"
• prints instructions telling you to open the agent yourself and paste the prompt

So, to your two questions

a) Why doesn't it work automatically?

Because for adapterless agents, Entire never controls the process spawn. The automatic path is: "Entire spawns agent → sets env vars → hook tags session." If Entire doesn't do the spawning, there are no env vars, and the hook just sees a normal session with no idea it was a review. There's no hook to fire because you opened Cursor by hand. The manual attach step is the only way to retroactively say "that session I just ran was the review."

b) Why is it another command?

It honestly mostly isn't — it's the same attach plumbing. It exists as a separate verb for two reasons:

• It's the back-half of the marker flow: it reads the marker to auto-fill the agent/skills/prompt so you don't retype them, then clears it.
• Discoverability — so people in the review world find it without knowing about attach --review.

My honest take

Your frustration is legitimate — this is a leaky abstraction. The real reason is a hard technical limit (can't programmatically drive Cursor), but the UX fallout ("go do it by hand, then come back and run a second command") is clunky, and the marker dance is mostly there to make that clunky path slightly less annoying. It's also self-admittedly transitional — the comments keep saying "does not have an adapter yet." The intent is clearly that as more agents get adapters, this whole manual path (and review attach with it) becomes vestigial.

So the mental model is: if your review agent has an adapter, ignore review attach entirely. It only exists to rescue the manual case.

Sharp question — and it gets at whether the env-var handshake is even necessary. The answer hinges on when we learn the session ID. Let me check what we actually know at spawn time.

Your premise is the right thing to challenge — but it turns out to be mostly false for exactly these agents, and the evidence is right here in env.go:

ENTIRE_REVIEW_SESSION=1 is a flag, not an ID. If we knew the session ID at spawn, that's exactly where it would go. We don't put it there because we don't have it yet.

Why we don't know the ID at spawn

For Claude Code / Codex / Gemini, the agent mints its own session ID internally. Entire forks the process, but the agent doesn't hand its session UUID back to us synchronously. We learn the ID only later, when the agent's lifecycle hook (e.g. UserPromptSubmit) fires — and that hook carries the ID the agent chose.

So the handshake is built the only way it can be:

1. Entire spawns the agent with ENTIRE_REVIEW_* env vars riding along.
2. The child process inherits them.
3. When the agent later announces a session via the hook, the hook process also inherited those env vars → it tags whatever session ID just appeared as a review.

The env var is essentially a note pinned to the process tree saying "any session born under here is a review" — precisely because we can't name the session in advance. The comment spells this out: each spawned agent inherits its own copy of the env, so multi-agent / multi-worktree attribution holds "by construction" without ever needing the ID up front.

"Couldn't we correlate it ourselves?"

In principle, yes — we spawned the process, so we know its PID and start time, and the agent eventually drops a transcript file. We could guess "newest session file from that PID after time T." But that's fragile timestamp/PID correlation, and it breaks with concurrent sessions, retries, slow writes, etc.

And notably — that guessing is exactly what entire attach auto-detection already does as the manual fallback. The env-var handshake is the robust version of the same idea: instead of guessing which session was the review after the fact, it lets the session tag itself from inside, at the moment it's created.

And the adapterless case

For Cursor et al., we don't spawn the process at all — you do, by hand. So we have neither channel: no env vars riding the process, and no PID to correlate against. That's the one case left with no automatic attribution, which is the entire reason review attach + the marker exist.

So the honest summary: for spawned adapter agents it is automatic, just not via a known session ID — via env inheritance, because the ID doesn't exist until the agent decides it does.

Good — this is the crux, and I shouldn't guess. Let me check how monitoring actually identifies sessions, because that determines whether your point lands.

Yes — and you've actually put your finger on the precise mechanism. We do identify sessions. The question is just where that identification happens, and once you see it, the env-var thing stops looking redundant. Look at how the hook tags a review (lifecycle.go, adoptReviewEnv):

The key: identification happens in the hook, not in entire review

The session ID comes in through the hook input JSON (hooks.go: SessionID string). The agent tells the hook its session ID. So the process that knows "which session this is" is the hook — and the hook is a grandchild of the review command:

So your instinct is exactly right: we identify the session. But we identify it at the hook, which is the moment the session announces itself. The env var isn't how we identify the session — it's how the spawn-time fact "this is a review" travels down to the place where identification already happens. The session tags itself from the inside, the instant it becomes known.

Why not let entire review do the attribution?

That's the real question, and the answer is: it could, but it would have to match a freshly-appeared session in shared state back to its own spawn — by agent + time + base SHA. That's the fragile correlation, and the codebase knows it:

• The marker code explicitly warns that "multiple worktrees in one repo share .git/entire-sessions/, so without [scoping] any session in any worktree could race to claim the marker."
• And they literally do fall back to that fuzzy matching — but only as a last resort. In manifest.go, matchReviewSessionState correlates by worktree path → start time → agent → model to build the findings manifest after the run. The comments around it flag it as the degraded path.

So the design is: prefer self-tagging via env inheritance (impossible to mis-attribute across concurrent agents/worktrees — it rides the process tree), and fall back to fuzzy correlation only when that's unavailable.

Notice the three guards in adoptReviewEnv — EnvSession==1 and agent match and SHA match. Even inside the "safe" inheritance model they triple-check, because attribution-by-matching is precisely the thing that goes wrong. Env inheritance lets them avoid matching for the primary path entirely.

So tying it back to your original complaint

• Spawned adapter agents: identified by the hook, tagged from the inside via inherited env. Fully automatic. ✓
• Adapterless agents (Cursor): we never spawn them → no env rides along → the hook (if any) has nothing to read → no automatic tag. The session still gets identified, it just can't be attributed to a review automatically. That gap is exactly what review attach + the marker fill, using the same fuzzy-match-ish adoption after the fact.

So you're correct that identity exists. The reason it's not automatic for the manual case isn't that we can't identify the session — it's that we have no trustworthy channel to say "this identified session was the review" without the env inheritance, and the alternative (correlate-and-guess) is the fragile path they deliberately demote to fallback.

You're right about the timing — and that's worth saying clearly, because it means the env var isn't doing what I implied. Let me concede that part and then show where the proposal breaks.

You're correct: attribution is already lazy

The env var is set at spawn but consumed lazily — adoptReviewEnv runs inside the hook (UserPromptSubmit), i.e. once the session exists and is identified. Nothing is attributed at spawn time. So "do it when the hook fires, not at spawn" isn't a change to the timing — that's already how it works. The env var is just a durable label sitting there waiting for whenever the hook eventually fires.

So the real disagreement isn't when — it's what key we match on:

• Today: process ancestry, encoded as inherited env (ENTIRE_REVIEW_SESSION=1).
• Your proposal: content — "next session for agent X at base SHA Y."

Where "match on next hook by agent + SHA" breaks

It can't disambiguate concurrent or subsequent same-agent sessions. Concretely:

• You run entire review (expecting a claude-code review at HEAD abc).
• In another pane/worktree you've also got a normal claude-code coding session going, same repo, same HEAD.
• Both fire hooks as agent claude-code with BaseCommit == abc.

Your matcher matches both. Whichever hook fires first wins — so a plain coding session gets silently mis-tagged as the review, and the actual review is missed. The three guards in adoptReviewEnv (session flag + agent + SHA) deliberately aren't enough on their own; the thing that makes it unambiguous is that only the process born under entire review inherited the flag. That's a property no content-match can reconstruct.

This is exactly why the marker is not auto-adopted

This isn't hypothetical — it's the documented reason the adapterless path is manual. From marker_fallback.go:

The marker is NOT auto-adopted by anything — the lifecycle hook reads ENTIRE_REVIEW_* env vars on the spawned process, not the marker file.

They have a pending-review record (the marker) and the hook could read it. They chose not to, precisely because "tag the next claude-code session" would mis-fire on any unrelated same-agent session. So they require entire review attach <session-id> — and the whole point of that command is that the user supplies the one piece of disambiguating identity the system can't infer: the specific session ID.

So the answer to your whole thread collapses to this: the manual step isn't there because we can't identify or can't defer. It's there because, for a process we didn't spawn, only the user knows which of the candidate sessions was the review.

But — honestly, your instinct points at a real gap

The naive "next hook" auto-adopt is unsafe. A guarded version is not:

• If, at the moment a hook fires, there's a pending marker for that agent+SHA and exactly one candidate session in scope → auto-adopt. Fall back to manual attach only when it's genuinely ambiguous (≥2 candidates).

That covers the overwhelmingly common solo-developer, one-session-at-a-time case and would make review attach unnecessary most of the time — while preserving correctness in the concurrent case. The current design just picks the always-safe-but-always-manual end of that tradeoff, probably because the env path already makes the spawned agents fully automatic, leaving only Cursor-style agents in the cold and not worth a heuristic yet.

That'd be a reasonable thing to actually build. Want me to sketch where it'd hook in (adoptReviewEnv is the natural seam — add a marker-based adoption pass after the env pass fails, gated on a single-candidate check)?

1. Make attach automatic

Reuse what's already there — the marker + the hook seam.

• entire review already writes the pending marker (agent, base SHA, skills, prompt, worktree, time). Keep that for every review, not just adapterless ones.
• In the hook, right after the env-var pass fails (adoptReviewEnv), add a marker pass:
  1. Read the pending marker.
  2. Does it match this hook's agent + base SHA + worktree?
  3. Count candidate sessions that fit. If exactly one (this one) → tag it agent_review, copy skills/prompt from the marker, clear the marker.
  4. If two or more match → do nothing, leave the marker. (ambiguous, stay safe)
• Result: spawned agents still self-tag via env (unchanged). Manually-started agents now self-tag too, the moment their hook fires — as long as nothing else is ambiguous.

2. Drop entire review attach

Agreed. It earns nothing:

• The user needs the session ID anyway.
• entire attach --review <id> already does the tagging.
• Add one small touch: when --review runs, if a pending marker matches, auto-fill skills/prompt from it and clear it (the only useful bit review attach had).

So: delete the review attach subcommand, fold its marker-fill into attach --review, and let the hook handle the common case automatically.

Net effect: most reviews need no attach at all; the rare ambiguous case falls back to entire attach --review <id>.

Want me to implement the marker pass in adoptReviewEnv and remove the subcommand?

Fair, the word is bad. Let me fix it.

There are two ways the agent process gets launched:

1. Entire launches it. You type entire review, and Entire itself starts claude-code/codex/gemini for you. These get the env vars → self-tag. (already works)

2. You launch it. For agents Entire can't drive (e.g. Cursor), entire review can't start them. It just prints "open Cursor yourself and paste this prompt." You open the app by hand. No env vars → today it can't self-tag, so you must run attach afterward.

By "manually-started" I meant case 2 — you opened the agent, Entire didn't.

You're right that you type entire review either way. The difference isn't who runs the command — it's who starts the actual agent process: Entire, or you. My fix (the marker pass in the hook) is about making case 2 self-tag too, so you don't have to attach by hand.

Right, let me be blunt about what entire review actually does, because it's not one thing:

• For claude-code / codex / gemini: entire review really runs the review. It launches the agent, feeds it the prompt, captures the result. Fully automatic. This is the case you're picturing.

• For Cursor (and similar): entire review can't run anything. Entire has no code to drive Cursor. So the command just prints a message like "Cursor has no review runner yet — open it yourself and paste this prompt" and writes the marker. That's it. Nothing actually reviews until you go do it by hand in Cursor.

That second case is what I clumsily called "you launch it." It's still entire review — but for those agents the command is basically a glorified "here's your prompt, go paste it" note, not an actual review run.

So:

• entire review = automatic only for agents that have an adapter.
• For the rest, it's manual, and that's the only reason attach exists at all.

Is that the part that wasn't clear — that entire review doesn't actually run the review for every agent?

Checked the code. Here's the real status of Pi:

Pi is integrated as an agent, but has no review adapter yet.

• cmd/entire/cli/agent/pi/pi.go exists — Pi has hooks, session capture, token counting, transcript parsing. Sessions are identified and monitored, just like Claude.
• But launchableReviewerFor (review_bridge.go) only returns a reviewer for claude-code, codex, gemini. For "pi" it hits default: return nil.

So today entire review --agent pi falls into the marker fallback — same dead-end as Cursor.

The good news: Pi already has the two things Cursor lacks — it fires hooks and inherits env. So the only missing piece is a pi.NewReviewer() that knows how to launch Pi with the review prompt. Add that, wire one case "pi": line, and:

• entire review launches Pi itself → sets ENTIRE_REVIEW_* env → Pi's hook self-tags the session.
• Fully automatic. No attach, no marker, no second command.

So for Pi the whole "attach" problem we've been arguing about simply disappears once the adapter exists. Want me to write pi.NewReviewer() and wire it into launchableReviewerFor?

Let me find exactly what a reviewer needs and whether Pi can satisfy it (can Pi run headless with a prompt?).

Let me confirm Pi can run headless with streaming output (the one hard requirement for an automatic reviewer).

Pi has headless -p mode and a JSON output mode. Let me confirm the streaming JSON format the parser would consume.

Good news: I checked everything. Nothing fundamental is missing. Pi already has the three things required for fully automatic review:

1. Headless mode: pi --mode json "<prompt>" — runs non-interactive, streams JSON events (agent_start, message_update, tool_execution, agent_end with token usage).
2. Hooks: Pi fires lifecycle hooks → the shared adoptReviewEnv already self-tags any agent's session. Agent-agnostic, works for Pi for free.
3. Env inheritance: Pi is a child process of whatever spawns it → inherits ENTIRE_REVIEW_*.

So no half-assed marker/attach dance is needed for Pi. It can be exactly as automatic as claude-code.

Also: part 2 is already done. entire attach --review <id> already exists (with --skills). Nothing to add there.

What's actually left to build

A. The Pi reviewer adapter (cmd/entire/cli/agent/pi/reviewer.go) — mirror claude's:

• BuildCmd: exec(ctx, "pi", "--mode", "json", prompt), add --model if set, env = review.AppendReviewEnv(os.Environ(), "pi", cfg, prompt).
• Parser: map Pi's JSON lines → review Events:
  • agent_start → Started
  • message_end / text deltas → AssistantText
  • tool_execution_start → ToolCall
  • agent_end → sum usage → Tokens, then Finished{Success:true}
  • scanner error → RunError + Finished{false}

B. Wire it in (review_bridge.go), one line:

C. Drop entire review attach: remove newReviewAttachCmd, its marker bridge, and the AttachCmd wiring. Keep entire attach --review as the only manual fallback.

That's the whole job. Result: entire review --agent pi launches Pi, it self-tags, no attach ever needed.

Want me to implement A, B, and C now?

Let me gather what I need to implement correctly — the ReviewerTemplate, event types, and Pi's content shape.

Let me read the exact Event types and Pi's content parsing to map them correctly.

Let me verify Pi's --model flag and the content block shape before writing.

Confirmed: pi --mode json "<prompt>" with --model <pattern>. I have everything.

First, your question — what --skills does on entire attach:

It's declarative metadata only. It records which review skills you claim were run in that session, stored as state.ReviewSkills. From attach.go:

Key points (from resolveReviewSkills):

• Only meaningful with --review.
• It does not run or verify anything — it's a label you assert after the fact.
• It deliberately ignores your configured profile skills, because "what I'd run" ≠ "what actually ran." Lying would misrepresent the session.
• Empty is fine — the session is still tagged a review via Kind + the recorded prompt. Skills are just a queryable convenience.

So: --review = "this was a review"; --skills = "...and these specific review skills were used."

Now let me implement A, B, C.

Now B — wire it into launchableReviewerFor:

Now C — drop entire review attach. Let me find all its wiring.

I'll fold the marker auto-fill into entire attach --review (the only unique behavior review attach had), then remove the subcommand. Let me read attach.go's RunE.

Now I'll fold the marker logic into entire attach's RunE and remove the review attach wiring. Editing attach.go:

Now add the cliReview import to attach.go and trim review_helpers.go to just the marker helper:

Now remove the AttachCmd plumbing from review_bridge.go, root.go, and review/cmd.go:

Now remove the AttachCmd field and its registration in review/cmd.go:

Let me build and see what breaks.

Builds clean. Now let me check the test files that referenced the removed wiring:

These tests exercise the removed review attach subcommand. Let me migrate them to entire attach --review. Let me read them all.

This file only tests the removed subcommand. Deleting it (the attach --review flag coverage lives in attach_test.go):

These two tests verify the marker-adoption and skills behaviors I folded into attach --review. Migrate their args:

Let me check for any remaining references and run the affected tests.

Let me update the now-stale doc comments referencing the removed subcommand:

Now let me run the relevant tests (reviewer parser, attach, review):

All passing. Now let me add a test for the Pi reviewer parser to lock in the behavior (no half-assed integration), and verify it against a realistic Pi JSON stream:

Now I'll write the Pi reviewer test mirroring the claude one, covering name, env, argv, model, and parser behavior:

Let me run gofmt, vet, and the broader review/cli test suite to confirm nothing else regressed:

Let me run the integration review test to confirm the migrated attach --review path works end-to-end:

All green. Done — A, B, and C are implemented and tested.

Summary

A. Pi reviewer adapter — cmd/entire/cli/agent/pi/reviewer.go

• NewReviewer() builds pi --mode json <prompt> [--model …], attaching ENTIRE_REVIEW_* env via AppendReviewEnv.
• Parser maps Pi's JSON event stream → review events: agent_start→Started, assistant message_end→AssistantText, tool_execution_start→ToolCall, summed usage→Tokens, agent_end→Finished{true}. Handles array or string content, torn streams, and non-assistant messages.
• Full test coverage in reviewer_test.go (name/registry lock, env, argv±model, stream parsing, failure & edge cases).

B. Wired in — review_bridge.go: added case "pi": return pi.NewReviewer(). Now entire review --agent pi launches Pi, which self-tags via the existing hook + env path — no marker, no attach.

C. Dropped entire review attach

• Removed the subcommand, Deps.AttachCmd, and all its wiring (root.go, review_bridge.go, review/cmd.go, review_helpers.go).
• Folded its one unique behavior (consuming a pending-review marker: auto-fill agent/skills/prompt, then clear it) into entire attach --review, which already existed.
• Migrated/removed the tests that drove the old subcommand; updated stale doc comments.

Re: your --skills question — it's declarative metadata only (state.ReviewSkills): asserts which review skills were run, valid only with --review, runs/verifies nothing.

Net result: for Pi (and claude/codex/gemini) review is fully automatic. entire attach --review <id> is the only manual fallback, and it's needed solely for agents Entire can't launch itself (e.g. Cursor).

Everything builds, gofmt/go vet clean, unit + integration tests pass.

Good question — let me check whether skills are already extracted from the transcript automatically.

This is the key distinction. There are two separate fields — let me confirm how attach populates them.

Sharp catch. There are actually two different skill fields, and they behave differently:

1. state.SkillEvents — evidence-based, auto-extracted from the transcript (slash-commands, skill tool-use). But it's only populated by the live hook path (lifecycle.go:1130). And only some agents implement the extractor — today just claude-code.

2. state.ReviewSkills — what --skills sets. It's a declared snapshot ("this review covers these skills"), not derived from the transcript.

So to your question, split by path:

• Automatic review (spawned agents incl. Pi): yes — ReviewSkills is already filled in for you. It comes from the profile config through the env handshake (EnvSkills). --skills is irrelevant there; you never touch it.

• entire attach --review: no — and this is the gap. Attach currently:

  • does not auto-derive ReviewSkills, and
  • does not run skill-event extraction either.

  So --skills is the only source of skill info on attach today. That's exactly why the flag exists.

But your instinct is right: attach has the transcript, and claude-code has an extractor. We could run ExtractSkillEvents on attach, take the distinct skill names, and default ReviewSkills from them — turning --skills into an override instead of the sole source. Evidence-based, fully automatic.

Caveat: only works for agents with an extractor (claude-code now; Pi/codex/gemini would need one), so --skills stays as the manual fallback for the rest.

Want me to wire that auto-derivation into attach --review (default skills from extracted SkillEvents, --skills overrides)?

Let me verify properly — I may have grepped too narrowly.

You're right — I was wrong. Skill identification isn't claude-only. There are three mechanisms:

1. Claude Code — transcript extraction. ClaudeCodeAgent.ExtractSkillEvents (implements the SkillEventExtractor interface). Parses the transcript after the fact to find skill tool-use.

2. Pi — live hook payload. pi/lifecycle.go — Pi's hook reports skill_events directly in its payload (piSkillEventInput → piSkillEvents). Skills come in live, not by re-parsing a transcript.

3. Generic pipeline. Any agent's event.SkillEvents flow into state.SkillEvents via appendEventSkillEventsToState, plus the skilldiscovery package (keyword matcher: review/audit/inspect/…) that the review picker uses to find review-adjacent skills.

So skill capture is general — claude, pi, and a shared pipeline. My mistake.

But one important nuance for the attach question: those two agent-specific paths capture skills differently —

• Claude: from the transcript → usable at attach time (you only have the transcript on disk).
• Pi: from the live hook payload → not re-derivable at attach time, because Pi doesn't implement SkillEventExtractor (only claude does).

So for auto-deriving ReviewSkills on attach --review:

• claude-code: works now (transcript extractor exists).
• pi: would need a transcript-based ExtractSkillEvents (its JSONL does record slash-command/skill invocations, so it's doable — just not implemented).
• others: fall back to --skills.

So the corrected picture: skills are identified across agents during live runs; the gap is specifically transcript-time re-extraction for attach, which only claude has today. Want me to (a) wire claude's auto-derive into attach --review now, and (b) add a Pi transcript ExtractSkillEvents so Pi gets it too?

I'll commit and push the current changes first, then investigate Pi's skill extraction.

Committed. Now let me push.

Pushed successfully. Now let me investigate Pi's skill handling — first confirm the current state precisely.

Now the picture is clear:

• Live entire review --agent pi: skills are captured (hook payload → piSkillEvents → state.SkillEvents). ✓
• Transcript/attach-time: Pi does not implement SkillEventExtractor, so invoked skills can't be re-derived from a saved transcript. ✗

Let me understand how skill invocations appear in Pi's transcript so I can implement extraction. Let me look at claude's extractor as the model and Pi's transcript format.

Let me check how skill invocations actually appear in a Pi transcript — looking for test fixtures.

Let me check how skill invocations are recorded in the transcript vs. only in the live hook, by examining the embedded Pi extension and session format.

Let me check the working tree state and how I should branch for the separate PR.

Clean tree, gh available. Let me create a separate branch from origin/main for the Pi skills work.

Now on a clean branch from main. Let me understand how skill invocations are recorded in the Pi transcript by examining the extension that produces skill_events.

The extension detects skills via regex ^/skill:name on raw input text. The critical question: does the transcript preserve that literal /skill:name, or only the expanded content? Let me determine this from the session format and fixtures.

Let me verify empirically whether Pi stores the literal /skill: in the transcript by checking pi's own source for how skill commands are recorded.

Definitive finding. When a user runs /skill:name, Pi's _expandSkillCommand replaces the literal with an expanded block before storing the user message:

So the literal /skill:name is gone from the transcript — but the expansion leaves an explicit, structured <skill name="..."> marker. That's the transcript signal to extract on. Let me check who calls the generic extractor, to avoid duplicating the live-captured events.

There's already an AppendPromptSlashCommandSkillEvent at lifecycle.go:463 and a mergeSkillEvents dedup. Let me examine both — this may already cover Pi.

Big finding — there's already generic slash-command skill detection (SkillEventFromPromptSlashCommand) that even normalizes Pi's /skill:<name>. But it runs on the prompt, which for Pi is the expanded text (<skill name=...>), not /skill:name. Let me confirm the dedup paths and whether the generic path helps at transcript time.

This is genuinely intricate — there's a real duplicate-vs-native risk through mergeSkillEvents (keyed by ts-based ID). Let me check what the native Pi event's ID scheme is depended on, so I can design dedup-safe extraction.

The native Pi event uses a timestamp-based ID for cross-turn uniqueness, which won't match a transcript-derived event — so duplicates would appear in live runs through mergeSkillEvents. The robust fix is semantic dedup (empty ID → composite key including TurnID). Let me check nothing depends on the pi-skill- ID format.

The grep exit was just "no matches" on the last one. Two important findings:

1. Nothing depends on the pi-skill- ID format (only a synthetic fixture).
2. attach.go never calls ExtractSkillEvents — attach doesn't populate skill events for any agent.

This reframes everything. Let me confirm the architecture: does claude-code use live hook capture too, or only transcript extraction? That determines whether Pi adding an extractor would duplicate.

I went deep before writing any code — and the "double check" paid off. Pi already supports invoked skills. Adding a transcript extractor would actually be harmful, not just redundant. Here's the evidence:

The two models

The codebase uses one skill-capture model per agent, not both:

So Pi already captures every /skill:name invocation in all live sessions, including entire review --agent pi — via the hook path (piSkillEvents), confirmed by TestParseHookEvent_BeforeAgentStart_WithSkillEvent.

Why I should NOT add a Pi transcript extractor

1. The literal is gone from the transcript. Pi's _expandSkillCommand rewrites /skill:name into a <skill name="...">…</skill> block before storing the user message. The only durable signal is that expansion block.

2. It would duplicate. At condensation, agent.ExtractSkillEvents is merged with the live state.SkillEvents via mergeSkillEvents. Pi already has the native event there. A transcript-derived event can't dedup against it cleanly:

   • Native IDs are timestamp-based (per-invocation) — unreproducible from the transcript.
   • Falling back to the composite key fails too: the extractor runs in the current turn and stamps all extracted events with the current TurnID, while native events carry their original TurnID. Skills from earlier turns → mismatched keys → duplicate skill events in checkpoint metadata.

3. attach calls neither path — for any agent. So the attach-time gap I mentioned earlier isn't Pi-specific; it's the same for claude. Nothing to fix in Pi for that.

So: nothing to implement here

The honest conclusion is that this separate PR shouldn't add an extractor. The branch pi-invoked-skills-extraction is currently empty (clean off main).

What would be genuinely valuable and safe:

• A regression test locking Pi's live skill-capture for the review flow (guards the "already works" guarantee), and
• A short comment on launchableReviewerFor/piSkillEvents noting Pi uses live-capture so no future dev adds a duplicating transcript extractor.

Want me to make that the PR? Or, if you'd still like transcript extraction (e.g. for a future attach-side backfill), I can do it properly — but it requires reworking skill-event dedup to be identity-stable across the live/transcript boundary, which is a bigger change than it looks.

I'll make that the PR. Note this branch is off main, so the Pi reviewer (the review flow) lives on the other branch — here I'll lock the live-capture mechanism and the architectural decision (Pi is intentionally not a transcript extractor), with a guard comment. Let me confirm the capability helper.

Now let me add the guard comment to piSkillEvents and the architectural regression test.

Now the regression test locking both the live-capture guarantee and the no-transcript-extractor decision:

Tests pass, gofmt clean. Let me commit and push, then open the PR.

The apostrophes broke the heredoc. Let me write the body to a file.

PR opened: https://github.com/entireio/cli/pull/1383

Now let me switch back to the review-profiles branch as you asked.

Done. Summary:

Separate PR (Pi invoked-skills) — opened

PR #1383 on branch pi-invoked-skills-extraction (off main).

After deep investigation, the honest outcome: Pi already captures invoked skills via its live-capture model, and adding a transcript extractor would double-count at condensation. So the PR documents and guards that decision rather than implementing a harmful extractor:

• Guard comment on piSkillEvents.
• TestPiAgent_UsesLiveSkillCaptureNotTranscriptExtraction — fails if someone makes PiAgent a SkillEventExtractor.
• TestParseHookEvent_BeforeAgentStart_MultipleSkillEvents — locks multi-invocation live capture.

No behavior change; tests + docs only. Build/gofmt/tests green.

Back on review-profiles

Switched back, working tree clean, up to date with origin. The earlier work here (Pi review adapter + dropping entire review attach) is already committed and pushed.

Ready for whatever's next on the review-profiles PR.

Trimmed and pushed to PR #1383. The comments are now concise:

• piSkillEvents: 4 lines (was ~15) — states it's Pi's only path and points to the guard test.
• The two test comments: 2–3 lines each, just the intent + cross-reference.

Tests still pass, gofmt clean. Want me to switch back to review-profiles?

Back on review-profiles. Let me read the interactive flow code to describe the menus accurately.

Back on review-profiles. Here's the complete picture of entire review.

entire review — full reference

Flags

Rules: --configure/--edit/--findings are mutually exclusive · --model requires --agent · profile via positional or --profile, not both · at most one positional arg · command is Hidden (works, but not in entire help).

Decision flow (RunE)

Use cases

Single worker → runs that agent. Multiple workers → fan-out: all run concurrently, then the master synthesizes the final report.

Interactive flow & menu output

A. First run (no profile yet) — entire review

Step 1 — review type:

Step 2 — worker agents (only if >1 launchable agent installed):

Step 3 — models?:

If "Choose models", per worker:

Then optionally model variants (run same agent twice w/ different models):

Step 4 — master (only if >1 worker):

Saved + confirm:

B. Configure without running — entire review --configure

Same wizard as above, but ends with:

Non-interactive (no TTY) prints the discovery catalog instead:

C. Advanced edit — entire review --edit

Per installed agent, multi-select menus:

D. Re-review guard (HEAD already reviewed)

(Non-interactive: prints a note and proceeds.)

E. The run itself

Before agents start, a scope banner:

• TTY: live TUI dashboard (per-agent progress), then narrative dump, then — for fan-out — the master's synthesized final report.
• Non-TTY (CI / agent host): Running review with <agent>... + narrative dump (+ auto-synthesis for fan-out). No TUI.

Afterward, findings are persisted (footer points to entire review --findings); if persistence fails you get a "findings were not persisted" note.

Note: entire review attach no longer exists — manual tagging is entire attach --review <id> (which auto-consumes any pending-review marker).

I'll merge the latest main into the current review-profiles branch.

Note: PR #1383 (Pi skills work) is already merged into main. Let me merge main in.

A fast-forward is unexpected — that implies my review-profiles commits weren't on this branch. Let me verify my work is intact.

My work got bypassed — the local review-profiles was at bfb5baca5, not my commit 1d8ceda2d, so the merge fast-forwarded over it. My commit still exists. Let me investigate before doing anything destructive.

Conflicting signals — origin/review-profiles correctly has my commit 1d8ceda2d. Let me get ground truth on the current state.

Found it — I'm on the wrong branch. HEAD is on feat/trail-review-cli (someone else's merged branch), not review-profiles. My earlier git switch review-profiles evidently didn't stick (the repo HEAD has been moving due to other activity in this checkout).

Good news: review-profiles and origin/review-profiles both correctly point at my commit 1d8ceda2d. My accidental fast-forward only moved the local feat/trail-review-cli branch (not pushed, harmless). Let me switch to the right branch and merge there.

Now correctly on review-profiles with my work intact. Let me merge origin/main.

Just an import-ordering conflict — I keep my added paths + cliReview imports:

paths is no longer used in attach.go (main removed its usage); cliReview is used. Remove the stale paths import:

Build passes. Let me run the affected tests, then commit the merge.

All green. Let me stage the resolved file and commit the merge.

Merge committed. Let me verify everything is intact and push.

Done. origin/main is merged into review-profiles and pushed.

What happened

• ⚠️ Caught a problem first: HEAD was actually on feat/trail-review-cli (the checkout had drifted to another branch from external activity), so my first git merge accidentally fast-forwarded that branch. No harm — it wasn't pushed, and your work was safe on review-profiles/origin/review-profiles.
• Switched to the correct review-profiles branch and merged origin/main (19 commits) there.

Merge resolution

• One conflict, in cmd/entire/cli/attach.go — just import ordering. Kept the cliReview import (used by the marker-fill logic), and dropped paths since main removed its usage there.

Verified

• Build passes, gofmt clean.
• Tests green: cli, agent/pi, review.
• My work intact: pi/reviewer.go present, Pi wired into launchableReviewerFor, entire review attach still removed.
• origin/main fully merged.

Merge commit 6b6b776e1 pushed to origin/review-profiles.

Good idea — it maps better to the actual data model ("a worker = agent + model") than the current 4-step flow. Let me ground my assessment in the real code constraints before saying it's easy.

This is a genuine improvement, and importantly it matches the data model you already have. Today a worker is keyed by agent+model: profile.Agents[workerName] where workerName = workerIDForAgentModel(agent, model, …), so the same agent on two models is already two workers. Your mock is just a faithful UI for that — checking two models under Claude Code = two claude-code workers. No model changes needed.

It also collapses the current 4 steps — "Which agents?" → "Choose models?" (y/n) → per-agent model picker → "Add another variant?" loop — into one screen. The variant loop especially is awkward today; your design makes multi-model obvious (just check two boxes).

Feasibility — two tiers

Tier 1 — native huh, ~90% of the mock (easy, recommended first): One huh.Group containing one MultiSelect per agent, stacked on a single screen with the agent name as each field's title:

Pre-check defaults via .Selected(true). This is straightforward and accessible-mode works for free.

Tier 2 — exact mock (custom bubbletea component): A single flowing list with non-selectable group headers, one cursor crossing groups, and the live 5 worker(s) selected footer. huh MultiSelect can't do non-selectable header rows or a cross-field computed counter, so this needs a custom component (like the review TUI sink) plus its own accessible fallback. Noticeably more code.

Edge cases either tier must handle

• custom… can't collect text inside a multiselect → a short follow-up Input per agent that has custom checked (still far fewer screens than today).
• Agents with no advertised models (ListModels empty) → show only custom… (+ optional default). Need to fetch model lists up front for all candidate agents.
• default semantics: today "Default" stores "" (track the agent's own default); your mock pre-checks an explicit id like sonnet. Either keep a default pseudo-row that stores "", or accept storing explicit ids (clearer, minor behavior shift).
• Master step still follows when ≥2 workers (mock doesn't show it).
• Validation: ≥1 worker total; zero models under an agent = that agent isn't a worker (this is what replaces the separate agent-select step).

My take

Worth doing. I'd ship Tier 1 first — it captures the entire UX win (combined agent+model, multi-model via checkboxes, one screen) with native components and low risk, replacing promptForSimpleReviewAgents + promptForSimpleReviewModels + the variant loop. Tier 2 is a polish pass if the exact single-list/live-counter look matters.

Want me to implement Tier 1 in RunReviewGuidedSetup?

Let me read the full set of functions I'll be replacing and the helpers I'll reuse.

Now let me check ModelInfo fields and what references the functions I'll replace.

ModelInfo has no default flag, so I'll pre-check a "Default" pseudo-entry (stores "") — clean and matches current semantics. Let me check tests referencing the functions I'll remove.

No test references — clean to remove the three replaced functions. One concern surfaced: defaultReviewAgentConfig returns a zero config for Pi (and anything outside claude/codex/gemini). Let me check ReviewConfig / IsZero and defaultReviewMaster to handle Pi correctly.

IsZero() is false when Agent is set — so setting cfg.Agent makes every worker valid, including Pi (whose default config is empty skills). That also fixes a latent gap where guided setup silently skipped Pi. Let me read the exact region to replace.

modelInList is used by a test, so I'll keep it; agentNamesToTypes and promptForModelChoice become dead and must go (lint). Let me read their exact text for clean removal.

I have the full picture. Now making the edits — replacing the agent-select + "choose models?" + variant loop with a single combined crew picker.

Now replace the two old functions with the combined crew picker:

Build passes. Let me run gofmt, vet, and the review/cli tests.

Let me run the linter to confirm no unused issues (e.g. modelInList now only used by a test).

0 lint issues. Let me run the broader CLI tests then commit.

Done — Tier 1 implemented and pushed to review-profiles.

What changed

Replaced the 4-step guided flow with a single "Build the review crew" screen — one multiselect per launchable agent, listing its models plus Default and Custom…:

• Multi-model = multi-worker is now native (check two boxes), replacing the hidden "Add another variant?" loop.
• Default pre-checked → pressing Enter gives one worker per agent on its default model.
• Custom… triggers a short follow-up text input per agent.

Code

• Added promptForReviewCrew + resolveCrewModel; removed promptForSimpleReviewAgents, promptForSimpleReviewModels, promptForModelChoice, agentNamesToTypes (net −72 lines).
• Workers always set cfg.Agent, which (since IsZero() is false once Agent is set) fixes a latent gap: prompt/model-only agents like Pi are now selectable in guided setup — they were previously silently skipped because their default config is empty.
• Master selection still follows when ≥2 workers; the advanced --edit picker is untouched.

Verified

Build, gofmt, go vet, golangci-lint (0 issues), and cli + review tests all pass. Commit 7382fdb42 pushed.

One note: the interactive crew function itself isn't unit-tested (huh forms can't run headless — same as the functions it replaced). The worker-keying/dedup primitives it relies on (workerIDForAgentModel, defaultReviewAgentConfig) are already covered. If you want, I can extract the selection→profile mapping into a pure helper and add a focused test.

Good — this is a real modeling question. Let me separate what already works from what doesn't.

The data model already supports it

profile.Agents is map[workerKey]ReviewConfig, keyed by an arbitrary worker name, not by agent. Each worker = {Agent, Model, Skills, Prompt}, and workerIDForAgentModel already disambiguates collisions (claude-code:opus, claude-code-2, …). So "5 Claudes with different models" — or even 5 identical ones — is fully representable in settings today. The name Agents is misleading; it's really a list of worker slots.

So the gap isn't the storage — it's the two configuration surfaces that are still agent-keyed:

1. The crew picker (Tier 1) — it's organized as "per agent, check models," and dedupes by model. So you can make 5 Claude workers (check 5 models), but: you can't have two slots with the same model, you're limited to advertised models unless you use Custom, and the mental model is "models per agent," not "N slots."

2. The scripted path (--set-agents a,b,c) — builds a map keyed by agent name, so --set-agents claude-code,claude-code collapses to one. No way to express repeats.

Proposed reframe: slots

Make both surfaces slot-first.

Picker — replace the model grid with a slot list:

Each slot: pick agent → pick model (list/Default/Custom). Add/remove freely. This directly expresses "define N slots," allows any number of same-agent slots, and drops the dependence on advertised model counts.

Scripted — add a repeatable slot flag:

--set-slot agent[=model], repeatable. Keep --set-agents as a shorthand (one default-model slot per agent).

One thing to confirm before I rip out Tier 1

Do you actually need duplicate identical slots (same agent and model, e.g. two claude-code:sonnet for ensemble/voting), or only multiple-same-agent-different-model (which Tier 1 nearly does already)?

• If only different-model: I can keep Tier 1's grid and just lift the model-dedup + add the scripted --set-slot — smaller change.
• If you want true arbitrary slots (incl. duplicates): the slot-list picker above is the right model and I'd replace Tier 1's grid with it.

Which one — and should master selection also become slot-targeted (pick the slot that writes the final report)?

Got it — true arbitrary slots, duplicates included. I'll replace Tier 1's grid with a slot-list picker and add a scripted --set-slot. Let me first check the existing config tests so I don't break the scripted path's contract.

Crucial constraint from TestBuildConfiguredProfile_FromFlags: --set-agents must keep worker keys equal to the agent name (so --set-model claude-code=opus lands on key claude-code). I'll keep that path intact and add --set-slot alongside. Now the picker — replacing the grid with a slot list:

Now the scripted --set-slot flag. Let me read the relevant cmd.go regions.

Now wire --set-slot into cmd.go:

Now let me add a test for the scripted --set-slot path (duplicates included), then run lint + tests.

All green, lint clean. Let me run the broader CLI tests, then commit and push.

Done — review crew is now slot-based, supporting arbitrary workers including duplicate agent+model. Pushed to review-profiles (commit 1481e83c2).

What changed

A slot = agent + model, and you can have any number — including the same agent repeated on different or identical models.

Interactive picker — replaced the per-agent model grid with a slot list:

Seeded with one default-model slot per launchable agent (so Enter-through = today's baseline), then add/remove freely. Each "Add" = pick agent → pick model (Default / advertised / Custom…).

Scripted — new repeatable flag:

--set-agents stays as the one-default-slot-per-agent shorthand (preserves --set-model targeting). Duplicates become distinct workers via workerIDForAgentModel (claude-code, claude-code-2, claude-code:opus, …).

Why this works cleanly

The storage layer already modeled workers as an arbitrary-keyed map, so no settings/schema change was needed — only the two agent-keyed config surfaces. Master selection was already worker/slot-based.

Verified

• Build, gofmt, go vet, golangci-lint (0 issues).
• New test TestBuildConfiguredProfile_FromSlots_AllowsDuplicateAgents (4 workers: opus, sonnet, +2 identical defaults).
• Existing --set-agents/--set-model test still passes; full review + cli suites green.

One caveat worth flagging: when you create duplicate slots for the same agent, --set-model agent=model becomes ambiguous (it errors, listing the workers) — by design, since --set-slot carries the model inline. The interactive slot picker itself isn't unit-tested (huh forms can't run headless, same as before); the slot→profile mapping (buildCrewProfile / workerIDForAgentModel) is covered via the scripted path test.

Agreed — the Add/Remove/Done menu is clunky. Let me make it linear: configure the first slot, then keep asking "add another?". Let me read the current crew functions to replace them precisely.

Now let me apply the linear flow. Three edits: rewrite promptForReviewCrew (+ drop the action consts), replace promptCrewAction/crewSummary with promptAddAnotherSlot, and remove promptCrewRemove.

Now remove the now-unused promptCrewRemove:

All clean. Let me commit and push.

Done — the crew picker is now linear. Pushed to review-profiles (commit ef60a7a67, net −65 lines).

New flow

Choosing "Add another" loops back to agent → model; "Done" finishes. The first slot is mandatory so the crew is never empty, and you can add the same agent repeatedly for duplicates.

Tradeoff

I dropped slot removal (the old Remove action). The linear flow has no natural place for it, and you get a confirmation line per add (Added … (N in crew)). To fix a mistake, re-run entire review --configure. If you'd rather keep an explicit remove/undo, I can add a lightweight "edit the crew" review step at the end (show the list, offer to drop any) — but that reintroduces a menu, so I left it out per your "keep adding more" intent.

Verified

Build, gofmt, go vet, golangci-lint (0 issues), and review tests all pass. The scripted --set-slot path (and its duplicate-agents test) is unchanged and still green.

Solid direction — master-as-lead is a nice framing. But before endorsing, let me check one thing that the mock quietly changes: whether the master is still required to be one of the worker slots.

Confirmed the key hidden change: today profile.Master must be one of the worker slots (selectProfileWorker(profile, profile.Master) errors otherwise), and that master also runs as a worker. Your mock shows the master (Claude Code) not in the slot list — so it's quietly reframing master as a separate "judge" role. That's the crux of my feedback.

Biggest concerns

1. Master-first front-loads ceremony onto the common case. Most runs are "just review my branch with one agent." Synthesis/master only matters with ≥2 workers. Asking for the master first forces every user through a judge picker they often don't need. I'd either:

• make master conditional (only ask when there are ≥2 slots), or
• keep it first but offer an explicit "single reviewer / no master" path.

My instinct: task → slots → master(if ≥2 slots) reads more naturally (decide what to review, who reviews, then who judges — and only when judging is meaningful). If you keep master-first for the "lead" framing, at least skip synthesis when only one slot ends up configured.

2. Decide master's semantics — and the schema cost. The mock implies master is separate from slots. Two sub-questions:

• Does the master also produce a worker review, or is it a pure synthesizer? (Cleaner: pure judge. But it's a behavior change — today the master competes as a worker too.)
• Schema: Master is currently a worker key (+ MasterModel). A standalone master needs its own MasterAgent+MasterModel independent of Agents. Worth adding, but it's a real migration touchpoint and changes the selectProfileWorker(master) validation everywhere.

3. Step 3 contradicts what we just shipped. The mock's slot screen is a list + "+ Add slot" + "Done · N" — i.e. the single-screen menu I removed last commit in favor of the linear "add another" you asked for. To be fair, this version is nicer (shows the running list in one place, and selecting a row could give back edit/remove — which you lost). But pick a lane:

• linear "add another" (current), or
• single-screen list-with-add/done (mock) — I'd lean this way if selecting a slot row lets you edit/remove it (recovers the removal capability).

Smaller suggestions

• Step 2 conflates task vs skill. "What to review" (profile Task) and "which skill to run" (per-slot Skills) are different axes. A preset should map to both (e.g. Security → task text and /security-review per slot). Clarify whether "BYO .md skill" is profile-wide or per-slot — today skills are per-worker.
• Master picker UI: the grouped agent-headers-with-indented-models look needs a custom component (huh can't render non-selectable headers in a select). Simpler and consistent: a flat select of Claude Code · sonnet, Codex · gpt-5-codex, … custom….
• Reuse one agent→model sub-flow for both master (step 1) and slots (step 3) — same picker, less code, consistent UX.
• Master defaulting: pre-select a sensible master (first text-gen-capable slot) so Enter-through works, rather than forcing a choice.

My recommendation

1. Task (preset / BYO) — universal, set it first.
2. Slots (agent+model, linear or list-style — your call).
3. Master — only if ≥2 slots; default to a sensible slot; allow "separate judge" with its own model.

This keeps the simple single-reviewer path one-or-two prompts, and only surfaces master when it actually does something.

Want me to (a) adopt this ordering as-is, or (b) implement your master-first version with the schema change for a standalone master? Either way I'd nail down the task↔skill mapping first.

Love it. Big pivot, so let me lock the plan, then start on the foundation. First let me confirm the master/validation code I'll need to change.

This is a multi-stage pivot. Let me lock the plan and decisions, then build the foundation. First, the remaining validation block:

I have all the touch points. Here's the plan, then I'll build the foundation.

Plan — scout (multi-agent crew runner; review = one preset)

Decisions (yours + my recommendation):

• Command: scout becomes the user-facing command, reusing the review engine (package stays internal to avoid a destabilizing rename). review kept as a hidden alias for back-compat.
• Master: standalone judge — its own agent+model, not a worker slot, pure synthesizer. New schema field MasterAgent (legacy worker-Master still honored).
• Flow: Task (preset/BYO) → Slots (single-screen list, add/edit/remove) → Master (only when ≥2 slots).

Stages:

1. Foundation: standalone master (engine — this turn). Everything else needs it.
2. scout command wrapping the engine; review → hidden alias.
3. New guided flow: task→slots→master, single-screen slot list.

Starting Stage 1 now.

Now the three cmd.go validation/run sites:

Build OK. Let me add tests for the standalone-master resolution.

Lint's goconst tripped on repeated literals in the test. Let me check the count and use the existing agent-name constants.

golangci-lint runs in new-code mode, so my added test literals trip goconst. Let me factor them into constants.

Clean. Let me run the broader CLI tests, then commit Stage 1.

Stage 1 done and pushed (6f9da77f6). The master can now be a standalone judge.

Where we are

✅ Stage 1 — Standalone master (foundation)

• New MasterAgent field: master = its own agent+model, not required to be a worker slot, pure synthesizer.
• profileMasterIdentity resolves master (standalone wins, legacy worker Master still honored); validation now accepts a standalone master (checked for text-generation capability) instead of forcing master ∈ workers.
• Tests for resolution precedence; build/lint/tests green.

Next stages (confirming before I build)

Stage 2 — scout command

• scout becomes the user-facing command, reusing the review engine (NewCommand). review kept as a hidden alias (back-compat, reversible).
• Re-brand help/text from "review" → "scout" at the command layer; keep internal package/settings/env names as-is (implementation detail) to avoid a destabilizing rename.

Stage 3 — New guided flow

1. Task — preset (General / Security / Accessibility) or BYO (custom text / point at a .md skill).
2. Slots — single-screen list (+ Add slot, select a row to edit/remove, Done · N).
3. Master — only when ≥2 slots; standalone agent+model (reusing the slot's agent→model sub-flow); sensible default pre-selected.

Two quick checks before Stage 3 so I don't guess:

1. Task ↔ skill mapping: when a preset is chosen (e.g. Security), should it set both the profile Task text and seed each slot's default skill (e.g. /security-review for Claude)? And for BYO ".md skill" — apply it to all slots, or is that a per-slot thing?
2. scout naming: OK to keep review as a hidden alias (vs. removing it outright)? And keep settings keys/env (review_profiles, ENTIRE_REVIEW_*) internal for now, or do you want those renamed to scout_* too (bigger, migration-y change)?

Want me to proceed to Stage 2 (the scout command) now while you answer the Stage 3 questions?

Stage 2: introduce scout as the command, reusing the engine, with review as a hidden alias. Let me find the command definition and all user-facing entire review ... guidance strings.

Many user-facing strings. Let me read the command's Use/Short/Long and flag block to rebrand precisely.

Now rebrand the command to scout with review as a hidden alias:

Now update the user-facing printed guidance strings (entire review … → entire scout …) across cmd.go, picker.go, profile.go, and fix.go:

Now fix.go — let me view its command-name references.

Let me build, then sweep for any remaining user-facing entire review references (incl. root.go/labs.go).

The Long uses a raw string (backticks), so the backticks I added inside it broke it. Let me fix that line.

Fix the remaining references — picker.go:864 and the labs entry:

entire scout works. Let me verify the review alias still works, then run tests/lint.

Both work. Let me update the root.go comment, then run tests + lint.

Let me find the actual failing test (the --last lines are likely unrelated expected output).

The labs tests assert the old entire review wording. Let me see all labs failures and the overview format.

The Try: block in labsOverview hardcodes entire review --help. Let me update it and the labs tests to scout wording.

Let me check TestRootHelp_ShowsLabsButHidesReview and re-run.

Two more test assertions to update to scout wording:

All green. Let me run the integration tests (which invoke review/attach) to confirm the alias path works end-to-end, plus a final lint.

The 2 lint issues are in agent/architecture_test.go — a file I didn't touch (pre-existing nolintlint noise). My changed packages lint clean (0 issues, verified above). Let me commit Stage 2.

Stage 2 done and pushed (77e4c8d6d).

✅ Stage 2 — scout command

• entire scout is now the command (multi-agent crew), with review as a hidden cobra alias — both work, fully back-compatible.
• Rebranded Use/Short/Long (now describes the crew model: workers in parallel → master synthesizes), all user-facing guidance strings (entire review … → entire scout …), the labs entry, and --findings footer.
• No behavior change — internal package, settings keys (review_profiles), and env vars (ENTIRE_REVIEW_*) stay as-is (branding only).
• Updated the affected test expectations (labs, manifest footer/warning).

Verified: entire scout --help and entire review --help both work; build, gofmt, scoped lint (0 issues), unit + integration tests green. (The 2 lint warnings in the broad run are in agent/architecture_test.go, a pre-existing file I didn't touch.)

Progress

• ✅ Stage 1 — standalone master
• ✅ Stage 2 — scout command (review alias)
• ⏭️ Stage 3 — new guided flow: Task (preset/BYO) → Slots (single-screen list, edit/remove) → Master (standalone, only when ≥2 slots)

Before I build Stage 3, the two questions from before still stand (they shape the implementation):

1. Task ↔ skill mapping: when a preset is picked (e.g. Security), set both the profile Task text and seed each slot's default skill (e.g. /security-review for Claude)? And BYO ".md skill" — apply to all slots, or per-slot?
2. Anything to change about the single-screen slot list interaction now that master is separate (e.g. should the master row show at the top of that screen, or stay its own step)?

Answer those and I'll implement Stage 3.

Clear. Stage 3 changes: (a) slots → single-screen list with edit/remove, and (b) master → standalone separate step (sets MasterAgent, any installed text-gen agent). Task stays uniform-to-all-slots (per-slot is the advanced --edit/--set-slot path). Let me read the current guided-setup + crew functions to edit precisely.

Now I'll implement: single-screen crew list (add/edit/remove, seeded with all agents), a standalone-master step, and remove the now-unused helpers. Let me grep exact current text of the helpers I'll replace.

Now apply all Stage 3 edits:

The endText field slipped in. Resending without it.

Build OK. Let me run tests + lint.

All green. Let me verify the run path treats a standalone master correctly (master not in worker slots → only synthesizes), then commit. The reviewers are built from profile.Agents (workers) and the master is resolved separately via resolveProfileMaster, so a standalone MasterAgent not in Agents only synthesizes — correct.

Stage 3 done and pushed (a9ea5bf7d). The full pivot is now in place.

✅ Stage 3 — new guided flow

The entire scout guided setup is now Task → Slots → Master:

1. Task (existing step) — General / Security / Accessibility. The chosen task applies to the whole crew by default; per-slot skills stay available via entire scout --edit.

2. Slots — single-screen list, seeded with all launchable agents (default model):

Selecting a slot row → Edit / Remove / Cancel. + Add slot → agent → model. Duplicates allowed.

3. Master — separate, standalone step (only when ≥2 slots):

Sets MasterAgent/MasterModel — its own agent+model, not one of the worker slots, pure synthesizer.

Full pivot recap

• ✅ Stage 1 — standalone master schema + run path
• ✅ Stage 2 — scout command (review = hidden alias)
• ✅ Stage 3 — guided flow: task → single-screen slots (edit/remove) → standalone master

Build, gofmt, lint (0 issues in the review package), unit + integration tests all green across every stage.

Notes / possible follow-ups

• BYO task ("Custom… / point at a .md skill") from your mock isn't built yet — the task step still has the 3 presets. Small add (custom task text); the per-slot .md skill needs more design. Say the word and I'll add it.
• The interactive pickers remain not-unit-tested (huh can't run headless — unchanged constraint); the data-layer pieces (profileMasterIdentity, buildCrewProfile, workerIDForAgentModel, scripted --set-slot) are covered.
• Settings keys/env are still internally review_*/ENTIRE_REVIEW_* (branding-only rename per Stage 2). If you want those migrated to scout_*, that's a separate, migration-y change.

What's next — the BYO custom task, or something else?

Two things — first the verbosity (a code fix), then the "groups" question.

1. Trimming the guided-flow text

You're right, it's over-explained. The huh form titles/descriptions already carry the meaning, so I'll drop the prose banners.

Removing those banners makes out unused in promptForStandaloneMaster. Let me drop the param.

out is now unused in promptForReviewCrew too. Let me drop it there as well.

All clean. Let me commit the trim.

Trimmed and pushed (9eae501d9). The guided flow is now just the form prompts — What kind of review? → Review crew (with a one-line hint) → Choose the review master (one line, only when there's a choice) → Start review now?. No prose walls.

2. Groups = profiles (yes, multiple, run independently)

Each configured crew is a named profile — that is your "group." You can have as many as you want and run each separately.

How the guided flow names the group: the "What kind of review?" step picks the profile name:

• General → general (the default)
• Security → security
• Accessibility → accessibility

So choosing the type = choosing/creating that group.

Running a specific group:

Configuring multiple groups:

They coexist in settings (review_profiles map) with a default (review_default_profile). entire scout --configure (no --profile) shows all configured profiles.

Two clarifications

1. One group per invocation — there's no "run all groups" in a single command. Run them in sequence: entire scout general && entire scout security. Each is its own crew run, recorded on the checkpoint.
2. entire scout review won't work as you'd expect — review is the command alias (entire review == entire scout), not a profile name. There's no review profile by default, so entire scout review would look for a profile called "review" and error. The actual group names are general / security / accessibility (or whatever you pass to --profile).

So: yes, multiple groups, fully supported — just address them by profile name (entire scout security), not "review".

Honest take: keep scout, don't switch to squad — and the reason is grammatical, not aesthetic.

Command names want to be verbs

Look at the neighbors: review, investigate, attach, resume — all verbs/actions you do to the branch. scout fits that perfectly: "scout this change." squad is a noun, and entire squad reads like a management namespace (git branch, git remote) — i.e. "create/list squads," not "go assess my code now." That fights the run-now semantics you actually have.

So as the top-level command, scout is the stronger choice.

On naming the slots

Here's the tension with your idea: if the command is scout and the workers are "scouts," you get an overload — "the scout command runs scouts" is muddy in docs and --help. One word doing two jobs.

Two clean ways out:

• Don't name them (my lean). Slots are "workers" / "slots" — functional, unambiguous. You don't need a flavor noun for them, and not having one keeps scout unambiguous.
• Commit to the metaphor, but only where it doesn't collide. Keep scout as the verb, and name the master the scoutmaster (the real term for a scout troop's leader). That reinforces the theme, reads great in the UI ("Choose the scoutmaster — evaluates the scouts' reports"), and crucially doesn't overload the worker term. The workers can stay "workers"; the lead is the scoutmaster.

What I'd avoid: squad as the command, or simultaneously calling the command scout and the workers scouts.

If you want a team-noun anywhere

The group (the profile) is the natural place for a collective noun if you want one — internally I already call it the "crew." So you could say "scout runs a crew of workers, led by the scoutmaster." That gives you the squad-ish flavor without breaking the verb command.

My recommendation: scout (command, unchanged) + workers stay "workers"/"slots" + rename the master to "scoutmaster" in the UI. Small, cohesive, no overload.

Want me to apply just the master → scoutmaster relabel in the user-facing strings (keeping the schema field MasterAgent as-is)?

Switching to debugging. Let me look at Codex's reviewer command and model lister.

Codex's example models are by design (Codex has no model-enumeration command). Now the failure — let me find Codex's reviewer command construction.

Codex's example models are intentional (no enumeration command). The failure is the real issue. Let me check whether the reviewer captures Codex's stderr — "exit status 1" with no reason suggests the actual error is being swallowed.

The engine does capture stderr into ProcessError. So either it's empty or the sink isn't showing it. Let me check ProcessError.Error() and how the sinks render a failed run.

The engine does surface stderr (dump + TUI detail). The inline ✗ failed exit status 1 is just the summary row — the real reason should be in the post-run dump or the TUI detail pane. But Codex exec --json likely writes its error as a stdout JSON line and exits 1 with empty stderr. Let me check whether the parser captures Codex error envelopes.

Found the gap. Two separate things:

1. Codex "example" models — by design

Codex has no model-enumeration command, so ListModels returns a hardcoded advisory list (models.go), all tagged example. --model just forwards whatever you pick to the Codex CLI. So the picker can only ever show examples for Codex (unlike Claude/Gemini which advertise real models).

2. The exit status 1 with no reason

The Codex reviewer runs codex exec --skip-git-repo-check --json [--model X] -. When it fails, Codex typically writes its error as a stdout JSON envelope (e.g. {"type":"error",...}) and exits 1 — with empty stderr. But the parser only handles thread.started/turn.started/item.*/turn.completed and drops everything else (the default arm just Debug-logs the type, not the message). So the actual reason ("model not found" / "401" / "stream error") is thrown away, leaving only exit status 1.

Let me fix that — capture Codex error envelopes so the real message surfaces:

Let me add a test for the error-surfacing and verify lint.

Tests pass, lint clean. Let me commit.

Pushed (9a30e1d7a).

Answers

Why only example models for Codex? By design — Codex has no model-list command, so ListModels returns a fixed advisory set (all tagged example). --model forwards whatever you pick straight to the Codex CLI. Claude/Gemini advertise real models; Codex can't.

Why the bare exit status 1? Codex reports failures as a stdout JSON envelope and exits non-zero with empty stderr. The parser only understood the success envelopes and silently dropped everything else — so the actual reason was lost. Fixed: it now captures the error message ({"type":"error","message":…} or {"error":{"message":…}}) and surfaces it as a RunError, which shows in the post-run dump and TUI detail.

What to do now to see the real cause

Re-run and check the dump / TUI detail (not just the status row) — you should now get a real message instead of exit status 1.

If you want to confirm the underlying cause directly, reproduce the exact invocation manually:

That JSON output will show the real error. Most likely one of:

• gpt-5-codex isn't valid for your Codex account/CLI version — try --model gpt-5 (or entire scout --agent codex --model gpt-5).
• Not authenticated — codex login.
• Flag mismatch — an older/newer Codex where exec --json differs (a 4.2s run before failing argues against this; it points more at model/auth/API).

Tell me what that command prints and I can pin the exact fix. Also, if it turns out gpt-5-codex is just wrong, I can update the example list in codex/models.go.

Fair — fake "examples" that look pickable but may be invalid is bad UX. Let me check what the other agents advertise (real vs. also fabricated) before deciding the fix.

Clear distinction: Claude advertises real aliases (opus/sonnet/haiku — valid, keep), while Codex and Gemini advertise fabricated "example" guesses. Your instinct is right — those should be Default + Custom only. The picker already does exactly that for agents that aren't model-listers. Let me check what references the Codex/Gemini model listers before removing them.

No references anywhere — safe to remove. Deleting the two fabricated-example listers (keeping Claude's real aliases):

Let me run the affected tests and verify --models output now does the Default/Custom guidance for Codex/Gemini.

A test asserts the old example models in --models output. Let me read and update it.

Two tests assert the removed examples. Updating them to the new Default/Custom behavior:

All green, lint clean. Let me commit.

Done (724f5797e).

What the "example" thing was — and the fix

Those example-tagged entries were hardcoded guesses. Codex and Gemini have no model-enumeration command, so the code just made up a plausible list (gpt-5-codex, gemini-2.5-pro, …) and presented them as if pickable — even though they might not be valid for your CLI/account (which is exactly how gpt-5-codex failed).

You're right: if there's no real list, don't fabricate one. So I removed the Codex and Gemini model listers entirely. Now:

• Model picker (guided flow) for Codex/Gemini shows just:
• entire scout --models shows for them:
• Claude keeps its real aliases (opus/sonnet/haiku) — those are genuine, valid Claude Code aliases, not guesses.

This relies on the picker's existing "no advertised models → Default + Custom" path, so it was just a matter of not lying about the list.

Net effect for your earlier failure: you'd now pick Default (uses whatever your Codex is actually configured with) or type a model you know works via Custom…, instead of being handed a fake gpt-5-codex. And if it still fails, the previous commit makes Codex surface the real error message instead of exit status 1.

Build, lint, and tests all green.

Let me verify there aren't other hardcoded model/example lists I missed.

Checked the whole tree. Here's the complete picture:

Removed (fabricated guesses): ✅ Codex and Gemini — both gone.

Remaining hardcoded model list: only Claude (claudecode/models.go):

These are real, stable, documented Claude Code aliases (note they're tagged alias, not example) — valid values the CLI actually accepts, not made-up examples. So I kept them intentionally.

Everything else that matched "example" is just documentation, not data:

• agent.go — a comment ("Pi, for example, …").
• model_lister.go — the doc comment for the Note field describing "example" as one possible hint.

And ListModels now exists only on Claude — no other agent (codex, gemini, pi, copilot, cursor, opencode, droid) advertises a model list, so they all fall to Default + Custom.

So: all fabricated example lists are removed. The one surviving list is Claude's genuine aliases.

If you'd prefer zero hardcoding anywhere — i.e., Claude also reduced to just Default + Custom — I can drop its lister too. But I'd lean against it: those three aliases are real and a nice convenience, unlike the Codex/Gemini guesses. Want me to remove Claude's as well, or leave it?

Correct — right now nothing is queried live. Let me confirm what each agent's CLI actually offers before answering definitively.

Correct — today nothing is queried live. The doc on ModelLister even spells out the intended design, but it's only partially realized:

• Claude → hardcoded curated aliases (opus/sonnet/haiku) — real and valid, but static.
• Codex, Gemini → now nothing (we removed the fabricated lists) → Default + Custom.
• Pi → never had a lister → Default + Custom.

So no agent shells out for a live list right now.

The irony / the real opportunity

The interface comment literally says:

Agents whose CLI can enumerate models live (e.g. Pi's pi --list-models) may shell out instead.

…but that path was never implemented — and Pi is the one agent that actually has a real enumeration command (pi --list-models, confirmed in its docs). So the only agent that could give a genuine live list is the one currently showing nothing.

Capability per agent:

• Pi — ✅ pi --list-models exists → we could fetch a real, current list.
• Codex — ❌ no list command (confirmed).
• Claude Code — ❌ no list command; the aliases are the documented interface.
• Gemini — ❌ no reliable list command.

So the honest ceiling is: live list for Pi, static aliases for Claude, Default+Custom for Codex/Gemini.

Offer

I can wire a live ListModels for Pi — shell out to pi --list-models, parse the output, and return the real models (with Default + Custom still always available, and a graceful fallback to Default+Custom if the call fails). That'd make Pi the one agent with an accurate, current list instead of guesses.

I'd also fix the now-stale ModelLister doc comment (it still claims codex/gemini "return a curated list," which we removed).

Want me to implement Pi live model enumeration?

Let me actually probe the installed CLIs rather than rely on assumptions.

I actually probed the installed CLIs to be sure (not guessing):

• claude --help — subcommands are agents, auth, doctor, mcp, plugin, ultrareview, … no models/list-models. --model just takes an alias or full name.
• codex --help — subcommands exec, review, login, mcp, doctor, … no model-enumeration command. -m/--model takes a value.
• gemini — not installed here, but it has no documented list command either.
• pi — has --list-models. ✅

So through the agent CLIs themselves: yes, Pi is the only one. Claude and Codex genuinely have no command that enumerates models — verified from their own --help.

The only other way (and why I'd skip it)

The models ultimately belong to the providers, and those do have list endpoints:

• OpenAI GET /v1/models
• Anthropic GET /v1/models
• Google models.list

So in principle we could fetch live lists by calling the provider APIs directly with the user's keys. But I'd recommend against it:

• It bypasses the agent CLI abstraction (we'd need to discover/handle each provider's auth + endpoints ourselves).
• The returned model IDs don't necessarily match what a given CLI/account/tier actually accepts — so we'd be back to "looks valid but may fail," the exact problem we just removed.
• It couples Entire to provider APIs it otherwise doesn't touch.

So the honest model-list story

• Pi: can be live (pi --list-models) — worth wiring.
• Claude: static real aliases (opus/sonnet/haiku) — fine.
• Codex / Gemini: Default + Custom — no reliable source short of provider APIs.

Net: I'd implement live enumeration for Pi and leave the rest as-is. Want me to do the Pi one?

Plan: Pi's live ListModels goes in the Pi PR (pi-invoked-skills-extraction, off main); "Default + Custom for others" is already done on review-profiles. Let me first see Pi's actual --list-models output format, and how the Pi agent execs commands.

Output is a clean table (provider model context …). I'll parse provider+model → provider/model IDs. Let me switch to the Pi PR branch and check how the Pi agent execs.

On the Pi branch (off main), PiAgent is a plain struct. I'll exec pi --list-models directly and keep parsing in a pure, testable function. Writing pi/models.go:

agent.ModelLister is undefined here — meaning the whole ModelLister capability lives on review-profiles, not main. Let me confirm.

Important finding — this changes the plan. The entire ModelLister capability lives only on review-profiles, not main:

• agent.ModelLister, ModelInfo, AsModelLister → review-profiles only
• claude/codex/gemini listers + the picker that consumes them → review-profiles only

The Pi PR (pi-invoked-skills-extraction) is off main, so Pi's ListModels can't compile there — it depends on the review feature's interface. So model-listing can't go in that off-main PR.

The honest place for it is review-profiles, alongside the Pi reviewer (which already lives there, not in the Pi PR) and the rest of the model-list machinery. Let me move it there.

On review-profiles now (where ModelLister exists). Recreating Pi's live lister here:

Now fix the stale ModelLister doc comment (codex/gemini no longer list on this branch):

Let me verify end-to-end that scout --models --agent pi now shows the live list.

Pi's lister errored at runtime. Let me debug the exec.

Pi writes the table to stderr, not stdout — so .Output() (stdout only) got nothing. Switching to CombinedOutput:

Live Pi list works now. Let me run tests/lint and commit.

Done (6471b1b5e).

Outcome

Pi now lists models live via pi --list-models — verified end-to-end:

Parsed into provider/model ids that Pi's --model accepts. Falls back to Default + Custom if the call fails. (Gotcha caught along the way: Pi prints the table to stderr, so I had to use combined output.)

Everyone else unchanged: Claude keeps its real aliases; Codex/Gemini show only Default + Custom.

One important deviation from your instruction — please note

You asked to put the Pi model-listing in the separate Pi PR. I couldn't: that PR (pi-invoked-skills-extraction) is branched off main, and the entire ModelLister capability (agent.ModelLister, ModelInfo, AsModelLister, the picker that consumes it) exists only on review-profiles, not main. So Pi's ListModels literally won't compile off main — it's part of the review/scout feature.

So I put it on review-profiles, where it belongs alongside:

• the Pi reviewer (pi/reviewer.go) — also already on review-profiles, not the Pi PR, and
• the claude/codex/gemini listers + picker.

The off-main Pi PR (#1383) stays as just the skill-capture guard (which genuinely is main-compatible).

If you'd still prefer the model-listing isolated, the only way would be to first land the ModelLister interface on main separately — but that interface is intrinsically part of the scout feature, so I don't think it's worth splitting. Let me know if you disagree and I'll restructure.

Let me check what PR #1313 actually is and its base branch.

PR #1313 is based off review-profiles (head review-pi-reviewer) — so it does have ModelLister, and that's the right home for Pi-specifics. I conflated it with the off-main #1383. Let me inspect what's on that branch vs. what I put on review-profiles.

That clears it up — and reveals a problem I created. PR #1313 (review-pi-reviewer, stacked on review-profiles) is the proper home for all Pi-specifics, and it already contains pi/reviewer.go, pi/models.go, pi/generate.go, etc. But I've been committing Pi work directly onto review-profiles — so it's now duplicated. Let me assess the overlap precisely.

You're right, and this exposes a mistake I made. Here's the actual topology:

• PR #1313 (review-pi-reviewer) is based on review-profiles (not main) — so it does have ModelLister, and it's the correct home for all Pi-specifics.
• It already contains a complete, better Pi integration: reviewer.go, models.go (live pi --list-models via the isolated CLI runner, with notes), models_test.go, and generate.go (Pi text-gen, so Pi can be a scoutmaster too).

The problem: I've been committing Pi work directly onto review-profiles (the reviewer in 1d8ceda2d, the model lister just now in 6471b1b5e). So Pi is now duplicated — and worse, #1313 is stale (branched at b9663b514, before all my scout/slot/master/rename work), so it has its own divergent edits to cmd.go, picker.go, profile.go, review_bridge.go, settings.go.

So two things are now tangled:

1. Pi duplication I created on review-profiles (should only be on #1313).
2. #1313 is far behind review-profiles and will conflict heavily on the shared scout files when rebased.

What I recommend

1. Strip my Pi-specific additions from review-profiles (delete pi/reviewer.go, pi/reviewer_test.go, pi/models.go, pi/models_test.go; undo the pi case in review_bridge.go). That makes review-profiles the clean general-scout branch with no Pi, exactly as you want. Nothing is lost — #1313 has all of it (and better).
2. Rebase #1313 onto the updated review-profiles so it carries only Pi-specifics on top. This is where the real conflict work is (the overlapping scout edits), and it’s #1313’s job to absorb them.

Net end state: review-profiles = scout feature, no Pi; #1313 = Pi reviewer + Pi live models + Pi generate, stacked cleanly.

Before I start deleting from a shared branch: want me to (1) strip Pi from review-profiles now, and do you want me to (2) also rebase #1313 onto it, or will you/its author handle the rebase? I don't want to rewrite that branch without you confirming.

Let me do it. Step 1: strip Pi from review-profiles. First confirm clean state and see the exact review_bridge.go wiring.

Branch drifted again (external activity). Let me get firmly onto review-profiles.

Now on review-profiles (head 6471b1b5e), clean. Stripping Pi:

Now revert the Pi mention in the ModelLister doc comment (Pi's lister is leaving this branch):

Build + tests pass. Committing the strip:

Step 2: rebase #1313 onto the updated review-profiles. First let me see #1313's commits and how its shared-file edits relate to the new scout code.

#1313's content is clean: 2 substantive commits (Pi adapter + Pi live models) plus 8 sync-merges, and all its shared-file edits are trivial "add pi to a list/switch/string." A plain rebase (dropping the merge commits) onto review-profiles will flatten it to those 2 Pi commits. Let me do it on a local branch and verify before force-pushing.

Only one conflict — a docs file; all code auto-merged. Let me resolve it.

I'll keep both paragraphs and update the --models one to reflect the current reality (Claude aliases, Pi live, codex/gemini Default+Custom):

The rebase --continue likely tried to open an editor. Let me check state and continue non-interactively.

Rebase succeeded — review-pi-reviewer is now review-profiles + the 2 Pi commits. Let me verify it builds and tests/lints cleanly before force-pushing.

Let me identify the failing test.

Pi's live list likely contains a model whose ID includes gpt-5-codex, so my naive global substring assertion misfires. Let me confirm and make the check codex-section-specific.

Confirmed — Pi legitimately lists openai/gpt-5-codex. The fix: assert the codex section specifically, not a global substring.

All green. Let me fold the test fix into the Pi live-models commit (it's the cause), then verify the final branch shape.

Clean two-commit stack on top of review-profiles. Final full build/test, then force-push.

Force-push succeeded. Let me verify the final state of both branches/PR and confirm the diff is now Pi-only.

Both branches are now clean. Let me switch back to review-profiles (the working branch) and confirm state.

Done. Everything is in its right place now.

Final state

review-profiles (head 34a4b8305) — the clean general scout feature, no Pi:

• No pi/reviewer.go, no pi/models.go, no pi case in launchableReviewerFor (verified: 0 matches).
• Keeps all the general work: scout rename, slot picker, standalone master, dropped review attach, codex/gemini example-list removal, etc.

PR #1313 (review-pi-reviewer, base review-profiles) — now a clean 2-commit stack carrying only Pi:

1. Add Pi review runner adapter
2. Implement Pi live model listing for entire review --models

The 8 stale sync-merge commits are gone (flattened by the rebase), and the diff vs review-profiles is Pi-only — the new Pi files (reviewer.go, models.go, generate.go + tests) plus trivial "add pi to the list" edits to shared files.

What I did

1. Stripped Pi from review-profiles (deleted the duplicated reviewer/models I'd mistakenly added, removed the wiring, de-Pi'd the ModelLister doc) → pushed.
2. Rebased #1313 onto the updated review-profiles — one trivial docs conflict (merged both paragraphs), and one test fix: TestReviewCmd_ListModels had a naive global gpt-5-codex substring check that now false-positives because Pi's live list legitimately includes openai/gpt-5-codex; made it codex-section-specific instead. Folded that into the live-models commit.
3. Verified build / gofmt / golangci-lint (0 issues) / unit tests green on both branches, then force-pushed #1313 with --force-with-lease.

Net: #1313 keeps the superior Pi integration (live pi --list-models with notes, plus generate.go so Pi can also be the scoutmaster), and it now stacks cleanly on the Pi-free review-profiles. The off-main PR #1383 remains just the skill-capture guard.

It's hardcoded — same mechanism as the codex/gemini examples were, just with valid values.

Source: cmd/entire/cli/agent/claudecode/models.go:

Note it ignores ctx and makes no CLI call — it's a static list, not enumerated from claude.

So why did I keep it but remove codex/gemini? The difference is validity, not mechanism:

• These three are documented Claude Code aliases. claude --help literally says --model accepts "an alias for the latest model" — opus/sonnet/haiku are those stable aliases that always resolve to the current model of each tier. So they don't go stale the way a specific id (gpt-5-codex) can.
• The codex/gemini entries were specific, version-y model ids we guessed at — looked valid, could fail.

But your underlying point is fair: Claude's list is still hardcoded, and Claude has no enumeration command (verified earlier — no models subcommand in claude --help), so we can't make it live like Pi. So the honest options for Claude are exactly: keep these three stable aliases, or drop to Default + Custom (zero hardcoding).

I kept them because three never-stale aliases are a genuinely useful convenience. If you'd rather have zero hardcoded models anywhere — only Pi live, everyone else Default + Custom — I'll remove Claude's lister too. Want that?

Good catch. The "What kind of review?" (type → task + default skills) picker exists (promptForSimpleReviewProfile), but it only runs when no profile name is set. The --configure path defaults the name to general before calling the guided setup, so the picker gets skipped. Let me confirm.

Confirmed the gap: --configure defaults the name to general before calling the guided setup, so the type picker (promptForSimpleReviewProfile) is skipped. The fix is to pass the raw --profile value (empty when not given) to the interactive setup, so the type step runs unless the user explicitly named a profile:

Let me confirm I'm on review-profiles and commit.

Fixed and pushed to review-profiles (39ef30522).

What was wrong

The "What kind of review?" picker (General / Security / Accessibility → sets the task + each slot's default skill) does exist in the guided flow — but --configure defaulted the profile name to general before calling the setup, and the setup only shows the type step when no profile name is set. So it was silently skipped.

The fix

--configure now passes the raw --profile value (empty when you didn't give one) into the guided setup, so the flow is:

Behavior by invocation:

• entire scout --configure → type picker shown (you pick the type/skill).
• entire scout --configure --profile security → type picker skipped (you already named it → it uses the security defaults).
• entire scout first run (no profile yet) → already showed the type picker (unchanged).

The chosen type drives the skill uniformly across the crew (e.g. Security → /security-review for Claude). Per-slot skill differences remain in entire scout --edit.

Build / tests / lint green.

Two notes:

• This is a general scout fix, so it's on review-profiles (not Pi). It does make #1313 one commit stale again — normal for the stacked PR; I can re-rebase it whenever you want.
• The mock's "Custom… — write a task or point at a .md skill" option in that type step is still not built (we deferred it). Want me to add at least the custom-task-text option now?

Let me add a Custom option (free-text task) to the type picker and thread it through. Let me read the exact call site first.

Let me check what listing actually exists in the code before answering.

Short answer: there's no dedicated command for it — that's a gap.

What exists today

The only place profiles are listed is the discovery view inside --configure, and it only renders in a non-interactive context (piped/CI), because interactively --configure opens the wizard instead:

→ prints:

That's awkward. The other list flags aren't this:

• entire scout --agents → workers of one profile
• entire scout --models → models per agent

Otherwise the profiles live in settings as review_profiles (+ review_default_profile) in your Entire settings / clone-local preferences, which you'd have to read directly.

The gap

There's no clean entire scout --list to see your "groups" (profiles) at a glance interactively.

I'd add a --list flag (no repo needed, like --models) that prints each configured profile, marks the default, and shows its workers + master — e.g.:

It'd reuse the existing nonZeroProfiles / sortedProfileNames helpers, so it's small.

Want me to add entire scout --list?

Let me verify branch and read the exact flag/RunE blocks to insert --list.

Now add the runReviewListProfiles function. Let me place it near runReviewListAgents:

Let me add a test and verify behavior, then commit. First, let me check how existing tests set up profiles to reuse the pattern.

Let me verify it runs and add --list to the help test. First, a manual run:

Works. Let me add --list to the help-flags test for coverage.

Let me keep attach (still in help text) and add --list:

All green. Let me commit and push.

Added and pushed to review-profiles (23f389e95).

entire scout --list

• Lists every configured profile with its workers (agent · model) and master, marks the default, and tells you how to run one.
• No repo run / agents launched — just reads settings (like --models/--agents).
• Empty case: No scout profiles configured. Create one with 'entire scout --configure'.

Verified end-to-end (shows real profiles); build, tests, lint (0 issues) green. Help test now asserts --list is present.

As before, this is a general scout change so it's on review-profiles — it does leave #1313 another commit behind; I can re-rebase that stack whenever you want to sync it.

The flow always starts fresh and never shows which profile you're editing. Let me make the guided setup edit the existing profile — pre-select the current type, seed the crew from the existing config, and pre-select the existing master. Let me read the current setup + callers.

Let me read the current promptForReviewCrew seed and promptForStandaloneMaster start to edit precisely.

Now I have what I need. Applying all the "edit existing profile" changes to picker.go:

Now update promptForStandaloneMaster to pre-select the existing master:

Now update the two callers in cmd.go to pass s:

Let me check for any other callers (tests) of the changed signatures.

All green, no test callers to update. Let me commit.

Done — pushed to review-profiles (51bd6a09f).

What changed

The guided setup now reflects and edits the existing profile instead of always starting from scratch:

1. Type picker pre-selects and marks the current default:
2. Crew picker is seeded from the existing profile's workers (agent · model), so you see/edit your current crew rather than a fresh "all agents" default.
3. Master step pre-selects the existing master.

Mechanically: I threaded the loaded EntireSettings into RunReviewGuidedSetup so it can look up the profile being configured (s.ReviewDefaultProfile for the current marker, s.ReviewProfiles[name] to seed crew + master).

Behavior notes:

• This is the "simple" guided path, so per-agent skill details still reset to the profile-type defaults (advanced per-slot skills remain in entire scout --edit) — but crew composition and master are now preserved/shown.
• entire scout --configure --profile <name> editing an existing profile will seed from it; a brand-new name seeds all-agents.

Build, tests, lint (0 issues) green.

Two still-open follow-ups from earlier you may want:

• The Custom… task option in the type picker (you said yes, then we pivoted to listing/this) — still not built.
• Re-syncing #1313 onto the now-several-commits-ahead review-profiles.

Want me to knock out the Custom task option next?

Two fair points. Let me answer both, then fix.

Why no "Custom" yet: honestly — I just never added it. It got deferred twice (we pivoted to --list, then to "indicate selected profile"). No technical reason; let me add it now.

Is "type" the right name: No, I don't think so. That step sets the profile's Task (what to look for) plus default skills — it's the review's focus/task, not a "type." Your own mock framed it as "What should they review?" with Custom = write a task, which matches the data model exactly. So I'll rename the step to that and make Custom a free-text task.

Let me implement both — rename the step and add Custom:

All green. Let me commit.

Done — pushed to review-profiles (e4afce4d4).

Both addressed

1. "type" → renamed. That step actually sets the profile's Task (+ default skills), so it's the review focus, not a type. Retitled to match your mock:

2. Custom now exists. Picking Custom… prompts for free text:

That text becomes the crew's shared Task, saved under the custom profile (run it with entire scout custom). It's also pre-selected/marked (current) when you're editing the custom profile. Empty input falls back to General.

Build, tests, lint (0 issues) green.

Notes:

• I went with free-text task for Custom. The mock also mentioned "point at a .md skill" — that's a separate, bigger feature (per-agent skill-file resolution); not built. Say the word if you want it.
• Custom always saves as the profile name custom (one custom profile). For multiple named custom profiles, the scripted path works: entire scout --configure --profile myname --set-task "…". Could make the picker ask for a name if you'd prefer.
• Still outstanding whenever you want: re-syncing #1313 onto the now-further-ahead review-profiles.

Good instinct — for a command that spawns a whole crew of agents (real time + tokens), silently running the default profile on a bare entire scout is surprising and undiscoverable. Right now:

• entire scout (profiles exist) → immediately runs the default profile
• entire scout (no profiles) → first-run setup
• entire scout <name> → runs that profile

So bare scout is a "fire the crew now" with a profile you may not remember choosing.

My recommendation

Don't make it always config (that breaks the verb), and don't make it a read-only list (then you still need a second command to run). Instead, bare interactive entire scout should open a lightweight chooser that combines both:

• Enter runs the pre-selected default → the fast path is preserved (one keypress, same as today).
• You can pick a different profile to run, or jump to Configure.
• It lists your profiles (discoverability) and lets you act — covering both "config" and "list."

Behavior matrix:

• entire scout <name> → runs it directly (explicit, no menu).
• entire scout interactive, profiles exist → the chooser above.
• entire scout interactive, no profiles → first-run setup (unchanged).
• entire scout non-interactive (CI / agent host) → keep running the default silently (no TTY to prompt) — important so automated callers don't hang.

This makes the expensive action explicit and discoverable without slowing down the common case.

It's a meaningful change to default behavior, so before I build it: do you want the Enter-to-run-default fast path kept (my rec), or should bare scout never auto-run and always require an explicit pick/confirm? And should the chooser show the scope banner (as above) so you know what you're about to review?

Fair challenge — let me be concrete about it, because it changes my recommendation.

How a non-interactive run actually works

Scout doesn't need a TTY to run — the workers run headless: claude -p, codex exec --json, pi --mode json, gemini headless. Those take a prompt on argv/stdin and stream output; no terminal required. The only parts that need a TTY are the wizard, the profile chooser, and confirm prompts.

So "non-interactive" means no usable TTY on stdin/stdout — and it's detected via interactive.CanPromptInteractively() / IsTerminalWriter, which already switch the run from the TUI to the plain DumpSink.

Realistic non-interactive callers:

• CI reviewing a PR: entire scout security 2>&1 | tee report.md, with agent creds in env (ANTHROPIC_API_KEY, codex login, etc.).
• An agent host (e.g. Claude Code) invoking entire scout as a command — stdout is captured, stdin isn't a terminal.
• Any piped/redirected invocation.

(And yes — if the agents aren't authenticated headlessly in that environment, they exit non-zero, exactly like the codex … exit status 1 we hit.)

But you've exposed a real flaw in my plan

You're right to push on it. "Non-interactive → silently run the default profile" is a footgun: an automated caller that forgot --profile would quietly fire whatever the default crew happens to be. That's worse than failing.

So I'd revise: don't guess in automation. Behavior should be:

That way the explicit path always works headless, the interactive path is discoverable, and nothing silently spawns a crew you didn't ask for — in either mode.

So the bare, no-profile, non-interactive case becomes a clear error rather than a silent default run. Want me to implement it that way?

Got it — bare entire scout always requires a selection: interactive → a chooser, non-interactive → print the profile list and error (no run). Let me read the current runReview top to restructure it cleanly.

This restructures the core run path. Let me apply it: a non-interactive bare guard, set the profile after first-run, and an interactive chooser when profiles exist.

The stray endText again. Resending.

Now add promptForProfileToRun to picker.go:

These smoke tests run review bare non-interactively, relying on the auto-default that I just removed. Let me see how they invoke it and pass an explicit profile.

These tests configure a general profile and relied on auto-default. Now they must name it. Let me add general as the positional profile to all four:

Another bare-invocation test. Let me find it.

Several review/cmd_test.go tests run bare non-interactively. Let me see all bare invocations and the profile name they configure.

seedReviewConfig and the context settings both use the general profile. The bare-invocation tests relied on auto-default. Let me identify each test at those lines (some may intentionally test the no-profile path) before editing.

All 13 are run-the-profile tests using the general profile — none test the no-profile path. Let me add general to each.

Two more with a different --agent value. Let me find and fix them.

Line 203 is the --findings table test (not a run) — fine to leave. Let me re-run.

All green. Let me add a regression test locking the new non-interactive bare behavior. Let me view a template test's setup.

errcheck flags the ignored marker error. Let me handle it.

All green, lint clean. Let me commit and push.

Done — pushed to review-profiles (7ebae0894).

New behavior for bare entire scout

It never silently runs a default crew anymore:

Interactive chooser:

Non-interactive (CI / agent host):

→ exits non-zero, spawns nothing.

Implementation

• Non-interactive bare → error with the profile list (before any setup/run).
• Interactive bare with profiles → new promptForProfileToRun chooser.
• First-run setup sets the chosen profile so the chooser doesn't double-prompt.
• Updated the existing smoke/dispatch tests to name general explicitly, and added TestRunReview_BareNonInteractiveRequiresProfile to lock the new behavior (asserts it errors, lists profiles, and starts no review).

Build, tests, lint (0 issues) green.

Note: review-profiles has now moved several commits past #1313 (review-pi-reviewer). Whenever you want, I can re-rebase that Pi stack onto the current tip so it stays clean.

Let me check exactly what the code uses to decide, so I can tell you precisely how to trigger it.

Here's exactly what flips it. The scout flow treats a run as non-interactive when either check fails:

• IsTerminalWriter(out) → false when stdout isn't a terminal (piped/redirected).
• CanPromptInteractively() → false when any of: CI is set (non-empty, not false), running under go test, invoked as an agent subprocess (Claude Code/Codex/etc.), /dev/tty can't be opened, or the ENTIRE_TEST_TTY override is non-1.

How to trigger it manually

Easiest:

Any of those → you'll get:

(exit non-zero, nothing spawned).

When it happens "for real"

• CI pipelines (CI is set; output captured).
• An agent invoking it — e.g. Claude Code running entire scout as a command (isAgentSubprocessEnv() → true), stdin/stdout not a terminal.
• cron / systemd / Docker without -t — no controlling terminal.
• Anything that pipes or redirects scout's output.

So to see the interactive chooser, just run entire scout directly in your terminal; to see the non-interactive list+error, pipe it (entire scout | cat) or set CI=1.

Want me to make the non-interactive message even more explicit (e.g., also print the exact entire scout <name> for each configured profile)?

Renaming scout → inspect. Let me find every occurrence (the command name lives only in strings/comments).

Many occurrences, all in strings/comments. Let me rename scout → inspect across the non-test files, then fix the alias and tests.

Now add scout (the now-previous name) as an alias too, and update the alias comment:

All three names work. Now fix test assertions referencing scout:

Let me check docs and the rest of the repo for any scout references.

No scout in docs. Let me do a final full check and commit.

The only remaining scout references are the intentional alias + its comment. Committing:

Done — pushed to review-profiles (ffef7206d).

Renamed scout → inspect

entire inspect is now the primary command. Verified all three work:

Updated everywhere it's user-facing:

• Use: "inspect", Aliases: ["scout", "review"] (both old names kept as hidden aliases for back-compat).
• All help text, guidance strings, error messages (e.g. entire inspect <name>, entire inspect --configure, --list, the bare-invocation list, findings footer).
• The labs entry (inspect / entire inspect).
• ModelLister doc comment, reviewCommandBinary, root.go comment.
• Test assertions (labs, manifest, cmd).

The only remaining scout strings are the intentional alias and its comment. Internal package/identifiers (review package, settings keys, env vars) are unchanged — branding only, no behavior change.

Build, tests, lint (0 issues) green.

Note: review-profiles keeps moving ahead of #1313 (the Pi stack). Whenever you're ready I can re-rebase #1313 onto the current tip (it'll pick up all these renames/behavior changes cleanly).

Fair — scout only ever existed on this branch, so there's nothing to be compatible with. Dropping it. (review, by contrast, shipped on main as the hidden labs command, so I'll keep that one as the back-compat alias.)

scout is gone (now an unknown command), inspect + review work, no test depended on it. Committing.

Done — pushed to review-profiles (3f9f03925).

• Dropped scout — it only ever existed on this branch, so there's nothing to be compatible with. entire scout is now an unknown command.
• Kept review as the sole alias — it shipped on main as the hidden labs command, so that one has real back-compat value.

Final state: entire inspect (primary) + entire review (alias). Lint clean, tests green.

I'll merge latest main into review-profiles. Let me fetch and check the divergence first.

Working tree clean, 75 commits behind. Merging origin/main:

Merge ran without printed conflicts. Let me confirm state and check for any conflicts.

Clean merge, no conflicts. Now build and run the test suite.

All packages pass (no failures printed). Let me confirm explicitly and run lint on the touched packages.

72 packages pass, 0 failures, lint clean. Let me push the merge.

Done.

Merged latest main into review-profiles

• 75 commits from main (auth/login changes, investigate flowchart, userdirs, discovery, etc.) merged cleanly — no conflicts.
• Merge commit c628f722e pushed to origin/review-profiles.

Tests passing

• go build ./... — clean.
• go test ./... — 72 packages ok, 0 failures.
• golangci-lint on the touched packages (review, cli, agent/pi) — 0 issues.

The inspect/review command, profile-selection behavior, --list, and Pi live-models all survived the merge intact.

Reminder: #1313 (review-pi-reviewer) is now well behind this updated review-profiles. Want me to re-rebase that Pi stack onto the new tip so it stays clean and mergeable?

Let me ground this in the actual code rather than guessing — I'll look at the investigate command and compare it to the inspect/review engine.

Let me confirm the execution model (turn-based loop vs parallel fan-out) and shared infrastructure.

Key clarification first: inspect is review — same command, same review package/engine; review is just a back-compat alias. So there's no behavioral difference between inspect and review. The real comparison is inspect/review vs investigate.

Both are hidden labs, multi-agent, "against the current branch," and share a lot of plumbing — but they answer different questions with different execution models.

Purpose

Execution model (the biggest difference)

• inspect = parallel fan-out, one round. All worker slots run concurrently on the same task (RunMulti), then a single standalone master synthesizes their reports into a final verdict. Workers don't see each other; the master judges.
• investigate = sequential round-robin loop. Agents take turns appending to a shared findings document (loop.go), single-threaded for deterministic ordering. It iterates over --max-turns until a --quorum of agents approve (or it stalls). Peers collaborate and converge; no master — termination is vote-based.

Inputs & flags

• inspect: a profile (workers + master), --base, --prompt, --agent/--model, --list, --configure/--edit, --findings. Bare run requires picking a profile.
• investigate: a seed — [seed-doc], --issue-link <url>, or interactive topic — plus --agents, --max-turns, --quorum, --continue <run-id>, --edit, --findings, --allow-untrusted-seed.

State, resumability, subcommands

• inspect: one-shot. Produces per-agent reports + master synthesis + a findings manifest. No run id, not resumable; manual tagging via entire attach --review.
• investigate: a persistent run with an id — resumable via --continue, with fix (launch a coding agent grounded in the findings), show, and clean subcommands. It maintains loop state (state.json, recorded turn "stances").

Roles & termination

• inspect: workers + a distinct master/judge; finishes when workers complete and the master synthesizes once.
• investigate: equal peers; finishes on quorum/turn-budget/stall.

Security

• investigate has --allow-untrusted-seed: an --issue-link pulls attacker-influenced GitHub content and agents run with permission/sandbox bypass, so non-interactive issue-link runs are refused without the flag.
• inspect reviews your own diff — lower injection surface (no external seed).

What they share

Both spawn agent CLIs and tag the resulting session via an env-var handshake read by the lifecycle hook — ENTIRE_REVIEW_* → Kind=agent_review (adoptReviewEnv) vs ENTIRE_INVESTIGATE_* → Kind=agent_investigate (adoptInvestigateEnv); review wins if both are somehow set. Both record onto entire/checkpoints/v1, both have TUI dashboards/sinks, multi-agent pickers, --edit config, and --findings browsers. So architecturally they're siblings built on the same multi-agent substrate.

When to use which

• Changed code and want a graded review/verdict → inspect.
• Have a bug, question, incident, or issue and want agents to dig in, gather evidence, and converge on findings (then optionally fix) → investigate.

One honest observation

Both Long descriptions now say "against the current branch," and inspect's even says workers "inspect the change" — so the surface framing overlaps and could confuse users. The crisp mental distinction worth surfacing in help: inspect = parallel evaluation of a diff → verdict; investigate = iterative, quorum-driven inquiry into a topic → findings. If you want, I can sharpen the two Short/Long strings so the difference is obvious at --help.

Stripped down, it's one thing: what the agents are doing.

• inspect — agents look at your diff in parallel, once; a master grades it. → You get a verdict on a code change.
• investigate — agents take turns building a shared findings doc, looping until they vote to stop. → You get a researched answer to a question.

So:

• Point it at a change → inspect. Point it at a question/issue → investigate.
• Parallel + judge (one round) → inspect. Sequential + quorum (many rounds) → investigate.
• Output is a grade/report → inspect. Output is accumulated evidence → investigate.

That's the whole difference. Everything else (profiles, TUI, env handshake, checkpoints) is shared plumbing.

I'll merge the latest main into review-profiles.

Clean merge — just a Go dependency bump. Let me verify build + tests before pushing.

Build and tests pass after the dependency bump. Pushing.

Done.

• Merged origin/main (3 commits — a Go dependency bump: go.mod/go.sum) into review-profiles. Clean, no conflicts.
• Verified go build ./... and tests (review, cli, agent/pi) pass with the updated deps.
• Merge commit 2b7770d154 pushed to origin/review-profiles.

Let me read the actual loop logic so I explain it precisely, not hand-wave.

Here's the actual mechanism (from investigate/loop.go):

Each turn, an agent casts a "stance"

Agents run round-robin, one at a time. On its turn an agent reads the shared findings doc, appends its evidence/analysis, and records a stance — one of:

• approve — "I'm satisfied; we can stop."
• request-changes — "not done, here's what's missing."
• reject — "this is wrong."

That stance is the agent's vote, recorded as a TurnStance.

A "round" is one full pass through all agents

The loop only checks for termination at the end of a round (when it wraps back to the first agent):

So it counts the approve votes in that round. If they reach --quorum (default = all agents), the run ends successfully (OutcomeQuorum). If not, it starts another round — the doc has grown, and everyone re-votes on the new state.

The "vote to stop" is re-cast every round

It's not a one-time vote. Each round the shared findings evolve, and every agent votes again. Consensus has to hold in a single round — e.g. with quorum = all, every agent must approve in the same pass. One request-changes keeps the investigation going.

What ends it besides agreement

• Quorum reached → OutcomeQuorum (the "they voted to stop" case).
• Turn budget exhausted → OutcomeStalled. Total turns are capped at --max-turns × number of agents (default 2 each). Findings exist, but no consensus was reached.
• Repeated failures → OutcomePaused (resumable via --continue).
• Ctrl-C / cancel → OutcomeCancelled.

So "looping until they vote to stop" = the agents keep taking turns enriching a shared doc, and each round they re-vote; the loop exits the moment enough of them approve in the same round — or it gives up when the turn budget runs out.

Contrast with inspect: there's no voting or rounds — workers run once in parallel and a master writes the verdict. investigate is the iterative, consensus-driven one.

Let me check what the agents are actually asked to decide — the prompt defines the vote's meaning.

Now it's concrete. Here's exactly what the vote means and a worked example.

What they're voting on

Whether the shared findings document is complete and correct — i.e., "is this investigation done and right?" Not the code, not each other — the state of the findings doc (its root cause / conclusion). The prompt pins the meaning:

• approve — only if "you have independently verified all findings and confirm the investigation is complete and correct."
• request-changes — "remaining gaps, unverified claims, or alternative explanations not yet considered."
• reject — "fundamentally wrong (e.g. wrong root cause)."

Each turn an agent: reads the whole doc → does its own digging → edits/refines the findings (with file:line evidence, a TLDR, a diagram) → casts its stance.

Worked example

Question: "Why do image uploads intermittently fail?" Crew: claude-code + codex (2 agents) · quorum = all (default 2) · max-turns = 2 each (so ≤ 2 rounds).

Round 1

• claude-code reads the empty doc, investigates, writes Finding 1: uploads >5 MB hit a 10 s client timeout; no S3 multipart (upload.go:42), fills the TLDR. → request-changes ("need to confirm retries' role").
• codex reads the doc, confirms Finding 1, adds Finding 2: retry retries the whole upload, amplifying load (retry.go:88). → request-changes ("timeout value's source still unverified").

End of round 1: 0 approves of 2 needed → continue. ⟳

Round 2

• claude-code verifies Finding 2, traces the 10 s timeout to a config default (config.go:15), marks everything verified, rewrites the TLDR as a direct answer. → approve.
• codex independently re-checks, agrees. → approve.

End of round 2: 2 approves ≥ quorum 2 → OutcomeQuorum, stop. ✅

What makes them "continue" vs stop

• Continue: a round finishes without enough approve votes. With quorum = all, a single request-changes/reject forces another round. Agents vote that way whenever there are gaps, unverified claims, or unconsidered alternatives — so the loop keeps going until everything is independently confirmed.
• Stop (success): enough agents approve in the same round (they agree it's complete + correct).
• Stop (stall): the turn budget runs out first (max-turns × agents) — e.g., if in round 2 codex had found the timeout was actually 30 s (contradicting Finding 1) and voted request-changes, round 2 is also the last round, so it ends OutcomeStalled: findings exist, but no consensus.

So the "vote" is really a per-round consensus check on "have we nailed the answer and verified it?" — and disagreement (anything other than approve) is what keeps them looping.

You've identified real weaknesses. Let me separate the part that's a genuine structural reason from the part where your critique lands.

Why round 2 is structurally forced (the one real reason)

An agent only sees the findings that existed before its turn. So the last finding added in round 1 has been seen by no one else — it's unverified. And an agent can't honestly satisfy the approve rule ("I independently verified all findings") for a finding it just wrote itself — self-verification isn't independent.

So "everyone has independently checked the complete set" is impossible to reach in round 1 by construction: verification always lags contribution by at least one turn. Round 2 is the first point where every agent has seen the final doc. That's the only hard reason rounds exist.

And to your "why not just do it in round 1" — an agent can fill the gap it spots, but then its own fix is the unverified thing. The model is "propose, and let someone else confirm," not "do it all yourself."

Where your critique is correct

Pretty much everywhere else:

• No dedicated challenger. Every agent runs the same prompt — "contribute + verify + vote." There's no red-team role whose job is to falsify the conclusion. The only "challenge" is an optional request-changes, and the whole loop is biased toward approve because that's what terminates it.
• "Validation" is soft, and anchoring works against it. The 2nd agent reads the 1st's findings before forming its "independent" opinion. LLMs anchor on and agree with provided context — so "I independently verified all findings" easily degrades into rubber-stamping. The mechanism asks for independence; it doesn't enforce it (no blind re-derivation, no required disconfirmation attempt).
• It is sequential single-agent turns + a vote. You're right. There's no debate, no defend/rebut exchange, no agent forced to argue the opposing case. It's an ensemble writing into one doc with a consensus gate bolted on.

So what's the actual point (honestly, modest)

Two things, and only two:

1. Cross-model heterogeneity. codex re-checking claude catches different errors than claude re-checking claude (different training, different blind spots). That's a real but limited ensemble benefit.
2. A consensus gate that blocks a single model's premature/overconfident conclusion from being the answer, plus a persistent, evidence-cited audit doc.

That's it. It's "ensemble + vote + shared scratchpad," not "adversarial validation."

If the goal is real validation

The design that would deliver what you're implying is missing:

• a dedicated skeptic/red-team turn that must try to break the current conclusion (find a counterexample, a contradicting commit/test) rather than approve;
• blind contribution (form your hypothesis before reading others') to kill anchoring;
• an explicit defend-then-revise exchange, not a silent vote.

As built, investigate doesn't do any of those — so your read is fair: it's mostly parallel-ish single agents looping into a shared doc, with cross-model verification that's weaker in practice than the prompt claims. The strongest honest justification for the loop is just "a different model independently re-touched every finding before we accepted it."

Here's a concise spec for inspect with a master panel (instead of a single master). It directly addresses the validation gap: multiple judges, cross-model, with dissent surfaced.

Schema (reuse the slot model)

Masters are just slots, like workers — reuse workerIDForAgentModel, the crew picker, and profileMasterIdentity.

Flow

1. Workers run in parallel → worker reports (unchanged).
2. Master panel runs in parallel: each master independently reads all worker reports and writes its own verdict (different models → different blind spots).
3. Chair merge (only if ≥2 masters):
   • the chair reads the panel verdicts and writes the single final verdict, explicitly calling out where masters agreed vs disagreed.
   • 1 master → its verdict is final (today's behavior). ≥2 masters, no chair → show all verdicts + a one-line consensus/divergence note, no merge.

Output

• Final verdict (chair's, or the lone master's).
• Panel appendix: each master's verdict verbatim, so dissent is visible, not hidden.

Config surfaces

• Guided: master step becomes "Add masters" (the existing slot list) + "Choose chair" when ≥2.
• Scripted: --set-master agent[=model] repeatable (panel); --set-chair agent[=model].
• --list: shows masters: claude·opus, codex·gpt-5 (chair: claude·opus).

Example

The split is the point — a second judge caught something the first (and the workers) missed, and the chair reconciles into one actionable verdict.

Why it's better than one master

• Cross-model judging at the verdict stage, not just workers.
• Disagreement is surfaced (the challenge/validation the single master lacked).
• Still yields one verdict via the chair, so it stays actionable.

Effort

Small: synthesis already exists (AgentSynthesisProvider) — run it once per master (parallel), then one extra synthesis pass for the chair over the verdicts. Masters reuse the slot/picker plumbing. Mostly a schema field + a second synthesis stage in runMultiAgentPath.

Want me to implement it (schema + panel/chair synthesis + config surfaces)?

Half yes, half I'd push back.

masters → judges: yes, strongly. It's exactly what they do — weigh the reports and render a verdict. It also kills the baggage of "master," and the panel reads perfectly: a panel of judges with a chair (presiding judge). Very coherent.

workers → investigators: no — that's the one to avoid. Two problems:

1. It collides head-on with the investigate command. We already flagged that inspect and investigate sound too alike; calling inspect's workers "investigators" makes them basically indistinguishable. "Investigator" should belong to investigate.
2. It misdescribes them. In inspect they examine a specific diff and report — that's inspection/review, not open-ended investigation.

Better fit: inspectors (or reviewers). Since the command is inspect, inspectors inspect → judges rule is clean, self-consistent, and keeps the two commands' vocabularies separate:

So my recommendation:

• judges (+ chair) ✓
• inspectors for the workers (reserve "investigators" for the other command)

If you want a stricter single courtroom metaphor you could do "examiners + judges," but I think inspectors + judges reads most naturally for a command literally named inspect.

Want me to spec/implement with inspectors + judges (+ chair)?

inspector — clearly the best of the three.

Reasoning:

• Matches the command. entire inspect → inspectors. Verb-and-actor agree, so it's self-documenting; no one has to learn a mapping.
• Semantically exact. An inspector examines a specific thing against criteria and flags problems — that's precisely what these agents do to a diff. (An investigator digs into an open unknown; that's a different, broader activity even setting the other command aside — it overstates what's happening here.)
• Pairs cleanly with judges. "Inspectors examine → judges rule" is one coherent assessment metaphor.

Ranking for inspect:

1. inspector — best fit, on-theme, accurate.
2. worker — safe and clear, but generic/bland and connects to nothing; "workers + judges" mixes a labor metaphor with a court one.
3. investigator — weakest: implies open-ended inquiry rather than evaluating a known change.

So: inspectors + judges (+ chair).

This is a sizable feature. Let me look at the current synthesis machinery to see how much the panel/chair changes, then implement.

This is a substantial feature. I'll build it in a clear order: schema → resolution → panel/chair synthesis (with tests) → wire into the run path → config surfaces → terminology renames. Let me start.

1. Schema — add Judges + Chair (keep legacy master fields for back-compat):

2. Resolution — profileJudges + reimplement profileMasterIdentity on top of it. Let me view the current resolution code:

3. Panel/chair synthesis — new synthesis_panel.go:

Panel logic works. 4. Wire into runMultiAgentPath — build the panel provider from the judges:

Build passes; two now-dead helpers. Let me remove them.

Now update the multi-agent validation to require all judges be text-gen capable (not just the chair). Let me find it:

The new all-judges text-gen gate is stricter than before and breaks tests that use stub agents (the legacy worker-master path was never text-gen-validated). Since the panel already drops incapable judges at runtime and single-judge synthesis fails gracefully, I'll keep just the "has a judge" check and drop the hard text-gen gate:

Now fix the wrapcheck lint in the panel passthrough:

The core engine is done and tested. Now the config surfaces. Let me look at the scripted-config plumbing to add --set-judge/--set-chair.

Now the scripted surface. Let me view scripted() and the configure construction:

Now update buildConfiguredProfile to handle judges:

Tests reference the removed reviewConfigureOptions.Master. Let me view and update them.

gocritic wants a switch. Let me convert it:

Core engine + scripted config are working. Let me add a panel-config test, then commit this slice before tackling the guided picker + remaining renames.

Use the existing test constants:

Solid, tested slice. Let me commit the engine + scripted config, then continue with the guided picker and remaining renames.

Now the guided picker. Let me read the current crew picker so I can extract a reusable slot-list and add a judges picker.

Now I'll extract a reusable pickSlotList and refactor promptForReviewCrew, then add promptForJudges:

Now replace promptForStandaloneMaster with a judges-panel picker, and update the guided-setup call site:

Now update the guided-setup call site to use promptForJudges:

Now the remaining terminology in user-facing strings. Let me find worker/master in string literals (not identifiers/JSON):

Let me update the --list output to show inspectors + the judge panel, then batch the remaining string renames.

Now batch the remaining user-facing string renames in cmd.go, picker.go, profile.go:

Let me update the command's Long help text (worker→inspector, master→judge):

Let me smoke-test the help/list output, then commit.

Looks good. Full test sweep, then commit.

72 ok, 0 failures. Committing the guided picker + terminology renames.

Both parts are done and pushed to review-profiles.

What shipped

Judges panel (feature) — commit 2595b9efb:

• Schema: ReviewProfileConfig.Judges []ReviewConfig + Chair (legacy Master/MasterAgent kept for back-compat).
• profileJudges resolves the panel + chair index.
• PanelSynthesisProvider: judges synthesize in parallel → chair merges into one final verdict, with each judge's verdict appended as a Panel and dissent surfaced. Failed judges are dropped; all-fail errors. Unit-tested with stub providers.
• Run path builds the panel; single judge collapses to today's behavior.
• Scripted: --set-judge (repeatable) + --set-chair (replacing --set-master).

Guided picker + terminology — commit 1fa7124ba:

• Guided setup configures a judge panel: a single-screen add/edit/remove list (reused pickSlotList) + chair pick when ≥2, seeded from the existing profile.
• Renames: workers → inspectors, master → judge across help, flags, --list, the focus step, and error messages. (Go identifiers + legacy Master JSON keys untouched.)

Verified:

And help now reads "inspector agents inspect … a panel of judges renders the final verdict (a chair merges …)."

Full suite: 72 packages ok, 0 failures, lint clean.

Configure a panel:

Note: this is a big terminology/feature change on review-profiles, so #1313 (the Pi stack) is now significantly behind — let me know when to re-rebase it.

Let me merge main first, then do a thorough self-review of the branch's diff to catch issues a reviewer would flag.

Clean merge. Let me verify and build/test.

Clean merge, builds, tests pass. Now a self-review. First, check the auto-merged labs.go and whether the architecture doc went stale.

The architecture doc is badly stale (still entire review, master/worker, --set-master, review attach). Let me read it fully and bring it in line.

This doc needs a full refresh. Let me check for references to its path before deciding whether to rename it, then rewrite.

I'll keep the filename (package is still review, and CLAUDE.md links it) but rewrite the content for the current state.

Let me update the CLAUDE.md reference line to match:

Let me add a clarifying comment about the chair's double role (a likely review question) and scan for other stale terminology in docs.

No other stale docs. Now add the clarifying comment about the chair's two passes:

Let me scan cmd.go for any remaining user-facing master/worker text in the list/catalog helpers:

Two legacy leftovers ([master] marker, master= catalog line). Let me view the catalog block and fix both:

A test asserts the removed [master] marker. Let me view and update it.

All green. Final sweep — gofmt, full test suite, and lint on touched packages:

Let me lint the cli package too (touched via merge/earlier changes), then commit.

All clean. Committing the self-review fixes (the merge commit is already in place):

The push had a hiccup. Let me verify and retry.

The branch ref may just be a stale local tracking ref. Let me first re-confirm tests pass, and fetch the real remote state.

The branch is in fact pushed — git ls-remote shows origin/review-profiles = ead8c12dc, matching local HEAD. The earlier "Everything up-to-date" was correct; only the local tracking ref was stale, and the signal: killed was Entire's separate checkpoint-remote sync (unrelated to the branch push). Let me refresh the tracking ref to confirm.

All green and pushed.

Status

Tests: go test ./... → 72 packages ok, 0 failures Lint: golangci-lint → 0 issues on touched packages (review, settings, cli) gofmt: clean Branch: local HEAD == origin/review-profiles == ead8c12dc ✓ in sync

The earlier signal: killed / "Everything up-to-date" was just Entire's checkpoint-remote sync hook (a separate, best-effort push of entire/checkpoints/v1) being killed — your actual code branch pushed fine, which git ls-remote confirmed.

What landed this round

1. Merged origin/main (35 commits, incl. the new attribution.go/trail work) — clean merge, no conflicts (134efa937).
2. Self-review fixes (ead8c12dc):
   • Rewrote docs/architecture/review-command.md for entire inspect (review alias), inspectors + judge panel + chair, --list, bare-invocation selection, panel synthesis, judges/chair schema, removed review attach, accurate ModelLister status.
   • Synced the CLAUDE.md summary.
   • Dropped the legacy [master] marker in --agents; show judges= in the --configure catalog; fixed the stale comment.
   • Documented the chair's two passes in PanelSynthesisProvider.
   • Updated the affected test.

Checkpoint sync to your remote is failing (git push: signal: killed) — that's an infra/access issue with the configured checkpoint remote, not your branch. Let me know if you want me to look into it.

Pushed cleanly this time — the checkpoint remote also synced successfully (done).

• Branch review-profiles: local ead8c12dc == remote ead8c12dc ✓
• Checkpoint sync: ✓ done
• "Everything up-to-date" because the branch was already current from the prior successful push.

Created the handoff at docs/handoff/inspect-judges-handoff.md.

It covers:

• TL;DR + status (branch review-profiles @ ead8c12dc, green build/tests/lint, pushed).
• Final terminology (inspectors / judges / chair; inspect command, review alias, scout removed; internal keys still review_*).
• Command surface and settings schema with examples (incl. back-compat for legacy master).
• How the judge panel works — profileJudges, PanelSynthesisProvider, the deliberate chair-runs-twice behavior, graceful degradation.
• Done checklist and Pending/next steps (top item: rebase PR #1313 review-pi-reviewer onto ead8c12dc; Pi is the natural first real second judge).
• Key files, verify commands, and gotchas (the checkpoint-remote push noise vs. actual branch push).

Want me to commit/push this handoff doc to the branch?

Those other modified files appeared unexpectedly and I committed them under a misleading message. Let me inspect what actually changed before deciding how to fix the commit.

These are substantive, intended code changes (dead --fix manifest lookup removed, IsZero fixed for judges/chair, picker/manifest cleanup) — but I wrongly bundled them under the "handoff doc" message. Let me undo that commit, verify they build/test, then split into honest commits.

Builds and tests pass. Let me review the remaining changed files to write an accurate message, then run full lint/tests.

Coherent cleanup. Let me run full tests + lint, then split into two honest commits.

72 ok, 0 fail, 0 lint issues. Now split into two commits and push.

Done. Working tree is clean, everything committed and pushed.

Result

• Pushed: ead8c12dc..d93bcfb1d → origin/review-profiles; local == remote == d93bcfb1d ✓
• Checkpoint remote also synced (done).

Two honest commits (I unbundled my earlier mislabeled commit)

1. 007228bd8 — Inspect cleanup: drop dead --fix manifest lookup, fix IsZero for judges
   • Removed the unused resolveLocalReviewManifestBySessionID + its tests (leftover from the removed entire review --fix).
   • Updated the agentlaunch package doc (now entire investigate fix only).
   • Fixed ReviewProfileConfig.IsZero to count judges/chair/master_agent so judge-only profiles aren't treated as empty.
   • Wording/test tidy-ups; t.Parallel() on panel tests.
2. d93bcfb1d — Add inspect + judge-panel handoff doc (docs/handoff/inspect-judges-handoff.md)

Checks

• go build ./... ✓
• go test ./... → 72 ok, 0 fail ✓
• golangci-lint (review, cli, settings, agentlaunch) → 0 issues ✓

Note: I caught that those cleanup changes had appeared in the working tree and I'd initially committed them under the wrong (handoff-doc) message — I reset and re-split them so history is accurate. They're legitimate and consistent with the judge work, build clean, and are now properly described.