From 76dc027cdbeb0bbfcb3e372c5bbbf7b2159a5afc Mon Sep 17 00:00:00 2001 From: Jeremy Daer Date: Wed, 22 Jul 2026 14:48:03 -0700 Subject: [PATCH 1/5] Fix #528: non-TTY installer hardcodes the Claude agent MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The piped install (`curl … | bash`) takes the non-TTY branch of scripts/install.sh, which unconditionally ran `basecamp setup claude`. The post-login nudge likewise pointed at the first detected-unhealthy agent in registry order (Claude-first when both were present). Add `basecamp setup agents`: a non-interactive command that installs the baseline skill and connects coding agents by intent, not registry order. Selection is driven by BASECAMP_SETUP_AGENT (claude|codex|all|none); unset auto-detects — 0 detected installs the skill only, 1 connects it, ≥2 stays neutral and surfaces the per-agent commands. Every safety-critical outcome lives in a top-level flat field of the result envelope so it survives the styled renderer (which skips nested map/[]map); the `agents` array is JSON-only detail. `all` forces every registered handler and synthesizes symmetric missing-binary remediation (the Claude handler treats an absent binary as no-op success while Codex errors). - install.sh / install.ps1: both non-TTY and skip branches now run `setup agents`; install.sh gains an if-form source guard and documents BASECAMP_SETUP_AGENT. - auth.go: rewrite printAgentNudge — one detected-unhealthy agent prints its `setup `; several print every choice directly, never guessing. - Regenerate .surface; add smoke declaration and e2e/installer.bats. --- .surface | 23 ++ e2e/installer.bats | 79 +++++++ e2e/smoke/smoke_lifecycle.bats | 4 + install.md | 12 +- internal/commands/auth.go | 29 ++- internal/commands/auth_test.go | 63 +++++ internal/commands/setup_agents_test.go | 292 +++++++++++++++++++++++ internal/commands/wizard.go | 1 + internal/commands/wizard_agents.go | 315 +++++++++++++++++++++++++ scripts/install.ps1 | 15 ++ scripts/install.sh | 35 ++- 11 files changed, 855 insertions(+), 13 deletions(-) create mode 100644 e2e/installer.bats create mode 100644 internal/commands/auth_test.go create mode 100644 internal/commands/setup_agents_test.go diff --git a/.surface b/.surface index 84f0339bb..751fac6dd 100644 --- a/.surface +++ b/.surface @@ -850,6 +850,7 @@ CMD basecamp search CMD basecamp search metadata CMD basecamp search types CMD basecamp setup +CMD basecamp setup agents CMD basecamp setup claude CMD basecamp setup codex CMD basecamp show @@ -11428,6 +11429,27 @@ FLAG basecamp setup --stats type=bool FLAG basecamp setup --styled type=bool FLAG basecamp setup --todolist type=string FLAG basecamp setup --verbose type=count +FLAG basecamp setup agents --account type=string +FLAG basecamp setup agents --agent type=bool +FLAG basecamp setup agents --cache-dir type=string +FLAG basecamp setup agents --count type=bool +FLAG basecamp setup agents --help type=bool +FLAG basecamp setup agents --hints type=bool +FLAG basecamp setup agents --ids-only type=bool +FLAG basecamp setup agents --in type=string +FLAG basecamp setup agents --jq type=string +FLAG basecamp setup agents --json type=bool +FLAG basecamp setup agents --markdown type=bool +FLAG basecamp setup agents --md type=bool +FLAG basecamp setup agents --no-hints type=bool +FLAG basecamp setup agents --no-stats type=bool +FLAG basecamp setup agents --profile type=string +FLAG basecamp setup agents --project type=string +FLAG basecamp setup agents --quiet type=bool +FLAG basecamp setup agents --stats type=bool +FLAG basecamp setup agents --styled type=bool +FLAG basecamp setup agents --todolist type=string +FLAG basecamp setup agents --verbose type=count FLAG basecamp setup claude --account type=string FLAG basecamp setup claude --agent type=bool FLAG basecamp setup claude --cache-dir type=string @@ -16470,6 +16492,7 @@ SUB basecamp search SUB basecamp search metadata SUB basecamp search types SUB basecamp setup +SUB basecamp setup agents SUB basecamp setup claude SUB basecamp setup codex SUB basecamp show diff --git a/e2e/installer.bats b/e2e/installer.bats new file mode 100644 index 000000000..a0983b6ea --- /dev/null +++ b/e2e/installer.bats @@ -0,0 +1,79 @@ +#!/usr/bin/env bats +# installer.bats - Tests for the install scripts' post-install agent setup. +# +# #528: the non-TTY and skip branches must run `setup agents` (baseline skill + +# best-effort agent connection), never the hardcoded `setup claude`. + +setup() { + # The installer contract keys off these; a leaked value would skew results. + unset BASECAMP_SKIP_SETUP BASECAMP_SETUP_AGENT + + INSTALL_SH="${BATS_TEST_DIRNAME}/../scripts/install.sh" + INSTALL_PS1="${BATS_TEST_DIRNAME}/../scripts/install.ps1" + + STUB_DIR="$(mktemp -d)" + LOG="$STUB_DIR/calls.log" + + # Stub `basecamp` that logs its argv so we can see which subcommand ran. + cat > "$STUB_DIR/basecamp" <> "$LOG" +EOF + chmod +x "$STUB_DIR/basecamp" +} + +teardown() { + [[ -n "${STUB_DIR:-}" ]] && rm -rf "$STUB_DIR" +} + +# The if-form guard must let sourcing define functions without running main. +@test "install.sh can be sourced without running the installer" { + run bash -c "set -euo pipefail; source '$INSTALL_SH'; echo sourced-ok" + [[ "$status" -eq 0 ]] + [[ "$output" == *"sourced-ok"* ]] + [[ "$output" != *"Basecamp CLI"* ]] # banner would print if main ran +} + +@test "post_install_setup dispatches to 'setup agents', never 'setup claude'" { + run bash -c " + set -euo pipefail + source '$INSTALL_SH' + BIN_DIR='$STUB_DIR' + post_install_setup basecamp + cat '$LOG' + " + [[ "$status" -eq 0 ]] + [[ "$output" == *"setup agents"* ]] + [[ "$output" != *"setup claude"* ]] +} + +@test "post_install_setup honors BASECAMP_SKIP_SETUP path (still 'setup agents')" { + run bash -c " + set -euo pipefail + export BASECAMP_SKIP_SETUP=1 + source '$INSTALL_SH' + BIN_DIR='$STUB_DIR' + post_install_setup basecamp + cat '$LOG' + " + [[ "$status" -eq 0 ]] + [[ "$output" == *"setup agents"* ]] + [[ "$output" != *"setup claude"* ]] +} + +@test "install.sh has no residual 'setup claude'" { + run grep -n "setup claude" "$INSTALL_SH" + [[ "$status" -ne 0 ]] # grep exits non-zero when nothing matches +} + +@test "install.sh skip and non-tty branches both dispatch via post_install_setup" { + run grep -c 'post_install_setup "\$binary_name"' "$INSTALL_SH" + [[ "$status" -eq 0 ]] + [[ "$output" -ge 2 ]] +} + +@test "install.ps1 skip and non-interactive branches each invoke 'setup agents'" { + run grep -c '& \$installedBinary setup agents' "$INSTALL_PS1" + [[ "$status" -eq 0 ]] + [[ "$output" -eq 2 ]] +} diff --git a/e2e/smoke/smoke_lifecycle.bats b/e2e/smoke/smoke_lifecycle.bats index 9c85f225a..e0d4ff7fb 100644 --- a/e2e/smoke/smoke_lifecycle.bats +++ b/e2e/smoke/smoke_lifecycle.bats @@ -32,6 +32,10 @@ load smoke_helper mark_out_of_scope "Modifies Claude Code config" } +@test "setup agents is out of scope" { + mark_out_of_scope "Modifies coding-agent config" +} + @test "quick-start is out of scope" { mark_out_of_scope "Interactive onboarding wizard" } diff --git a/install.md b/install.md index 7f4c2d385..6fe8b7d69 100644 --- a/install.md +++ b/install.md @@ -2,6 +2,8 @@ Install the Basecamp CLI and connect it to your AI agent. Execute all steps autonomously. +The piped installer (Step 1) already installs the baseline agent skill and, on a best-effort basis, attempts to connect a single detected coding agent. The optional "Connect Your Agent" section below is for forcing a specific agent, handling multiple detected agents, or verifying the connection. + **DONE WHEN:** `basecamp --version && basecamp auth status` both succeed. --- @@ -10,7 +12,7 @@ Install the Basecamp CLI and connect it to your AI agent. Execute all steps auto - [ ] Install Basecamp CLI - [ ] Authenticate with Basecamp -- [ ] Connect agent (optional) +- [ ] Connect agent (optional — the piped installer already attempts one detected agent) --- @@ -28,7 +30,11 @@ curl -fsSL https://basecamp.com/install-cli | bash irm https://raw.githubusercontent.com/basecamp/basecamp-cli/main/scripts/install.ps1 | iex ``` -> **Note:** The install scripts auto-detect non-interactive environments (CI, piped input, coding agents) and skip the interactive setup wizard. You can also explicitly skip it with `BASECAMP_SKIP_SETUP=1`. +> **Note:** The install scripts auto-detect non-interactive environments (CI, piped input, coding agents) and skip the interactive setup wizard. In that case they still run `basecamp setup agents`, which installs the baseline agent skill and **attempts to connect** a single detected coding agent (best effort). If several agents are detected, or none is, only the baseline skill is installed and the per-agent commands are surfaced. Explicitly skipping the wizard with `BASECAMP_SKIP_SETUP=1` still runs `setup agents`. +> +> Choose which agent to connect with `BASECAMP_SETUP_AGENT` (`claude`, `codex`, `all`, or `none`). Set it for the interpreter, not the fetch: +> - Bash: `curl -fsSL https://basecamp.com/install-cli | BASECAMP_SETUP_AGENT=codex bash` +> - PowerShell: `$env:BASECAMP_SETUP_AGENT='codex'; irm https://raw.githubusercontent.com/basecamp/basecamp-cli/main/scripts/install.ps1 | iex` > > **Windows note:** if `curl` fails with a `schannel` / `CRYPT_E_NO_REVOCATION_CHECK` TLS error, prefer the PowerShell installer, Scoop, or Git Bash's `/usr/bin/curl` instead of the system `curl.exe`. @@ -112,7 +118,7 @@ EXECUTE NOW: Start with Step 1. Mark TODO items complete as you go. Stop when `b **Do not execute this section unless explicitly requested.** The core installation is complete when DONE WHEN passes. -Both agent plugins require the `basecamp` CLI installed above — the plugin invokes it for every Basecamp operation. +The piped installer (Step 1) already installs the baseline skill and attempts to connect a single detected agent. Run the commands here to force a specific agent, connect a second one, or when several agents were detected and the installer connected none. Both agent plugins require the `basecamp` CLI installed above — the plugin invokes it for every Basecamp operation. ### Claude Code diff --git a/internal/commands/auth.go b/internal/commands/auth.go index 2a78e7884..8ddd54ee6 100644 --- a/internal/commands/auth.go +++ b/internal/commands/auth.go @@ -5,6 +5,7 @@ import ( "fmt" "io" "os" + "sort" "strings" "time" @@ -332,18 +333,38 @@ func buildLogoutCmd(use string) *cobra.Command { } // printAgentNudge prints a hint about coding agent setup after login. +// +// Detection proves presence, not intent: with a single detected-unhealthy agent +// it points at that agent; with several, it never guesses — it prints every +// `basecamp setup ` choice so the user picks. func printAgentNudge(w io.Writer, r *output.Renderer) { + type nudgeAgent struct{ id, name string } + var unhealthy []nudgeAgent for _, agent := range harness.DetectedAgents() { if agent.Checks == nil { continue } for _, c := range agent.Checks() { if c.Status != "pass" { - fmt.Fprintln(w) - fmt.Fprintln(w, r.Muted.Render(fmt.Sprintf(" %s detected. Connect it to Basecamp:", agent.Name))) - fmt.Fprintln(w, r.Data.Render(fmt.Sprintf(" basecamp setup %s", agent.ID))) - return // one nudge is enough + unhealthy = append(unhealthy, nudgeAgent{id: agent.ID, name: agent.Name}) + break } } } + sort.Slice(unhealthy, func(i, j int) bool { return unhealthy[i].id < unhealthy[j].id }) + + switch len(unhealthy) { + case 0: + return + case 1: + fmt.Fprintln(w) + fmt.Fprintln(w, r.Muted.Render(fmt.Sprintf(" %s detected. Connect it to Basecamp:", unhealthy[0].name))) + fmt.Fprintln(w, r.Data.Render(fmt.Sprintf(" basecamp setup %s", unhealthy[0].id))) + default: + fmt.Fprintln(w) + fmt.Fprintln(w, r.Muted.Render(" Multiple coding agents detected. Choose one:")) + for _, a := range unhealthy { + fmt.Fprintln(w, r.Data.Render(fmt.Sprintf(" basecamp setup %s", a.id))) + } + } } diff --git a/internal/commands/auth_test.go b/internal/commands/auth_test.go new file mode 100644 index 000000000..14a219242 --- /dev/null +++ b/internal/commands/auth_test.go @@ -0,0 +1,63 @@ +package commands + +import ( + "bytes" + "io" + "os" + "path/filepath" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + "github.com/basecamp/basecamp-cli/internal/output" +) + +// nudgeOutput runs printAgentNudge against a non-styled renderer and returns +// the plain text written for the login hint. +func nudgeOutput(t *testing.T) string { + t.Helper() + buf := &bytes.Buffer{} + printAgentNudge(buf, output.NewRenderer(io.Discard, false)) + return buf.String() +} + +// TestPrintAgentNudgeNoneDetected: no detected agent → no nudge. +func TestPrintAgentNudgeNoneDetected(t *testing.T) { + home := t.TempDir() + t.Setenv("HOME", home) + t.Setenv("PATH", filepath.Join(home, "empty-bin")) + + assert.Empty(t, nudgeOutput(t)) +} + +// TestPrintAgentNudgeSingle: one detected-unhealthy agent → its `setup `. +func TestPrintAgentNudgeSingle(t *testing.T) { + home := t.TempDir() + t.Setenv("HOME", home) + t.Setenv("PATH", filepath.Join(home, "empty-bin")) + require.NoError(t, os.MkdirAll(filepath.Join(home, ".codex"), 0o755)) + + out := nudgeOutput(t) + + assert.Contains(t, out, "Codex detected") + assert.Contains(t, out, "basecamp setup codex") + assert.NotContains(t, out, "basecamp setup claude") +} + +// TestPrintAgentNudgeMultiple: ≥2 detected-unhealthy → every choice printed +// directly (never Claude-first, never routed to `setup agents`). +func TestPrintAgentNudgeMultiple(t *testing.T) { + home := t.TempDir() + t.Setenv("HOME", home) + t.Setenv("PATH", filepath.Join(home, "empty-bin")) + require.NoError(t, os.MkdirAll(filepath.Join(home, ".claude"), 0o755)) + require.NoError(t, os.MkdirAll(filepath.Join(home, ".codex"), 0o755)) + + out := nudgeOutput(t) + + assert.Contains(t, out, "Multiple coding agents detected") + assert.Contains(t, out, "basecamp setup claude") + assert.Contains(t, out, "basecamp setup codex") + assert.NotContains(t, out, "basecamp setup agents") +} diff --git a/internal/commands/setup_agents_test.go b/internal/commands/setup_agents_test.go new file mode 100644 index 000000000..7f6a09b03 --- /dev/null +++ b/internal/commands/setup_agents_test.go @@ -0,0 +1,292 @@ +package commands + +import ( + "bytes" + "context" + "encoding/json" + "os" + "path/filepath" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + "github.com/basecamp/basecamp-cli/internal/appctx" + "github.com/basecamp/basecamp-cli/internal/output" +) + +// setupAgentsEnvelope mirrors the `setup agents` typed result contract. +type setupAgentsEnvelope struct { + Summary string `json:"summary"` + Data struct { + SkillInstalled bool `json:"skill_installed"` + Selector string `json:"selector"` + Ambiguous bool `json:"ambiguous"` + DetectedBefore []string `json:"detected_before"` + AttemptedAgents []string `json:"attempted_agents"` + Errors []string `json:"errors"` + Warnings []string `json:"warnings"` + ManualCommands []string `json:"manual_commands"` + Agents []struct { + ID string `json:"id"` + Name string `json:"name"` + DetectedBefore bool `json:"detected_before"` + DetectedAfter bool `json:"detected_after"` + PluginInstalled bool `json:"plugin_installed"` + Errors []string `json:"errors"` + ManualCommands []string `json:"manual_commands"` + } `json:"agents"` + } `json:"data"` +} + +// runSetupAgentsJSON executes `setup agents` in machine mode and parses the envelope. +func runSetupAgentsJSON(t *testing.T) setupAgentsEnvelope { + t.Helper() + app, out := setupQuickstartTestApp(t, "", "") + app.Flags.JSON = true + app.Flags.Hints = true + t.Cleanup(app.Close) + + cmd := NewSetupCmd() + cmd.SetArgs([]string{"agents"}) + cmd.SetContext(appctx.WithApp(context.Background(), app)) + cmd.SetOut(&bytes.Buffer{}) + cmd.SetErr(&bytes.Buffer{}) + require.NoError(t, cmd.Execute()) + + var envelope setupAgentsEnvelope + require.NoError(t, json.Unmarshal(out.Bytes(), &envelope), out.String()) + return envelope +} + +// runSetupAgentsStyled executes `setup agents` rendering the styled (ANSI) +// output — the real installer path (curl | bash: piped stdin, TTY stdout). +func runSetupAgentsStyled(t *testing.T) string { + t.Helper() + app, out := setupQuickstartTestApp(t, "", "") + app.Output = output.New(output.Options{Format: output.FormatStyled, Writer: out}) + t.Cleanup(app.Close) + + cmd := NewSetupCmd() + cmd.SetArgs([]string{"agents"}) + cmd.SetContext(appctx.WithApp(context.Background(), app)) + cmd.SetOut(&bytes.Buffer{}) + cmd.SetErr(&bytes.Buffer{}) + require.NoError(t, cmd.Execute()) + return out.String() +} + +// emptyHome points HOME and PATH at empty temp dirs so no agent is detected. +func emptyHome(t *testing.T) string { + t.Helper() + home := t.TempDir() + t.Setenv("HOME", home) + t.Setenv("PATH", filepath.Join(home, "empty-bin")) + return home +} + +func TestNewSetupCmdHasAgentsSubcommand(t *testing.T) { + assert.NotNil(t, findSubcommand(NewSetupCmd(), "agents")) +} + +// TestSetupAgentsStyledSurfacesFailure is the styled-output regression guard: +// a per-agent failure must reach the human-facing output via a top-level flat +// field, since the styled renderer skips the nested `agents` array. +func TestSetupAgentsStyledSurfacesFailure(t *testing.T) { + installCodexStub(t, codexStubOptions{marketplaceFailure: true}) + t.Setenv("BASECAMP_SETUP_AGENT", "codex") + + out := runSetupAgentsStyled(t) + + assert.Contains(t, out, "codex") + assert.Contains(t, out, "marketplace add", "per-agent failure must survive styled rendering via top-level errors") +} + +func TestSetupAgentsZeroDetected(t *testing.T) { + emptyHome(t) + t.Setenv("BASECAMP_SETUP_AGENT", "") + + env := runSetupAgentsJSON(t) + + assert.True(t, env.Data.SkillInstalled) + assert.Equal(t, "auto", env.Data.Selector) + assert.False(t, env.Data.Ambiguous) + assert.Empty(t, env.Data.DetectedBefore) + assert.Empty(t, env.Data.AttemptedAgents) +} + +// TestSetupAgentsBaselineSkillFailure forces installSkillFiles to fail by making +// ~/.agents a regular file, so MkdirAll under it errors. +func TestSetupAgentsBaselineSkillFailure(t *testing.T) { + home := emptyHome(t) + t.Setenv("BASECAMP_SETUP_AGENT", "none") + require.NoError(t, os.WriteFile(filepath.Join(home, ".agents"), []byte("blocker"), 0o644)) + + env := runSetupAgentsJSON(t) + + assert.False(t, env.Data.SkillInstalled) + require.NotEmpty(t, env.Data.Errors) + assert.Contains(t, env.Data.Errors[0], "skill:") +} + +func TestSetupAgentsSingleDetectedCodex(t *testing.T) { + installCodexStub(t, codexStubOptions{}) + t.Setenv("BASECAMP_SETUP_AGENT", "") + + env := runSetupAgentsJSON(t) + + assert.Equal(t, "auto", env.Data.Selector) + assert.False(t, env.Data.Ambiguous) + assert.Equal(t, []string{"codex"}, env.Data.AttemptedAgents) + require.Len(t, env.Data.Agents, 1) + assert.Equal(t, "codex", env.Data.Agents[0].ID) + assert.True(t, env.Data.Agents[0].DetectedBefore) + assert.True(t, env.Data.Agents[0].DetectedAfter) + assert.True(t, env.Data.Agents[0].PluginInstalled) +} + +// TestSetupAgentsAmbiguous verifies that ≥2 detected agents with no selector +// never guesses — the flat data surfaces both choices. +func TestSetupAgentsAmbiguous(t *testing.T) { + home := emptyHome(t) + require.NoError(t, os.MkdirAll(filepath.Join(home, ".claude"), 0o755)) + require.NoError(t, os.MkdirAll(filepath.Join(home, ".codex"), 0o755)) + t.Setenv("BASECAMP_SETUP_AGENT", "") + + env := runSetupAgentsJSON(t) + + assert.Equal(t, "auto", env.Data.Selector) + assert.True(t, env.Data.Ambiguous) + assert.Empty(t, env.Data.AttemptedAgents) + assert.Equal(t, []string{"claude", "codex"}, env.Data.DetectedBefore) + assert.Equal(t, []string{"basecamp setup claude", "basecamp setup codex"}, env.Data.ManualCommands) + assert.NotEmpty(t, env.Data.Warnings) +} + +// TestSetupAgentsAllForcesEveryHandler covers =all with zero, one, and two +// detected agents: every registered handler is attempted, and absent-binary +// agents produce synthesized remediation. +func TestSetupAgentsAllForcesEveryHandler(t *testing.T) { + t.Run("zero detected", func(t *testing.T) { + emptyHome(t) + t.Setenv("BASECAMP_SETUP_AGENT", "all") + + env := runSetupAgentsJSON(t) + + assert.Equal(t, "all", env.Data.Selector) + assert.Equal(t, []string{"claude", "codex"}, env.Data.AttemptedAgents) + // Both binaries absent → symmetric synthesized remediation. + assert.Contains(t, env.Data.ManualCommands, "basecamp setup claude") + assert.Contains(t, env.Data.ManualCommands, "basecamp setup codex") + require.GreaterOrEqual(t, len(env.Data.Warnings), 2) + }) + + t.Run("one detected", func(t *testing.T) { + installCodexStub(t, codexStubOptions{}) // codex binary present, claude absent + t.Setenv("BASECAMP_SETUP_AGENT", "all") + + env := runSetupAgentsJSON(t) + + assert.Equal(t, []string{"claude", "codex"}, env.Data.AttemptedAgents) + // Claude binary absent → synthesized; codex present and healthy → not. + assert.Contains(t, env.Data.ManualCommands, "basecamp setup claude") + assert.NotContains(t, env.Data.ManualCommands, "basecamp setup codex") + }) + + t.Run("two detected", func(t *testing.T) { + home := emptyHome(t) + require.NoError(t, os.MkdirAll(filepath.Join(home, ".claude"), 0o755)) + require.NoError(t, os.MkdirAll(filepath.Join(home, ".codex"), 0o755)) + t.Setenv("BASECAMP_SETUP_AGENT", "all") + + env := runSetupAgentsJSON(t) + + assert.False(t, env.Data.Ambiguous, "explicit selector is never ambiguous") + assert.Equal(t, []string{"claude", "codex"}, env.Data.AttemptedAgents) + assert.Contains(t, env.Data.ManualCommands, "basecamp setup claude") + assert.Contains(t, env.Data.ManualCommands, "basecamp setup codex") + }) +} + +// TestSetupAgentsCodexMissingBinary asserts the real missing-binary contract: +// the agentSetupError is unioned into top-level errors + a warning, and the +// deduped remediation is the single `basecamp setup codex` (not the 3-command seq). +func TestSetupAgentsCodexMissingBinary(t *testing.T) { + emptyHome(t) + t.Setenv("BASECAMP_SETUP_AGENT", "codex") + + env := runSetupAgentsJSON(t) + + assert.Equal(t, "codex", env.Data.Selector) + require.NotEmpty(t, env.Data.Errors) + assert.Contains(t, env.Data.Errors[0], "codex: ") + assert.Contains(t, env.Data.Errors[0], "Codex executable not found") + assert.NotEmpty(t, env.Data.Warnings) + assert.Equal(t, []string{"basecamp setup codex"}, env.Data.ManualCommands) +} + +// TestSetupAgentsCodexPreservesManualOrder guards against string-sorting: when +// marketplace add fails, the aggregate manual_commands must preserve Codex's own +// ordered remediation sequence (marketplace add → upgrade → plugin add). +func TestSetupAgentsCodexPreservesManualOrder(t *testing.T) { + installCodexStub(t, codexStubOptions{marketplaceFailure: true}) + t.Setenv("BASECAMP_SETUP_AGENT", "codex") + + env := runSetupAgentsJSON(t) + + assert.Equal(t, []string{ + "codex plugin marketplace add basecamp/claude-plugins", + "codex plugin marketplace upgrade 37signals", + "codex plugin add basecamp@37signals", + }, env.Data.ManualCommands) +} + +// TestSetupAgentsClaudeNoBinary asserts the real Claude contract: no binary means +// plugin_installed:false, but linkSkillToClaude creates ~/.claude/skills/basecamp +// so detection flips to true, and missing-binary remediation is synthesized. +func TestSetupAgentsClaudeNoBinary(t *testing.T) { + home := emptyHome(t) + t.Setenv("BASECAMP_SETUP_AGENT", "claude") + + env := runSetupAgentsJSON(t) + + require.Len(t, env.Data.Agents, 1) + claude := env.Data.Agents[0] + assert.Equal(t, "claude", claude.ID) + assert.False(t, claude.DetectedBefore) + assert.True(t, claude.DetectedAfter, "linkSkillToClaude creates ~/.claude, flipping detection") + assert.False(t, claude.PluginInstalled) + + // linkSkillToClaude created the skill link. + _, statErr := os.Stat(filepath.Join(home, ".claude", "skills", "basecamp")) + assert.NoError(t, statErr) + + assert.NotEmpty(t, env.Data.Warnings) + assert.Contains(t, env.Data.ManualCommands, "basecamp setup claude") +} + +func TestSetupAgentsNoneSelector(t *testing.T) { + home := emptyHome(t) + require.NoError(t, os.MkdirAll(filepath.Join(home, ".codex"), 0o755)) + t.Setenv("BASECAMP_SETUP_AGENT", "none") + + env := runSetupAgentsJSON(t) + + assert.Equal(t, "none", env.Data.Selector) + assert.True(t, env.Data.SkillInstalled) + assert.Empty(t, env.Data.AttemptedAgents) + assert.Equal(t, []string{"codex"}, env.Data.DetectedBefore) +} + +func TestSetupAgentsInvalidSelector(t *testing.T) { + emptyHome(t) + t.Setenv("BASECAMP_SETUP_AGENT", "frobnicate") + + env := runSetupAgentsJSON(t) + + assert.Equal(t, "invalid", env.Data.Selector) + assert.True(t, env.Data.SkillInstalled) + assert.Empty(t, env.Data.AttemptedAgents) + require.NotEmpty(t, env.Data.Warnings) + assert.Contains(t, env.Data.Warnings[0], "frobnicate") +} diff --git a/internal/commands/wizard.go b/internal/commands/wizard.go index ed821be4c..35c1cab60 100644 --- a/internal/commands/wizard.go +++ b/internal/commands/wizard.go @@ -45,6 +45,7 @@ func NewSetupCmd() *cobra.Command { for _, sub := range newSetupAgentCmds() { cmd.AddCommand(sub) } + cmd.AddCommand(newSetupAgentsCmd()) return cmd } diff --git a/internal/commands/wizard_agents.go b/internal/commands/wizard_agents.go index fc3fe8bf8..75f490b51 100644 --- a/internal/commands/wizard_agents.go +++ b/internal/commands/wizard_agents.go @@ -7,6 +7,7 @@ import ( "os" "os/exec" "path/filepath" + "sort" "strings" "github.com/spf13/cobra" @@ -544,6 +545,320 @@ func newSetupAgentCmds() []*cobra.Command { return cmds } +// agentSetupEnv selects which coding agents `setup agents` targets. +// Values: claude | codex | all | none. Empty (unset) means auto-detect. +const agentSetupEnv = "BASECAMP_SETUP_AGENT" + +// newSetupAgentsCmd builds `setup agents`. It always runs non-interactively: +// it installs the baseline skill, connects agents per the BASECAMP_SETUP_AGENT +// selector (or auto-detection), and emits a structured envelope. It never +// prompts, so it is safe for the piped installer and coding-agent shells. +func newSetupAgentsCmd() *cobra.Command { + return &cobra.Command{ + Use: "agents", + Short: "Install the Basecamp skill and connect detected coding agents", + Long: "Install the baseline Basecamp agent skill and attempt to connect coding agents.\n\n" + + "Selection is controlled by " + agentSetupEnv + ": claude, codex, all, or none. When\n" + + "unset, a single detected agent is connected; when several are detected none is\n" + + "guessed — the per-agent `basecamp setup ` commands are surfaced instead.", + RunE: func(cmd *cobra.Command, args []string) error { + app := appctx.FromContext(cmd.Context()) + if app == nil { + return fmt.Errorf("app not initialized") + } + return runNonInteractiveAgentSetup(cmd, app) + }, + } +} + +// agentSetupRecord is the per-agent outcome captured while running handlers. +// errors/manualCommands are stored bare (not id-prefixed); the top-level union +// prefixes errors with the agent id. +type agentSetupRecord struct { + id, name string + detectedBefore bool + detectedAfter bool + pluginInstalled bool + binaryAbsent bool + errors []string + manualCommands []string +} + +// runNonInteractiveAgentSetup installs the baseline skill, resolves the agent +// selector, runs each targeted handler, and returns a structured envelope where +// every safety-critical outcome lives in a top-level flat field (the styled +// renderer skips nested map/[]map, so the `agents` array is JSON-only detail). +func runNonInteractiveAgentSetup(cmd *cobra.Command, app *appctx.App) error { + // Baseline skill: installed regardless of selector. + _, skillErr := installSkillFiles() + skillInstalled := skillErr == nil + + // Pre-run detection snapshot (set-like → sorted by id). + detectedBefore := detectedAgentIDs() + + selectorRaw := strings.TrimSpace(os.Getenv(agentSetupEnv)) + selector := strings.ToLower(selectorRaw) + + var warnings []string + var ambiguous bool + var ambiguousManual []string + var targets []harness.AgentInfo + + switch selector { + case "", "auto": + selector = "auto" + detected := harness.DetectedAgents() + switch len(detected) { + case 0: + // baseline skill only + case 1: + targets = detected + default: + ambiguous = true + ambiguousManual = agentChoiceCommands(detected) + warnings = append(warnings, "Multiple coding agents detected; installed the baseline skill only. Choose one: "+strings.Join(ambiguousManual, ", ")) + } + case "all": + targets = harness.AllAgents() + case "none": + // baseline skill only + case "claude", "codex": + if a := harness.FindAgent(selector); a != nil { + targets = []harness.AgentInfo{*a} + } + default: + selector = "invalid" + warnings = append(warnings, fmt.Sprintf("Unknown %s value %q; installed the baseline skill only (expected claude, codex, all, or none)", agentSetupEnv, selectorRaw)) + } + + // Run handlers in id order so aggregation is deterministic. + sort.Slice(targets, func(i, j int) bool { return targets[i].ID < targets[j].ID }) + records := make([]agentSetupRecord, 0, len(targets)) + for _, agent := range targets { + records = append(records, runAgentSetupHandler(cmd, agent)) + } + + attempted := make([]string, 0, len(records)) + for _, r := range records { + attempted = append(attempted, r.id) + } + + // errors: union of baseline + every per-agent error (id-prefixed), stable + // first-seen dedup, in sorted-agent order. Never string-sorted. + errUnion := newOrderedStringSet() + if skillErr != nil { + errUnion.add(fmt.Sprintf("skill: %s", skillErr)) + } + for _, r := range records { + for _, e := range r.errors { + errUnion.add(fmt.Sprintf("%s: %s", r.id, e)) + } + } + + // manual_commands: ambiguous → both `setup `; else union of each + // handler's own ordered sequence plus a synthesized hint for absent + // binaries. Stable first-seen dedup preserves each handler's order. + manualUnion := newOrderedStringSet() + if ambiguous { + for _, m := range ambiguousManual { + manualUnion.add(m) + } + } else { + for _, r := range records { + for _, m := range r.manualCommands { + manualUnion.add(m) + } + if r.binaryAbsent { + manualUnion.add("basecamp setup " + r.id) + } + } + } + + // warnings: synthesized missing-binary remediation, sorted-agent order. + // The Claude handler treats a missing binary as no-op success while Codex + // returns an error, so synthesizing here keeps remediation symmetric. + for _, r := range records { + if r.binaryAbsent { + warnings = append(warnings, fmt.Sprintf("%s: %s binary not found; install %s, then run: basecamp setup %s", r.id, r.name, r.name, r.id)) + } + } + + agentsDetail := make([]map[string]any, 0, len(records)) + for _, r := range records { + agentsDetail = append(agentsDetail, map[string]any{ + "id": r.id, + "name": r.name, + "detected_before": r.detectedBefore, + "detected_after": r.detectedAfter, + "plugin_installed": r.pluginInstalled, + "errors": orEmptyStrings(r.errors), + "manual_commands": orEmptyStrings(r.manualCommands), + }) + } + + result := map[string]any{ + "skill_installed": skillInstalled, + "selector": selector, + "ambiguous": ambiguous, + "detected_before": detectedBefore, + "attempted_agents": attempted, + "errors": errUnion.slice(), + "warnings": orEmptyStrings(warnings), + "manual_commands": manualUnion.slice(), + "agents": agentsDetail, + } + + manual := manualUnion.slice() + breadcrumbs := make([]output.Breadcrumb, 0, 1+len(manual)) + breadcrumbs = append(breadcrumbs, output.Breadcrumb{Action: "doctor", Cmd: "basecamp doctor", Description: "Check CLI health"}) + for i, m := range manual { + breadcrumbs = append(breadcrumbs, output.Breadcrumb{ + Action: fmt.Sprintf("manual_step_%d", i+1), + Cmd: m, + Description: "Manual setup step", + }) + } + + return app.OK(result, + output.WithSummary(agentSetupSummary(selector, ambiguous, skillInstalled, records)), + output.WithBreadcrumbs(breadcrumbs...), + ) +} + +// runAgentSetupHandler runs one agent's non-interactive handler and captures +// its before/after detection, plugin health, and remediation. +func runAgentSetupHandler(cmd *cobra.Command, agent harness.AgentInfo) agentSetupRecord { + rec := agentSetupRecord{ + id: agent.ID, + name: agent.Name, + detectedBefore: agent.Detect != nil && agent.Detect(), + binaryAbsent: !agentBinaryPresent(agent.ID), + } + + if handler, ok := agentSetupHandlers[agent.ID]; ok && handler.RunNonInteractive != nil { + if err := handler.RunNonInteractive(cmd); err != nil { + rec.errors = append(rec.errors, err.Error()) + var setupErr *agentSetupError + if errors.As(err, &setupErr) { + rec.manualCommands = append(rec.manualCommands, setupErr.Manual...) + } + } + } + + rec.detectedAfter = agent.Detect != nil && agent.Detect() + rec.pluginInstalled = agentChecksPass(agent) + return rec +} + +// agentBinaryPresent reports whether the agent's executable is on disk. +// Unknown agents are assumed present so no bogus remediation is synthesized. +func agentBinaryPresent(id string) bool { + switch id { + case "claude": + return harness.FindClaudeBinary() != "" + case "codex": + return harness.FindCodexBinary() != "" + default: + return true + } +} + +// agentChecksPass reports whether every health check for the agent passes. +func agentChecksPass(agent harness.AgentInfo) bool { + if agent.Checks == nil { + return false + } + checks := agent.Checks() + if len(checks) == 0 { + return false + } + for _, c := range checks { + if c.Status != "pass" { + return false + } + } + return true +} + +// detectedAgentIDs returns the ids of currently detected agents, sorted. +func detectedAgentIDs() []string { + agents := harness.DetectedAgents() + ids := make([]string, 0, len(agents)) + for _, a := range agents { + ids = append(ids, a.ID) + } + sort.Strings(ids) + return ids +} + +// agentChoiceCommands returns `basecamp setup ` for each agent, sorted by id. +func agentChoiceCommands(agents []harness.AgentInfo) []string { + ids := make([]string, 0, len(agents)) + for _, a := range agents { + ids = append(ids, a.ID) + } + sort.Strings(ids) + cmds := make([]string, 0, len(ids)) + for _, id := range ids { + cmds = append(cmds, "basecamp setup "+id) + } + return cmds +} + +// agentSetupSummary names the resulting state in one line. +func agentSetupSummary(selector string, ambiguous, skillInstalled bool, records []agentSetupRecord) string { + switch { + case !skillInstalled: + return "Baseline skill installation failed" + case selector == "invalid": + return "Unknown " + agentSetupEnv + " value; installed baseline skill only" + case ambiguous: + return "Multiple coding agents detected; installed baseline skill only" + case len(records) == 0: + return "Installed baseline skill; no coding agents connected" + } + names := make([]string, 0, len(records)) + connected := 0 + for _, r := range records { + names = append(names, r.name) + if r.pluginInstalled { + connected++ + } + } + if connected == len(records) { + return "Installed baseline skill; connected " + joinNames(names) + } + return "Installed baseline skill; attempted " + joinNames(names) +} + +// orderedStringSet accumulates strings with stable first-seen dedup. +type orderedStringSet struct { + seen map[string]bool + items []string +} + +func newOrderedStringSet() *orderedStringSet { + return &orderedStringSet{seen: map[string]bool{}, items: []string{}} +} + +func (s *orderedStringSet) add(v string) { + if !s.seen[v] { + s.seen[v] = true + s.items = append(s.items, v) + } +} + +func (s *orderedStringSet) slice() []string { return s.items } + +// orEmptyStrings replaces a nil slice with a non-nil empty one so JSON renders +// `[]` rather than `null`. +func orEmptyStrings(ss []string) []string { + if ss == nil { + return []string{} + } + return ss +} + // baselineSkillInstalled returns true if ~/.agents/skills/basecamp/SKILL.md exists. func baselineSkillInstalled() bool { home, err := os.UserHomeDir() diff --git a/scripts/install.ps1 b/scripts/install.ps1 index e8eeb70fe..664290809 100644 --- a/scripts/install.ps1 +++ b/scripts/install.ps1 @@ -6,6 +6,15 @@ try { # Ignore when the runtime manages TLS defaults. } +# Environment options: +# BASECAMP_VERSION Specific version to install (default: latest) +# BASECAMP_BIN_DIR Where to install the binary +# BASECAMP_SKIP_SETUP Set to 1 to skip the interactive wizard (still runs +# `basecamp setup agents`) +# BASECAMP_SETUP_AGENT Which coding agent(s) `setup agents` connects: +# claude | codex | all | none. Unset = auto-detect. +# Piped install sets it for the interpreter, not the fetch: +# $env:BASECAMP_SETUP_AGENT='codex'; irm https://raw.githubusercontent.com/basecamp/basecamp-cli/main/scripts/install.ps1 | iex $Repo = 'basecamp/basecamp-cli' $Version = $env:BASECAMP_VERSION $SkipSetup = $env:BASECAMP_SKIP_SETUP @@ -260,6 +269,9 @@ function Main { Write-Host '' if ($SkipSetup -eq '1') { Step 'Skipping setup wizard (BASECAMP_SKIP_SETUP=1)' + # Still install the baseline skill and connect coding agents (never prompts). + # Honors BASECAMP_SETUP_AGENT (claude|codex|all|none; unset = auto-detect). + & $installedBinary setup agents Write-Host '' Write-Host ' Next steps:' Write-Host ' basecamp auth login Authenticate with Basecamp' @@ -273,6 +285,9 @@ function Main { Write-Host '' } else { Info 'Skipping interactive setup because PowerShell is running non-interactively.' + # Install the baseline skill and connect coding agents (never prompts). + # Honors BASECAMP_SETUP_AGENT (claude|codex|all|none; unset = auto-detect). + & $installedBinary setup agents Write-Host '' Write-Host ' Installed executable:' Write-Host " $installedBinary" diff --git a/scripts/install.sh b/scripts/install.sh index 55e8514e0..60abfeb5b 100755 --- a/scripts/install.sh +++ b/scripts/install.sh @@ -10,6 +10,15 @@ # otherwise ~/bin on Windows, ~/.local/bin elsewhere) # BASECAMP_VERSION Specific version to install (default: latest) # BASECAMP_SKIP_SETUP Set to 1 to skip the interactive setup wizard after install +# (still runs `basecamp setup agents` to install the skill +# and connect coding agents) +# BASECAMP_SETUP_AGENT +# Which coding agent(s) `setup agents` connects: +# claude | codex | all | none. Unset = auto-detect (connect +# a single detected agent; if several, install the skill +# only and surface the per-agent commands). +# Piped install sets it for the interpreter, not the fetch: +# curl -fsSL https://basecamp.com/install-cli | BASECAMP_SETUP_AGENT=codex bash set -euo pipefail @@ -443,12 +452,13 @@ main() { echo "" # Run interactive setup wizard only when stdin is a TTY and not explicitly skipped. - # Non-interactive environments (CI, piped input, coding agents like Claude Code) - # get the agent skill installed and next-step instructions instead — the wizard - # requires interactive prompts that don't work without a terminal. + # Non-interactive environments (CI, piped input, coding agents like Claude Code + # or Codex) get the baseline skill installed, a best-effort agent connection via + # `setup agents`, and next-step instructions instead — the wizard requires + # interactive prompts that don't work without a terminal. if [[ "${BASECAMP_SKIP_SETUP:-}" == "1" ]]; then step "Skipping setup wizard (BASECAMP_SKIP_SETUP=1)" - "$BIN_DIR/$binary_name" setup claude || true + post_install_setup "$binary_name" echo "" echo " Next steps:" echo " $(bold "basecamp auth login") Authenticate with Basecamp" @@ -458,7 +468,7 @@ main() { "$BIN_DIR/$binary_name" setup else info "Skipping interactive setup (no terminal detected)." - "$BIN_DIR/$binary_name" setup claude || true + post_install_setup "$binary_name" echo "" echo " Next steps:" echo " $(bold "basecamp auth login") Authenticate with Basecamp" @@ -467,4 +477,17 @@ main() { fi } -main "$@" +# post_install_setup installs the baseline skill and connects coding agents +# without prompting. It honors BASECAMP_SETUP_AGENT (claude|codex|all|none; +# unset = auto-detect). Never runs the interactive wizard. +post_install_setup() { + local binary_name="$1" + "$BIN_DIR/$binary_name" setup agents || true +} + +# Guard so sourcing the script (e.g. from tests) doesn't run the installer. +# The if-form is required: `[[ … ]] && main` returns 1 when sourced, which +# trips `set -e` in the sourcing shell. +if [[ "${BASH_SOURCE[0]}" == "$0" ]]; then + main "$@" +fi From 267c7efebf19389f07c30e7822be441babcd6fca Mon Sep 17 00:00:00 2001 From: Jeremy Daer Date: Wed, 22 Jul 2026 16:04:39 -0700 Subject: [PATCH 2/5] Address PR #558 review: cross-version installers, guarded PS setup, skill docs Three follow-up fixes from review of #558: 1. Cross-version installer strategy (install.sh + install.ps1). The hosted install script from main can outrun the latest release, whose binary lacks `setup agents` (v0.7.2 has only `setup`/`setup claude`/`skill install`). Both scripts now probe the installed binary for `setup agents` support and, on an older binary, fall back WITHOUT reintroducing the Claude-first bug: only an explicitly selected agent is connected; an unset/auto/ambiguous selector installs the shared skill via `skill install`. Added old-binary regression tests in e2e/installer.bats. 2. Guard PowerShell post-install setup. Both calls now route through a single best-effort Invoke-PostInstallSetup helper wrapped in try/catch, so a nonzero native exit (under $ErrorActionPreference='Stop' + $PSNativeCommandUseErrorActionPreference) can no longer terminate an otherwise-successful install. 3. Skill docs. Document `setup agents` + BASECAMP_SETUP_AGENT in skills/basecamp/SKILL.md and add it to the remediation list in skills/basecamp-doctor/SKILL.md. --- e2e/installer.bats | 103 +++++++++++++++++++++++--------- scripts/install.ps1 | 37 ++++++++++-- scripts/install.sh | 30 +++++++++- skills/basecamp-doctor/SKILL.md | 1 + skills/basecamp/SKILL.md | 10 ++++ 5 files changed, 145 insertions(+), 36 deletions(-) diff --git a/e2e/installer.bats b/e2e/installer.bats index a0983b6ea..035509f11 100644 --- a/e2e/installer.bats +++ b/e2e/installer.bats @@ -2,7 +2,9 @@ # installer.bats - Tests for the install scripts' post-install agent setup. # # #528: the non-TTY and skip branches must run `setup agents` (baseline skill + -# best-effort agent connection), never the hardcoded `setup claude`. +# best-effort agent connection), never the hardcoded `setup claude`. Old release +# binaries (which lack `setup agents`) must fall back without reintroducing the +# Claude-first bug. setup() { # The installer contract keys off these; a leaked value would skew results. @@ -13,57 +15,93 @@ setup() { STUB_DIR="$(mktemp -d)" LOG="$STUB_DIR/calls.log" - - # Stub `basecamp` that logs its argv so we can see which subcommand ran. - cat > "$STUB_DIR/basecamp" <> "$LOG" -EOF - chmod +x "$STUB_DIR/basecamp" + write_stub new # default: a binary that supports `setup agents` } teardown() { [[ -n "${STUB_DIR:-}" ]] && rm -rf "$STUB_DIR" } -# The if-form guard must let sourcing define functions without running main. -@test "install.sh can be sourced without running the installer" { - run bash -c "set -euo pipefail; source '$INSTALL_SH'; echo sourced-ok" - [[ "$status" -eq 0 ]] - [[ "$output" == *"sourced-ok"* ]] - [[ "$output" != *"Basecamp CLI"* ]] # banner would print if main ran +# write_stub emits a `basecamp` stub that logs its argv. mode=new advertises the +# `setup agents` subcommand in `setup --help`; mode=old omits it and fails an +# actual `setup agents` invocation, mimicking a pre-v0.7.3 release binary. +write_stub() { + local mode="$1" + { + echo '#!/usr/bin/env bash' + echo "echo \"\$@\" >> \"$LOG\"" + echo 'if [[ "$1 $2" == "setup --help" ]]; then' + echo ' echo " claude Install the Basecamp plugin for Claude Code"' + if [[ "$mode" == "new" ]]; then + echo ' echo " agents Install the Basecamp skill and connect detected coding agents"' + fi + echo ' exit 0' + echo 'fi' + if [[ "$mode" == "old" ]]; then + echo 'if [[ "$1 $2" == "setup agents" ]]; then echo "unknown command \"agents\"" >&2; exit 1; fi' + fi + echo 'exit 0' + } > "$STUB_DIR/basecamp" + chmod +x "$STUB_DIR/basecamp" } -@test "post_install_setup dispatches to 'setup agents', never 'setup claude'" { +run_post_install_setup() { run bash -c " set -euo pipefail + ${1:-} source '$INSTALL_SH' BIN_DIR='$STUB_DIR' post_install_setup basecamp cat '$LOG' " +} + +# The if-form guard must let sourcing define functions without running main. +@test "install.sh can be sourced without running the installer" { + run bash -c "set -euo pipefail; source '$INSTALL_SH'; echo sourced-ok" + [[ "$status" -eq 0 ]] + [[ "$output" == *"sourced-ok"* ]] + [[ "$output" != *"Basecamp CLI"* ]] # banner would print if main ran +} + +@test "new binary: post_install_setup dispatches to 'setup agents', never 'setup claude'" { + run_post_install_setup [[ "$status" -eq 0 ]] [[ "$output" == *"setup agents"* ]] [[ "$output" != *"setup claude"* ]] } -@test "post_install_setup honors BASECAMP_SKIP_SETUP path (still 'setup agents')" { - run bash -c " - set -euo pipefail - export BASECAMP_SKIP_SETUP=1 - source '$INSTALL_SH' - BIN_DIR='$STUB_DIR' - post_install_setup basecamp - cat '$LOG' - " +@test "new binary: BASECAMP_SKIP_SETUP path still runs 'setup agents'" { + run_post_install_setup "export BASECAMP_SKIP_SETUP=1" [[ "$status" -eq 0 ]] [[ "$output" == *"setup agents"* ]] [[ "$output" != *"setup claude"* ]] } -@test "install.sh has no residual 'setup claude'" { - run grep -n "setup claude" "$INSTALL_SH" - [[ "$status" -ne 0 ]] # grep exits non-zero when nothing matches +# Cross-version regression: an old release binary (no `setup agents`) must NOT +# silently fall back to Claude when the selector is unset — it installs the +# shared skill only. +@test "old binary + unset selector falls back to 'skill install', never 'setup claude'" { + write_stub old + run_post_install_setup + [[ "$status" -eq 0 ]] + [[ "$output" == *"skill install"* ]] + [[ "$output" != *"setup claude"* ]] + [[ "$output" != *"setup agents"$'\n'* ]] # the unknown command is never left as the outcome +} + +@test "old binary + BASECAMP_SETUP_AGENT=claude connects claude explicitly" { + write_stub old + run_post_install_setup "export BASECAMP_SETUP_AGENT=claude" + [[ "$status" -eq 0 ]] + [[ "$output" == *"setup claude"* ]] + [[ "$output" != *"skill install"* ]] +} + +@test "install.sh has no residual 'setup claude' dispatch" { + # `setup claude` may appear only inside the explicit-selector fallback case. + run grep -n 'setup claude' "$INSTALL_SH" + [[ "$status" -ne 0 ]] # no literal `setup claude` string in the script } @test "install.sh skip and non-tty branches both dispatch via post_install_setup" { @@ -72,8 +110,15 @@ teardown() { [[ "$output" -ge 2 ]] } -@test "install.ps1 skip and non-interactive branches each invoke 'setup agents'" { - run grep -c '& \$installedBinary setup agents' "$INSTALL_PS1" +@test "install.ps1 routes both branches through the guarded best-effort helper" { + run grep -c 'Invoke-PostInstallSetup \$installedBinary' "$INSTALL_PS1" [[ "$status" -eq 0 ]] [[ "$output" -eq 2 ]] } + +@test "install.ps1 helper is guarded and cross-version aware" { + grep -q 'function Invoke-PostInstallSetup' "$INSTALL_PS1" + grep -q 'setup agents' "$INSTALL_PS1" + grep -q 'skill install' "$INSTALL_PS1" + grep -q 'catch {' "$INSTALL_PS1" +} diff --git a/scripts/install.ps1 b/scripts/install.ps1 index 664290809..f7436b902 100644 --- a/scripts/install.ps1 +++ b/scripts/install.ps1 @@ -208,6 +208,33 @@ function Test-InteractiveSession { } } +# Invoke-PostInstallSetup installs the baseline skill and connects coding agents +# without prompting, honoring BASECAMP_SETUP_AGENT (claude|codex|all|none; +# unset = auto-detect). It is strictly best-effort: agent setup must never fail +# an otherwise-successful install, so every native call is wrapped so a nonzero +# exit (amplified by $ErrorActionPreference='Stop' + +# $PSNativeCommandUseErrorActionPreference) cannot terminate the installer. +# +# Cross-version: newer binaries expose the intent-neutral `setup agents`. Older +# release binaries (the hosted install.ps1 from main can outrun the latest +# release) fall back WITHOUT reintroducing the Claude-first bug — only an +# explicitly selected agent is connected; unset/auto/ambiguous installs the +# shared skill only. +function Invoke-PostInstallSetup([string]$Binary) { + try { + $help = & $Binary setup --help 2>$null + if ($help -match '(?m)^\s+agents\s') { + & $Binary setup agents + } elseif ($env:BASECAMP_SETUP_AGENT -in @('claude', 'codex')) { + & $Binary setup $env:BASECAMP_SETUP_AGENT + } else { + & $Binary skill install + } + } catch { + # Best-effort — swallow any failure from the agent/skill setup step. + } +} + function Main { $arch = Get-PlatformArch if (-not $BinDir) { @@ -269,9 +296,8 @@ function Main { Write-Host '' if ($SkipSetup -eq '1') { Step 'Skipping setup wizard (BASECAMP_SKIP_SETUP=1)' - # Still install the baseline skill and connect coding agents (never prompts). - # Honors BASECAMP_SETUP_AGENT (claude|codex|all|none; unset = auto-detect). - & $installedBinary setup agents + # Still install the baseline skill and connect coding agents (best-effort). + Invoke-PostInstallSetup $installedBinary Write-Host '' Write-Host ' Next steps:' Write-Host ' basecamp auth login Authenticate with Basecamp' @@ -285,9 +311,8 @@ function Main { Write-Host '' } else { Info 'Skipping interactive setup because PowerShell is running non-interactively.' - # Install the baseline skill and connect coding agents (never prompts). - # Honors BASECAMP_SETUP_AGENT (claude|codex|all|none; unset = auto-detect). - & $installedBinary setup agents + # Install the baseline skill and connect coding agents (best-effort). + Invoke-PostInstallSetup $installedBinary Write-Host '' Write-Host ' Installed executable:' Write-Host " $installedBinary" diff --git a/scripts/install.sh b/scripts/install.sh index 60abfeb5b..17c4fa8fe 100755 --- a/scripts/install.sh +++ b/scripts/install.sh @@ -477,12 +477,40 @@ main() { fi } +# binary_supports_setup_agents reports whether the installed binary exposes the +# `setup agents` subcommand (added after v0.7.2). The hosted install.sh from main +# can outrun the latest release, so we probe rather than assume. +binary_supports_setup_agents() { + "$1" setup --help 2>/dev/null | grep -qE '^[[:space:]]+agents[[:space:]]' +} + # post_install_setup installs the baseline skill and connects coding agents # without prompting. It honors BASECAMP_SETUP_AGENT (claude|codex|all|none; # unset = auto-detect). Never runs the interactive wizard. +# +# Cross-version: newer binaries get the intent-neutral `setup agents`. Older +# release binaries (no `setup agents`) fall back WITHOUT reintroducing the +# Claude-first bug — only an explicitly selected agent is connected; an unset, +# auto, or ambiguous selector installs the shared skill only (`skill install`). post_install_setup() { local binary_name="$1" - "$BIN_DIR/$binary_name" setup agents || true + local bin="$BIN_DIR/$binary_name" + + if binary_supports_setup_agents "$bin"; then + "$bin" setup agents || true + return 0 + fi + + case "${BASECAMP_SETUP_AGENT:-}" in + claude|codex) + "$bin" setup "${BASECAMP_SETUP_AGENT}" || true + ;; + *) + # Intent-neutral on old binaries: install the shared skill, never pick an + # agent. The user connects one via the printed "Next steps". + "$bin" skill install || true + ;; + esac } # Guard so sourcing the script (e.g. from tests) doesn't run the installer. diff --git a/skills/basecamp-doctor/SKILL.md b/skills/basecamp-doctor/SKILL.md index 9c476edc1..17311a368 100644 --- a/skills/basecamp-doctor/SKILL.md +++ b/skills/basecamp-doctor/SKILL.md @@ -22,6 +22,7 @@ Report failures and warnings with their `hint` fields. Also inspect the top-leve - Basecamp authentication: `basecamp auth login` - Agent plugin installation or version: `basecamp setup` +- Skill + all detected agents, non-interactively: `basecamp setup agents` (honors `BASECAMP_SETUP_AGENT`) - Codex plugin specifically: `basecamp setup codex` - Claude Code plugin specifically: `basecamp setup claude` diff --git a/skills/basecamp/SKILL.md b/skills/basecamp/SKILL.md index 36f3e696d..11a4391fb 100644 --- a/skills/basecamp/SKILL.md +++ b/skills/basecamp/SKILL.md @@ -972,6 +972,16 @@ cat .basecamp/config.json 2>/dev/null || echo "No project configured" basecamp doctor --json # Check CLI health, auth, connectivity ``` +**Coding agent setup (non-interactive):** +```bash +basecamp setup agents # Install skill + connect detected agent(s) +basecamp setup agents --json # Structured result envelope +``` +`setup agents` installs the baseline skill and connects coding agents without +prompting. Selection is driven by `BASECAMP_SETUP_AGENT` (`claude`, `codex`, +`all`, or `none`); unset auto-detects — one detected agent is connected, several +leave the skill only and surface the per-agent `basecamp setup ` commands. + **Rate limiting (429):** The CLI handles backoff automatically. If you see 429 errors, reduce request frequency. **Authentication errors:** From 3839df554dd4e4add11b44fc37536ddd2a5d87a2 Mon Sep 17 00:00:00 2001 From: Jeremy Daer Date: Wed, 22 Jul 2026 17:03:53 -0700 Subject: [PATCH 3/5] Address PR #558 review round 2: reject stray args, honor `all` on old binaries MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - setup agents: add cobra.NoArgs — selection is env-driven, so positional args (typos, or confusion with `setup `) are now rejected, not silently ignored. - install.sh / install.ps1: on old release binaries lacking `setup agents`, BASECAMP_SETUP_AGENT=all now dispatches every per-agent `setup ` the binary supports instead of collapsing to skill-only. Explicit "all" is honored intent, distinct from the intent-neutral unset/auto/none fallback. PowerShell calls are now individually guarded so one agent's failure can't skip the rest. - Tests: NoArgs rejection (Go) + `all` old-binary dispatch (installer.bats). --- e2e/installer.bats | 9 +++++++ internal/commands/setup_agents_test.go | 17 +++++++++++++ internal/commands/wizard_agents.go | 3 +++ scripts/install.ps1 | 34 +++++++++++++++++--------- scripts/install.sh | 23 +++++++++++++++-- 5 files changed, 72 insertions(+), 14 deletions(-) diff --git a/e2e/installer.bats b/e2e/installer.bats index 035509f11..1ff978f81 100644 --- a/e2e/installer.bats +++ b/e2e/installer.bats @@ -98,6 +98,15 @@ run_post_install_setup() { [[ "$output" != *"skill install"* ]] } +# Explicit `all` intent must dispatch every per-agent setup the old binary +# supports (here the stub advertises only `claude`), never collapse to skill-only. +@test "old binary + BASECAMP_SETUP_AGENT=all runs the supported per-agent setups" { + write_stub old + run_post_install_setup "export BASECAMP_SETUP_AGENT=all" + [[ "$status" -eq 0 ]] + [[ "$output" == *"setup claude"* ]] +} + @test "install.sh has no residual 'setup claude' dispatch" { # `setup claude` may appear only inside the explicit-selector fallback case. run grep -n 'setup claude' "$INSTALL_SH" diff --git a/internal/commands/setup_agents_test.go b/internal/commands/setup_agents_test.go index 7f6a09b03..167e48195 100644 --- a/internal/commands/setup_agents_test.go +++ b/internal/commands/setup_agents_test.go @@ -89,6 +89,23 @@ func TestNewSetupCmdHasAgentsSubcommand(t *testing.T) { assert.NotNil(t, findSubcommand(NewSetupCmd(), "agents")) } +// TestSetupAgentsRejectsPositionalArgs: selection is env-driven, so stray args +// (typos, or confusion with `setup `) are rejected rather than ignored. +func TestSetupAgentsRejectsPositionalArgs(t *testing.T) { + emptyHome(t) + app, _ := setupQuickstartTestApp(t, "", "") + app.Flags.JSON = true + t.Cleanup(app.Close) + + cmd := NewSetupCmd() + cmd.SetArgs([]string{"agents", "codex"}) + cmd.SetContext(appctx.WithApp(context.Background(), app)) + cmd.SetOut(&bytes.Buffer{}) + cmd.SetErr(&bytes.Buffer{}) + + assert.Error(t, cmd.Execute(), "unexpected positional arg should be rejected") +} + // TestSetupAgentsStyledSurfacesFailure is the styled-output regression guard: // a per-agent failure must reach the human-facing output via a top-level flat // field, since the styled renderer skips the nested `agents` array. diff --git a/internal/commands/wizard_agents.go b/internal/commands/wizard_agents.go index 75f490b51..619c6e513 100644 --- a/internal/commands/wizard_agents.go +++ b/internal/commands/wizard_agents.go @@ -561,6 +561,9 @@ func newSetupAgentsCmd() *cobra.Command { "Selection is controlled by " + agentSetupEnv + ": claude, codex, all, or none. When\n" + "unset, a single detected agent is connected; when several are detected none is\n" + "guessed — the per-agent `basecamp setup ` commands are surfaced instead.", + // Selection is env-driven; positional args are always a mistake (typo, + // or confusion with `setup `). Reject them rather than silently ignore. + Args: cobra.NoArgs, RunE: func(cmd *cobra.Command, args []string) error { app := appctx.FromContext(cmd.Context()) if app == nil { diff --git a/scripts/install.ps1 b/scripts/install.ps1 index f7436b902..c5117ccdb 100644 --- a/scripts/install.ps1 +++ b/scripts/install.ps1 @@ -218,20 +218,30 @@ function Test-InteractiveSession { # Cross-version: newer binaries expose the intent-neutral `setup agents`. Older # release binaries (the hosted install.ps1 from main can outrun the latest # release) fall back WITHOUT reintroducing the Claude-first bug — only an -# explicitly selected agent is connected; unset/auto/ambiguous installs the -# shared skill only. +# *explicitly* selected agent is connected. `all` runs every per-agent setup the +# binary supports; unset/auto/ambiguous installs the shared skill only. Each +# native call is individually guarded so a nonzero exit never aborts the install. function Invoke-PostInstallSetup([string]$Binary) { - try { - $help = & $Binary setup --help 2>$null - if ($help -match '(?m)^\s+agents\s') { - & $Binary setup agents - } elseif ($env:BASECAMP_SETUP_AGENT -in @('claude', 'codex')) { - & $Binary setup $env:BASECAMP_SETUP_AGENT - } else { - & $Binary skill install + try { $help = & $Binary setup --help 2>$null } catch { $help = '' } + + if ($help -match '(?m)^\s+agents\s') { + try { & $Binary setup agents } catch { } + return + } + + $selector = $env:BASECAMP_SETUP_AGENT + if ($selector -in @('claude', 'codex')) { + try { & $Binary setup $selector } catch { } + } elseif ($selector -eq 'all') { + $ranAgent = $false + foreach ($agent in @('claude', 'codex')) { + if ($help -match "(?m)^\s+$agent\s") { + try { & $Binary setup $agent; $ranAgent = $true } catch { } + } } - } catch { - # Best-effort — swallow any failure from the agent/skill setup step. + if (-not $ranAgent) { try { & $Binary skill install } catch { } } + } else { + try { & $Binary skill install } catch { } } } diff --git a/scripts/install.sh b/scripts/install.sh index 17c4fa8fe..cc75a20dc 100755 --- a/scripts/install.sh +++ b/scripts/install.sh @@ -484,14 +484,21 @@ binary_supports_setup_agents() { "$1" setup --help 2>/dev/null | grep -qE '^[[:space:]]+agents[[:space:]]' } +# binary_supports_setup_agent reports whether the binary exposes a per-agent +# `setup ` subcommand for the given agent id (claude or codex). +binary_supports_setup_agent() { + "$1" setup --help 2>/dev/null | grep -qE "^[[:space:]]+$2[[:space:]]" +} + # post_install_setup installs the baseline skill and connects coding agents # without prompting. It honors BASECAMP_SETUP_AGENT (claude|codex|all|none; # unset = auto-detect). Never runs the interactive wizard. # # Cross-version: newer binaries get the intent-neutral `setup agents`. Older # release binaries (no `setup agents`) fall back WITHOUT reintroducing the -# Claude-first bug — only an explicitly selected agent is connected; an unset, -# auto, or ambiguous selector installs the shared skill only (`skill install`). +# Claude-first bug — only an *explicitly* selected agent is connected. `all` +# runs every per-agent setup the binary supports; an unset, auto, or ambiguous +# selector installs the shared skill only (`skill install`). post_install_setup() { local binary_name="$1" local bin="$BIN_DIR/$binary_name" @@ -505,6 +512,18 @@ post_install_setup() { claude|codex) "$bin" setup "${BASECAMP_SETUP_AGENT}" || true ;; + all) + # Explicit "every agent": dispatch each per-agent setup the binary knows, + # falling back to the shared skill if it supports none of them. + local ran_agent=0 agent + for agent in claude codex; do + if binary_supports_setup_agent "$bin" "$agent"; then + "$bin" setup "$agent" || true + ran_agent=1 + fi + done + [[ "$ran_agent" -eq 1 ]] || "$bin" skill install || true + ;; *) # Intent-neutral on old binaries: install the shared skill, never pick an # agent. The user connects one via the printed "Next steps". From 4d6a9f6f3990dfaba6dc4fb38e9efde3738a5cf3 Mon Sep 17 00:00:00 2001 From: Jeremy Daer Date: Wed, 22 Jul 2026 17:12:54 -0700 Subject: [PATCH 4/5] Capability-check explicit claude|codex on old-binary installer fallback MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit On a pre-`setup agents` release (v0.7.2 advertises only `setup claude`), an explicit BASECAMP_SETUP_AGENT=codex ran `basecamp setup codex`. The old `setup` parent accepts `codex` as a stray positional arg and launches the INTERACTIVE setup/OAuth wizard — violating the non-interactive contract. Both installers now capability-check explicit claude|codex selectors the same way `all` already did: only dispatch `setup ` if the binary advertises it, otherwise degrade to `skill install` and never invoke the unknown subcommand. Tests: the old-binary stub now rejects every unadvertised `setup ` (not just `setup agents`), and new coverage asserts explicit `codex` degrades to `skill install` (install.sh) plus a static guard on install.ps1's capability check. --- e2e/installer.bats | 20 +++++++++++++++++++- scripts/install.ps1 | 8 +++++++- scripts/install.sh | 9 ++++++++- 3 files changed, 34 insertions(+), 3 deletions(-) diff --git a/e2e/installer.bats b/e2e/installer.bats index 1ff978f81..a8175d95b 100644 --- a/e2e/installer.bats +++ b/e2e/installer.bats @@ -38,7 +38,10 @@ write_stub() { echo ' exit 0' echo 'fi' if [[ "$mode" == "old" ]]; then - echo 'if [[ "$1 $2" == "setup agents" ]]; then echo "unknown command \"agents\"" >&2; exit 1; fi' + # A pre-`setup agents` binary advertises only `setup claude`. Reject every + # OTHER `setup `: the real old parent would swallow it as a stray arg + # and launch the interactive wizard — the exact bug the installer must avoid. + echo 'if [[ "$1" == "setup" && "$2" != "claude" && "$2" != "--help" ]]; then echo "unknown command \"$2\"" >&2; exit 1; fi' fi echo 'exit 0' } > "$STUB_DIR/basecamp" @@ -105,6 +108,18 @@ run_post_install_setup() { run_post_install_setup "export BASECAMP_SETUP_AGENT=all" [[ "$status" -eq 0 ]] [[ "$output" == *"setup claude"* ]] + [[ "$output" != *"setup codex"* ]] # codex unadvertised → never invoked +} + +# Explicit `codex` on an old binary that lacks `setup codex` must NOT run the +# unknown subcommand (which would launch the interactive wizard) — it degrades +# to the shared skill. +@test "old binary + BASECAMP_SETUP_AGENT=codex degrades to 'skill install', never 'setup codex'" { + write_stub old + run_post_install_setup "export BASECAMP_SETUP_AGENT=codex" + [[ "$status" -eq 0 ]] + [[ "$output" == *"skill install"* ]] + [[ "$output" != *"setup codex"* ]] } @test "install.sh has no residual 'setup claude' dispatch" { @@ -130,4 +145,7 @@ run_post_install_setup() { grep -q 'setup agents' "$INSTALL_PS1" grep -q 'skill install' "$INSTALL_PS1" grep -q 'catch {' "$INSTALL_PS1" + # Explicit claude|codex selectors must be capability-checked before dispatch, + # so an old binary never gets an unadvertised subcommand as a stray arg. + grep -qF 'match "(?m)^\s+$selector\s"' "$INSTALL_PS1" } diff --git a/scripts/install.ps1 b/scripts/install.ps1 index c5117ccdb..078ff7165 100644 --- a/scripts/install.ps1 +++ b/scripts/install.ps1 @@ -231,7 +231,13 @@ function Invoke-PostInstallSetup([string]$Binary) { $selector = $env:BASECAMP_SETUP_AGENT if ($selector -in @('claude', 'codex')) { - try { & $Binary setup $selector } catch { } + # Capability-check first: an old `setup` parent accepts an unadvertised agent + # id as a stray arg and launches the INTERACTIVE wizard. Degrade to the skill. + if ($help -match "(?m)^\s+$selector\s") { + try { & $Binary setup $selector } catch { } + } else { + try { & $Binary skill install } catch { } + } } elseif ($selector -eq 'all') { $ranAgent = $false foreach ($agent in @('claude', 'codex')) { diff --git a/scripts/install.sh b/scripts/install.sh index cc75a20dc..4e915c19f 100755 --- a/scripts/install.sh +++ b/scripts/install.sh @@ -510,7 +510,14 @@ post_install_setup() { case "${BASECAMP_SETUP_AGENT:-}" in claude|codex) - "$bin" setup "${BASECAMP_SETUP_AGENT}" || true + # Capability-check first: an old `setup` parent accepts an unadvertised + # agent id as a stray positional arg and launches the INTERACTIVE wizard, + # violating the non-interactive contract. Degrade to the shared skill. + if binary_supports_setup_agent "$bin" "${BASECAMP_SETUP_AGENT}"; then + "$bin" setup "${BASECAMP_SETUP_AGENT}" || true + else + "$bin" skill install || true + fi ;; all) # Explicit "every agent": dispatch each per-agent setup the binary knows, From 5cdc4ec99a0c2bd2c2fbce3f18870951973c55f8 Mon Sep 17 00:00:00 2001 From: Jeremy Daer Date: Wed, 22 Jul 2026 17:13:30 -0700 Subject: [PATCH 5/5] install.ps1: mark ranAgent on attempt, not success (match install.sh) --- scripts/install.ps1 | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/scripts/install.ps1 b/scripts/install.ps1 index 078ff7165..4efb14be8 100644 --- a/scripts/install.ps1 +++ b/scripts/install.ps1 @@ -242,7 +242,10 @@ function Invoke-PostInstallSetup([string]$Binary) { $ranAgent = $false foreach ($agent in @('claude', 'codex')) { if ($help -match "(?m)^\s+$agent\s") { - try { & $Binary setup $agent; $ranAgent = $true } catch { } + # Mark attempted (not succeeded) — matches install.sh's `ran_agent=1`, + # which is set regardless of the setup call's exit status. + $ranAgent = $true + try { & $Binary setup $agent } catch { } } } if (-not $ranAgent) { try { & $Binary skill install } catch { } }