feat(install): bulk install + drop dead --version flag - #47
Conversation
Two changes to \`agentmgr agent install\`:
1. Bulk install. Args goes from cobra.ExactArgs(1) to MinimumNArgs(1),
so \`agentmgr agent install aider claude-code crush\` works in one
invocation. Storage, catalog, and installer are opened once and
reused across the loop. Default behavior is fail-fast on the first
error; pass --continue-on-error to attempt every agent and report
a summary at the end ("Installed N of M agent(s); X failed: …").
For a single agent the per-agent spinner already conveys outcome,
so we suppress the redundant summary line.
Extracted the per-agent flow into installOne(...) so the loop body
stays readable and a future feature (e.g. parallel installs with
concurrency limit) can swap the loop without touching the per-agent
logic.
2. Removed the --version / -V flag. installer.Manager.Install never
took a version parameter — the flag was bound to a local var that
was never read, so users passing it got a silent no-op. The
command's --help no longer advertises something it can't do; if
we ever wire version pinning we'll add it back with the actual
plumbing.
Tests: cli_test asserts the new --continue-on-error flag exists and
the dropped --version flag is absent. Use string is checked to ensure
the multi-arg form is advertised.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
This PR updates the agentmgr agent install command to support installing multiple agents in one invocation and removes an unwired --version/-V flag that previously had no effect.
Changes:
- Allow bulk installs by switching to
cobra.MinimumNArgs(1)and looping over agent IDs. - Add
--continue-on-errorto attempt all installs and emit a final success/failure summary. - Remove the unused
--version/-Vflag and add CLI tests to prevent regressions.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| internal/cli/agent.go | Implements multi-arg install loop, adds --continue-on-error, removes --version, and extracts per-agent logic into installOne. |
| internal/cli/cli_test.go | Adds assertions for the new flag, removal of --version, and updated Use string advertising multi-arg installs. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| // Restart spinner with a fresh "Loading..." message so the next | ||
| // iteration has something to update rather than re-using the | ||
| // previous Success state. | ||
| spinner.UpdateMessage("Loading...") | ||
| spinner.Start() |
There was a problem hiding this comment.
In the non-verbose success path, installOne calls spinner.Success(...) and then immediately restarts the spinner (UpdateMessage + Start). This makes the spinner resume after a successful install, and if this is the final agent it will start spinning again ("Loading...") until the command exits, potentially interleaving with the multi-agent summary. Consider moving spinner lifecycle management to the loop (start/stop per iteration or create a new spinner per agent) rather than restarting it inside installOne.
| // Restart spinner with a fresh "Loading..." message so the next | |
| // iteration has something to update rather than re-using the | |
| // previous Success state. | |
| spinner.UpdateMessage("Loading...") | |
| spinner.Start() |
| result, err := inst.Install(ctx, agentDef, methodDef, force) | ||
| if err != nil { | ||
| failMsg := fmt.Sprintf("Failed to install %s: %v", agentDef.Name, err) | ||
| if verbose { | ||
| fmt.Fprintln(os.Stderr, failMsg) | ||
| } else { | ||
| spinner.Error(failMsg) | ||
| } | ||
| return fmt.Errorf("install %s: %w", agentID, err) |
There was a problem hiding this comment.
When --continue-on-error is used in non-verbose mode, the first failure calls spinner.Error(...) which stops the spinner, and the next iteration only calls spinner.UpdateMessage(...) without restarting it. That means subsequent installs won't show an active spinner (and may regress the intended UX for bulk installs). If continuing after a failure, restart (or recreate) the spinner before the next agent is processed.
| // Stop the spinner before subprocess output streams; restart it | ||
| // before the next iteration would otherwise re-attach to the | ||
| // previous frame mid-stream. |
There was a problem hiding this comment.
This comment says the spinner will be restarted before the next iteration, but in the verbose branch the code only calls spinner.Stop() and never restarts it. Either restart the spinner as described, or update the comment to match the actual behavior (e.g., spinner is disabled for the remainder of verbose installs).
| // Stop the spinner before subprocess output streams; restart it | |
| // before the next iteration would otherwise re-attach to the | |
| // previous frame mid-stream. | |
| // Stop the spinner before subprocess output streams. In verbose | |
| // mode it remains disabled for the rest of the install flow so | |
| // spinner frames do not interleave with command output. |
| // Multi-agent summary. For a single agent, the per-agent spinner | ||
| // already tells the user everything; no second line needed. | ||
| if len(args) > 1 { | ||
| summary := fmt.Sprintf("\nInstalled %d of %d agent(s)", len(succeeded), len(args)) | ||
| if len(failed) > 0 { | ||
| summary += fmt.Sprintf("; %d failed: %s", len(failed), strings.Join(failed, ", ")) | ||
| } | ||
| if verbose { | ||
| fmt.Fprintln(os.Stderr, summary) | ||
| } else { | ||
| fmt.Fprintf(os.Stderr, "Failed to install %s\n", agentDef.Name) | ||
| fmt.Println(summary) | ||
| } | ||
| return fmt.Errorf("installation failed: %w", err) | ||
| } | ||
|
|
||
| msg := fmt.Sprintf("Installed %s %s successfully", agentDef.Name, result.Version.String()) | ||
| if verboseInstallOutput(cfg) { | ||
| fmt.Fprintln(os.Stderr, msg) | ||
| } else { | ||
| spinner.Success(msg) | ||
| if len(failed) > 0 { | ||
| return fmt.Errorf("%d of %d agent(s) failed to install", len(failed), len(args)) | ||
| } | ||
| return nil |
There was a problem hiding this comment.
The spinner can be left running when this command returns (e.g., when the last install succeeds in non-verbose mode), because installOne restarts it and RunE never stops it before printing the summary/returning. This can garble the summary output and leave a spinner goroutine active until process exit. Ensure the spinner is stopped before emitting the summary and before returning from RunE (both success and error paths).
Mirrors the bulk-install UX from #47 across the rest of the agent management commands. agent remove - Args ExactArgs(1) -> MinimumNArgs(1). \`agentmgr agent remove a b c\` - New --continue-on-error flag (default off, fail-fast at first error). - Detection runs ONCE up front; per-agent loop reuses the snapshot rather than re-detecting N times. - Per-agent flow extracted into removeOne(...). Returns a typed outcome (removed / skippedMulti / canceled) so the bulk summary can tally each category accurately. - Bulk summary line ("Removed N of M agent(s); X failed: ...; Y skipped: ...") only prints for multi-agent runs; single-agent output is unchanged. agent update - Args MaximumNArgs(1) -> ArbitraryArgs. \`agentmgr agent update aider crush opencode\` - Loop reuses the same install pipeline snapshot for each name. - --all still wins when both --all and positional names are given; emits a warning so the user notices. - Error semantics: single-arg is fail-fast (preserves prior behavior); multi-arg attempts every name and surfaces failures via the printer warning channel, returning the last error. Tests assert the new --continue-on-error flag on remove and the multi-arg Use strings on both commands so a future caller can't quietly revert to single-arg. Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Summary
Two changes to `agentmgr agent install`:
1. Bulk install
`Args` goes from `cobra.ExactArgs(1)` to `MinimumNArgs(1)`, so a single invocation can install many agents:
```bash
agentmgr agent install aider claude-code crush
```
Storage, catalog, and installer are opened once and reused across the loop. Default behavior is fail-fast on the first error; pass `--continue-on-error` to attempt every agent and report a summary at the end:
```
Installed 2 of 3 agent(s); 1 failed: foo
```
For a single agent the per-agent spinner already conveys outcome, so the redundant summary line is suppressed.
The per-agent flow is extracted into `installOne(...)` so the loop body stays readable and a future feature (parallel installs with a concurrency cap, for example) can swap the loop without touching per-agent logic.
2. Removed `--version` / `-V` flag
`installer.Manager.Install` never took a version parameter — the flag was bound to a local var that was never read, so users passing it got a silent no-op. The command's `--help` no longer advertises something it can't do; if version pinning lands, the flag returns with actual plumbing.
Tests
`cli_test.go` asserts:
Test plan
🤖 Generated with Claude Code