Auto-select Detected Agents in Enable Flow
small fix: when doing entire enable we now auto select agents that we detect, we should still offer selecting more but make the found agents selected already so you can just hit enter
All green. Here's the change.
What I changed
The entire enable first‑run flow had a shortcut: when exactly one built‑in agent was detected, it auto‑selected it and skipped the picker entirely — so you couldn't add more agents without re‑running. (The multi‑agent case already showed the picker with detected agents pre‑checked.)
I removed that early‑return so a single detected agent now falls through to the same multi‑select form, where it's already pre‑selected via the existing preSelectedSet → hookAgentOptions → opt.Selected(true) machinery. You see the detected agent checked and can just hit enter, or tick more first.
cmd/entire/cli/setup.go (detectOrSelectAgent):
- The
len(detected) == 1case now only printsDetected agent: <name>and falls through to the picker instead ofreturn detected, nil. - Kept the
selectFn == nil && isBuiltInAgent(...)guard so--yes(non‑interactive) still uses the caller's selection logic, and a lone external agent still isn't announced/pre‑selected (unchanged, matches the existing external‑detection test). - Updated the doc comment to describe the new first‑run behavior.
cmd/entire/cli/setup_test.go:
- Added
TestDetectOrSelectAgent_FirstRun_SingleBuiltIn_PromptsWithDetectedPreSelected— with a TTY and one detected built‑in agent, the selection path runs (picker shown) and honors a user selection of more than the detected agent. This guards against re‑introducing an unconditional skip‑the‑picker shortcut. - Added a clarifying comment to
TestDetectOrSelectAgent_AgentDetectednoting it exercises the no‑TTY fallback (single detected agent used without a picker), which is still correct.
Notes / scope
- Behavioral boundary: The new "show the picker" behavior only kicks in with a real TTY. Non‑interactive runs (tests, CI,
--yes) keep using the detected/default agent without prompting — the agent‑safe fallback. The two existing single‑detection tests pass unchanged because they hit that non‑interactive path. - External detected agents are still shown but not pre‑selected (only built‑ins are). I kept that existing design as‑is since it's a separate decision from the picker‑skip fix — say the word if you want detected external agents pre‑checked too.
mise run fmt, mise run lint (0 issues), and the setup/enable test suite all pass.
/simplify → 4 cleanup agents in parallel → apply the fixes
You are improving the quality of the changed code, not hunting for bugs. Review
it for reuse, simplification, efficiency, and altitude issues, then fix what you
find. Do not look for correctness bugs — that is what /code-review is for.
Phase 0 — Gather the diff
Run git diff @{upstream}...HEAD (or git diff main...HEAD / git diff HEAD~1
if there's no upstream) to get the unified diff under review. If there are
uncommitted changes, or the range diff is empty, also run git diff HEAD and
include the working-tree changes in scope — the review often runs before the
commit. If a PR number, branch name, or file path was passed as an argument,
review that target instead. Treat this diff as the review scope.
Phase 1 — Review (4 cleanup agents in parallel)
Launch 4 independent review agents via the Agent tool, all in a
single message so they run concurrently. Pass each agent the diff and one of
the four angles below. Each returns its findings with file, line, a
one-line summary, and the concrete cost (what is duplicated, wasted, or
harder to maintain).
Reuse
Flag new code that re-implements something the codebase already has — Grep shared/utility modules and files adjacent to the change, and name the existing helper to call instead.
Simplification
Flag unnecessary complexity the diff adds: redundant or derivable state, copy-paste with slight variation, deep nesting, dead code left behind. Name the simpler form that does the same job.
Efficiency
Flag wasted work the diff introduces: redundant computation or repeated I/O, independent operations run sequentially, blocking work added to startup or hot paths. Also flag long-lived objects built from closures or captured environments — they keep the entire enclosing scope alive for the object's lifetime (a memory leak when that scope holds large values); prefer a class/struct that copies only the fields it needs. Name the cheaper alternative.
Altitude
Check that each change is implemented at the right depth, not as a fragile bandaid. Special cases layered on shared infrastructure are a sign the fix isn't deep enough — prefer generalizing the underlying mechanism over adding special cases.
Phase 2 — Apply the fixes
Wait for all four agents to complete, dedup findings that point at the same line or mechanism, and fix each remaining one directly. Skip any finding whose fix would change intended behavior, require changes well outside the reviewed diff, or that you judge to be a false positive — note the skip rather than arguing with it. Finish with a brief summary of what was fixed and what was skipped (or confirm the code was already clean).
All clean. Here's the /simplify summary.
Applied (2 cleanups)
- Trimmed the history-narrating comment (
setup.go, single-detected-agent case) — dropped the "Previously a lone built-in agent was used automatically…" narration (belongs in git history) and kept the two load-bearing facts: it's pre-selected in the form below, and--yesskips the announcement. 6 lines → 4. - Fixed the now-inconsistent re-run doc bullet — my diff added a "(when a TTY is available)" caveat + non-interactive sub-bullet to the first-run bullets, which made the adjacent re-run bullet's "Always shows the interactive multi-select" read as inaccurate (the no-TTY re-run path keeps installed agents without a picker). Added the matching TTY qualifier and non-interactive sub-bullet.
Skipped (with reasons)
- Reuse: clean. (One out-of-scope note: the
agentTypes-building loops duplicateconfig.go'sagentDisplayNames, but they're pre-existing and untouched by this diff.) - Efficiency: nothing material —
entire enableis a one-shot interactive path; the fall-through adds only bounded in-memory registry lookups, no new I/O or expensive closures. - Altitude Finding 1 (pre-select detected external agents too): skipped — the built-in-only pre-selection is pre-existing code and a deliberate, tested policy (external-only detection intentionally isn't auto-selected). Changing it alters intended behavior.
- Altitude Finding 2 (drop the announcement / unify announced vs pre-selected set): skipped — the announcement is the only feedback on the non-interactive path (no picker there), and two existing non-TTY tests assert it prints. Removing it would break behavior and tests. The multi-agent announcement it also flags is pre-existing and untouched.
- Simplification Findings 2 & 3 (switch shape; test precondition guard): confirmed already appropriately simple — the
switchidiomatically emits singular/plural messages, and the first-run precondition assertion is a cheap, worthwhile guard against fixture drift.
The core change itself was validated as the right altitude (special-case shortcut removed in favor of the shared picker, with the per-path difference now expressed as data via preSelectedSet). mise run fmt, mise run lint, and the detectOrSelectAgent tests all pass.
ok, can you open a PR
PR opened: https://github.com/entireio/cli/pull/1682
Summary of what I did:
- Branched off
main→soph/enable-preselect-detected-agents - Ran
mise run check— all passed (fmt, lint, unit + integration + E2E canary, 59 + 4 tests) - Committed the two changed files with the Entire/Co-Authored-By trailer
- Pushed (checkpoint refs went up too) and opened PR #1682
The PR covers the fix (single detected built-in agent now goes through the pre-selected picker instead of being auto-used) plus the new regression test, with the behavior matrix documented in the description.
Review target: — continue: collect altitude + simplification review findings, apply fixes
/simplify → 4 cleanup agents in parallel → apply the fixes
You are improving the quality of the changed code, not hunting for bugs. Review
it for reuse, simplification, efficiency, and altitude issues, then fix what you
find. Do not look for correctness bugs — that is what /code-review is for.
Phase 0 — Gather the diff
Run git diff @{upstream}...HEAD (or git diff main...HEAD / git diff HEAD~1
if there's no upstream) to get the unified diff under review. If there are
uncommitted changes, or the range diff is empty, also run git diff HEAD and
include the working-tree changes in scope — the review often runs before the
commit. If a PR number, branch name, or file path was passed as an argument,
review that target instead. Treat this diff as the review scope.
Phase 1 — Review (4 cleanup agents in parallel)
Launch 4 independent review agents via the Agent tool, all in a
single message so they run concurrently. Pass each agent the diff and one of
the four angles below. Each returns its findings with file, line, a
one-line summary, and the concrete cost (what is duplicated, wasted, or
harder to maintain).
Reuse
Flag new code that re-implements something the codebase already has — Grep shared/utility modules and files adjacent to the change, and name the existing helper to call instead.
Simplification
Flag unnecessary complexity the diff adds: redundant or derivable state, copy-paste with slight variation, deep nesting, dead code left behind. Name the simpler form that does the same job.
Efficiency
Flag wasted work the diff introduces: redundant computation or repeated I/O, independent operations run sequentially, blocking work added to startup or hot paths. Also flag long-lived objects built from closures or captured environments — they keep the entire enclosing scope alive for the object's lifetime (a memory leak when that scope holds large values); prefer a class/struct that copies only the fields it needs. Name the cheaper alternative.
Altitude
Check that each change is implemented at the right depth, not as a fragile bandaid. Special cases layered on shared infrastructure are a sign the fix isn't deep enough — prefer generalizing the underlying mechanism over adding special cases.
Phase 2 — Apply the fixes
Wait for all four agents to complete, dedup findings that point at the same line or mechanism, and fix each remaining one directly. Skip any finding whose fix would change intended behavior, require changes well outside the reviewed diff, or that you judge to be a false positive — note the skip rather than arguing with it. Finish with a brief summary of what was fixed and what was skipped (or confirm the code was already clean).
No new work to do — this fired from the stale fallback wakeup I scheduled earlier, and that continuation already ran to completion.
The /simplify review on this exact diff (cmd/entire/cli/setup.go + setup_test.go) is already done:
- All 4 cleanup agents completed (reuse, simplification, efficiency, altitude).
- Fixes applied: trimmed the history-narrating comment; fixed the inconsistent re-run doc bullet.
- Skipped (with reasons): pre-selecting external agents (pre-existing tested policy, behavior change); dropping the announcement (it's the only feedback on the non-TTY path, and tests assert it); switch shape and test precondition guard (already appropriately simple).
- Working tree is clean; the reviewed change is committed as
44e02205cand pushed in PR #1682.
The diff hasn't changed since that review, so re-running the four agents would just repeat work on unchanged code. Nothing further to apply. If you push new changes to the branch and want them reviewed, run /simplify again and I'll review the fresh diff.
can you check the comments on the PR?
That confirms it — pre-selection does work. With .claude/ present, the picker shows 1. ✓ Claude Code pre-checked (and it falls through to the picker as intended, rather than auto-using). Your enable-test-4 repo showed everything unchecked simply because it's brand new with no agent marker dir, so agent.DetectAll found nothing to pre-select.
The real question is whether "detection = a .claude/.gemini/… dir exists in the repo" is good enough. Let me check whether we could detect the agent you're actually running (e.g. via an env var) so a fresh repo still pre-selects sensibly:
it's fine, the comments on the PR are addressed?
Both PR comments are now addressed and pushed (commit c8b30c8f9 on the PR branch).
Comment 1 — misleading doc comment ✅
Reworded so it no longer claims the picker "always shows"; it now states the multi-select shows when a TTY is available and no selectFn override is supplied, and spells out that selectFn/--yes bypasses the prompt even on a TTY.
Comment 2 — test didn't guard the regression ✅ (the substantive one — the reviewer was right)
- Routed the real interactive form through a package-level
promptAgentSelectionseam, matching the repo's existingrunDispatchWizardForm/runTrailCreateFormconvention. - Collapsed the old
if selectFn != nil { … } else { form }branch into a singleselectFn(availableNames)call — so there's now exactly one selection step and noselectFn == nilshortcut for a lone detected agent to slip back into. - Rewrote the test to stub that seam and drive the
selectFn == nilpath (the real interactive path), asserting the picker is shown rather than the agent auto-used. - Verified it's a real guard: I temporarily reintroduced the old
return detectedshortcut and the test failed with "Expected the picker to be shown for a single detected agent, but it was auto-used", then passed again once restored. The old injected-selectFntest would have passed against the buggy code; this one doesn't.
mise run fmt, mise run lint (0 issues), and full mise run test:ci all pass.
One note from the earlier tangent (which you said is fine): the feature itself works — I proved that with a .claude/ dir present the picker renders 1. ✓ Claude Code pre-checked. Your enable-test-4 showed everything unchecked only because it's a fresh repo with no agent-marker directory, so detection found nothing to pre-select. That's pre-existing detection behavior, unrelated to these two review comments. Happy to enhance detection (e.g. pre-select the default agent when nothing is detected) as a follow-up if you want the "just hit enter" path to work in empty repos too — just say the word.