refactor: revert aimx agent-setup to positional-only (soft)

[uzyn]

Summary

Reverts the interactive menu added in e5ecc11 (`feat: guided menu for aimx agent-setup with MCP (General) option`). The menu adds friction for the common case (users who already know the agent name, scripts, documentation) and its discoverability role is already covered by `aimx agent-setup --list`. This is a soft revert: bare `aimx agent-setup` now prints the registry plus a one-line usage hint and exits 0, so interactive users still get a useful default instead of an error.

Requirements

• `aimx agent-setup ` continues to install for the named agent; unknown names still produce the `unknown agent '…'; run `aimx agent-setup --list`` error.
• `aimx agent-setup` (no args, TTY or non-TTY) prints the registry plus `Run `aimx agent-setup ` to install …` to stdout and exits 0. No branch on `is_stdin_tty`.
• `aimx agent-setup --list` is unchanged.
• Root is still refused up front on bare invocation (no registry dump for `sudo aimx agent-setup`).
• Menu surface removed: `Selection` enum, `prompt_agent_selection`, `print_mcp_general`, `render_mcp_general_snippet`, "MCP (General)" slot, and the now-unused `AgentSpec.display_name` field.
• `AgentEnv::is_stdin_tty` / `read_line` retained — still used by the `write_files` `Overwrite? [y/N]` prompt.
• `book/agent-integration.md` "Guided setup" section rewritten as "Discovering supported agents", describing the new bare-invocation behavior.
• Positional-arg and overwrite-prompt tests untouched and still green.

Out of scope

• No changes to the per-agent registry (same 7 agents, same destinations, same activation hints).
• No changes to `--data-dir`, `--force`, `--print`, install file modes, or the overwrite flow.
• No changes to `src/cli.rs` or `src/main.rs` — the clap definition already treated `` as optional.
• No generic-MCP-snippet CLI to replace the removed "MCP (General)" slot — users who need one can hand-author the JSON.

Technical approach

• `src/agent_setup.rs`: rewrote the `None` arm of the agent match in `run_with_env_to_writer` to call a new `print_registry_to_writer(env, out)` plus a `writeln!` of the usage hint, then return `Ok(())`. The `--list` path switched to call the same writer-parameterized helper (previously `print_registry` wrote with direct `println!`, making the bare-invocation path untestable via the `out: &mut dyn Write` parameter). Deleted `Selection`, `prompt_agent_selection`, `print_mcp_general`, `render_mcp_general_snippet`, and `AgentSpec.display_name`.
• `book/agent-integration.md`: replaced the "Guided setup" section with a short paragraph describing the bare-invocation registry dump.
• Root-check order: `--list` still skips the root check (pure static read, tests rely on this). Bare invocation runs the root check up front — a user who typed `sudo aimx agent-setup` by mistake gets the same friendly `per-user operation` error as the install paths, not a registry dump followed by silence.

Net: `src/agent_setup.rs` drops ~280 lines (2868 → 2588).

Test plan

TDD — new tests added first, confirmed failing against the menu code, then implementation landed.

New tests in `src/agent_setup.rs`:

• `bare_invocation_prints_registry_and_hint` — bare invocation with `tty=false` and `tty=true` each produce stdout containing every agent's registry key plus the `aimx agent-setup ` hint string, exit `Ok(())`, and write nothing to the temp filesystem.
• `bare_invocation_root_is_refused` — `root=true` + no agent → `Err` containing "per-user", no registry output.
• `bare_invocation_non_tty_still_prints_registry` — regression guard for the behavior inversion: pre-revert code errored here, post-revert code prints the registry and returns `Ok(())`.

Deleted: the 10 menu-mode tests (`run_menu` helper + `menu_*` cases) and two now-contradictory guards (`missing_agent_name_errors_when_not_listing`, `non_tty_without_agent_still_errors`).

Local run:

• `cargo test` — 936 pass (851 unit + 85 integration), 3 pre-existing ignored, 0 failed.
• `cargo clippy -- -D warnings` — clean.
• `cargo fmt -- --check` — clean.

Notes

• Behavior inversion for non-TTY callers: scripts that relied on bare `aimx agent-setup` exiting non-zero as an error signal will no longer hit that exit path. They should switch to passing `` explicitly (which was always the documented non-interactive form) or check for the registry output. This is the intended soft-revert behavior, not a regression.
• `AgentSpec.display_name` deleted: the field was added in e5ecc11 solely for menu rendering and had no other consumers. If a future `--list` expansion wants human-readable labels, adding the field back is cheap.

[uzyn]

Sprint Reviewer — Standalone Mode

Changes Overview

This PR is a targeted soft revert of commit e5ecc11 (which introduced an interactive numbered menu to aimx agent-setup). The menu path and the MCP (General) pseudo-option are gone; aimx agent-setup with no arguments now prints the supported-agents registry plus a one-line usage hint and exits 0 on both TTY and non-TTY. The --list, positional <agent>, --force, --print, and root-refusal paths are preserved. Two files change: src/agent_setup.rs (~280 line net reduction) and book/agent-integration.md (Guided setup → Discovering supported agents).

Scope Alignment

Matches the PR description one-for-one:

• Positional invocation unchanged — unknown agents still error.
• Bare invocation on TTY and non-TTY both print the registry plus hint, exit 0, write nothing to disk. Verified by running the built binary both interactively and with < /dev/null.
• --list unchanged (still prints registry, still bypasses the root check, no trailing hint).
• Root still refused up front on bare invocation (tested by bare_invocation_root_is_refused).
• Removed: Selection enum, prompt_agent_selection, print_mcp_general, render_mcp_general_snippet, AgentSpec.display_name, and the "MCP (General)" slot. Verified by rg — no residual references anywhere in source or tests.
• AgentEnv::is_stdin_tty / read_line retained — still used by the write_files overwrite prompt (agent_setup.rs:873,892).
• Book section rewritten correctly.

No scope creep.

Potential Bugs

None. Behavior is equivalent to the pre-menu state for --list and positional paths; the bare-invocation path is a straightforward "print registry + hint, return Ok(())" block with no conditional branching.

The print_registry → print_registry_to_writer refactor preserves output byte-for-byte: the old println!(...) / println!() sequences map to writeln!(out, ...)? / writeln!(out)?, which emit the same bytes (terminating \\n, blank line between agents, same leading blank line after the header). Confirmed by running the built binary against both --list and bare invocation.

Root-check ordering is correct for all three paths:

• --list → skips root check (returns early at line 450), same as before.
• Positional <agent> → root check fires first, then agent resolution.
• Bare → root check fires before the registry dump (matches PR description and is verified by bare_invocation_root_is_refused, which asserts out.is_empty() on the root-refusal path).

Security Issues

None. Removing the interactive code path reduces surface area slightly (no more read_line on the bare path); no new external inputs or privileged operations introduced.

Test Coverage

New tests cover the behavior well:

• bare_invocation_prints_registry_and_hint — loops over tty ∈ {false, true}, asserts every registry key appears, asserts the hint string appears, asserts nothing is installed. Exactly the regression guard the PR needs.
• bare_invocation_root_is_refused — asserts the error message contains "per-user" and that out is empty (no premature registry dump).
• bare_invocation_non_tty_still_prints_registry — explicit regression guard for the intentional behavior inversion called out in the PR body.

Deleted tests are all correctly obsolete:

• The 10 menu_* tests targeted the removed Selection / prompt code.
• missing_agent_name_errors_when_not_listing and non_tty_without_agent_still_errors are now contradicted by the new behavior; their deletion is intentional.

One minor gap worth mentioning (not required, non-blocker): there's no test asserting that the --list path does not emit the usage-hint footer. The distinction between bare-invocation output (with hint) and --list output (without hint) is a real behavior difference; a single-line assertion like assert!(!rendered.contains(\"Run \\aimx agent-setup"))on the existing--list` test would lock it in. I verified this manually against the built binary but tests don't enforce it.

Full test suite: 851 unit + 85 integration = 936 pass, 3 ignored, 0 failed. cargo clippy -- -D warnings and cargo fmt -- --check both clean — matches the PR's claims.

Code Quality

Two minor documentation nits (both non-blocker):

1. Stale doc comment at src/agent_setup.rs:437 — the docstring on run_with_env_to_writer still reads "writes menu / install output to out". The "menu" half is now fiction. Suggest something like "writes registry / install output to out".

2. Clap help text at src/cli.rs:112 — reads "Agent short name (e.g. claude-code). Omit with --list." Post-revert, omitting the agent without --list is also valid (it prints the registry). The current wording subtly implies --list is the only valid omission. Something like "Agent short name (e.g. claude-code). Omit to see the supported-agents registry." would be more accurate. Arguably pre-existing — even before e5ecc11 the bare path had behavior (just an error, not a registry) — but this PR is the right time to fix it since bare invocation is now user-facing output rather than an error.

Neither blocks merge. The core code is clean: the bare-invocation block is short, the refactor to print_registry_to_writer is mechanical and symmetric, and the test delta is well-scoped.

Summary and Recommended Actions

• Overall verdict: Needs minor fixes (documentation only — no functional changes required).
• Blockers: none.
• Non-blockers:
  1. Update stale docstring on run_with_env_to_writer (src/agent_setup.rs:437) — still says "menu".
  2. Update clap help text on the agent arg (src/cli.rs:112) to reflect the new bare-invocation behavior.
• Nice-to-haves:
  1. Add a small assertion in the --list test path that the usage-hint footer is absent, locking in the bare-vs-list output distinction.

The functional revert itself is correct, complete, and well-tested. If the author prefers to land as-is and address the two doc nits in a follow-up, that's defensible — both are string-only changes with no behavioral impact.

[uzyn]

Addressed the review feedback (commit 28baff3):

• Non-blocker [Sprint 1] Core Pipeline + Idea Validation #1 — refreshed the run_with_env_to_writer docstring (src/agent_setup.rs:437) so it describes the three real output shapes (install output, --list output, bare-invocation registry dump) instead of the now-fictional "menu / install output".
• Non-blocker [Sprint 2] DKIM + Production-Quality Outbound #2 — rewrote the clap help on agent: Option<String> (src/cli.rs:112) to Omit to print the supported-agent registry, or pass --list for the same view., removing the misleading "Omit with --list" wording.
• Nice-to-have — extended list_mode_runs_without_agent_name to capture stdout via run_with_env_to_writer and assert the bare-invocation usage hint Run \aimx agent-setup `is absent from--list` output, locking in the output distinction between the two paths.
• No behavior changes — docstring, help text, and one new assertion only.
• cargo fmt, cargo clippy -- -D warnings, and cargo test all clean (851 unit + 85 integration pass, 3 pre-existing ignored, 0 failed).

[uzyn]

Sprint Reviewer — Re-Review (Cycle 2)

All three cycle-1 items are addressed in commit 28baff3, with no new issues introduced and no regressions in the full test suite.

Resolved

1. Stale docstring on run_with_env_to_writer (src/agent_setup.rs:437) — rewritten to describe the three real output shapes (install output, --list output, bare-invocation registry dump plus usage-hint footer). No remaining mention of the deleted menu. Grammar and scanability are fine.

2. Clap help text on agent: Option<String> (src/cli.rs:112) — now reads "Agent short name (e.g. claude-code). Omit to print the supported-agent registry, or pass --list for the same view." Renders cleanly under aimx agent-setup --help (clap wraps the long line correctly, no layout weirdness). Accurately describes both omission paths — the "Omit with --list" misdirection is gone.

3. list_mode_runs_without_agent_name negative assertion — switched to capture stdout via run_with_env_to_writer(None, true, …, &mut out) and asserts !rendered.contains("Run \aimx agent-setup `"). I confirmed the assertion is meaningful: the hint string is emitted in exactly one place (src/agent_setup.rs:477, inside the bare-invocation arm of the agentmatch), and the--listpath returns early at line 451 before that arm is reached. If anyone later refactors the output paths and accidentally emits the hint on--list`, this test will fail.

Verification

• Built binary locally, confirmed empirically that aimx agent-setup --list ends at the registry entries with no Run \aimx agent-setup `footer, while bareaimx agent-setup` ends with exactly that footer line.
• cargo test — 851 unit + 85 integration = 936 pass, 3 pre-existing ignored, 0 failed.
• cargo clippy -- -D warnings — clean.
• cargo fmt -- --check — clean.
• The 28baff3 diff is strictly additive/doc-only: one docstring rewrite, one clap help string, one test body rewrite with one new assertion. No production behavior changes.

No new issues

The fix commit touches only prose (docstring, clap help, test comment) and one test-only code rewrite. Nothing that could introduce a regression in the agent-setup command surface itself.

Verdict: Ready to merge

No blockers, non-blockers, or nice-to-haves remaining.

Recommended merge commit message

refactor: revert aimx agent-setup to positional-only (soft)

Bare `aimx agent-setup` (no args, TTY or non-TTY) now prints the
supported-agent registry plus a one-line usage hint and exits 0,
replacing the interactive numbered menu added in e5ecc11. `--list`,
positional `<agent>`, `--force`, `--print`, `--data-dir`, and the
overwrite prompt are unchanged; root is still refused up front on
bare invocation. Removed: Selection enum, prompt_agent_selection,
print_mcp_general, render_mcp_general_snippet, "MCP (General)"
slot, and the now-unused AgentSpec.display_name field.
AgentEnv::is_stdin_tty / read_line stay on the trait because the
write_files overwrite prompt still uses them. Docs: replaced
book/agent-integration.md's "Guided setup" section with a short
paragraph describing the new bare-invocation behavior. Also
refreshed run_with_env_to_writer's docstring and rewrote the clap
help on <agent> so "Omit with --list" no longer implies --list is
the only valid omission path; extended list_mode_runs_without_agent_name
to lock in that --list output omits the bare-invocation usage hint.