From c699183321abb7d0669871c01af107a27c64e6f1 Mon Sep 17 00:00:00 2001 From: Jeremy Daer Date: Mon, 5 Oct 2026 15:15:11 -0700 Subject: [PATCH 01/13] =?UTF-8?q?Rename=20the=20agent=20plugin=20basecamp?= =?UTF-8?q?=20=E2=86=92=20basecamp-cli?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The 37signals marketplace gives the basecamp name to the hosted Basecamp connector plugin, so this CLI's Claude Code and Codex plugin becomes basecamp-cli@37signals. The CLI binary and skill names are unchanged. setup claude and setup codex migrate a pre-rename install: an entry under basecamp@37signals whose installed manifest names this repository is replaced with basecamp-cli@37signals (Claude at the same scopes; Codex removes the old copy only after the new one installs). An entry whose manifest names anything else, or can't be read, is the hosted connector or unknown, and is left alone. doctor reports the old name instead of 'Plugin not installed'. --- .claude-plugin/plugin.json | 2 +- .codex-plugin/plugin.json | 2 +- AGENTS.md | 4 +- README.md | 23 ++++- install.md | 6 +- internal/commands/setup_agents_test.go | 2 +- internal/commands/wizard_agents.go | 4 +- internal/commands/wizard_codex.go | 20 +++- internal/commands/wizard_codex_test.go | 71 +++++++++++--- internal/commands/wizard_test.go | 88 ++++++++++++++++-- internal/harness/claude.go | 118 ++++++++++++++++++++--- internal/harness/claude_test.go | 124 +++++++++++++++++++++---- internal/harness/codex.go | 80 +++++++++++++++- internal/harness/codex_test.go | 55 ++++++++++- internal/release/manifests_test.go | 19 ++-- 15 files changed, 550 insertions(+), 68 deletions(-) diff --git a/.claude-plugin/plugin.json b/.claude-plugin/plugin.json index 186f831dc..4d25e0bf2 100644 --- a/.claude-plugin/plugin.json +++ b/.claude-plugin/plugin.json @@ -1,5 +1,5 @@ { - "name": "basecamp", + "name": "basecamp-cli", "version": "0.12.0", "description": "Basecamp integration for Claude Code. Create todos, track work, link code to projects.", "author": { diff --git a/.codex-plugin/plugin.json b/.codex-plugin/plugin.json index 103f8031c..1b65f7d79 100644 --- a/.codex-plugin/plugin.json +++ b/.codex-plugin/plugin.json @@ -1,5 +1,5 @@ { - "name": "basecamp", + "name": "basecamp-cli", "version": "0.12.0", "description": "Use Basecamp from Codex to find work, create todos, and keep projects up to date.", "author": { diff --git a/AGENTS.md b/AGENTS.md index b8b671503..eb47ef615 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -44,7 +44,9 @@ Coding-agent integration lives in `internal/harness` (agent registry, detection, skill health checks) and `internal/commands/wizard_agents.go` (`basecamp setup claude|codex|grok|agents`). Claude Code and Codex each get a native plugin from the `basecamp/claude-plugins` marketplace and have registrations of their own (`claude.go`, -`codex.go`). Grok Build has no plugin: it reads the shared `~/.agents/skills/basecamp` skill +`codex.go`). The plugin is `basecamp-cli@37signals`; `basecamp@37signals` is the hosted +Basecamp connector plugin, so an install under that old key counts as this CLI's (and is +migrated by setup) only when its installed manifest's `repository` is this repo. Grok Build has no plugin: it reads the shared `~/.agents/skills/basecamp` skill directly, so it is a row of `harness.SkillAgent` (name, id, home env var, home directory, binary) in `skill_agent.go`, and everything in `internal/commands` that touches a shared-skill agent — the setup handler, the `BASECAMP_SETUP_AGENT` values, doctor's remediation — loops over diff --git a/README.md b/README.md index 35b00f8db..f1dcd314b 100644 --- a/README.md +++ b/README.md @@ -292,12 +292,33 @@ Manual Codex installation uses the same marketplace: ```bash codex plugin marketplace add basecamp/claude-plugins -codex plugin add basecamp@37signals +codex plugin add basecamp-cli@37signals ``` To pick up a newer plugin version later, refresh the marketplace with `codex plugin marketplace upgrade 37signals` (or re-run `basecamp setup codex`). +**Plugin name:** in the 37signals marketplace this plugin is `basecamp-cli` +(`basecamp-cli@37signals`). It was published as `basecamp` until that name +went to the hosted Basecamp connector plugin, which works through Basecamp's +own MCP server rather than this CLI. If you installed the plugin under the old +name, re-run `basecamp setup claude` or `basecamp setup codex`: setup recognizes +the old CLI install by its manifest, removes it, and installs `basecamp-cli` at +the same scope. To migrate by hand: + +```bash +claude plugin marketplace update 37signals +claude plugin uninstall basecamp@37signals +claude plugin install basecamp-cli@37signals + +codex plugin marketplace upgrade 37signals +codex plugin remove basecamp@37signals +codex plugin add basecamp-cli@37signals +``` + +Skills from the plugin are namespaced by its name, so `basecamp:basecamp` +becomes `basecamp-cli:basecamp`. The CLI itself is still `basecamp`. + **Grok Build:** `basecamp setup grok` — installs the shared skill and confirms it is healthy. There is no Grok plugin: Grok reads user skills from `~/.grok/skills/` and from the cross-agent `~/.agents/skills/`, so the shared `~/.agents/skills/basecamp` skill is the whole integration. Grok is detected by `$GROK_HOME` (default `~/.grok`) or a `grok` binary on `PATH`, in `~/.local/bin`, or in `$GROK_HOME/bin` where its installers put it. Start a new Grok session after setup to load the skill. **Other agents:** Point your agent at [`skills/basecamp/SKILL.md`](skills/basecamp/SKILL.md) for Basecamp workflow coverage. diff --git a/install.md b/install.md index 8d76d03bb..dc3fb6313 100644 --- a/install.md +++ b/install.md @@ -139,7 +139,9 @@ Interactive setup in Step 1 connects every detected agent. Without a controlling basecamp setup claude ``` -This registers the marketplace and installs the plugin with skills, hooks, and agent workflow support. +This registers the marketplace and installs the `basecamp-cli@37signals` plugin with skills, hooks, and agent workflow support. + +The plugin was named `basecamp` until that marketplace name went to the hosted Basecamp connector plugin. Re-running `basecamp setup claude` (or `basecamp setup codex`) replaces an old CLI install under `basecamp@37signals` with `basecamp-cli@37signals` at the same scope; it leaves a hosted-connector install under that name alone. The hooks call the CLI's `agent-hook` command, so they need a `basecamp` new enough to have it. If hook errors appear after installing or refreshing the @@ -158,7 +160,7 @@ For a manual install: ```bash codex plugin marketplace add basecamp/claude-plugins -codex plugin add basecamp@37signals +codex plugin add basecamp-cli@37signals ``` To pick up a newer plugin version later, refresh with `codex plugin marketplace upgrade 37signals` (or re-run `basecamp setup codex`). diff --git a/internal/commands/setup_agents_test.go b/internal/commands/setup_agents_test.go index d79ed7480..a7a0cbd36 100644 --- a/internal/commands/setup_agents_test.go +++ b/internal/commands/setup_agents_test.go @@ -377,7 +377,7 @@ func TestSetupAgentsCodexPreservesManualOrder(t *testing.T) { assert.Equal(t, []string{ "codex plugin marketplace add basecamp/claude-plugins", "codex plugin marketplace upgrade 37signals", - "codex plugin add basecamp@37signals", + "codex plugin add basecamp-cli@37signals", }, env.Data.ManualCommands) } diff --git a/internal/commands/wizard_agents.go b/internal/commands/wizard_agents.go index 70bc41030..0cdcc649d 100644 --- a/internal/commands/wizard_agents.go +++ b/internal/commands/wizard_agents.go @@ -129,7 +129,7 @@ func agentSetupHandlersFor(skillAgents []harness.SkillAgent) map[string]agentSet "claude": { Labels: []string{ "Add basecamp/claude-plugins marketplace to Claude Code", - "Install the basecamp plugin for Claude Code", + "Install the basecamp-cli plugin for Claude Code", }, Run: runClaudeSetup, RunNonInteractive: runClaudeSetupNonInteractive, @@ -137,7 +137,7 @@ func agentSetupHandlersFor(skillAgents []harness.SkillAgent) map[string]agentSet "codex": { Labels: []string{ "Add the 37signals marketplace to Codex", - "Install the basecamp plugin for Codex", + "Install the basecamp-cli plugin for Codex", }, Run: runCodexSetup, RunNonInteractive: runCodexSetupNonInteractive, diff --git a/internal/commands/wizard_codex.go b/internal/commands/wizard_codex.go index 33952dba3..6218bbf21 100644 --- a/internal/commands/wizard_codex.go +++ b/internal/commands/wizard_codex.go @@ -100,13 +100,31 @@ func installCodexPlugin(parent context.Context, stderr io.Writer, progress func( } } - progress("Installing basecamp plugin…") + progress("Installing " + harness.CodexPluginName + " plugin…") stdout, stderrOutput, err = runCodexStep(parent, stderr, codexInstallTimeout, codexPath, "plugin", "add", harness.CodexExpectedPluginKey, "--json") if err != nil && !codexPluginAlreadyInstalled(stdout, stderrOutput) { return codexSetupError("plugin add failed: " + codexCommandFailure(stdout, stderrOutput, err)) } + // The plugin was renamed basecamp → basecamp-cli. Remove a pre-rename + // install only once its replacement is in, so a failed install never + // leaves the user with neither. + legacyCtx, cancelLegacy := context.WithTimeout(parent, codexVerifyTimeout) + legacy := harness.CodexLegacyCLIInstalled(legacyCtx) + cancelLegacy() + if legacy { + progress("Removing the pre-rename " + harness.CodexLegacyPluginKey + " plugin…") + removeStdout, removeStderr, removeErr := runCodexStep(parent, stderr, codexInstallTimeout, codexPath, + "plugin", "remove", harness.CodexLegacyPluginKey, "--json") + if removeErr != nil { + return &agentSetupError{ + Summary: "removing the pre-rename plugin failed: " + codexCommandFailure(removeStdout, removeStderr, removeErr), + Manual: []string{"codex plugin remove " + harness.CodexLegacyPluginKey}, + } + } + } + progress("Verifying installation…") verifyCtx, cancel := context.WithTimeout(parent, codexVerifyTimeout) defer cancel() diff --git a/internal/commands/wizard_codex_test.go b/internal/commands/wizard_codex_test.go index 6c66dceab..ab05e8ff7 100644 --- a/internal/commands/wizard_codex_test.go +++ b/internal/commands/wizard_codex_test.go @@ -43,7 +43,7 @@ func TestSetupCodexFreshInstallCommandOrder(t *testing.T) { calls := readCodexSetupCalls(t, logPath) assertCallOrder(t, calls, "plugin marketplace add basecamp/claude-plugins --json", - "plugin add basecamp@37signals --json", + "plugin add basecamp-cli@37signals --json", "plugin list --available --json", ) } @@ -58,7 +58,7 @@ func TestSetupCodexAlreadyAddedRefreshesMarketplace(t *testing.T) { assertCallOrder(t, calls, "plugin marketplace add basecamp/claude-plugins --json", "plugin marketplace upgrade 37signals --json", - "plugin add basecamp@37signals --json", + "plugin add basecamp-cli@37signals --json", ) } @@ -72,7 +72,7 @@ func TestSetupCodexIdempotentAddRefreshesMarketplace(t *testing.T) { assertCallOrder(t, calls, "plugin marketplace add basecamp/claude-plugins --json", "plugin marketplace upgrade 37signals --json", - "plugin add basecamp@37signals --json", + "plugin add basecamp-cli@37signals --json", ) } @@ -83,7 +83,39 @@ func TestSetupCodexAlreadyInstalledIsIdempotent(t *testing.T) { assert.True(t, envelope.Data.PluginInstalled) assert.Empty(t, envelope.Data.Errors) - assert.Contains(t, readCodexSetupCalls(t, logPath), "plugin add basecamp@37signals --json") + assert.Contains(t, readCodexSetupCalls(t, logPath), "plugin add basecamp-cli@37signals --json") +} + +func TestSetupCodexReplacesLegacyCLIPlugin(t *testing.T) { + logPath := installCodexStub(t, codexStubOptions{legacyInstalled: true}) + + envelope := runSetupCodexJSON(t) + + assert.True(t, envelope.Data.PluginInstalled) + assert.Empty(t, envelope.Data.Errors) + assertCallOrder(t, readCodexSetupCalls(t, logPath), + "plugin add basecamp-cli@37signals --json", + "plugin remove basecamp@37signals --json", + "plugin list --available --json", + ) +} + +func TestSetupCodexLegacyRemovalFailureNamesTheManualStep(t *testing.T) { + installCodexStub(t, codexStubOptions{legacyInstalled: true, legacyRemoveFailure: true}) + + envelope := runSetupCodexJSON(t) + + require.NotEmpty(t, envelope.Data.Errors) + assert.Contains(t, envelope.Data.Errors[0], "pre-rename plugin") + assert.Contains(t, envelope.Data.ManualCommands, "codex plugin remove basecamp@37signals") +} + +func TestSetupCodexLeavesNonLegacyInstallsAlone(t *testing.T) { + logPath := installCodexStub(t, codexStubOptions{}) + + runSetupCodexJSON(t) + + assert.NotContains(t, readCodexSetupCalls(t, logPath), "plugin remove") } func TestSetupCodexMissingBinaryReturnsManualCommands(t *testing.T) { @@ -120,7 +152,7 @@ func TestSetupCodexMarketplaceFailureStopsInstall(t *testing.T) { require.NotEmpty(t, envelope.Data.Errors) assert.Contains(t, envelope.Data.Errors[0], "marketplace add") assert.NotEmpty(t, envelope.Data.ManualCommands) - assert.NotContains(t, readCodexSetupCalls(t, logPath), "plugin add basecamp@37signals --json") + assert.NotContains(t, readCodexSetupCalls(t, logPath), "plugin add basecamp-cli@37signals --json") } func TestSetupCodexPluginFailureReturnsStructuredError(t *testing.T) { @@ -155,7 +187,7 @@ func TestRunCodexSetupInteractiveExplainsNextSteps(t *testing.T) { require.NoError(t, runCodexSetup(cmd, styles)) assert.Contains(t, output.String(), "Registering 37signals marketplace") - assert.Contains(t, output.String(), "Installing basecamp plugin") + assert.Contains(t, output.String(), "Installing basecamp-cli plugin") // Codex lists untrusted hooks but does not run them until trusted, so // complete hook setup before starting the thread that loads the skills. trustAt := strings.Index(output.String(), "trust the plugin hooks with /hooks") @@ -178,7 +210,7 @@ func TestRunCodexSetupInteractiveFailureWarnsAndContinues(t *testing.T) { assert.Contains(t, output.String(), "Codex plugin setup failed") assert.Contains(t, output.String(), "codex plugin marketplace add basecamp/claude-plugins") - assert.Contains(t, output.String(), "codex plugin add basecamp@37signals") + assert.Contains(t, output.String(), "codex plugin add basecamp-cli@37signals") assert.Contains(t, output.String(), "basecamp doctor") } @@ -193,7 +225,7 @@ func TestInstallCodexPluginReturnsStructuredError(t *testing.T) { assert.Equal(t, []string{ "codex plugin marketplace add basecamp/claude-plugins", "codex plugin marketplace upgrade 37signals", - "codex plugin add basecamp@37signals", + "codex plugin add basecamp-cli@37signals", }, setupErr.Manual) } @@ -237,6 +269,8 @@ type codexStubOptions struct { pluginAlreadyInstalled bool pluginFailure bool verificationMissing bool + legacyInstalled bool + legacyRemoveFailure bool } func installCodexStub(t *testing.T, options codexStubOptions) string { @@ -248,6 +282,16 @@ func installCodexStub(t *testing.T, options codexStubOptions) string { if options.pluginAlreadyInstalled { require.NoError(t, os.WriteFile(statePath, []byte("installed"), 0o644)) } + // PATH holds only the stub, so it records state with shell builtins + // (no rm): the legacy file reads "installed" until `plugin remove`. + legacyPath := filepath.Join(home, "legacy") + if options.legacyInstalled { + require.NoError(t, os.WriteFile(legacyPath, []byte("installed"), 0o644)) + manifestDir := filepath.Join(home, ".codex", "plugins", "cache", "37signals", "basecamp", "0.11.0", ".codex-plugin") + require.NoError(t, os.MkdirAll(manifestDir, 0o755)) + require.NoError(t, os.WriteFile(filepath.Join(manifestDir, "plugin.json"), + []byte(`{"name":"basecamp","repository":"https://github.com/basecamp/basecamp-cli"}`), 0o644)) + } logPath := filepath.Join(home, "codex-calls.log") boolShell := func(value bool) string { if value { @@ -264,12 +308,17 @@ func installCodexStub(t *testing.T, options codexStubOptions) string { " if [ " + boolShell(options.marketplaceAlreadyAddedSuccess) + " = 1 ]; then echo '{\"marketplaceName\":\"37signals\",\"alreadyAdded\":true}'; exit 0; fi\n" + " echo '{\"name\":\"37signals\"}'; exit 0 ;;\n" + " \"plugin marketplace upgrade 37signals --json\") echo '{\"name\":\"37signals\"}'; exit 0 ;;\n" + - " \"plugin add basecamp@37signals --json\")\n" + + " \"plugin add basecamp-cli@37signals --json\")\n" + " if [ " + boolShell(options.pluginFailure) + " = 1 ]; then echo 'plugin failure' >&2; exit 1; fi\n" + " : > \"" + statePath + "\"; echo '{\"installed\":true}'; exit 0 ;;\n" + + " \"plugin remove basecamp@37signals --json\")\n" + + " if [ " + boolShell(options.legacyRemoveFailure) + " = 1 ]; then echo 'remove failure' >&2; exit 1; fi\n" + + " echo removed > \"" + legacyPath + "\"; echo '{}'; exit 0 ;;\n" + " \"plugin list --available --json\")\n" + - " if [ " + boolShell(options.verificationMissing) + " = 1 ]; then echo '{\"installed\":[],\"available\":[{\"pluginId\":\"basecamp@37signals\",\"version\":\"0.7.2\",\"installed\":false,\"enabled\":false}]}'; exit 0; fi\n" + - " if [ -f \"" + statePath + "\" ]; then echo '{\"installed\":[{\"pluginId\":\"basecamp@37signals\",\"version\":\"0.7.2\",\"installed\":true,\"enabled\":true}],\"available\":[]}'; else echo '{\"installed\":[],\"available\":[]}'; fi; exit 0 ;;\n" + + " legacy_state=''; [ -f \"" + legacyPath + "\" ] && read -r legacy_state < \"" + legacyPath + "\"\n" + + " if [ \"$legacy_state\" = installed ]; then legacy='{\"pluginId\":\"basecamp@37signals\",\"version\":\"0.11.0\",\"installed\":true,\"enabled\":true},'; else legacy=''; fi\n" + + " if [ " + boolShell(options.verificationMissing) + " = 1 ]; then echo '{\"installed\":[],\"available\":[{\"pluginId\":\"basecamp-cli@37signals\",\"version\":\"0.7.2\",\"installed\":false,\"enabled\":false}]}'; exit 0; fi\n" + + " if [ -f \"" + statePath + "\" ]; then echo \"{\\\"installed\\\":[$legacy{\\\"pluginId\\\":\\\"basecamp-cli@37signals\\\",\\\"version\\\":\\\"0.7.2\\\",\\\"installed\\\":true,\\\"enabled\\\":true}],\\\"available\\\":[]}\"; else echo \"{\\\"installed\\\":[${legacy%,}],\\\"available\\\":[]}\"; fi; exit 0 ;;\n" + " *) echo 'unexpected command' >&2; exit 1 ;;\n" + "esac\n" require.NoError(t, os.WriteFile(filepath.Join(binDir, "codex"), []byte(script), 0o755)) //nolint:gosec // test executable diff --git a/internal/commands/wizard_test.go b/internal/commands/wizard_test.go index 76e3fa19d..d7b7f6ca2 100644 --- a/internal/commands/wizard_test.go +++ b/internal/commands/wizard_test.go @@ -8,6 +8,7 @@ import ( "os" "path/filepath" "runtime" + "strconv" "strings" "testing" "time" @@ -732,7 +733,7 @@ func TestSetupClaudeNonInteractiveRepairsSkillLink(t *testing.T) { require.NoError(t, os.MkdirAll(filepath.Join(home, ".claude", "plugins"), 0o755)) require.NoError(t, os.WriteFile( filepath.Join(home, ".claude", "plugins", "installed_plugins.json"), - []byte(`[{"name":"basecamp","version":"1.0.0"}]`), 0o644)) + []byte(`[{"name":"basecamp-cli","version":"1.0.0"}]`), 0o644)) app, appBuf := setupQuickstartTestApp(t, "", "") app.Flags.JSON = true @@ -773,7 +774,7 @@ func TestRunClaudeSetupRepairsSkillLink(t *testing.T) { pluginDir := filepath.Join(home, ".claude", "plugins") require.NoError(t, os.MkdirAll(pluginDir, 0o755)) require.NoError(t, os.WriteFile(filepath.Join(pluginDir, "installed_plugins.json"), - []byte(`[{"name":"basecamp","version":"1.0.0"}]`), 0o644)) + []byte(`[{"name":"basecamp-cli","version":"1.0.0"}]`), 0o644)) // Install baseline skill files (source for the symlink) _, err := installSkillFiles() @@ -813,7 +814,7 @@ func TestSetupClaudeNonInteractiveRemovesStalePlugins(t *testing.T) { filepath.Join(pluginDir, "installed_plugins.json"), []byte(`{"version":2,"plugins":{`+ `"basecamp@basecamp":[{"scope":"user","version":"0.1.0"},{"scope":"project","version":"0.1.0"}],`+ - `"basecamp@37signals":[{"scope":"user","version":"0.1.0"}]}}`), + `"basecamp-cli@37signals":[{"scope":"user","version":"0.1.0"}]}}`), 0o644)) // Create stub claude binary that logs invocations. @@ -911,14 +912,87 @@ func TestSetupClaudeNonInteractiveScopeAwareReinstall(t *testing.T) { // Verify install calls preserve scopes from stale entries calls, readErr := os.ReadFile(logFile) require.NoError(t, readErr) - assert.Contains(t, string(calls), "plugin install basecamp@37signals --scope project") + assert.Contains(t, string(calls), "plugin install basecamp-cli@37signals --scope project") // The reinstall path must also refresh before installing: add → update → // scoped install, so a stale SSH-shorthand entry is replaced with the current // HTTPS source first (issue #417). assertCallOrder(t, string(calls), "plugin marketplace add basecamp/claude-plugins", "plugin marketplace update 37signals", - "plugin install basecamp@37signals --scope user") + "plugin install basecamp-cli@37signals --scope user") +} + +// runClaudeSetupWithStub runs non-interactive `setup claude` against a stub +// claude binary that logs its argv and succeeds, returning the call log. +func runClaudeSetupWithStub(t *testing.T, home string) string { + t.Helper() + binDir := filepath.Join(home, "bin") + require.NoError(t, os.MkdirAll(binDir, 0o755)) + logFile := filepath.Join(home, "claude-calls.log") + stubScript := "#!/bin/sh\necho \"$*\" >> \"" + logFile + "\"\nexit 0\n" + require.NoError(t, os.WriteFile(filepath.Join(binDir, "claude"), []byte(stubScript), 0o755)) //nolint:gosec // G306: test helper + t.Setenv("PATH", binDir) + + app, _ := setupQuickstartTestApp(t, "", "") + app.Flags.JSON = true + cmd := NewSetupCmd() + cmd.SetArgs([]string{"claude"}) + cmd.SetContext(appctx.WithApp(context.Background(), app)) + cmd.SetOut(&bytes.Buffer{}) + cmd.SetErr(&bytes.Buffer{}) + require.NoError(t, cmd.Execute()) + + calls, err := os.ReadFile(logFile) + require.NoError(t, err) + return string(calls) +} + +// seedLegacyBasecampPlugin records a basecamp@37signals install at the user +// and project scopes whose cached manifest names repo. +func seedLegacyBasecampPlugin(t *testing.T, home, repo string) { + t.Helper() + installPath := filepath.Join(home, ".claude", "plugins", "cache", "37signals", "basecamp", "0.11.0") + require.NoError(t, os.MkdirAll(filepath.Join(installPath, ".claude-plugin"), 0o755)) + require.NoError(t, os.WriteFile(filepath.Join(installPath, ".claude-plugin", "plugin.json"), + []byte(`{"name":"basecamp","repository":"`+repo+`"}`), 0o644)) + entry := func(scope string) string { + return `{"scope":"` + scope + `","version":"0.11.0","installPath":` + strconv.Quote(installPath) + `}` + } + require.NoError(t, os.WriteFile(filepath.Join(home, ".claude", "plugins", "installed_plugins.json"), + []byte(`{"version":2,"plugins":{"basecamp@37signals":[`+entry("user")+`,`+entry("project")+`]}}`), 0o644)) +} + +// TestSetupClaudeMigratesLegacyCLIPlugin verifies that the CLI plugin still +// installed as basecamp@37signals is replaced by basecamp-cli@37signals at the +// same scopes. +func TestSetupClaudeMigratesLegacyCLIPlugin(t *testing.T) { + t.Setenv("BASECAMP_NO_KEYRING", "1") + home := t.TempDir() + t.Setenv("HOME", home) + seedLegacyBasecampPlugin(t, home, "https://github.com/basecamp/basecamp-cli") + + calls := runClaudeSetupWithStub(t, home) + + assert.Contains(t, calls, "plugin uninstall basecamp@37signals --scope user") + assert.Contains(t, calls, "plugin uninstall basecamp@37signals --scope project") + assertCallOrder(t, calls, + "plugin marketplace update 37signals", + "plugin install basecamp-cli@37signals --scope user") + assert.Contains(t, calls, "plugin install basecamp-cli@37signals --scope project") +} + +// TestSetupClaudeLeavesHostedConnectorPluginAlone verifies that a +// basecamp@37signals install of the hosted connector is never uninstalled. +func TestSetupClaudeLeavesHostedConnectorPluginAlone(t *testing.T) { + t.Setenv("BASECAMP_NO_KEYRING", "1") + home := t.TempDir() + t.Setenv("HOME", home) + seedLegacyBasecampPlugin(t, home, "https://github.com/basecamp/basecamp-mcp-server") + + calls := runClaudeSetupWithStub(t, home) + + assert.NotContains(t, calls, "plugin uninstall") + assert.Contains(t, calls, "plugin install basecamp-cli@37signals") } // TestSetupClaudeNonInteractiveRefreshesMarketplace verifies the fresh-install @@ -961,7 +1035,7 @@ func TestSetupClaudeNonInteractiveRefreshesMarketplace(t *testing.T) { assertCallOrder(t, string(calls), "plugin marketplace add basecamp/claude-plugins", "plugin marketplace update 37signals", - "plugin install basecamp@37signals") + "plugin install basecamp-cli@37signals") } // TestRunClaudeSetupInteractiveRefreshOrder covers the interactive install path @@ -997,7 +1071,7 @@ func TestRunClaudeSetupInteractiveRefreshOrder(t *testing.T) { assertCallOrder(t, string(calls), "plugin marketplace add basecamp/claude-plugins", "plugin marketplace update 37signals", - "plugin install basecamp@37signals") + "plugin install basecamp-cli@37signals") } // TestJoinNames verifies name joining with commas and "and". diff --git a/internal/harness/claude.go b/internal/harness/claude.go index baf61ccd2..a29f3f97b 100644 --- a/internal/harness/claude.go +++ b/internal/harness/claude.go @@ -7,6 +7,7 @@ import ( "os" "os/exec" "path/filepath" + "strings" "github.com/basecamp/basecamp-cli/internal/version" ) @@ -40,8 +41,10 @@ func claudeChecks() []*StatusCheck { // Migrating from basecamp/basecamp-cli → basecamp/claude-plugins. const ClaudeMarketplaceSource = "basecamp/claude-plugins" -// ClaudePluginName is the plugin identifier to install. -const ClaudePluginName = "basecamp" +// ClaudePluginName is the plugin identifier to install. It was "basecamp" +// until the 37signals marketplace gave that name to the hosted-connector +// plugin; see ClaudeLegacyPluginKey. +const ClaudePluginName = "basecamp-cli" // ClaudeMarketplaceName is the marketplace name as it appears in plugin keys. const ClaudeMarketplaceName = "37signals" @@ -49,6 +52,17 @@ const ClaudeMarketplaceName = "37signals" // ClaudeExpectedPluginKey is the fully-qualified key for a correctly installed plugin. const ClaudeExpectedPluginKey = ClaudePluginName + "@" + ClaudeMarketplaceName +// ClaudeLegacyPluginKey is the key this plugin was installed under before the +// rename. The 37signals marketplace now publishes the hosted Basecamp +// connector plugin under that same key, so an entry with it is this CLI's +// plugin only when its installed manifest says so (see legacyCLIInstall). +const ClaudeLegacyPluginKey = "basecamp@" + ClaudeMarketplaceName + +// CLIRepository is the repository both plugin manifests in this repo name. +// An installed plugin whose manifest names it is this CLI's plugin, whatever +// key it was installed under. +const CLIRepository = "https://github.com/basecamp/basecamp-cli" + // DetectClaude returns true if Claude Code is installed. // Checks ~/.claude/ directory first, then falls back to binary on PATH. func DetectClaude() bool { @@ -129,6 +143,15 @@ func CheckClaudePlugin() *StatusCheck { } } + if len(legacyCLIPluginEntries(data)) > 0 { + return &StatusCheck{ + Name: "Claude Code Plugin", + Status: "fail", + Message: "Installed under the old name " + ClaudeLegacyPluginKey, + Hint: "Run: basecamp setup claude (reinstalls it as " + ClaudeExpectedPluginKey + ")", + } + } + return &StatusCheck{ Name: "Claude Code Plugin", Status: "fail", @@ -336,18 +359,19 @@ func matchesBasecamp(p map[string]any) bool { } // matchesPluginKey returns true if the key identifies a correctly installed -// basecamp plugin — either bare "basecamp" (legacy) or the expected -// marketplace-qualified key "basecamp@37signals". +// plugin — either the bare name or the expected marketplace-qualified key +// "basecamp-cli@37signals". The pre-rename "basecamp" names deliberately do +// not match: they now belong to the hosted-connector plugin. func matchesPluginKey(key string) bool { - return key == "basecamp" || key == ClaudeExpectedPluginKey + return key == ClaudePluginName || key == ClaudeExpectedPluginKey } func jsonContainsBasecamp(data []byte) bool { // Fallback: raw string search for the expected plugin key or bare name. - // Note: `"basecamp"` (with quotes) won't match `"basecamp@old"` since - // the closing quote requires an exact boundary. + // Note: `"basecamp-cli"` (with quotes) won't match `"basecamp-cli@old"` + // since the closing quote requires an exact boundary. s := string(data) - return len(s) > 0 && (contains(s, `"`+ClaudeExpectedPluginKey+`"`) || contains(s, `"basecamp"`)) + return len(s) > 0 && (contains(s, `"`+ClaudeExpectedPluginKey+`"`) || contains(s, `"`+ClaudePluginName+`"`)) } func contains(s, substr string) bool { @@ -369,8 +393,10 @@ type StalePlugin struct { Scopes []string // empty = unknown, fall back to unscoped uninstall } -// StalePluginKeys returns stale plugin entries from installed_plugins.json -// that belong to old/dead marketplaces. +// StalePluginKeys returns stale plugin entries from installed_plugins.json: +// entries from old/dead marketplaces, and this CLI's plugin still installed +// under its pre-rename key. Setup uninstalls each and reinstalls +// ClaudeExpectedPluginKey at the scopes it removed. func StalePluginKeys() []StalePlugin { home, err := os.UserHomeDir() if err != nil { @@ -380,7 +406,7 @@ func StalePluginKeys() []StalePlugin { if err != nil { return nil } - return stalePluginKeys(data) + return append(stalePluginKeys(data), legacyCLIPluginEntries(data)...) } // stalePluginKeys extracts known stale plugin entries (e.g. basecamp@basecamp from @@ -459,7 +485,7 @@ func stalePluginKeys(data []byte) []StalePlugin { } // claudeStalePluginKey is the known stale key from the basecamp → 37signals marketplace rename. -const claudeStalePluginKey = ClaudePluginName + "@" + "basecamp" +const claudeStalePluginKey = "basecamp@basecamp" // isStalePluginKey returns true only for the known stale marketplace key. func isStalePluginKey(key string) bool { @@ -474,3 +500,71 @@ func appendUnique(ss []string, s string) []string { } return append(ss, s) } + +// legacyCLIPluginEntries returns this CLI's plugin when it is still installed +// under ClaudeLegacyPluginKey. That key now also names the hosted-connector +// plugin, so the key alone proves nothing: only the scopes whose installed +// manifest names CLIRepository count. An entry whose manifest can't be read — +// or a file format that records no install path — is left alone, since +// removing a hosted-connector install would be worse than leaving a legacy +// CLI one in place. +func legacyCLIPluginEntries(data []byte) []StalePlugin { + var envelope struct { + Plugins map[string][]struct { + Scope string `json:"scope"` + InstallPath string `json:"installPath"` + } `json:"plugins"` + } + if err := json.Unmarshal(data, &envelope); err != nil { + return nil + } + entries, ok := envelope.Plugins[ClaudeLegacyPluginKey] + if !ok { + return nil + } + found := false + var scopes []string + for _, entry := range entries { + if entry.InstallPath == "" || !isCLIPluginManifest(filepath.Join(entry.InstallPath, ".claude-plugin", "plugin.json")) { + continue + } + found = true + if entry.Scope != "" { + scopes = appendUnique(scopes, entry.Scope) + } + } + if !found { + return nil + } + return []StalePlugin{{Key: ClaudeLegacyPluginKey, Scopes: scopes}} +} + +// isCLIPluginManifest reports whether the plugin manifest at path is this +// CLI's: its repository is CLIRepository. +func isCLIPluginManifest(path string) bool { + data, err := os.ReadFile(path) //nolint:gosec // G304: path from the agent's own plugin records + if err != nil { + return false + } + var manifest struct { + Repository any `json:"repository"` + } + if err := json.Unmarshal(data, &manifest); err != nil { + return false + } + var repo string + switch r := manifest.Repository.(type) { + case string: + repo = r + case map[string]any: + repo, _ = r["url"].(string) + } + return normalizeRepositoryURL(repo) == CLIRepository +} + +func normalizeRepositoryURL(url string) string { + url = strings.TrimSpace(strings.ToLower(url)) + url = strings.TrimPrefix(url, "git+") + url = strings.TrimSuffix(url, "/") + return strings.TrimSuffix(url, ".git") +} diff --git a/internal/harness/claude_test.go b/internal/harness/claude_test.go index 156c96139..5fd9bac9a 100644 --- a/internal/harness/claude_test.go +++ b/internal/harness/claude_test.go @@ -3,6 +3,8 @@ package harness import ( "os" "path/filepath" + "strconv" + "strings" "testing" "github.com/stretchr/testify/assert" @@ -12,7 +14,7 @@ import ( ) func TestPluginInstalled_ArrayFormat(t *testing.T) { - data := []byte(`[{"name": "basecamp", "version": "1.0.0"}]`) + data := []byte(`[{"name": "basecamp-cli", "version": "1.0.0"}]`) assert.True(t, pluginInstalled(data)) } @@ -34,7 +36,7 @@ func TestPluginInstalled_MapFormat_StaleMarketplace(t *testing.T) { } func TestPluginInstalled_MapFormat_Simple(t *testing.T) { - data := []byte(`{"basecamp": {"version": "1.0.0"}}`) + data := []byte(`{"basecamp-cli": {"version": "1.0.0"}}`) assert.True(t, pluginInstalled(data)) } @@ -70,7 +72,7 @@ func TestPluginInstalled_V2Envelope_StaleMarketplace(t *testing.T) { } func TestPluginInstalled_V2Envelope_AltMarketplace(t *testing.T) { - data := []byte(`{"version":2,"plugins":{"basecamp@37signals":[{"scope":"user","version":"0.1.0"}]}}`) + data := []byte(`{"version":2,"plugins":{"basecamp-cli@37signals":[{"scope":"user","version":"0.1.0"}]}}`) assert.True(t, pluginInstalled(data)) } @@ -85,12 +87,12 @@ func TestPluginInstalled_V2Envelope_EmptyPlugins(t *testing.T) { } func TestPluginInstalled_ArrayFormat_AltMarketplace(t *testing.T) { - data := []byte(`[{"package": "basecamp@37signals", "version": "0.1.0"}]`) + data := []byte(`[{"package": "basecamp-cli@37signals", "version": "0.1.0"}]`) assert.True(t, pluginInstalled(data)) } func TestPluginInstalled_MapFormat_AltMarketplace(t *testing.T) { - data := []byte(`{"basecamp@37signals": {"version": "0.1.0"}}`) + data := []byte(`{"basecamp-cli@37signals": {"version": "0.1.0"}}`) assert.True(t, pluginInstalled(data)) } @@ -160,13 +162,13 @@ func TestStalePluginKeys_V2_StaleOnly(t *testing.T) { } func TestStalePluginKeys_V2_Mixed(t *testing.T) { - data := []byte(`{"version":2,"plugins":{"basecamp@37signals":[{"scope":"user"}],"basecamp@basecamp":[{"scope":"user"}]}}`) + data := []byte(`{"version":2,"plugins":{"basecamp-cli@37signals":[{"scope":"user"}],"basecamp@basecamp":[{"scope":"user"}]}}`) plugins := stalePluginKeys(data) assert.Equal(t, []StalePlugin{{Key: "basecamp@basecamp", Scopes: []string{"user"}}}, plugins) } func TestStalePluginKeys_V2_CorrectOnly(t *testing.T) { - data := []byte(`{"version":2,"plugins":{"basecamp@37signals":[{"scope":"user"}]}}`) + data := []byte(`{"version":2,"plugins":{"basecamp-cli@37signals":[{"scope":"user"}]}}`) assert.Empty(t, stalePluginKeys(data)) } @@ -176,7 +178,7 @@ func TestStalePluginKeys_V1_Stale(t *testing.T) { } func TestStalePluginKeys_V1_Correct(t *testing.T) { - data := []byte(`{"basecamp@37signals":{"version":"1.0.0"}}`) + data := []byte(`{"basecamp-cli@37signals":{"version":"1.0.0"}}`) assert.Empty(t, stalePluginKeys(data)) } @@ -187,7 +189,7 @@ func TestStalePluginKeys_BareKey(t *testing.T) { } func TestStalePluginKeys_ArrayFormat_Stale(t *testing.T) { - data := []byte(`[{"package":"basecamp@basecamp","version":"1.0.0"},{"package":"basecamp@37signals","version":"0.1.0"}]`) + data := []byte(`[{"package":"basecamp@basecamp","version":"1.0.0"},{"package":"basecamp-cli@37signals","version":"0.1.0"}]`) assert.Equal(t, []StalePlugin{{Key: "basecamp@basecamp"}}, stalePluginKeys(data)) } @@ -221,18 +223,18 @@ func TestStalePluginKeys_ArrayFormat_WithScope(t *testing.T) { func TestIsStalePluginKey(t *testing.T) { assert.True(t, isStalePluginKey("basecamp@basecamp")) assert.False(t, isStalePluginKey("basecamp@old-marketplace")) - assert.False(t, isStalePluginKey("basecamp@37signals")) + assert.False(t, isStalePluginKey("basecamp-cli@37signals")) assert.False(t, isStalePluginKey("basecamp")) assert.False(t, isStalePluginKey("other@basecamp")) } func TestInstalledPluginVersion_V2(t *testing.T) { - data := []byte(`{"version":2,"plugins":{"basecamp@37signals":[{"scope":"user","version":"1.2.3"}]}}`) + data := []byte(`{"version":2,"plugins":{"basecamp-cli@37signals":[{"scope":"user","version":"1.2.3"}]}}`) assert.Equal(t, "1.2.3", installedPluginVersion(data)) } func TestInstalledPluginVersion_V2_BareKey(t *testing.T) { - data := []byte(`{"version":2,"plugins":{"basecamp":[{"scope":"user","version":"0.5.0"}]}}`) + data := []byte(`{"version":2,"plugins":{"basecamp-cli":[{"scope":"user","version":"0.5.0"}]}}`) assert.Equal(t, "0.5.0", installedPluginVersion(data)) } @@ -242,7 +244,7 @@ func TestInstalledPluginVersion_V2_NotFound(t *testing.T) { } func TestInstalledPluginVersion_Array(t *testing.T) { - data := []byte(`[{"name":"basecamp@37signals","version":"2.0.0"}]`) + data := []byte(`[{"name":"basecamp-cli@37signals","version":"2.0.0"}]`) assert.Equal(t, "2.0.0", installedPluginVersion(data)) } @@ -252,7 +254,7 @@ func TestInstalledPluginVersion_Array_NotFound(t *testing.T) { } func TestInstalledPluginVersion_V1FlatMap(t *testing.T) { - data := []byte(`{"basecamp@37signals":{"version":"1.0.0"}}`) + data := []byte(`{"basecamp-cli@37signals":{"version":"1.0.0"}}`) assert.Equal(t, "1.0.0", installedPluginVersion(data)) } @@ -274,7 +276,7 @@ func TestCheckClaudePluginVersion_UpToDate(t *testing.T) { require.NoError(t, os.MkdirAll(pluginsDir, 0o755)) require.NoError(t, os.WriteFile( filepath.Join(pluginsDir, "installed_plugins.json"), - []byte(`{"version":2,"plugins":{"basecamp@37signals":[{"scope":"user","version":"1.5.0"}]}}`), + []byte(`{"version":2,"plugins":{"basecamp-cli@37signals":[{"scope":"user","version":"1.5.0"}]}}`), 0o644, )) @@ -295,7 +297,7 @@ func TestCheckClaudePluginVersion_Outdated(t *testing.T) { require.NoError(t, os.MkdirAll(pluginsDir, 0o755)) require.NoError(t, os.WriteFile( filepath.Join(pluginsDir, "installed_plugins.json"), - []byte(`{"version":2,"plugins":{"basecamp@37signals":[{"scope":"user","version":"0.1.0"}]}}`), + []byte(`{"version":2,"plugins":{"basecamp-cli@37signals":[{"scope":"user","version":"0.1.0"}]}}`), 0o644, )) @@ -318,7 +320,7 @@ func TestCheckClaudePluginVersion_DevBuild(t *testing.T) { require.NoError(t, os.MkdirAll(pluginsDir, 0o755)) require.NoError(t, os.WriteFile( filepath.Join(pluginsDir, "installed_plugins.json"), - []byte(`{"version":2,"plugins":{"basecamp@37signals":[{"scope":"user","version":"0.1.0"}]}}`), + []byte(`{"version":2,"plugins":{"basecamp-cli@37signals":[{"scope":"user","version":"0.1.0"}]}}`), 0o644, )) @@ -335,3 +337,91 @@ func TestCheckClaudePluginVersion_NoFile(t *testing.T) { assert.Equal(t, "pass", check.Status) assert.Contains(t, check.Message, "not tracked") } + +// writePluginManifest writes a .claude-plugin/plugin.json under dir naming repo. +func writePluginManifest(t *testing.T, dir, manifestDir, repo string) { + t.Helper() + require.NoError(t, os.MkdirAll(filepath.Join(dir, manifestDir), 0o755)) + require.NoError(t, os.WriteFile(filepath.Join(dir, manifestDir, "plugin.json"), + []byte(`{"name":"basecamp","repository":"`+repo+`"}`), 0o644)) +} + +func legacyInstalledPlugins(entries ...string) []byte { + return []byte(`{"version":2,"plugins":{"basecamp@37signals":[` + strings.Join(entries, ",") + `]}}`) +} + +func legacyEntry(scope, installPath string) string { + return `{"scope":"` + scope + `","version":"0.11.0","installPath":` + strconv.Quote(installPath) + `}` +} + +func TestLegacyCLIPluginEntries_CLIManifest(t *testing.T) { + dir := t.TempDir() + writePluginManifest(t, dir, ".claude-plugin", "https://github.com/basecamp/basecamp-cli") + + got := legacyCLIPluginEntries(legacyInstalledPlugins(legacyEntry("user", dir), legacyEntry("project", dir))) + assert.Equal(t, []StalePlugin{{Key: "basecamp@37signals", Scopes: []string{"user", "project"}}}, got) +} + +func TestLegacyCLIPluginEntries_RepositoryVariants(t *testing.T) { + for _, repo := range []string{ + "https://github.com/basecamp/basecamp-cli.git", + "https://github.com/basecamp/basecamp-cli/", + "git+https://github.com/Basecamp/basecamp-cli.git", + } { + dir := t.TempDir() + writePluginManifest(t, dir, ".claude-plugin", repo) + assert.Len(t, legacyCLIPluginEntries(legacyInstalledPlugins(legacyEntry("user", dir))), 1, repo) + } +} + +// The hosted-connector plugin now owns basecamp@37signals: it must never be +// mistaken for a stale CLI install, or setup would uninstall it. +func TestLegacyCLIPluginEntries_HostedConnectorIsLeftAlone(t *testing.T) { + dir := t.TempDir() + writePluginManifest(t, dir, ".claude-plugin", "https://github.com/basecamp/basecamp-mcp-server") + assert.Empty(t, legacyCLIPluginEntries(legacyInstalledPlugins(legacyEntry("user", dir)))) +} + +func TestLegacyCLIPluginEntries_OnlyCLIScopesCount(t *testing.T) { + cli, hosted := t.TempDir(), t.TempDir() + writePluginManifest(t, cli, ".claude-plugin", "https://github.com/basecamp/basecamp-cli") + writePluginManifest(t, hosted, ".claude-plugin", "https://github.com/basecamp/basecamp-mcp-server") + + got := legacyCLIPluginEntries(legacyInstalledPlugins(legacyEntry("user", hosted), legacyEntry("project", cli))) + assert.Equal(t, []StalePlugin{{Key: "basecamp@37signals", Scopes: []string{"project"}}}, got) +} + +func TestLegacyCLIPluginEntries_UnreadableManifestIsLeftAlone(t *testing.T) { + assert.Empty(t, legacyCLIPluginEntries(legacyInstalledPlugins(legacyEntry("user", filepath.Join(t.TempDir(), "gone"))))) + assert.Empty(t, legacyCLIPluginEntries([]byte(`{"version":2,"plugins":{"basecamp@37signals":[{"scope":"user"}]}}`))) + assert.Empty(t, legacyCLIPluginEntries([]byte(`{"basecamp@37signals":{"version":"0.11.0"}}`))) +} + +func TestCheckClaudePlugin_LegacyCLIInstall(t *testing.T) { + home := t.TempDir() + t.Setenv("HOME", home) + installPath := filepath.Join(home, ".claude", "plugins", "cache", "37signals", "basecamp", "0.11.0") + writePluginManifest(t, installPath, ".claude-plugin", "https://github.com/basecamp/basecamp-cli") + require.NoError(t, os.WriteFile(filepath.Join(home, ".claude", "plugins", "installed_plugins.json"), + legacyInstalledPlugins(legacyEntry("user", installPath)), 0o644)) + + check := CheckClaudePlugin() + assert.Equal(t, "fail", check.Status) + assert.Contains(t, check.Message, "old name basecamp@37signals") + assert.Contains(t, check.Hint, "basecamp setup claude") + assert.Equal(t, []StalePlugin{{Key: "basecamp@37signals", Scopes: []string{"user"}}}, StalePluginKeys()) +} + +func TestCheckClaudePlugin_HostedConnectorAloneIsNotInstalled(t *testing.T) { + home := t.TempDir() + t.Setenv("HOME", home) + installPath := filepath.Join(home, ".claude", "plugins", "cache", "37signals", "basecamp", "0.1.0") + writePluginManifest(t, installPath, ".claude-plugin", "https://github.com/basecamp/basecamp-mcp-server") + require.NoError(t, os.WriteFile(filepath.Join(home, ".claude", "plugins", "installed_plugins.json"), + legacyInstalledPlugins(legacyEntry("user", installPath)), 0o644)) + + check := CheckClaudePlugin() + assert.Equal(t, "fail", check.Status) + assert.Equal(t, "Plugin not installed", check.Message) + assert.Empty(t, StalePluginKeys()) +} diff --git a/internal/harness/codex.go b/internal/harness/codex.go index d921ca551..278268b06 100644 --- a/internal/harness/codex.go +++ b/internal/harness/codex.go @@ -9,6 +9,7 @@ import ( "os" "os/exec" "path/filepath" + "strings" "time" "github.com/basecamp/basecamp-cli/internal/version" @@ -17,12 +18,18 @@ import ( const ( // CodexMarketplaceSource is the Git marketplace repository containing Basecamp. CodexMarketplaceSource = "basecamp/claude-plugins" - // CodexPluginName is the plugin identifier to install. - CodexPluginName = "basecamp" + // CodexPluginName is the plugin identifier to install. It was "basecamp" + // until the 37signals marketplace gave that name to the hosted-connector + // plugin; see CodexLegacyPluginKey. + CodexPluginName = "basecamp-cli" // CodexMarketplaceName is the marketplace name published by 37signals. CodexMarketplaceName = "37signals" // CodexExpectedPluginKey is the fully qualified Basecamp plugin ID. CodexExpectedPluginKey = CodexPluginName + "@" + CodexMarketplaceName + // CodexLegacyPluginKey is the pre-rename plugin ID. It now also names + // the hosted-connector plugin, so an install under it is this CLI's only + // when its cached manifest says so (see codexLegacyCLIInstalled). + CodexLegacyPluginKey = "basecamp@" + CodexMarketplaceName // codexQueryTimeout bounds how long the Codex probe may run. codexQueryTimeout = 5 * time.Second @@ -106,6 +113,10 @@ type codexPluginState struct { Version string `json:"version"` Installed bool `json:"installed"` Enabled bool `json:"enabled"` + + // legacyInstalled is set, on the state queryCodexPlugin returns, when + // this CLI's plugin is still installed under CodexLegacyPluginKey. + legacyInstalled bool } func init() { @@ -174,6 +185,14 @@ func codexPluginCheck(state codexPluginState, found bool, err error) *StatusChec return codexQueryFailure("Codex Plugin", err) } if !found || !state.Installed { + if state.legacyInstalled { + return &StatusCheck{ + Name: "Codex Plugin", + Status: "fail", + Message: "Installed under the old name " + CodexLegacyPluginKey, + Hint: "Run: basecamp setup codex (reinstalls it as " + CodexExpectedPluginKey + ")", + } + } return &StatusCheck{ Name: "Codex Plugin", Status: "fail", @@ -189,6 +208,14 @@ func codexPluginCheck(state codexPluginState, found bool, err error) *StatusChec Hint: "Run: basecamp setup codex", } } + if state.legacyInstalled { + return &StatusCheck{ + Name: "Codex Plugin", + Status: "warn", + Message: "Installed, but the old " + CodexLegacyPluginKey + " copy is still installed too", + Hint: "Run: basecamp setup codex", + } + } return &StatusCheck{ Name: "Codex Plugin", Status: "pass", @@ -270,9 +297,16 @@ func queryCodexPlugin(parent context.Context) (codexPluginState, bool, error) { if envelope.Installed == nil && envelope.Available == nil { return codexPluginState{}, false, fmt.Errorf("%w: missing installed and available fields", errCodexParse) } + legacy := false if envelope.Installed != nil { + for _, plugin := range *envelope.Installed { + if plugin.PluginID == CodexLegacyPluginKey && plugin.Installed && codexLegacyCLIInstalled(plugin.Version) { + legacy = true + } + } for _, plugin := range *envelope.Installed { if plugin.PluginID == CodexExpectedPluginKey { + plugin.legacyInstalled = legacy return plugin, true, nil } } @@ -280,11 +314,51 @@ func queryCodexPlugin(parent context.Context) (codexPluginState, bool, error) { if envelope.Available != nil { for _, plugin := range *envelope.Available { if plugin.PluginID == CodexExpectedPluginKey { + plugin.legacyInstalled = legacy return plugin, true, nil } } } - return codexPluginState{}, false, nil + return codexPluginState{legacyInstalled: legacy}, false, nil +} + +// CodexLegacyCLIInstalled reports whether this CLI's plugin is still installed +// in Codex under CodexLegacyPluginKey. It reads the plugin list itself, so +// callers that already hold a query result should use its state instead. +func CodexLegacyCLIInstalled(ctx context.Context) bool { + state, _, err := queryCodexPlugin(ctx) + return err == nil && state.legacyInstalled +} + +// codexLegacyCLIInstalled reports whether the cached copy of the +// CodexLegacyPluginKey install at version is this CLI's plugin. Codex's +// plugin list reports the marketplace's current source for an ID, not the +// source the installed copy came from — after the rename that is the hosted +// connector's — so the installed manifest is the only reliable witness. A +// manifest that can't be read is not counted: leaving a legacy CLI install +// in place beats removing a hosted-connector one. +func codexLegacyCLIInstalled(version string) bool { + if version == "" || strings.ContainsAny(version, `/\`) || version == "." || version == ".." { + return false + } + home := codexHome() + if home == "" { + return false + } + name := strings.TrimSuffix(CodexLegacyPluginKey, "@"+CodexMarketplaceName) + return isCLIPluginManifest(filepath.Join(home, "plugins", "cache", CodexMarketplaceName, name, version, ".codex-plugin", "plugin.json")) +} + +// codexHome is $CODEX_HOME, or ~/.codex. +func codexHome() string { + if home := os.Getenv("CODEX_HOME"); home != "" { + return filepath.Clean(home) + } + home, err := os.UserHomeDir() + if err != nil || home == "" { + return "" + } + return filepath.Join(filepath.Clean(home), ".codex") } func codexQueryFailure(name string, err error) *StatusCheck { diff --git a/internal/harness/codex_test.go b/internal/harness/codex_test.go index c02350650..8470e7e04 100644 --- a/internal/harness/codex_test.go +++ b/internal/harness/codex_test.go @@ -45,7 +45,7 @@ func TestCheckCodexPluginMissingBinary(t *testing.T) { } func TestCheckCodexPluginMissing(t *testing.T) { - stubCodexList(t, `{"installed":[],"available":[{"pluginId":"basecamp@37signals","name":"basecamp","marketplaceName":"37signals","version":"0.7.2","installed":false,"enabled":false}]}`, nil) + stubCodexList(t, `{"installed":[],"available":[{"pluginId":"basecamp-cli@37signals","name":"basecamp-cli","marketplaceName":"37signals","version":"0.7.2","installed":false,"enabled":false}]}`, nil) check := CheckCodexPlugin() @@ -192,7 +192,7 @@ func stubCodexList(t *testing.T, output string, commandErr error) { } func codexListFixture(pluginVersion string, installed, enabled bool) string { - return `{"installed":[{"pluginId":"basecamp@37signals","name":"basecamp","marketplaceName":"37signals","version":"` + pluginVersion + `","installed":` + boolJSON(installed) + `,"enabled":` + boolJSON(enabled) + `}],"available":[]}` + return `{"installed":[{"pluginId":"basecamp-cli@37signals","name":"basecamp-cli","marketplaceName":"37signals","version":"` + pluginVersion + `","installed":` + boolJSON(installed) + `,"enabled":` + boolJSON(enabled) + `}],"available":[]}` } func boolJSON(value bool) string { @@ -201,3 +201,54 @@ func boolJSON(value bool) string { } return "false" } + +const codexLegacyInstalledJSON = `{"pluginId":"basecamp@37signals","name":"basecamp","marketplaceName":"37signals","version":"0.11.0","installed":true,"enabled":true}` + +// writeCodexCachedManifest puts a cached basecamp@37signals 0.11.0 manifest +// naming repo under a fresh CODEX_HOME. +func writeCodexCachedManifest(t *testing.T, repo string) { + t.Helper() + codexHome := t.TempDir() + t.Setenv("CODEX_HOME", codexHome) + writePluginManifest(t, filepath.Join(codexHome, "plugins", "cache", "37signals", "basecamp", "0.11.0"), ".codex-plugin", repo) +} + +func TestCheckCodexPluginLegacyCLIInstall(t *testing.T) { + writeCodexCachedManifest(t, "https://github.com/basecamp/basecamp-cli") + stubCodexList(t, `{"installed":[`+codexLegacyInstalledJSON+`],"available":[{"pluginId":"basecamp-cli@37signals","version":"0.11.0","installed":false,"enabled":false}]}`, nil) + + check := CheckCodexPlugin() + + assert.Equal(t, "fail", check.Status) + assert.Contains(t, check.Message, "old name basecamp@37signals") + assert.Contains(t, check.Hint, "basecamp setup codex") + assert.True(t, CodexLegacyCLIInstalled(context.Background())) +} + +func TestCheckCodexPluginHostedConnectorIsNotLegacy(t *testing.T) { + writeCodexCachedManifest(t, "https://github.com/basecamp/basecamp-mcp-server") + stubCodexList(t, `{"installed":[`+codexLegacyInstalledJSON+`],"available":[]}`, nil) + + check := CheckCodexPlugin() + + assert.Equal(t, "Plugin not installed", check.Message) + assert.False(t, CodexLegacyCLIInstalled(context.Background())) +} + +func TestCheckCodexPluginBothInstalledWarns(t *testing.T) { + writeCodexCachedManifest(t, "https://github.com/basecamp/basecamp-cli") + stubCodexList(t, `{"installed":[`+codexLegacyInstalledJSON+`,{"pluginId":"basecamp-cli@37signals","version":"0.11.0","installed":true,"enabled":true}],"available":[]}`, nil) + + check := CheckCodexPlugin() + + assert.Equal(t, "warn", check.Status) + assert.Contains(t, check.Message, "old basecamp@37signals copy") +} + +func TestCodexLegacyCLIInstalledRejectsPathVersions(t *testing.T) { + writeCodexCachedManifest(t, "https://github.com/basecamp/basecamp-cli") + for _, v := range []string{"", ".", "..", "../0.11.0", `..\0.11.0`} { + assert.False(t, codexLegacyCLIInstalled(v), v) + } + assert.True(t, codexLegacyCLIInstalled("0.11.0")) +} diff --git a/internal/release/manifests_test.go b/internal/release/manifests_test.go index 5cf5a51cd..1d9290dc6 100644 --- a/internal/release/manifests_test.go +++ b/internal/release/manifests_test.go @@ -20,6 +20,8 @@ import ( "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + + "github.com/basecamp/basecamp-cli/internal/harness" ) // semverPattern is the strict semver 2.0.0 grammar (semver.org). Go's \d is @@ -39,10 +41,11 @@ type codexInterface struct { } type pluginManifest struct { - Name string `json:"name"` - Version string `json:"version"` - Skills string `json:"skills"` - Interface *codexInterface `json:"interface"` + Name string `json:"name"` + Version string `json:"version"` + Repository string `json:"repository"` + Skills string `json:"skills"` + Interface *codexInterface `json:"interface"` } func TestManifestsParseWithMatchingIdentity(t *testing.T) { @@ -50,8 +53,12 @@ func TestManifestsParseWithMatchingIdentity(t *testing.T) { claude := readManifest(t, filepath.Join(root, ".claude-plugin", "plugin.json")) codex := readManifest(t, filepath.Join(root, ".codex-plugin", "plugin.json")) - assert.Equal(t, "basecamp", claude.Name) - assert.Equal(t, "basecamp", codex.Name) + assert.Equal(t, harness.ClaudePluginName, claude.Name) + assert.Equal(t, harness.CodexPluginName, codex.Name) + // Setup tells a pre-rename install of this plugin from the hosted + // connector that now owns the old name by this field alone. + assert.Equal(t, harness.CLIRepository, claude.Repository) + assert.Equal(t, harness.CLIRepository, codex.Repository) assert.Regexp(t, semverPattern, claude.Version) assert.Regexp(t, semverPattern, codex.Version) assert.Equal(t, claude.Version, codex.Version, "manifest versions must stay in lockstep") From c3470d53ad08a89513514656cbc46fb223a2455a Mon Sep 17 00:00:00 2001 From: Jeremy Daer Date: Mon, 5 Oct 2026 15:22:49 -0700 Subject: [PATCH 02/13] Give the Codex legacy-plugin removal its own timeout --- internal/commands/wizard_codex.go | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/internal/commands/wizard_codex.go b/internal/commands/wizard_codex.go index 6218bbf21..ab4899d53 100644 --- a/internal/commands/wizard_codex.go +++ b/internal/commands/wizard_codex.go @@ -21,6 +21,9 @@ const ( codexMarketplaceTimeout = 20 * time.Second codexInstallTimeout = 20 * time.Second codexVerifyTimeout = 5 * time.Second + // codexRemoveTimeout bounds `plugin remove`, which only deletes local + // state — no clone — so it needs less than an install. + codexRemoveTimeout = 10 * time.Second ) // runCodexSetupCommand runs a codex subcommand, capturing stdout for --json @@ -115,7 +118,7 @@ func installCodexPlugin(parent context.Context, stderr io.Writer, progress func( cancelLegacy() if legacy { progress("Removing the pre-rename " + harness.CodexLegacyPluginKey + " plugin…") - removeStdout, removeStderr, removeErr := runCodexStep(parent, stderr, codexInstallTimeout, codexPath, + removeStdout, removeStderr, removeErr := runCodexStep(parent, stderr, codexRemoveTimeout, codexPath, "plugin", "remove", harness.CodexLegacyPluginKey, "--json") if removeErr != nil { return &agentSetupError{ From b779d644169ab4cf7e0d4baff21000b1a13e40b9 Mon Sep 17 00:00:00 2001 From: Jeremy Daer Date: Mon, 5 Oct 2026 15:30:25 -0700 Subject: [PATCH 03/13] Never let a legacy-plugin migration reach an unscoped uninstall; flag leftover copies A legacy basecamp@37signals entry counts only at a scope claude plugin uninstall --scope accepts, so setup can't fall back to an unscoped uninstall of the shared key that would also remove a hosted connector. doctor warns when the old copy is still installed next to basecamp-cli (Claude), or alongside a disabled one (Codex). The README's manual steps install first and remove the old key only after checking its manifest. --- README.md | 12 +++++++++--- internal/commands/wizard_codex_test.go | 3 +++ internal/harness/claude.go | 24 +++++++++++++++++++----- internal/harness/claude_test.go | 24 ++++++++++++++++++++++++ internal/harness/codex.go | 6 +++++- internal/harness/codex_test.go | 11 +++++++++++ 6 files changed, 71 insertions(+), 9 deletions(-) diff --git a/README.md b/README.md index f1dcd314b..277007e80 100644 --- a/README.md +++ b/README.md @@ -304,16 +304,22 @@ went to the hosted Basecamp connector plugin, which works through Basecamp's own MCP server rather than this CLI. If you installed the plugin under the old name, re-run `basecamp setup claude` or `basecamp setup codex`: setup recognizes the old CLI install by its manifest, removes it, and installs `basecamp-cli` at -the same scope. To migrate by hand: +the same scope. Setup is the safe way to migrate: `basecamp@37signals` now +also names the hosted connector, so uninstalling that key without checking +which plugin it is can remove the connector. If you migrate by hand, install +`basecamp-cli` first, and remove `basecamp@37signals` only if its installed +manifest (`~/.claude/plugins/cache/37signals/basecamp//.claude-plugin/plugin.json`, +or `~/.codex/plugins/cache/37signals/basecamp//.codex-plugin/plugin.json`) +names `https://github.com/basecamp/basecamp-cli` as its `repository`: ```bash claude plugin marketplace update 37signals -claude plugin uninstall basecamp@37signals claude plugin install basecamp-cli@37signals +claude plugin uninstall basecamp@37signals # only after the check above codex plugin marketplace upgrade 37signals -codex plugin remove basecamp@37signals codex plugin add basecamp-cli@37signals +codex plugin remove basecamp@37signals # only after the check above ``` Skills from the plugin are namespaced by its name, so `basecamp:basecamp` diff --git a/internal/commands/wizard_codex_test.go b/internal/commands/wizard_codex_test.go index ab05e8ff7..687e44a3f 100644 --- a/internal/commands/wizard_codex_test.go +++ b/internal/commands/wizard_codex_test.go @@ -285,6 +285,9 @@ func installCodexStub(t *testing.T, options codexStubOptions) string { // PATH holds only the stub, so it records state with shell builtins // (no rm): the legacy file reads "installed" until `plugin remove`. legacyPath := filepath.Join(home, "legacy") + // codexHome prefers $CODEX_HOME; pin it so an ambient value can't hide + // the fixture. + t.Setenv("CODEX_HOME", filepath.Join(home, ".codex")) if options.legacyInstalled { require.NoError(t, os.WriteFile(legacyPath, []byte("installed"), 0o644)) manifestDir := filepath.Join(home, ".codex", "plugins", "cache", "37signals", "basecamp", "0.11.0", ".codex-plugin") diff --git a/internal/harness/claude.go b/internal/harness/claude.go index a29f3f97b..f6e83f716 100644 --- a/internal/harness/claude.go +++ b/internal/harness/claude.go @@ -136,6 +136,14 @@ func CheckClaudePlugin() *StatusCheck { // Try as array of objects with "name" or "package" fields, // or as a map with plugin keys. if pluginInstalled(data) { + if len(legacyCLIPluginEntries(data)) > 0 { + return &StatusCheck{ + Name: "Claude Code Plugin", + Status: "warn", + Message: "Installed, but the old " + ClaudeLegacyPluginKey + " copy is still installed too", + Hint: "Run: basecamp setup claude", + } + } return &StatusCheck{ Name: "Claude Code Plugin", Status: "pass", @@ -507,7 +515,9 @@ func appendUnique(ss []string, s string) []string { // manifest names CLIRepository count. An entry whose manifest can't be read — // or a file format that records no install path — is left alone, since // removing a hosted-connector install would be worse than leaving a legacy -// CLI one in place. +// CLI one in place. So is an entry without a scope `claude plugin uninstall +// --scope` accepts: setup falls back to an unscoped uninstall for those, and +// for this shared key that would remove a hosted connector at every scope. func legacyCLIPluginEntries(data []byte) []StalePlugin { var envelope struct { Plugins map[string][]struct { @@ -525,13 +535,11 @@ func legacyCLIPluginEntries(data []byte) []StalePlugin { found := false var scopes []string for _, entry := range entries { - if entry.InstallPath == "" || !isCLIPluginManifest(filepath.Join(entry.InstallPath, ".claude-plugin", "plugin.json")) { + if !targetableScope(entry.Scope) || entry.InstallPath == "" || !isCLIPluginManifest(filepath.Join(entry.InstallPath, ".claude-plugin", "plugin.json")) { continue } found = true - if entry.Scope != "" { - scopes = appendUnique(scopes, entry.Scope) - } + scopes = appendUnique(scopes, entry.Scope) } if !found { return nil @@ -539,6 +547,12 @@ func legacyCLIPluginEntries(data []byte) []StalePlugin { return []StalePlugin{{Key: ClaudeLegacyPluginKey, Scopes: scopes}} } +// targetableScope reports whether scope is one `claude plugin uninstall +// --scope` accepts (the same set setup's validPluginScope allows). +func targetableScope(scope string) bool { + return scope == "user" || scope == "project" || scope == "local" +} + // isCLIPluginManifest reports whether the plugin manifest at path is this // CLI's: its repository is CLIRepository. func isCLIPluginManifest(path string) bool { diff --git a/internal/harness/claude_test.go b/internal/harness/claude_test.go index 5fd9bac9a..50db29de6 100644 --- a/internal/harness/claude_test.go +++ b/internal/harness/claude_test.go @@ -391,6 +391,30 @@ func TestLegacyCLIPluginEntries_OnlyCLIScopesCount(t *testing.T) { assert.Equal(t, []StalePlugin{{Key: "basecamp@37signals", Scopes: []string{"project"}}}, got) } +// An entry without a targetable scope would reach setup's unscoped uninstall, +// which removes the shared key at every scope — hosted connector included. +func TestLegacyCLIPluginEntries_UntargetableScopeIsLeftAlone(t *testing.T) { + dir := t.TempDir() + writePluginManifest(t, dir, ".claude-plugin", "https://github.com/basecamp/basecamp-cli") + assert.Empty(t, legacyCLIPluginEntries(legacyInstalledPlugins(legacyEntry("", dir)))) + assert.Empty(t, legacyCLIPluginEntries(legacyInstalledPlugins(legacyEntry("global", dir)))) + got := legacyCLIPluginEntries(legacyInstalledPlugins(legacyEntry("global", dir), legacyEntry("local", dir))) + assert.Equal(t, []StalePlugin{{Key: "basecamp@37signals", Scopes: []string{"local"}}}, got) +} + +func TestCheckClaudePlugin_BothInstalledWarns(t *testing.T) { + home := t.TempDir() + t.Setenv("HOME", home) + installPath := filepath.Join(home, ".claude", "plugins", "cache", "37signals", "basecamp", "0.11.0") + writePluginManifest(t, installPath, ".claude-plugin", "https://github.com/basecamp/basecamp-cli") + require.NoError(t, os.WriteFile(filepath.Join(home, ".claude", "plugins", "installed_plugins.json"), + []byte(`{"version":2,"plugins":{"basecamp-cli@37signals":[{"scope":"user","version":"0.12.0"}],"basecamp@37signals":[`+legacyEntry("user", installPath)+`]}}`), 0o644)) + + check := CheckClaudePlugin() + assert.Equal(t, "warn", check.Status) + assert.Contains(t, check.Message, "old basecamp@37signals copy") +} + func TestLegacyCLIPluginEntries_UnreadableManifestIsLeftAlone(t *testing.T) { assert.Empty(t, legacyCLIPluginEntries(legacyInstalledPlugins(legacyEntry("user", filepath.Join(t.TempDir(), "gone"))))) assert.Empty(t, legacyCLIPluginEntries([]byte(`{"version":2,"plugins":{"basecamp@37signals":[{"scope":"user"}]}}`))) diff --git a/internal/harness/codex.go b/internal/harness/codex.go index 278268b06..47f4f0047 100644 --- a/internal/harness/codex.go +++ b/internal/harness/codex.go @@ -201,10 +201,14 @@ func codexPluginCheck(state codexPluginState, found bool, err error) *StatusChec } } if !state.Enabled { + message := "Installed but disabled" + if state.legacyInstalled { + message += "; the old " + CodexLegacyPluginKey + " copy is still installed too" + } return &StatusCheck{ Name: "Codex Plugin", Status: "fail", - Message: "Installed but disabled", + Message: message, Hint: "Run: basecamp setup codex", } } diff --git a/internal/harness/codex_test.go b/internal/harness/codex_test.go index 8470e7e04..1ea1f5856 100644 --- a/internal/harness/codex_test.go +++ b/internal/harness/codex_test.go @@ -252,3 +252,14 @@ func TestCodexLegacyCLIInstalledRejectsPathVersions(t *testing.T) { } assert.True(t, codexLegacyCLIInstalled("0.11.0")) } + +func TestCheckCodexPluginDisabledMentionsLegacyCopy(t *testing.T) { + writeCodexCachedManifest(t, "https://github.com/basecamp/basecamp-cli") + stubCodexList(t, `{"installed":[`+codexLegacyInstalledJSON+`,{"pluginId":"basecamp-cli@37signals","version":"0.11.0","installed":true,"enabled":false}],"available":[]}`, nil) + + check := CheckCodexPlugin() + + assert.Equal(t, "fail", check.Status) + assert.Contains(t, check.Message, "disabled") + assert.Contains(t, check.Message, "old basecamp@37signals copy") +} From a7568038cc2627e9d7184f54f2d860a86b5955ff Mon Sep 17 00:00:00 2001 From: Jeremy Daer Date: Mon, 5 Oct 2026 15:39:07 -0700 Subject: [PATCH 04/13] README: leave removing the old plugin key to setup's per-scope check --- README.md | 16 +++++++--------- 1 file changed, 7 insertions(+), 9 deletions(-) diff --git a/README.md b/README.md index 277007e80..e78747efd 100644 --- a/README.md +++ b/README.md @@ -304,22 +304,20 @@ went to the hosted Basecamp connector plugin, which works through Basecamp's own MCP server rather than this CLI. If you installed the plugin under the old name, re-run `basecamp setup claude` or `basecamp setup codex`: setup recognizes the old CLI install by its manifest, removes it, and installs `basecamp-cli` at -the same scope. Setup is the safe way to migrate: `basecamp@37signals` now -also names the hosted connector, so uninstalling that key without checking -which plugin it is can remove the connector. If you migrate by hand, install -`basecamp-cli` first, and remove `basecamp@37signals` only if its installed -manifest (`~/.claude/plugins/cache/37signals/basecamp//.claude-plugin/plugin.json`, -or `~/.codex/plugins/cache/37signals/basecamp//.codex-plugin/plugin.json`) -names `https://github.com/basecamp/basecamp-cli` as its `repository`: +the same scope. Use setup rather than removing the old key by hand: +`basecamp@37signals` now also names the hosted connector, and Claude Code can +hold the old CLI plugin at one scope and the connector at another under that +one key. Setup checks each installed entry's manifest and uninstalls only the +CLI's scopes. A manual `claude plugin uninstall basecamp@37signals` or +`codex plugin remove basecamp@37signals` does no such check. To install the +renamed plugin by hand: ```bash claude plugin marketplace update 37signals claude plugin install basecamp-cli@37signals -claude plugin uninstall basecamp@37signals # only after the check above codex plugin marketplace upgrade 37signals codex plugin add basecamp-cli@37signals -codex plugin remove basecamp@37signals # only after the check above ``` Skills from the plugin are namespaced by its name, so `basecamp:basecamp` From 4b6b7f245ad371e12de294b7edfb3f49f4ae46ae Mon Sep 17 00:00:00 2001 From: Jeremy Daer Date: Mon, 5 Oct 2026 15:49:57 -0700 Subject: [PATCH 05/13] Migrate a project or local legacy plugin only from the directory it belongs to Claude resolves project and local scopes against the working directory, so an uninstall run from another checkout would reach that checkout's entry under the shared key, which may be the hosted connector. Such entries now count as legacy only when their projectPath is the current directory. --- internal/commands/wizard_test.go | 5 ++++- internal/harness/claude.go | 34 +++++++++++++++++++++++++++++++- internal/harness/claude_test.go | 28 +++++++++++++++++++++++++- 3 files changed, 64 insertions(+), 3 deletions(-) diff --git a/internal/commands/wizard_test.go b/internal/commands/wizard_test.go index d7b7f6ca2..cb9e056c0 100644 --- a/internal/commands/wizard_test.go +++ b/internal/commands/wizard_test.go @@ -955,8 +955,11 @@ func seedLegacyBasecampPlugin(t *testing.T, home, repo string) { require.NoError(t, os.MkdirAll(filepath.Join(installPath, ".claude-plugin"), 0o755)) require.NoError(t, os.WriteFile(filepath.Join(installPath, ".claude-plugin", "plugin.json"), []byte(`{"name":"basecamp","repository":"`+repo+`"}`), 0o644)) + cwd, err := os.Getwd() + require.NoError(t, err) entry := func(scope string) string { - return `{"scope":"` + scope + `","version":"0.11.0","installPath":` + strconv.Quote(installPath) + `}` + return `{"scope":"` + scope + `","version":"0.11.0","installPath":` + strconv.Quote(installPath) + + `,"projectPath":` + strconv.Quote(cwd) + `}` } require.NoError(t, os.WriteFile(filepath.Join(home, ".claude", "plugins", "installed_plugins.json"), []byte(`{"version":2,"plugins":{"basecamp@37signals":[`+entry("user")+`,`+entry("project")+`]}}`), 0o644)) diff --git a/internal/harness/claude.go b/internal/harness/claude.go index f6e83f716..3133f8645 100644 --- a/internal/harness/claude.go +++ b/internal/harness/claude.go @@ -518,11 +518,16 @@ func appendUnique(ss []string, s string) []string { // CLI one in place. So is an entry without a scope `claude plugin uninstall // --scope` accepts: setup falls back to an unscoped uninstall for those, and // for this shared key that would remove a hosted connector at every scope. +// A project or local entry counts only when its projectPath is the current +// directory: Claude resolves those scopes against the working directory, so +// an uninstall run elsewhere would target another checkout's entry — which +// may be the hosted connector. func legacyCLIPluginEntries(data []byte) []StalePlugin { var envelope struct { Plugins map[string][]struct { Scope string `json:"scope"` InstallPath string `json:"installPath"` + ProjectPath string `json:"projectPath"` } `json:"plugins"` } if err := json.Unmarshal(data, &envelope); err != nil { @@ -535,7 +540,7 @@ func legacyCLIPluginEntries(data []byte) []StalePlugin { found := false var scopes []string for _, entry := range entries { - if !targetableScope(entry.Scope) || entry.InstallPath == "" || !isCLIPluginManifest(filepath.Join(entry.InstallPath, ".claude-plugin", "plugin.json")) { + if !targetableScope(entry.Scope) || !scopeResolvesHere(entry.Scope, entry.ProjectPath) || entry.InstallPath == "" || !isCLIPluginManifest(filepath.Join(entry.InstallPath, ".claude-plugin", "plugin.json")) { continue } found = true @@ -553,6 +558,33 @@ func targetableScope(scope string) bool { return scope == "user" || scope == "project" || scope == "local" } +// scopeResolvesHere reports whether `claude plugin uninstall --scope scope`, +// run from the current directory, reaches the entry recorded for projectPath. +// User scope is global; project and local scopes belong to one directory. +func scopeResolvesHere(scope, projectPath string) bool { + if scope == "user" { + return true + } + if projectPath == "" { + return false + } + cwd, err := os.Getwd() + if err != nil { + return false + } + return samePath(cwd, projectPath) +} + +func samePath(a, b string) bool { + resolve := func(p string) string { + if r, err := filepath.EvalSymlinks(p); err == nil { + return r + } + return filepath.Clean(p) + } + return resolve(a) == resolve(b) +} + // isCLIPluginManifest reports whether the plugin manifest at path is this // CLI's: its repository is CLIRepository. func isCLIPluginManifest(path string) bool { diff --git a/internal/harness/claude_test.go b/internal/harness/claude_test.go index 50db29de6..868634b79 100644 --- a/internal/harness/claude_test.go +++ b/internal/harness/claude_test.go @@ -350,8 +350,19 @@ func legacyInstalledPlugins(entries ...string) []byte { return []byte(`{"version":2,"plugins":{"basecamp@37signals":[` + strings.Join(entries, ",") + `]}}`) } +// legacyEntry records an install at scope; project and local entries belong +// to the current directory. func legacyEntry(scope, installPath string) string { - return `{"scope":"` + scope + `","version":"0.11.0","installPath":` + strconv.Quote(installPath) + `}` + cwd, _ := os.Getwd() + return legacyEntryIn(scope, installPath, cwd) +} + +func legacyEntryIn(scope, installPath, projectPath string) string { + entry := `{"scope":"` + scope + `","version":"0.11.0","installPath":` + strconv.Quote(installPath) + if scope != "user" { + entry += `,"projectPath":` + strconv.Quote(projectPath) + } + return entry + `}` } func TestLegacyCLIPluginEntries_CLIManifest(t *testing.T) { @@ -402,6 +413,21 @@ func TestLegacyCLIPluginEntries_UntargetableScopeIsLeftAlone(t *testing.T) { assert.Equal(t, []StalePlugin{{Key: "basecamp@37signals", Scopes: []string{"local"}}}, got) } +// Claude resolves project and local scopes against the working directory, so +// an entry recorded for another checkout can't be targeted from here: an +// uninstall would reach this directory's entry, which may be the connector. +func TestLegacyCLIPluginEntries_OtherCheckoutIsLeftAlone(t *testing.T) { + dir := t.TempDir() + writePluginManifest(t, dir, ".claude-plugin", "https://github.com/basecamp/basecamp-cli") + other := t.TempDir() + assert.Empty(t, legacyCLIPluginEntries(legacyInstalledPlugins(legacyEntryIn("project", dir, other)))) + assert.Empty(t, legacyCLIPluginEntries(legacyInstalledPlugins(legacyEntryIn("local", dir, "")))) + + t.Chdir(other) + got := legacyCLIPluginEntries(legacyInstalledPlugins(legacyEntryIn("project", dir, other))) + assert.Equal(t, []StalePlugin{{Key: "basecamp@37signals", Scopes: []string{"project"}}}, got) +} + func TestCheckClaudePlugin_BothInstalledWarns(t *testing.T) { home := t.TempDir() t.Setenv("HOME", home) From 04dce4929ba7da395cb748474aeabf42062cb4a8 Mon Sep 17 00:00:00 2001 From: Jeremy Daer Date: Mon, 5 Oct 2026 15:59:36 -0700 Subject: [PATCH 06/13] Point a failed Codex legacy removal at setup, not a bare plugin remove --- internal/commands/wizard_codex.go | 4 +++- internal/commands/wizard_codex_test.go | 3 ++- 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/internal/commands/wizard_codex.go b/internal/commands/wizard_codex.go index ab4899d53..97001816f 100644 --- a/internal/commands/wizard_codex.go +++ b/internal/commands/wizard_codex.go @@ -123,7 +123,9 @@ func installCodexPlugin(parent context.Context, stderr io.Writer, progress func( if removeErr != nil { return &agentSetupError{ Summary: "removing the pre-rename plugin failed: " + codexCommandFailure(removeStdout, removeStderr, removeErr), - Manual: []string{"codex plugin remove " + harness.CodexLegacyPluginKey}, + // Not `codex plugin remove`: the key is shared with the hosted + // connector, so removal must stay behind setup's manifest check. + Manual: []string{"basecamp setup codex"}, } } } diff --git a/internal/commands/wizard_codex_test.go b/internal/commands/wizard_codex_test.go index 687e44a3f..b60eb4051 100644 --- a/internal/commands/wizard_codex_test.go +++ b/internal/commands/wizard_codex_test.go @@ -107,7 +107,8 @@ func TestSetupCodexLegacyRemovalFailureNamesTheManualStep(t *testing.T) { require.NotEmpty(t, envelope.Data.Errors) assert.Contains(t, envelope.Data.Errors[0], "pre-rename plugin") - assert.Contains(t, envelope.Data.ManualCommands, "codex plugin remove basecamp@37signals") + assert.Equal(t, []string{"basecamp setup codex"}, envelope.Data.ManualCommands, + "removal of the shared key stays behind setup's manifest check") } func TestSetupCodexLeavesNonLegacyInstallsAlone(t *testing.T) { From 3bf8a397a0763cb91f9aceb1664618a6095f97dd Mon Sep 17 00:00:00 2001 From: Jeremy Daer Date: Mon, 5 Oct 2026 16:02:01 -0700 Subject: [PATCH 07/13] Treat the pre-rename plugin key as stale instead of telling it apart by manifest The 37signals marketplace now lists no basecamp plugin during the rename, so basecamp@37signals can only be this CLI's pre-rename plugin. Setup removes it by key through the existing stale-entry path, and doctor reports it, without the manifest, scope and projectPath checks that existed only to tell it apart from a hosted connector under the same key. Re-adding basecamp to the marketplace has to wait until this handling is dropped. --- AGENTS.md | 7 +- README.md | 18 ++- install.md | 2 +- internal/commands/wizard_codex.go | 4 +- internal/commands/wizard_codex_test.go | 10 +- internal/commands/wizard_test.go | 36 +----- internal/harness/claude.go | 148 +++++-------------------- internal/harness/claude_test.go | 138 ++++------------------- internal/harness/codex.go | 40 +------ internal/harness/codex_test.go | 29 +---- internal/release/manifests_test.go | 13 +-- 11 files changed, 76 insertions(+), 369 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index eb47ef615..df198ddab 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -44,9 +44,10 @@ Coding-agent integration lives in `internal/harness` (agent registry, detection, skill health checks) and `internal/commands/wizard_agents.go` (`basecamp setup claude|codex|grok|agents`). Claude Code and Codex each get a native plugin from the `basecamp/claude-plugins` marketplace and have registrations of their own (`claude.go`, -`codex.go`). The plugin is `basecamp-cli@37signals`; `basecamp@37signals` is the hosted -Basecamp connector plugin, so an install under that old key counts as this CLI's (and is -migrated by setup) only when its installed manifest's `repository` is this repo. Grok Build has no plugin: it reads the shared `~/.agents/skills/basecamp` skill +`codex.go`). The plugin is `basecamp-cli@37signals`. Setup treats the pre-rename `basecamp@37signals` as +stale and replaces it by key, which is safe only while the marketplace lists no `basecamp` +plugin. Drop that key from `isStalePluginKey`/`CodexLegacyPluginKey` handling before the +marketplace re-adds `basecamp` for the hosted connector. Grok Build has no plugin: it reads the shared `~/.agents/skills/basecamp` skill directly, so it is a row of `harness.SkillAgent` (name, id, home env var, home directory, binary) in `skill_agent.go`, and everything in `internal/commands` that touches a shared-skill agent — the setup handler, the `BASECAMP_SETUP_AGENT` values, doctor's remediation — loops over diff --git a/README.md b/README.md index e78747efd..bbbead0af 100644 --- a/README.md +++ b/README.md @@ -300,24 +300,20 @@ To pick up a newer plugin version later, refresh the marketplace with **Plugin name:** in the 37signals marketplace this plugin is `basecamp-cli` (`basecamp-cli@37signals`). It was published as `basecamp` until that name -went to the hosted Basecamp connector plugin, which works through Basecamp's -own MCP server rather than this CLI. If you installed the plugin under the old -name, re-run `basecamp setup claude` or `basecamp setup codex`: setup recognizes -the old CLI install by its manifest, removes it, and installs `basecamp-cli` at -the same scope. Use setup rather than removing the old key by hand: -`basecamp@37signals` now also names the hosted connector, and Claude Code can -hold the old CLI plugin at one scope and the connector at another under that -one key. Setup checks each installed entry's manifest and uninstalls only the -CLI's scopes. A manual `claude plugin uninstall basecamp@37signals` or -`codex plugin remove basecamp@37signals` does no such check. To install the -renamed plugin by hand: +was set aside for the hosted Basecamp connector plugin, which works through +Basecamp's own MCP server rather than this CLI. If you installed the plugin +under the old name, re-run `basecamp setup claude` or `basecamp setup codex`: +setup installs `basecamp-cli` and removes `basecamp@37signals` at the scopes it +was installed in. To migrate by hand: ```bash claude plugin marketplace update 37signals claude plugin install basecamp-cli@37signals +claude plugin uninstall basecamp@37signals codex plugin marketplace upgrade 37signals codex plugin add basecamp-cli@37signals +codex plugin remove basecamp@37signals ``` Skills from the plugin are namespaced by its name, so `basecamp:basecamp` diff --git a/install.md b/install.md index dc3fb6313..067550cf3 100644 --- a/install.md +++ b/install.md @@ -141,7 +141,7 @@ basecamp setup claude This registers the marketplace and installs the `basecamp-cli@37signals` plugin with skills, hooks, and agent workflow support. -The plugin was named `basecamp` until that marketplace name went to the hosted Basecamp connector plugin. Re-running `basecamp setup claude` (or `basecamp setup codex`) replaces an old CLI install under `basecamp@37signals` with `basecamp-cli@37signals` at the same scope; it leaves a hosted-connector install under that name alone. +The plugin was named `basecamp` until that name was set aside for the hosted Basecamp connector plugin. Re-running `basecamp setup claude` (or `basecamp setup codex`) replaces an old install under `basecamp@37signals` with `basecamp-cli@37signals` at the same scope. The hooks call the CLI's `agent-hook` command, so they need a `basecamp` new enough to have it. If hook errors appear after installing or refreshing the diff --git a/internal/commands/wizard_codex.go b/internal/commands/wizard_codex.go index 97001816f..ab4899d53 100644 --- a/internal/commands/wizard_codex.go +++ b/internal/commands/wizard_codex.go @@ -123,9 +123,7 @@ func installCodexPlugin(parent context.Context, stderr io.Writer, progress func( if removeErr != nil { return &agentSetupError{ Summary: "removing the pre-rename plugin failed: " + codexCommandFailure(removeStdout, removeStderr, removeErr), - // Not `codex plugin remove`: the key is shared with the hosted - // connector, so removal must stay behind setup's manifest check. - Manual: []string{"basecamp setup codex"}, + Manual: []string{"codex plugin remove " + harness.CodexLegacyPluginKey}, } } } diff --git a/internal/commands/wizard_codex_test.go b/internal/commands/wizard_codex_test.go index b60eb4051..38bac91ec 100644 --- a/internal/commands/wizard_codex_test.go +++ b/internal/commands/wizard_codex_test.go @@ -107,8 +107,7 @@ func TestSetupCodexLegacyRemovalFailureNamesTheManualStep(t *testing.T) { require.NotEmpty(t, envelope.Data.Errors) assert.Contains(t, envelope.Data.Errors[0], "pre-rename plugin") - assert.Equal(t, []string{"basecamp setup codex"}, envelope.Data.ManualCommands, - "removal of the shared key stays behind setup's manifest check") + assert.Equal(t, []string{"codex plugin remove basecamp@37signals"}, envelope.Data.ManualCommands) } func TestSetupCodexLeavesNonLegacyInstallsAlone(t *testing.T) { @@ -286,15 +285,8 @@ func installCodexStub(t *testing.T, options codexStubOptions) string { // PATH holds only the stub, so it records state with shell builtins // (no rm): the legacy file reads "installed" until `plugin remove`. legacyPath := filepath.Join(home, "legacy") - // codexHome prefers $CODEX_HOME; pin it so an ambient value can't hide - // the fixture. - t.Setenv("CODEX_HOME", filepath.Join(home, ".codex")) if options.legacyInstalled { require.NoError(t, os.WriteFile(legacyPath, []byte("installed"), 0o644)) - manifestDir := filepath.Join(home, ".codex", "plugins", "cache", "37signals", "basecamp", "0.11.0", ".codex-plugin") - require.NoError(t, os.MkdirAll(manifestDir, 0o755)) - require.NoError(t, os.WriteFile(filepath.Join(manifestDir, "plugin.json"), - []byte(`{"name":"basecamp","repository":"https://github.com/basecamp/basecamp-cli"}`), 0o644)) } logPath := filepath.Join(home, "codex-calls.log") boolShell := func(value bool) string { diff --git a/internal/commands/wizard_test.go b/internal/commands/wizard_test.go index cb9e056c0..f9057c669 100644 --- a/internal/commands/wizard_test.go +++ b/internal/commands/wizard_test.go @@ -8,7 +8,6 @@ import ( "os" "path/filepath" "runtime" - "strconv" "strings" "testing" "time" @@ -947,22 +946,13 @@ func runClaudeSetupWithStub(t *testing.T, home string) string { return string(calls) } -// seedLegacyBasecampPlugin records a basecamp@37signals install at the user -// and project scopes whose cached manifest names repo. -func seedLegacyBasecampPlugin(t *testing.T, home, repo string) { +// seedLegacyBasecampPlugin records the pre-rename basecamp@37signals plugin +// at the user and project scopes. +func seedLegacyBasecampPlugin(t *testing.T, home string) { t.Helper() - installPath := filepath.Join(home, ".claude", "plugins", "cache", "37signals", "basecamp", "0.11.0") - require.NoError(t, os.MkdirAll(filepath.Join(installPath, ".claude-plugin"), 0o755)) - require.NoError(t, os.WriteFile(filepath.Join(installPath, ".claude-plugin", "plugin.json"), - []byte(`{"name":"basecamp","repository":"`+repo+`"}`), 0o644)) - cwd, err := os.Getwd() - require.NoError(t, err) - entry := func(scope string) string { - return `{"scope":"` + scope + `","version":"0.11.0","installPath":` + strconv.Quote(installPath) + - `,"projectPath":` + strconv.Quote(cwd) + `}` - } + require.NoError(t, os.MkdirAll(filepath.Join(home, ".claude", "plugins"), 0o755)) require.NoError(t, os.WriteFile(filepath.Join(home, ".claude", "plugins", "installed_plugins.json"), - []byte(`{"version":2,"plugins":{"basecamp@37signals":[`+entry("user")+`,`+entry("project")+`]}}`), 0o644)) + []byte(`{"version":2,"plugins":{"basecamp@37signals":[{"scope":"user","version":"0.11.0"},{"scope":"project","version":"0.11.0","projectPath":"/somewhere"}]}}`), 0o644)) } // TestSetupClaudeMigratesLegacyCLIPlugin verifies that the CLI plugin still @@ -972,7 +962,7 @@ func TestSetupClaudeMigratesLegacyCLIPlugin(t *testing.T) { t.Setenv("BASECAMP_NO_KEYRING", "1") home := t.TempDir() t.Setenv("HOME", home) - seedLegacyBasecampPlugin(t, home, "https://github.com/basecamp/basecamp-cli") + seedLegacyBasecampPlugin(t, home) calls := runClaudeSetupWithStub(t, home) @@ -984,20 +974,6 @@ func TestSetupClaudeMigratesLegacyCLIPlugin(t *testing.T) { assert.Contains(t, calls, "plugin install basecamp-cli@37signals --scope project") } -// TestSetupClaudeLeavesHostedConnectorPluginAlone verifies that a -// basecamp@37signals install of the hosted connector is never uninstalled. -func TestSetupClaudeLeavesHostedConnectorPluginAlone(t *testing.T) { - t.Setenv("BASECAMP_NO_KEYRING", "1") - home := t.TempDir() - t.Setenv("HOME", home) - seedLegacyBasecampPlugin(t, home, "https://github.com/basecamp/basecamp-mcp-server") - - calls := runClaudeSetupWithStub(t, home) - - assert.NotContains(t, calls, "plugin uninstall") - assert.Contains(t, calls, "plugin install basecamp-cli@37signals") -} - // TestSetupClaudeNonInteractiveRefreshesMarketplace verifies the fresh-install // path refreshes the marketplace cache before `plugin install`, so a stale // `source: github` entry (which would clone over SSH) is replaced with the diff --git a/internal/harness/claude.go b/internal/harness/claude.go index 3133f8645..7893e0183 100644 --- a/internal/harness/claude.go +++ b/internal/harness/claude.go @@ -7,7 +7,6 @@ import ( "os" "os/exec" "path/filepath" - "strings" "github.com/basecamp/basecamp-cli/internal/version" ) @@ -53,16 +52,15 @@ const ClaudeMarketplaceName = "37signals" const ClaudeExpectedPluginKey = ClaudePluginName + "@" + ClaudeMarketplaceName // ClaudeLegacyPluginKey is the key this plugin was installed under before the -// rename. The 37signals marketplace now publishes the hosted Basecamp -// connector plugin under that same key, so an entry with it is this CLI's -// plugin only when its installed manifest says so (see legacyCLIInstall). +// rename. During the migration window the 37signals marketplace publishes no +// "basecamp" plugin at all, so an install under this key is always this CLI's +// pre-rename plugin and setup removes it by key, like any stale entry. +// +// The marketplace may later list the hosted Basecamp connector as "basecamp" +// again. That step has to wait until this key is no longer treated as stale +// here (isStalePluginKey); otherwise setup would uninstall the connector. const ClaudeLegacyPluginKey = "basecamp@" + ClaudeMarketplaceName -// CLIRepository is the repository both plugin manifests in this repo name. -// An installed plugin whose manifest names it is this CLI's plugin, whatever -// key it was installed under. -const CLIRepository = "https://github.com/basecamp/basecamp-cli" - // DetectClaude returns true if Claude Code is installed. // Checks ~/.claude/ directory first, then falls back to binary on PATH. func DetectClaude() bool { @@ -136,7 +134,7 @@ func CheckClaudePlugin() *StatusCheck { // Try as array of objects with "name" or "package" fields, // or as a map with plugin keys. if pluginInstalled(data) { - if len(legacyCLIPluginEntries(data)) > 0 { + if hasLegacyPluginKey(data) { return &StatusCheck{ Name: "Claude Code Plugin", Status: "warn", @@ -151,7 +149,7 @@ func CheckClaudePlugin() *StatusCheck { } } - if len(legacyCLIPluginEntries(data)) > 0 { + if hasLegacyPluginKey(data) { return &StatusCheck{ Name: "Claude Code Plugin", Status: "fail", @@ -402,8 +400,8 @@ type StalePlugin struct { } // StalePluginKeys returns stale plugin entries from installed_plugins.json: -// entries from old/dead marketplaces, and this CLI's plugin still installed -// under its pre-rename key. Setup uninstalls each and reinstalls +// entries from old/dead marketplaces, and this plugin still installed under +// its pre-rename key. Setup uninstalls each and reinstalls // ClaudeExpectedPluginKey at the scopes it removed. func StalePluginKeys() []StalePlugin { home, err := os.UserHomeDir() @@ -414,7 +412,7 @@ func StalePluginKeys() []StalePlugin { if err != nil { return nil } - return append(stalePluginKeys(data), legacyCLIPluginEntries(data)...) + return stalePluginKeys(data) } // stalePluginKeys extracts known stale plugin entries (e.g. basecamp@basecamp from @@ -495,9 +493,21 @@ func stalePluginKeys(data []byte) []StalePlugin { // claudeStalePluginKey is the known stale key from the basecamp → 37signals marketplace rename. const claudeStalePluginKey = "basecamp@basecamp" -// isStalePluginKey returns true only for the known stale marketplace key. +// isStalePluginKey returns true for the known stale keys: the old marketplace +// name, and this plugin's pre-rename key (see ClaudeLegacyPluginKey). func isStalePluginKey(key string) bool { - return key == claudeStalePluginKey + return key == claudeStalePluginKey || key == ClaudeLegacyPluginKey +} + +// hasLegacyPluginKey reports whether this plugin is still installed under +// ClaudeLegacyPluginKey. +func hasLegacyPluginKey(data []byte) bool { + for _, p := range stalePluginKeys(data) { + if p.Key == ClaudeLegacyPluginKey { + return true + } + } + return false } func appendUnique(ss []string, s string) []string { @@ -508,109 +518,3 @@ func appendUnique(ss []string, s string) []string { } return append(ss, s) } - -// legacyCLIPluginEntries returns this CLI's plugin when it is still installed -// under ClaudeLegacyPluginKey. That key now also names the hosted-connector -// plugin, so the key alone proves nothing: only the scopes whose installed -// manifest names CLIRepository count. An entry whose manifest can't be read — -// or a file format that records no install path — is left alone, since -// removing a hosted-connector install would be worse than leaving a legacy -// CLI one in place. So is an entry without a scope `claude plugin uninstall -// --scope` accepts: setup falls back to an unscoped uninstall for those, and -// for this shared key that would remove a hosted connector at every scope. -// A project or local entry counts only when its projectPath is the current -// directory: Claude resolves those scopes against the working directory, so -// an uninstall run elsewhere would target another checkout's entry — which -// may be the hosted connector. -func legacyCLIPluginEntries(data []byte) []StalePlugin { - var envelope struct { - Plugins map[string][]struct { - Scope string `json:"scope"` - InstallPath string `json:"installPath"` - ProjectPath string `json:"projectPath"` - } `json:"plugins"` - } - if err := json.Unmarshal(data, &envelope); err != nil { - return nil - } - entries, ok := envelope.Plugins[ClaudeLegacyPluginKey] - if !ok { - return nil - } - found := false - var scopes []string - for _, entry := range entries { - if !targetableScope(entry.Scope) || !scopeResolvesHere(entry.Scope, entry.ProjectPath) || entry.InstallPath == "" || !isCLIPluginManifest(filepath.Join(entry.InstallPath, ".claude-plugin", "plugin.json")) { - continue - } - found = true - scopes = appendUnique(scopes, entry.Scope) - } - if !found { - return nil - } - return []StalePlugin{{Key: ClaudeLegacyPluginKey, Scopes: scopes}} -} - -// targetableScope reports whether scope is one `claude plugin uninstall -// --scope` accepts (the same set setup's validPluginScope allows). -func targetableScope(scope string) bool { - return scope == "user" || scope == "project" || scope == "local" -} - -// scopeResolvesHere reports whether `claude plugin uninstall --scope scope`, -// run from the current directory, reaches the entry recorded for projectPath. -// User scope is global; project and local scopes belong to one directory. -func scopeResolvesHere(scope, projectPath string) bool { - if scope == "user" { - return true - } - if projectPath == "" { - return false - } - cwd, err := os.Getwd() - if err != nil { - return false - } - return samePath(cwd, projectPath) -} - -func samePath(a, b string) bool { - resolve := func(p string) string { - if r, err := filepath.EvalSymlinks(p); err == nil { - return r - } - return filepath.Clean(p) - } - return resolve(a) == resolve(b) -} - -// isCLIPluginManifest reports whether the plugin manifest at path is this -// CLI's: its repository is CLIRepository. -func isCLIPluginManifest(path string) bool { - data, err := os.ReadFile(path) //nolint:gosec // G304: path from the agent's own plugin records - if err != nil { - return false - } - var manifest struct { - Repository any `json:"repository"` - } - if err := json.Unmarshal(data, &manifest); err != nil { - return false - } - var repo string - switch r := manifest.Repository.(type) { - case string: - repo = r - case map[string]any: - repo, _ = r["url"].(string) - } - return normalizeRepositoryURL(repo) == CLIRepository -} - -func normalizeRepositoryURL(url string) string { - url = strings.TrimSpace(strings.ToLower(url)) - url = strings.TrimPrefix(url, "git+") - url = strings.TrimSuffix(url, "/") - return strings.TrimSuffix(url, ".git") -} diff --git a/internal/harness/claude_test.go b/internal/harness/claude_test.go index 868634b79..189013f22 100644 --- a/internal/harness/claude_test.go +++ b/internal/harness/claude_test.go @@ -3,8 +3,6 @@ package harness import ( "os" "path/filepath" - "strconv" - "strings" "testing" "github.com/stretchr/testify/assert" @@ -338,140 +336,42 @@ func TestCheckClaudePluginVersion_NoFile(t *testing.T) { assert.Contains(t, check.Message, "not tracked") } -// writePluginManifest writes a .claude-plugin/plugin.json under dir naming repo. -func writePluginManifest(t *testing.T, dir, manifestDir, repo string) { - t.Helper() - require.NoError(t, os.MkdirAll(filepath.Join(dir, manifestDir), 0o755)) - require.NoError(t, os.WriteFile(filepath.Join(dir, manifestDir, "plugin.json"), - []byte(`{"name":"basecamp","repository":"`+repo+`"}`), 0o644)) -} - -func legacyInstalledPlugins(entries ...string) []byte { - return []byte(`{"version":2,"plugins":{"basecamp@37signals":[` + strings.Join(entries, ",") + `]}}`) -} - -// legacyEntry records an install at scope; project and local entries belong -// to the current directory. -func legacyEntry(scope, installPath string) string { - cwd, _ := os.Getwd() - return legacyEntryIn(scope, installPath, cwd) -} - -func legacyEntryIn(scope, installPath, projectPath string) string { - entry := `{"scope":"` + scope + `","version":"0.11.0","installPath":` + strconv.Quote(installPath) - if scope != "user" { - entry += `,"projectPath":` + strconv.Quote(projectPath) - } - return entry + `}` -} - -func TestLegacyCLIPluginEntries_CLIManifest(t *testing.T) { - dir := t.TempDir() - writePluginManifest(t, dir, ".claude-plugin", "https://github.com/basecamp/basecamp-cli") - - got := legacyCLIPluginEntries(legacyInstalledPlugins(legacyEntry("user", dir), legacyEntry("project", dir))) - assert.Equal(t, []StalePlugin{{Key: "basecamp@37signals", Scopes: []string{"user", "project"}}}, got) -} - -func TestLegacyCLIPluginEntries_RepositoryVariants(t *testing.T) { - for _, repo := range []string{ - "https://github.com/basecamp/basecamp-cli.git", - "https://github.com/basecamp/basecamp-cli/", - "git+https://github.com/Basecamp/basecamp-cli.git", - } { - dir := t.TempDir() - writePluginManifest(t, dir, ".claude-plugin", repo) - assert.Len(t, legacyCLIPluginEntries(legacyInstalledPlugins(legacyEntry("user", dir))), 1, repo) - } -} - -// The hosted-connector plugin now owns basecamp@37signals: it must never be -// mistaken for a stale CLI install, or setup would uninstall it. -func TestLegacyCLIPluginEntries_HostedConnectorIsLeftAlone(t *testing.T) { - dir := t.TempDir() - writePluginManifest(t, dir, ".claude-plugin", "https://github.com/basecamp/basecamp-mcp-server") - assert.Empty(t, legacyCLIPluginEntries(legacyInstalledPlugins(legacyEntry("user", dir)))) -} - -func TestLegacyCLIPluginEntries_OnlyCLIScopesCount(t *testing.T) { - cli, hosted := t.TempDir(), t.TempDir() - writePluginManifest(t, cli, ".claude-plugin", "https://github.com/basecamp/basecamp-cli") - writePluginManifest(t, hosted, ".claude-plugin", "https://github.com/basecamp/basecamp-mcp-server") - - got := legacyCLIPluginEntries(legacyInstalledPlugins(legacyEntry("user", hosted), legacyEntry("project", cli))) - assert.Equal(t, []StalePlugin{{Key: "basecamp@37signals", Scopes: []string{"project"}}}, got) -} - -// An entry without a targetable scope would reach setup's unscoped uninstall, -// which removes the shared key at every scope — hosted connector included. -func TestLegacyCLIPluginEntries_UntargetableScopeIsLeftAlone(t *testing.T) { - dir := t.TempDir() - writePluginManifest(t, dir, ".claude-plugin", "https://github.com/basecamp/basecamp-cli") - assert.Empty(t, legacyCLIPluginEntries(legacyInstalledPlugins(legacyEntry("", dir)))) - assert.Empty(t, legacyCLIPluginEntries(legacyInstalledPlugins(legacyEntry("global", dir)))) - got := legacyCLIPluginEntries(legacyInstalledPlugins(legacyEntry("global", dir), legacyEntry("local", dir))) - assert.Equal(t, []StalePlugin{{Key: "basecamp@37signals", Scopes: []string{"local"}}}, got) +func TestIsStalePluginKey_LegacyName(t *testing.T) { + assert.True(t, isStalePluginKey("basecamp@37signals")) + assert.False(t, isStalePluginKey("basecamp-cli@37signals")) } -// Claude resolves project and local scopes against the working directory, so -// an entry recorded for another checkout can't be targeted from here: an -// uninstall would reach this directory's entry, which may be the connector. -func TestLegacyCLIPluginEntries_OtherCheckoutIsLeftAlone(t *testing.T) { - dir := t.TempDir() - writePluginManifest(t, dir, ".claude-plugin", "https://github.com/basecamp/basecamp-cli") - other := t.TempDir() - assert.Empty(t, legacyCLIPluginEntries(legacyInstalledPlugins(legacyEntryIn("project", dir, other)))) - assert.Empty(t, legacyCLIPluginEntries(legacyInstalledPlugins(legacyEntryIn("local", dir, "")))) - - t.Chdir(other) - got := legacyCLIPluginEntries(legacyInstalledPlugins(legacyEntryIn("project", dir, other))) - assert.Equal(t, []StalePlugin{{Key: "basecamp@37signals", Scopes: []string{"project"}}}, got) +// The pre-rename key is removed by key at every scope it was installed in, +// whatever the file format. +func TestStalePluginKeys_LegacyName(t *testing.T) { + data := []byte(`{"version":2,"plugins":{"basecamp@37signals":[{"scope":"user"},{"scope":"project","projectPath":"/x"}],"basecamp-cli@37signals":[{"scope":"user"}]}}`) + assert.Equal(t, []StalePlugin{{Key: "basecamp@37signals", Scopes: []string{"user", "project"}}}, stalePluginKeys(data)) + assert.Equal(t, []StalePlugin{{Key: "basecamp@37signals"}}, stalePluginKeys([]byte(`{"basecamp@37signals":{"version":"0.11.0"}}`))) } -func TestCheckClaudePlugin_BothInstalledWarns(t *testing.T) { - home := t.TempDir() - t.Setenv("HOME", home) - installPath := filepath.Join(home, ".claude", "plugins", "cache", "37signals", "basecamp", "0.11.0") - writePluginManifest(t, installPath, ".claude-plugin", "https://github.com/basecamp/basecamp-cli") - require.NoError(t, os.WriteFile(filepath.Join(home, ".claude", "plugins", "installed_plugins.json"), - []byte(`{"version":2,"plugins":{"basecamp-cli@37signals":[{"scope":"user","version":"0.12.0"}],"basecamp@37signals":[`+legacyEntry("user", installPath)+`]}}`), 0o644)) - - check := CheckClaudePlugin() - assert.Equal(t, "warn", check.Status) - assert.Contains(t, check.Message, "old basecamp@37signals copy") -} - -func TestLegacyCLIPluginEntries_UnreadableManifestIsLeftAlone(t *testing.T) { - assert.Empty(t, legacyCLIPluginEntries(legacyInstalledPlugins(legacyEntry("user", filepath.Join(t.TempDir(), "gone"))))) - assert.Empty(t, legacyCLIPluginEntries([]byte(`{"version":2,"plugins":{"basecamp@37signals":[{"scope":"user"}]}}`))) - assert.Empty(t, legacyCLIPluginEntries([]byte(`{"basecamp@37signals":{"version":"0.11.0"}}`))) +func writeInstalledPlugins(t *testing.T, home, data string) { + t.Helper() + require.NoError(t, os.MkdirAll(filepath.Join(home, ".claude", "plugins"), 0o755)) + require.NoError(t, os.WriteFile(filepath.Join(home, ".claude", "plugins", "installed_plugins.json"), []byte(data), 0o644)) } -func TestCheckClaudePlugin_LegacyCLIInstall(t *testing.T) { +func TestCheckClaudePlugin_LegacyNameOnly(t *testing.T) { home := t.TempDir() t.Setenv("HOME", home) - installPath := filepath.Join(home, ".claude", "plugins", "cache", "37signals", "basecamp", "0.11.0") - writePluginManifest(t, installPath, ".claude-plugin", "https://github.com/basecamp/basecamp-cli") - require.NoError(t, os.WriteFile(filepath.Join(home, ".claude", "plugins", "installed_plugins.json"), - legacyInstalledPlugins(legacyEntry("user", installPath)), 0o644)) + writeInstalledPlugins(t, home, `{"version":2,"plugins":{"basecamp@37signals":[{"scope":"user","version":"0.11.0"}]}}`) check := CheckClaudePlugin() assert.Equal(t, "fail", check.Status) assert.Contains(t, check.Message, "old name basecamp@37signals") assert.Contains(t, check.Hint, "basecamp setup claude") - assert.Equal(t, []StalePlugin{{Key: "basecamp@37signals", Scopes: []string{"user"}}}, StalePluginKeys()) } -func TestCheckClaudePlugin_HostedConnectorAloneIsNotInstalled(t *testing.T) { +func TestCheckClaudePlugin_BothInstalledWarns(t *testing.T) { home := t.TempDir() t.Setenv("HOME", home) - installPath := filepath.Join(home, ".claude", "plugins", "cache", "37signals", "basecamp", "0.1.0") - writePluginManifest(t, installPath, ".claude-plugin", "https://github.com/basecamp/basecamp-mcp-server") - require.NoError(t, os.WriteFile(filepath.Join(home, ".claude", "plugins", "installed_plugins.json"), - legacyInstalledPlugins(legacyEntry("user", installPath)), 0o644)) + writeInstalledPlugins(t, home, `{"version":2,"plugins":{"basecamp-cli@37signals":[{"scope":"user","version":"0.12.0"}],"basecamp@37signals":[{"scope":"user","version":"0.11.0"}]}}`) check := CheckClaudePlugin() - assert.Equal(t, "fail", check.Status) - assert.Equal(t, "Plugin not installed", check.Message) - assert.Empty(t, StalePluginKeys()) + assert.Equal(t, "warn", check.Status) + assert.Contains(t, check.Message, "old basecamp@37signals copy") } diff --git a/internal/harness/codex.go b/internal/harness/codex.go index 47f4f0047..bad2c22f0 100644 --- a/internal/harness/codex.go +++ b/internal/harness/codex.go @@ -9,7 +9,6 @@ import ( "os" "os/exec" "path/filepath" - "strings" "time" "github.com/basecamp/basecamp-cli/internal/version" @@ -26,9 +25,9 @@ const ( CodexMarketplaceName = "37signals" // CodexExpectedPluginKey is the fully qualified Basecamp plugin ID. CodexExpectedPluginKey = CodexPluginName + "@" + CodexMarketplaceName - // CodexLegacyPluginKey is the pre-rename plugin ID. It now also names - // the hosted-connector plugin, so an install under it is this CLI's only - // when its cached manifest says so (see codexLegacyCLIInstalled). + // CodexLegacyPluginKey is the pre-rename plugin ID. The marketplace lists + // no "basecamp" plugin during the migration window, so an install under it + // is always this CLI's (see ClaudeLegacyPluginKey). CodexLegacyPluginKey = "basecamp@" + CodexMarketplaceName // codexQueryTimeout bounds how long the Codex probe may run. @@ -304,7 +303,7 @@ func queryCodexPlugin(parent context.Context) (codexPluginState, bool, error) { legacy := false if envelope.Installed != nil { for _, plugin := range *envelope.Installed { - if plugin.PluginID == CodexLegacyPluginKey && plugin.Installed && codexLegacyCLIInstalled(plugin.Version) { + if plugin.PluginID == CodexLegacyPluginKey && plugin.Installed { legacy = true } } @@ -334,37 +333,6 @@ func CodexLegacyCLIInstalled(ctx context.Context) bool { return err == nil && state.legacyInstalled } -// codexLegacyCLIInstalled reports whether the cached copy of the -// CodexLegacyPluginKey install at version is this CLI's plugin. Codex's -// plugin list reports the marketplace's current source for an ID, not the -// source the installed copy came from — after the rename that is the hosted -// connector's — so the installed manifest is the only reliable witness. A -// manifest that can't be read is not counted: leaving a legacy CLI install -// in place beats removing a hosted-connector one. -func codexLegacyCLIInstalled(version string) bool { - if version == "" || strings.ContainsAny(version, `/\`) || version == "." || version == ".." { - return false - } - home := codexHome() - if home == "" { - return false - } - name := strings.TrimSuffix(CodexLegacyPluginKey, "@"+CodexMarketplaceName) - return isCLIPluginManifest(filepath.Join(home, "plugins", "cache", CodexMarketplaceName, name, version, ".codex-plugin", "plugin.json")) -} - -// codexHome is $CODEX_HOME, or ~/.codex. -func codexHome() string { - if home := os.Getenv("CODEX_HOME"); home != "" { - return filepath.Clean(home) - } - home, err := os.UserHomeDir() - if err != nil || home == "" { - return "" - } - return filepath.Join(filepath.Clean(home), ".codex") -} - func codexQueryFailure(name string, err error) *StatusCheck { if errors.Is(err, errCodexBinaryMissing) { return &StatusCheck{ diff --git a/internal/harness/codex_test.go b/internal/harness/codex_test.go index 1ea1f5856..43aaf9b3f 100644 --- a/internal/harness/codex_test.go +++ b/internal/harness/codex_test.go @@ -204,17 +204,7 @@ func boolJSON(value bool) string { const codexLegacyInstalledJSON = `{"pluginId":"basecamp@37signals","name":"basecamp","marketplaceName":"37signals","version":"0.11.0","installed":true,"enabled":true}` -// writeCodexCachedManifest puts a cached basecamp@37signals 0.11.0 manifest -// naming repo under a fresh CODEX_HOME. -func writeCodexCachedManifest(t *testing.T, repo string) { - t.Helper() - codexHome := t.TempDir() - t.Setenv("CODEX_HOME", codexHome) - writePluginManifest(t, filepath.Join(codexHome, "plugins", "cache", "37signals", "basecamp", "0.11.0"), ".codex-plugin", repo) -} - func TestCheckCodexPluginLegacyCLIInstall(t *testing.T) { - writeCodexCachedManifest(t, "https://github.com/basecamp/basecamp-cli") stubCodexList(t, `{"installed":[`+codexLegacyInstalledJSON+`],"available":[{"pluginId":"basecamp-cli@37signals","version":"0.11.0","installed":false,"enabled":false}]}`, nil) check := CheckCodexPlugin() @@ -225,18 +215,14 @@ func TestCheckCodexPluginLegacyCLIInstall(t *testing.T) { assert.True(t, CodexLegacyCLIInstalled(context.Background())) } -func TestCheckCodexPluginHostedConnectorIsNotLegacy(t *testing.T) { - writeCodexCachedManifest(t, "https://github.com/basecamp/basecamp-mcp-server") - stubCodexList(t, `{"installed":[`+codexLegacyInstalledJSON+`],"available":[]}`, nil) - - check := CheckCodexPlugin() +func TestCheckCodexPluginLegacyAvailableOnlyIsNotInstalled(t *testing.T) { + stubCodexList(t, `{"installed":[],"available":[{"pluginId":"basecamp@37signals","version":"0.11.0","installed":false,"enabled":false}]}`, nil) - assert.Equal(t, "Plugin not installed", check.Message) + assert.Equal(t, "Plugin not installed", CheckCodexPlugin().Message) assert.False(t, CodexLegacyCLIInstalled(context.Background())) } func TestCheckCodexPluginBothInstalledWarns(t *testing.T) { - writeCodexCachedManifest(t, "https://github.com/basecamp/basecamp-cli") stubCodexList(t, `{"installed":[`+codexLegacyInstalledJSON+`,{"pluginId":"basecamp-cli@37signals","version":"0.11.0","installed":true,"enabled":true}],"available":[]}`, nil) check := CheckCodexPlugin() @@ -245,16 +231,7 @@ func TestCheckCodexPluginBothInstalledWarns(t *testing.T) { assert.Contains(t, check.Message, "old basecamp@37signals copy") } -func TestCodexLegacyCLIInstalledRejectsPathVersions(t *testing.T) { - writeCodexCachedManifest(t, "https://github.com/basecamp/basecamp-cli") - for _, v := range []string{"", ".", "..", "../0.11.0", `..\0.11.0`} { - assert.False(t, codexLegacyCLIInstalled(v), v) - } - assert.True(t, codexLegacyCLIInstalled("0.11.0")) -} - func TestCheckCodexPluginDisabledMentionsLegacyCopy(t *testing.T) { - writeCodexCachedManifest(t, "https://github.com/basecamp/basecamp-cli") stubCodexList(t, `{"installed":[`+codexLegacyInstalledJSON+`,{"pluginId":"basecamp-cli@37signals","version":"0.11.0","installed":true,"enabled":false}],"available":[]}`, nil) check := CheckCodexPlugin() diff --git a/internal/release/manifests_test.go b/internal/release/manifests_test.go index 1d9290dc6..3e2bd5184 100644 --- a/internal/release/manifests_test.go +++ b/internal/release/manifests_test.go @@ -41,11 +41,10 @@ type codexInterface struct { } type pluginManifest struct { - Name string `json:"name"` - Version string `json:"version"` - Repository string `json:"repository"` - Skills string `json:"skills"` - Interface *codexInterface `json:"interface"` + Name string `json:"name"` + Version string `json:"version"` + Skills string `json:"skills"` + Interface *codexInterface `json:"interface"` } func TestManifestsParseWithMatchingIdentity(t *testing.T) { @@ -55,10 +54,6 @@ func TestManifestsParseWithMatchingIdentity(t *testing.T) { assert.Equal(t, harness.ClaudePluginName, claude.Name) assert.Equal(t, harness.CodexPluginName, codex.Name) - // Setup tells a pre-rename install of this plugin from the hosted - // connector that now owns the old name by this field alone. - assert.Equal(t, harness.CLIRepository, claude.Repository) - assert.Equal(t, harness.CLIRepository, codex.Repository) assert.Regexp(t, semverPattern, claude.Version) assert.Regexp(t, semverPattern, codex.Version) assert.Equal(t, claude.Version, codex.Version, "manifest versions must stay in lockstep") From e53d5f77b67e76cdf95ec3b9e6eb76f3f702d2fc Mon Sep 17 00:00:00 2001 From: Jeremy Daer Date: Mon, 5 Oct 2026 16:11:49 -0700 Subject: [PATCH 08/13] Seed the project-scope legacy fixture in the directory setup runs from --- internal/commands/wizard_test.go | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/internal/commands/wizard_test.go b/internal/commands/wizard_test.go index f9057c669..659f63c1d 100644 --- a/internal/commands/wizard_test.go +++ b/internal/commands/wizard_test.go @@ -8,6 +8,7 @@ import ( "os" "path/filepath" "runtime" + "strconv" "strings" "testing" "time" @@ -947,12 +948,15 @@ func runClaudeSetupWithStub(t *testing.T, home string) string { } // seedLegacyBasecampPlugin records the pre-rename basecamp@37signals plugin -// at the user and project scopes. +// at user scope and at project scope for the directory setup runs from, which +// is the project a scoped uninstall reaches. func seedLegacyBasecampPlugin(t *testing.T, home string) { t.Helper() + cwd, err := os.Getwd() + require.NoError(t, err) require.NoError(t, os.MkdirAll(filepath.Join(home, ".claude", "plugins"), 0o755)) require.NoError(t, os.WriteFile(filepath.Join(home, ".claude", "plugins", "installed_plugins.json"), - []byte(`{"version":2,"plugins":{"basecamp@37signals":[{"scope":"user","version":"0.11.0"},{"scope":"project","version":"0.11.0","projectPath":"/somewhere"}]}}`), 0o644)) + []byte(`{"version":2,"plugins":{"basecamp@37signals":[{"scope":"user","version":"0.11.0"},{"scope":"project","version":"0.11.0","projectPath":`+strconv.Quote(cwd)+`}]}}`), 0o644)) } // TestSetupClaudeMigratesLegacyCLIPlugin verifies that the CLI plugin still From 97c0896bbac87948723fa18877fb2ecb199dc3b6 Mon Sep 17 00:00:00 2001 From: Jeremy Daer Date: Mon, 5 Oct 2026 16:53:16 -0700 Subject: [PATCH 09/13] Tell installs under the pre-rename plugin id to switch to basecamp-cli, once During the deprecation window the marketplace keeps basecamp as an alias of basecamp-cli with the same source, so existing installs keep working. A SessionStart hook, agent-hook plugin-notice, recognizes an install under the old id by its plugin root (.../37signals/basecamp/), since the manifest is the same for both, and says once that the plugin is now basecamp-cli and which setup command switches it. Any other install stays silent. --- hooks/hooks.json | 13 ++- internal/commands/agent_hook.go | 2 +- internal/commands/agent_hook_rename.go | 117 +++++++++++++++++++++++++ internal/commands/agent_hook_test.go | 63 +++++++++++++ internal/harness/claude.go | 5 +- internal/release/manifests_test.go | 9 +- 6 files changed, 205 insertions(+), 4 deletions(-) create mode 100644 internal/commands/agent_hook_rename.go diff --git a/hooks/hooks.json b/hooks/hooks.json index 91e64a90b..3714fcc5f 100644 --- a/hooks/hooks.json +++ b/hooks/hooks.json @@ -1,6 +1,17 @@ { - "description": "Basecamp agent hooks: commit-reference nudges.", + "description": "Basecamp agent hooks: commit-reference nudges, and a one-time notice for installs under the plugin's pre-rename id.", "hooks": { + "SessionStart": [ + { + "hooks": [ + { + "type": "command", + "command": "basecamp agent-hook plugin-notice", + "timeout": 5 + } + ] + } + ], "PreToolUse": [ { "matcher": "Bash", diff --git a/internal/commands/agent_hook.go b/internal/commands/agent_hook.go index db6b9782d..59f2596b0 100644 --- a/internal/commands/agent_hook.go +++ b/internal/commands/agent_hook.go @@ -72,7 +72,7 @@ func NewAgentHookCmd() *cobra.Command { return nil }, } - cmd.AddCommand(newAgentHookSessionStartCmd(), newAgentHookPreCommitSnapshotCmd(), newAgentHookPostCommitCmd()) + cmd.AddCommand(newAgentHookSessionStartCmd(), newAgentHookPluginNoticeCmd(), newAgentHookPreCommitSnapshotCmd(), newAgentHookPostCommitCmd()) return cmd } diff --git a/internal/commands/agent_hook_rename.go b/internal/commands/agent_hook_rename.go new file mode 100644 index 000000000..8f1b3ccea --- /dev/null +++ b/internal/commands/agent_hook_rename.go @@ -0,0 +1,117 @@ +package commands + +import ( + "encoding/json" + "os" + "path/filepath" + + "github.com/spf13/cobra" + + "github.com/basecamp/basecamp-cli/internal/harness" +) + +// pluginRenameNoticeMarker records that the rename notice was shown for one +// plugin install, so it appears once rather than every session. +const pluginRenameNoticeMarker = "plugin-rename-notice-shown" + +// newAgentHookPluginNoticeCmd tells someone still running this plugin under +// its pre-rename id that it is now basecamp-cli. During the deprecation window +// the marketplace keeps "basecamp" as an alias with the same source, so the +// plugin keeps working; this nudges toward `basecamp setup`, once. +// +// The alias and basecamp-cli install the same files, so the manifest can't +// tell them apart. The plugin root can: agents cache each install under +// //. Anything other than an install as +// basecamp@37signals stays silent, so the hook injects nothing into normal +// sessions. +func newAgentHookPluginNoticeCmd() *cobra.Command { + return &cobra.Command{ + Use: "plugin-notice", + Short: "Tell a pre-rename plugin install to switch to basecamp-cli, once", + Args: cobra.NoArgs, + RunE: func(cmd *cobra.Command, args []string) error { + root := agentPluginRoot() + if !isLegacyPluginRoot(root) { + return nil + } + dir := agentPluginDataDir() + if dir == "" { + dir = agentHookStateDir(cmd.Context()) + } + if dir == "" || !claimOnce(filepath.Join(dir, pluginRenameNoticeMarker)) { + return nil + } + message := "The Basecamp plugin is now `" + harness.ClaudePluginName + "`. " + + "This copy still runs under its old name `" + harness.ClaudeLegacyPluginKey + "`, which keeps working for now. " + + "Run `" + legacyPluginSetupCommand(root) + "` to switch." + _ = json.NewEncoder(cmd.OutOrStdout()).Encode(map[string]any{ + "systemMessage": message, + "hookSpecificOutput": map[string]string{ + "hookEventName": "SessionStart", + "additionalContext": "Mention this to the user once: " + message, + }, + }) + return nil + }, + } +} + +// agentPluginRoot is the plugin's install directory as the agent reports it: +// Claude Code sets CLAUDE_PLUGIN_ROOT, Codex sets PLUGIN_ROOT (and the +// Claude name too). +func agentPluginRoot() string { + if root := os.Getenv("CLAUDE_PLUGIN_ROOT"); root != "" { + return root + } + return os.Getenv("PLUGIN_ROOT") +} + +func agentPluginDataDir() string { + if dir := os.Getenv("CLAUDE_PLUGIN_DATA"); dir != "" { + return dir + } + return os.Getenv("PLUGIN_DATA") +} + +// isLegacyPluginRoot reports whether root is a cached install of +// basecamp@37signals: .../37signals/basecamp/. +func isLegacyPluginRoot(root string) bool { + if root == "" { + return false + } + plugin := filepath.Dir(filepath.Clean(root)) + return filepath.Base(plugin) == harness.LegacyPluginName && filepath.Base(filepath.Dir(plugin)) == harness.ClaudeMarketplaceName +} + +// legacyPluginSetupCommand names the setup command for the agent that owns +// root, falling back to setting up every detected agent. +func legacyPluginSetupCommand(root string) string { + if home, err := os.UserHomeDir(); err == nil { + for agent, dir := range map[string]string{"claude": ".claude", "codex": ".codex"} { + if rel, err := filepath.Rel(filepath.Join(home, dir), root); err == nil && filepath.IsLocal(rel) { + return "basecamp setup " + agent + } + } + } + if codexHome := os.Getenv("CODEX_HOME"); codexHome != "" { + if rel, err := filepath.Rel(codexHome, root); err == nil && filepath.IsLocal(rel) { + return "basecamp setup codex" + } + } + return "basecamp setup agents" +} + +// claimOnce creates marker, reporting whether this call created it. Any +// failure counts as already shown: a notice that can't be recorded would +// otherwise repeat every session. +func claimOnce(marker string) bool { + if err := os.MkdirAll(filepath.Dir(marker), 0o700); err != nil { + return false + } + f, err := os.OpenFile(marker, os.O_CREATE|os.O_EXCL|os.O_WRONLY, 0o600) //nolint:gosec // G304: marker under the agent's plugin data dir + if err != nil { + return false + } + _ = f.Close() + return true +} diff --git a/internal/commands/agent_hook_test.go b/internal/commands/agent_hook_test.go index 2175d0bf0..3ee24290a 100644 --- a/internal/commands/agent_hook_test.go +++ b/internal/commands/agent_hook_test.go @@ -28,6 +28,7 @@ func TestAgentHookCommandIsHidden(t *testing.T) { assert.True(t, cmd.Hidden) assert.Equal(t, "agent-hook", cmd.Name()) assert.NotNil(t, findSubcommand(cmd, "session-start")) + assert.NotNil(t, findSubcommand(cmd, "plugin-notice")) assert.NotNil(t, findSubcommand(cmd, "pre-commit-snapshot")) assert.NotNil(t, findSubcommand(cmd, "post-commit")) } @@ -558,3 +559,65 @@ func runGit(t *testing.T, dir string, args ...string) { output, err := cmd.CombinedOutput() require.NoError(t, err, string(output)) } + +func TestAgentHookPluginNoticeOnceForLegacyClaudeInstall(t *testing.T) { + data := t.TempDir() + t.Setenv("PLUGIN_ROOT", "") + t.Setenv("PLUGIN_DATA", "") + t.Setenv("CLAUDE_PLUGIN_DATA", data) + home, _ := os.UserHomeDir() + t.Setenv("CLAUDE_PLUGIN_ROOT", filepath.Join(home, ".claude", "plugins", "cache", "37signals", "basecamp", "0.12.0")) + + var payload struct { + SystemMessage string `json:"systemMessage"` + } + output := runAgentHookPreservingHome(t, "plugin-notice") + require.NoError(t, json.Unmarshal([]byte(output), &payload)) + assert.Contains(t, payload.SystemMessage, "now `basecamp-cli`") + assert.Contains(t, payload.SystemMessage, "basecamp setup claude") + event, context := hookSpecificOutput(t, output) + assert.Equal(t, "SessionStart", event) + assert.Contains(t, context, "basecamp setup claude") + + assert.Empty(t, runAgentHookPreservingHome(t, "plugin-notice"), "the notice appears once") +} + +func TestAgentHookPluginNoticeNamesCodexSetup(t *testing.T) { + t.Setenv("CLAUDE_PLUGIN_ROOT", "") + t.Setenv("CLAUDE_PLUGIN_DATA", "") + t.Setenv("PLUGIN_DATA", t.TempDir()) + home, _ := os.UserHomeDir() + t.Setenv("PLUGIN_ROOT", filepath.Join(home, ".codex", "plugins", "cache", "37signals", "basecamp", "0.12.0")) + + assert.Contains(t, runAgentHookPreservingHome(t, "plugin-notice"), "basecamp setup codex") +} + +func TestAgentHookPluginNoticeSilentForRenamedInstall(t *testing.T) { + t.Setenv("CLAUDE_PLUGIN_DATA", t.TempDir()) + t.Setenv("PLUGIN_ROOT", "") + for _, root := range []string{ + "/home/u/.claude/plugins/cache/37signals/basecamp-cli/0.12.0", + "/home/u/.claude/plugins/cache/other/basecamp/0.12.0", + "", + } { + t.Setenv("CLAUDE_PLUGIN_ROOT", root) + assert.Empty(t, runAgentHookPreservingHome(t, "plugin-notice"), root) + } +} + +// runAgentHookPreservingHome runs a hook subcommand with no stdin under the +// test's current HOME, so plugin roots built from it stay under it. +func runAgentHookPreservingHome(t *testing.T, subcommand string) string { + t.Helper() + app := appctx.NewApp(config.Default()) + t.Cleanup(app.Close) + cmd := NewAgentHookCmd() + var stdout bytes.Buffer + cmd.SetIn(strings.NewReader("")) + cmd.SetOut(&stdout) + cmd.SetErr(&bytes.Buffer{}) + cmd.SetArgs([]string{subcommand}) + cmd.SetContext(appctx.WithApp(context.Background(), app)) + require.NoError(t, cmd.Execute()) + return strings.TrimSpace(stdout.String()) +} diff --git a/internal/harness/claude.go b/internal/harness/claude.go index 7893e0183..9c5950fec 100644 --- a/internal/harness/claude.go +++ b/internal/harness/claude.go @@ -59,7 +59,10 @@ const ClaudeExpectedPluginKey = ClaudePluginName + "@" + ClaudeMarketplaceName // The marketplace may later list the hosted Basecamp connector as "basecamp" // again. That step has to wait until this key is no longer treated as stale // here (isStalePluginKey); otherwise setup would uninstall the connector. -const ClaudeLegacyPluginKey = "basecamp@" + ClaudeMarketplaceName +const ClaudeLegacyPluginKey = LegacyPluginName + "@" + ClaudeMarketplaceName + +// LegacyPluginName is the plugin's pre-rename name. +const LegacyPluginName = "basecamp" // DetectClaude returns true if Claude Code is installed. // Checks ~/.claude/ directory first, then falls back to binary on PATH. diff --git a/internal/release/manifests_test.go b/internal/release/manifests_test.go index 3e2bd5184..669c0fa72 100644 --- a/internal/release/manifests_test.go +++ b/internal/release/manifests_test.go @@ -143,7 +143,14 @@ func TestHooksFileCommandsInvokeBasecamp(t *testing.T) { } require.NoError(t, json.Unmarshal(data, &config)) require.NotEmpty(t, config.Hooks) - assert.NotContains(t, config.Hooks, "SessionStart", "plugins must not inject Basecamp context into every agent session") + // Plugins must not inject Basecamp context into every agent session. The + // one SessionStart hook allowed is the rename notice, which stays silent + // unless the plugin runs under its pre-rename id, and then speaks once. + for _, matcher := range config.Hooks["SessionStart"] { + for _, hook := range matcher.Hooks { + assert.Equal(t, "basecamp agent-hook plugin-notice", hook.Command, "only the rename notice may run at session start") + } + } for event, matchers := range config.Hooks { require.NotEmpty(t, matchers, event) From f454d060e1de9978cb41b83d989d4b50f032c555 Mon Sep 17 00:00:00 2001 From: Jeremy Daer Date: Mon, 5 Oct 2026 16:55:14 -0700 Subject: [PATCH 10/13] Deliver the rename notice through pre-commit-snapshot so older CLIs stay silent A new agent-hook subcommand fails with unknown command, on every session start, for anyone whose CLI is older than the plugin. pre-commit-snapshot exists in every hook-capable CLI and ignores a payload with no tool call, so the SessionStart hook runs it and only CLIs that know the notice act on the SessionStart event. --- hooks/hooks.json | 2 +- internal/commands/agent_hook.go | 11 ++++- internal/commands/agent_hook_rename.go | 58 +++++++++++--------------- internal/commands/agent_hook_test.go | 32 ++++++++++---- internal/release/manifests_test.go | 5 ++- 5 files changed, 63 insertions(+), 45 deletions(-) diff --git a/hooks/hooks.json b/hooks/hooks.json index 3714fcc5f..89d7646cf 100644 --- a/hooks/hooks.json +++ b/hooks/hooks.json @@ -6,7 +6,7 @@ "hooks": [ { "type": "command", - "command": "basecamp agent-hook plugin-notice", + "command": "basecamp agent-hook pre-commit-snapshot", "timeout": 5 } ] diff --git a/internal/commands/agent_hook.go b/internal/commands/agent_hook.go index 59f2596b0..ca6700c57 100644 --- a/internal/commands/agent_hook.go +++ b/internal/commands/agent_hook.go @@ -72,7 +72,7 @@ func NewAgentHookCmd() *cobra.Command { return nil }, } - cmd.AddCommand(newAgentHookSessionStartCmd(), newAgentHookPluginNoticeCmd(), newAgentHookPreCommitSnapshotCmd(), newAgentHookPostCommitCmd()) + cmd.AddCommand(newAgentHookSessionStartCmd(), newAgentHookPreCommitSnapshotCmd(), newAgentHookPostCommitCmd()) return cmd } @@ -165,6 +165,15 @@ func newAgentHookPreCommitSnapshotCmd() *cobra.Command { Args: cobra.NoArgs, RunE: func(cmd *cobra.Command, args []string) error { input, ok := readAgentHookInput(cmd.InOrStdin()) + // The plugin's SessionStart hook runs this subcommand too, for the + // rename notice: CLIs that predate the notice already have this + // subcommand and ignore a payload with no tool call, so a plugin + // newer than the CLI stays silent instead of failing every + // session start with "unknown command". + if ok && input.HookEventName == "SessionStart" { + emitPluginRenameNotice(cmd) + return nil + } if !ok || !agentHookHasSnapshotKey(input) || !strings.Contains(strings.ToLower(agentHookCommand(input.ToolInput)), "commit") { return nil diff --git a/internal/commands/agent_hook_rename.go b/internal/commands/agent_hook_rename.go index 8f1b3ccea..a25a70b8f 100644 --- a/internal/commands/agent_hook_rename.go +++ b/internal/commands/agent_hook_rename.go @@ -14,46 +14,38 @@ import ( // plugin install, so it appears once rather than every session. const pluginRenameNoticeMarker = "plugin-rename-notice-shown" -// newAgentHookPluginNoticeCmd tells someone still running this plugin under -// its pre-rename id that it is now basecamp-cli. During the deprecation window -// the marketplace keeps "basecamp" as an alias with the same source, so the -// plugin keeps working; this nudges toward `basecamp setup`, once. +// emitPluginRenameNotice tells someone still running this plugin under its +// pre-rename id that it is now basecamp-cli. During the deprecation window the +// marketplace keeps "basecamp" as an alias with the same source, so the plugin +// keeps working; this nudges toward `basecamp setup`, once. // // The alias and basecamp-cli install the same files, so the manifest can't // tell them apart. The plugin root can: agents cache each install under // //. Anything other than an install as // basecamp@37signals stays silent, so the hook injects nothing into normal // sessions. -func newAgentHookPluginNoticeCmd() *cobra.Command { - return &cobra.Command{ - Use: "plugin-notice", - Short: "Tell a pre-rename plugin install to switch to basecamp-cli, once", - Args: cobra.NoArgs, - RunE: func(cmd *cobra.Command, args []string) error { - root := agentPluginRoot() - if !isLegacyPluginRoot(root) { - return nil - } - dir := agentPluginDataDir() - if dir == "" { - dir = agentHookStateDir(cmd.Context()) - } - if dir == "" || !claimOnce(filepath.Join(dir, pluginRenameNoticeMarker)) { - return nil - } - message := "The Basecamp plugin is now `" + harness.ClaudePluginName + "`. " + - "This copy still runs under its old name `" + harness.ClaudeLegacyPluginKey + "`, which keeps working for now. " + - "Run `" + legacyPluginSetupCommand(root) + "` to switch." - _ = json.NewEncoder(cmd.OutOrStdout()).Encode(map[string]any{ - "systemMessage": message, - "hookSpecificOutput": map[string]string{ - "hookEventName": "SessionStart", - "additionalContext": "Mention this to the user once: " + message, - }, - }) - return nil - }, +func emitPluginRenameNotice(cmd *cobra.Command) { + root := agentPluginRoot() + if !isLegacyPluginRoot(root) { + return } + dir := agentPluginDataDir() + if dir == "" { + dir = agentHookStateDir(cmd.Context()) + } + if dir == "" || !claimOnce(filepath.Join(dir, pluginRenameNoticeMarker)) { + return + } + message := "The Basecamp plugin is now `" + harness.ClaudePluginName + "`. " + + "This copy still runs under its old name `" + harness.ClaudeLegacyPluginKey + "`, which keeps working for now. " + + "Run `" + legacyPluginSetupCommand(root) + "` to switch." + _ = json.NewEncoder(cmd.OutOrStdout()).Encode(map[string]any{ + "systemMessage": message, + "hookSpecificOutput": map[string]string{ + "hookEventName": "SessionStart", + "additionalContext": "Mention this to the user once: " + message, + }, + }) } // agentPluginRoot is the plugin's install directory as the agent reports it: diff --git a/internal/commands/agent_hook_test.go b/internal/commands/agent_hook_test.go index 3ee24290a..7793014e4 100644 --- a/internal/commands/agent_hook_test.go +++ b/internal/commands/agent_hook_test.go @@ -28,7 +28,6 @@ func TestAgentHookCommandIsHidden(t *testing.T) { assert.True(t, cmd.Hidden) assert.Equal(t, "agent-hook", cmd.Name()) assert.NotNil(t, findSubcommand(cmd, "session-start")) - assert.NotNil(t, findSubcommand(cmd, "plugin-notice")) assert.NotNil(t, findSubcommand(cmd, "pre-commit-snapshot")) assert.NotNil(t, findSubcommand(cmd, "post-commit")) } @@ -571,7 +570,7 @@ func TestAgentHookPluginNoticeOnceForLegacyClaudeInstall(t *testing.T) { var payload struct { SystemMessage string `json:"systemMessage"` } - output := runAgentHookPreservingHome(t, "plugin-notice") + output := runAgentHookPreservingHome(t, sessionStartPayload) require.NoError(t, json.Unmarshal([]byte(output), &payload)) assert.Contains(t, payload.SystemMessage, "now `basecamp-cli`") assert.Contains(t, payload.SystemMessage, "basecamp setup claude") @@ -579,7 +578,7 @@ func TestAgentHookPluginNoticeOnceForLegacyClaudeInstall(t *testing.T) { assert.Equal(t, "SessionStart", event) assert.Contains(t, context, "basecamp setup claude") - assert.Empty(t, runAgentHookPreservingHome(t, "plugin-notice"), "the notice appears once") + assert.Empty(t, runAgentHookPreservingHome(t, sessionStartPayload), "the notice appears once") } func TestAgentHookPluginNoticeNamesCodexSetup(t *testing.T) { @@ -589,7 +588,7 @@ func TestAgentHookPluginNoticeNamesCodexSetup(t *testing.T) { home, _ := os.UserHomeDir() t.Setenv("PLUGIN_ROOT", filepath.Join(home, ".codex", "plugins", "cache", "37signals", "basecamp", "0.12.0")) - assert.Contains(t, runAgentHookPreservingHome(t, "plugin-notice"), "basecamp setup codex") + assert.Contains(t, runAgentHookPreservingHome(t, sessionStartPayload), "basecamp setup codex") } func TestAgentHookPluginNoticeSilentForRenamedInstall(t *testing.T) { @@ -601,22 +600,37 @@ func TestAgentHookPluginNoticeSilentForRenamedInstall(t *testing.T) { "", } { t.Setenv("CLAUDE_PLUGIN_ROOT", root) - assert.Empty(t, runAgentHookPreservingHome(t, "plugin-notice"), root) + assert.Empty(t, runAgentHookPreservingHome(t, sessionStartPayload), root) } } -// runAgentHookPreservingHome runs a hook subcommand with no stdin under the +const sessionStartPayload = `{"hook_event_name":"SessionStart","session_id":"s","source":"startup"}` + +// A SessionStart payload carries no tool call, so the snapshot path ignores +// it — which is what keeps older CLIs silent on the plugin's SessionStart hook. +func TestAgentHookSessionStartPayloadTakesNoSnapshot(t *testing.T) { + t.Setenv("CLAUDE_PLUGIN_ROOT", "/home/u/.claude/plugins/cache/37signals/basecamp-cli/0.12.0") + data := t.TempDir() + t.Setenv("CLAUDE_PLUGIN_DATA", data) + + assert.Empty(t, runAgentHookPreservingHome(t, sessionStartPayload)) + entries, err := os.ReadDir(data) + require.NoError(t, err) + assert.Empty(t, entries) +} + +// runAgentHookPreservingHome runs pre-commit-snapshot with input under the // test's current HOME, so plugin roots built from it stay under it. -func runAgentHookPreservingHome(t *testing.T, subcommand string) string { +func runAgentHookPreservingHome(t *testing.T, input string) string { t.Helper() app := appctx.NewApp(config.Default()) t.Cleanup(app.Close) cmd := NewAgentHookCmd() var stdout bytes.Buffer - cmd.SetIn(strings.NewReader("")) + cmd.SetIn(strings.NewReader(input)) cmd.SetOut(&stdout) cmd.SetErr(&bytes.Buffer{}) - cmd.SetArgs([]string{subcommand}) + cmd.SetArgs([]string{"pre-commit-snapshot"}) cmd.SetContext(appctx.WithApp(context.Background(), app)) require.NoError(t, cmd.Execute()) return strings.TrimSpace(stdout.String()) diff --git a/internal/release/manifests_test.go b/internal/release/manifests_test.go index 669c0fa72..5fcdf6c7a 100644 --- a/internal/release/manifests_test.go +++ b/internal/release/manifests_test.go @@ -146,9 +146,12 @@ func TestHooksFileCommandsInvokeBasecamp(t *testing.T) { // Plugins must not inject Basecamp context into every agent session. The // one SessionStart hook allowed is the rename notice, which stays silent // unless the plugin runs under its pre-rename id, and then speaks once. + // It goes through pre-commit-snapshot, which every hook-capable CLI has + // and which older ones answer silently: a new subcommand would fail + // every session start for anyone whose CLI is older than the plugin. for _, matcher := range config.Hooks["SessionStart"] { for _, hook := range matcher.Hooks { - assert.Equal(t, "basecamp agent-hook plugin-notice", hook.Command, "only the rename notice may run at session start") + assert.Equal(t, "basecamp agent-hook pre-commit-snapshot", hook.Command, "only the rename notice may run at session start") } } From e1f9930542e98ced77e619d2bc026aac42896c2a Mon Sep 17 00:00:00 2001 From: Jeremy Daer Date: Mon, 5 Oct 2026 16:55:45 -0700 Subject: [PATCH 11/13] Document the basecamp alias window and the one-time rename notice --- AGENTS.md | 11 +++++++---- README.md | 15 +++++++++------ install.md | 2 +- 3 files changed, 17 insertions(+), 11 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index df198ddab..f9713f73c 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -44,10 +44,13 @@ Coding-agent integration lives in `internal/harness` (agent registry, detection, skill health checks) and `internal/commands/wizard_agents.go` (`basecamp setup claude|codex|grok|agents`). Claude Code and Codex each get a native plugin from the `basecamp/claude-plugins` marketplace and have registrations of their own (`claude.go`, -`codex.go`). The plugin is `basecamp-cli@37signals`. Setup treats the pre-rename `basecamp@37signals` as -stale and replaces it by key, which is safe only while the marketplace lists no `basecamp` -plugin. Drop that key from `isStalePluginKey`/`CodexLegacyPluginKey` handling before the -marketplace re-adds `basecamp` for the hosted connector. Grok Build has no plugin: it reads the shared `~/.agents/skills/basecamp` skill +`codex.go`). The plugin is `basecamp-cli@37signals`. During the deprecation window the marketplace keeps +`basecamp` as an alias with the same source; setup treats `basecamp@37signals` as stale and +replaces it by key, and the SessionStart hook (routed through `agent-hook pre-commit-snapshot` +so older CLIs stay silent) tells alias installs once to switch. That is safe only while +`basecamp` in the marketplace means this plugin: drop the `isStalePluginKey` / +`CodexLegacyPluginKey` handling before the marketplace points `basecamp` at the hosted +connector. Grok Build has no plugin: it reads the shared `~/.agents/skills/basecamp` skill directly, so it is a row of `harness.SkillAgent` (name, id, home env var, home directory, binary) in `skill_agent.go`, and everything in `internal/commands` that touches a shared-skill agent — the setup handler, the `BASECAMP_SETUP_AGENT` values, doctor's remediation — loops over diff --git a/README.md b/README.md index bbbead0af..09f86fc27 100644 --- a/README.md +++ b/README.md @@ -299,12 +299,15 @@ To pick up a newer plugin version later, refresh the marketplace with `codex plugin marketplace upgrade 37signals` (or re-run `basecamp setup codex`). **Plugin name:** in the 37signals marketplace this plugin is `basecamp-cli` -(`basecamp-cli@37signals`). It was published as `basecamp` until that name -was set aside for the hosted Basecamp connector plugin, which works through -Basecamp's own MCP server rather than this CLI. If you installed the plugin -under the old name, re-run `basecamp setup claude` or `basecamp setup codex`: -setup installs `basecamp-cli` and removes `basecamp@37signals` at the scopes it -was installed in. To migrate by hand: +(`basecamp-cli@37signals`). It was published as `basecamp`, a name set aside +for the hosted Basecamp connector plugin, which works through Basecamp's own +MCP server rather than this CLI. During a deprecation window the marketplace +keeps `basecamp` as an alias of `basecamp-cli`, so an existing +`basecamp@37signals` install keeps working and updating; once per agent it +says the plugin has been renamed. Switch with `basecamp setup claude` or +`basecamp setup codex` (or `basecamp setup agents`), which installs +`basecamp-cli` and removes `basecamp@37signals` at the scopes it was +installed in. To switch by hand: ```bash claude plugin marketplace update 37signals diff --git a/install.md b/install.md index 067550cf3..ce16e0fe4 100644 --- a/install.md +++ b/install.md @@ -141,7 +141,7 @@ basecamp setup claude This registers the marketplace and installs the `basecamp-cli@37signals` plugin with skills, hooks, and agent workflow support. -The plugin was named `basecamp` until that name was set aside for the hosted Basecamp connector plugin. Re-running `basecamp setup claude` (or `basecamp setup codex`) replaces an old install under `basecamp@37signals` with `basecamp-cli@37signals` at the same scope. +The plugin was named `basecamp` until that name was set aside for the hosted Basecamp connector plugin. During a deprecation window `basecamp@37signals` remains an alias that keeps working; re-running `basecamp setup claude` (or `basecamp setup codex`) replaces it with `basecamp-cli@37signals` at the same scope. The hooks call the CLI's `agent-hook` command, so they need a `basecamp` new enough to have it. If hook errors appear after installing or refreshing the From dcd43bbe2863145fe6e5127db00a0f9c064001f2 Mon Sep 17 00:00:00 2001 From: Jeremy Daer Date: Mon, 5 Oct 2026 17:06:56 -0700 Subject: [PATCH 12/13] Pin the rename-notice hook count, decode its JSON in tests, and scope the same-scope claim to Claude --- README.md | 7 ++++--- install.md | 2 +- internal/commands/agent_hook_test.go | 6 +++++- internal/release/manifests_test.go | 9 ++++----- 4 files changed, 14 insertions(+), 10 deletions(-) diff --git a/README.md b/README.md index 09f86fc27..94c0abfd7 100644 --- a/README.md +++ b/README.md @@ -305,9 +305,10 @@ MCP server rather than this CLI. During a deprecation window the marketplace keeps `basecamp` as an alias of `basecamp-cli`, so an existing `basecamp@37signals` install keeps working and updating; once per agent it says the plugin has been renamed. Switch with `basecamp setup claude` or -`basecamp setup codex` (or `basecamp setup agents`), which installs -`basecamp-cli` and removes `basecamp@37signals` at the scopes it was -installed in. To switch by hand: +`basecamp setup codex` (or `basecamp setup agents`). For Claude Code, setup +replaces `basecamp@37signals` with `basecamp-cli` at the scopes it was +installed in. Codex installs have no scopes, so setup adds `basecamp-cli` and +then removes the old ID. To switch by hand: ```bash claude plugin marketplace update 37signals diff --git a/install.md b/install.md index ce16e0fe4..5b6d328ce 100644 --- a/install.md +++ b/install.md @@ -141,7 +141,7 @@ basecamp setup claude This registers the marketplace and installs the `basecamp-cli@37signals` plugin with skills, hooks, and agent workflow support. -The plugin was named `basecamp` until that name was set aside for the hosted Basecamp connector plugin. During a deprecation window `basecamp@37signals` remains an alias that keeps working; re-running `basecamp setup claude` (or `basecamp setup codex`) replaces it with `basecamp-cli@37signals` at the same scope. +The plugin was named `basecamp` until that name was set aside for the hosted Basecamp connector plugin. During a deprecation window `basecamp@37signals` remains an alias that keeps working; re-running `basecamp setup claude` replaces it with `basecamp-cli@37signals` at the same scopes, and `basecamp setup codex` adds `basecamp-cli@37signals` and then removes the old ID (Codex installs have no scopes to preserve). The hooks call the CLI's `agent-hook` command, so they need a `basecamp` new enough to have it. If hook errors appear after installing or refreshing the diff --git a/internal/commands/agent_hook_test.go b/internal/commands/agent_hook_test.go index 7793014e4..638f62095 100644 --- a/internal/commands/agent_hook_test.go +++ b/internal/commands/agent_hook_test.go @@ -588,7 +588,11 @@ func TestAgentHookPluginNoticeNamesCodexSetup(t *testing.T) { home, _ := os.UserHomeDir() t.Setenv("PLUGIN_ROOT", filepath.Join(home, ".codex", "plugins", "cache", "37signals", "basecamp", "0.12.0")) - assert.Contains(t, runAgentHookPreservingHome(t, sessionStartPayload), "basecamp setup codex") + var payload struct { + SystemMessage string `json:"systemMessage"` + } + require.NoError(t, json.Unmarshal([]byte(runAgentHookPreservingHome(t, sessionStartPayload)), &payload)) + assert.Contains(t, payload.SystemMessage, "basecamp setup codex") } func TestAgentHookPluginNoticeSilentForRenamedInstall(t *testing.T) { diff --git a/internal/release/manifests_test.go b/internal/release/manifests_test.go index 5fcdf6c7a..52c14f726 100644 --- a/internal/release/manifests_test.go +++ b/internal/release/manifests_test.go @@ -149,11 +149,10 @@ func TestHooksFileCommandsInvokeBasecamp(t *testing.T) { // It goes through pre-commit-snapshot, which every hook-capable CLI has // and which older ones answer silently: a new subcommand would fail // every session start for anyone whose CLI is older than the plugin. - for _, matcher := range config.Hooks["SessionStart"] { - for _, hook := range matcher.Hooks { - assert.Equal(t, "basecamp agent-hook pre-commit-snapshot", hook.Command, "only the rename notice may run at session start") - } - } + sessionStart := config.Hooks["SessionStart"] + require.Len(t, sessionStart, 1, "exactly one SessionStart matcher: the rename notice") + require.Len(t, sessionStart[0].Hooks, 1, "exactly one SessionStart hook: the rename notice") + assert.Equal(t, "basecamp agent-hook pre-commit-snapshot", sessionStart[0].Hooks[0].Command, "only the rename notice may run at session start") for event, matchers := range config.Hooks { require.NotEmpty(t, matchers, event) From 917e4510a305e3c43f72c701d1939c7c62a529eb Mon Sep 17 00:00:00 2001 From: Jeremy Daer Date: Tue, 6 Oct 2026 18:27:31 -0700 Subject: [PATCH 13/13] Remove the old Codex plugin ID only once basecamp-cli is enabled Codex keeps a disabled plugin installed, so setup could remove the working basecamp@37signals while basecamp-cli sat disabled beside it. Setup now removes the old ID only when the replacement is installed and enabled. Also correct the legacy-key comments: during the window the marketplace lists basecamp as an alias of basecamp-cli, not nothing at all. --- internal/commands/wizard_codex.go | 6 +++--- internal/commands/wizard_codex_test.go | 17 ++++++++++++++++- internal/harness/claude.go | 11 ++++++----- internal/harness/codex.go | 24 +++++++++++++----------- internal/harness/codex_test.go | 6 ++++-- 5 files changed, 42 insertions(+), 22 deletions(-) diff --git a/internal/commands/wizard_codex.go b/internal/commands/wizard_codex.go index ab4899d53..7b24c7372 100644 --- a/internal/commands/wizard_codex.go +++ b/internal/commands/wizard_codex.go @@ -111,10 +111,10 @@ func installCodexPlugin(parent context.Context, stderr io.Writer, progress func( } // The plugin was renamed basecamp → basecamp-cli. Remove a pre-rename - // install only once its replacement is in, so a failed install never - // leaves the user with neither. + // install only once its replacement is installed and enabled, so a failed + // or disabled install never leaves the user without a working plugin. legacyCtx, cancelLegacy := context.WithTimeout(parent, codexVerifyTimeout) - legacy := harness.CodexLegacyCLIInstalled(legacyCtx) + legacy := harness.CodexLegacyReplaced(legacyCtx) cancelLegacy() if legacy { progress("Removing the pre-rename " + harness.CodexLegacyPluginKey + " plugin…") diff --git a/internal/commands/wizard_codex_test.go b/internal/commands/wizard_codex_test.go index 38bac91ec..522d6ef53 100644 --- a/internal/commands/wizard_codex_test.go +++ b/internal/commands/wizard_codex_test.go @@ -100,6 +100,19 @@ func TestSetupCodexReplacesLegacyCLIPlugin(t *testing.T) { ) } +// TestSetupCodexKeepsLegacyWhenReplacementDisabled covers a basecamp-cli +// install Codex keeps but has disabled: the working old ID stays until the +// replacement is enabled, and verification reports the disabled plugin. +func TestSetupCodexKeepsLegacyWhenReplacementDisabled(t *testing.T) { + logPath := installCodexStub(t, codexStubOptions{legacyInstalled: true, pluginAlreadyInstalled: true, pluginDisabled: true}) + + envelope := runSetupCodexJSON(t) + + assert.NotContains(t, readCodexSetupCalls(t, logPath), "plugin remove") + require.NotEmpty(t, envelope.Data.Errors) + assert.Contains(t, envelope.Data.Errors[0], "disabled") +} + func TestSetupCodexLegacyRemovalFailureNamesTheManualStep(t *testing.T) { installCodexStub(t, codexStubOptions{legacyInstalled: true, legacyRemoveFailure: true}) @@ -271,6 +284,7 @@ type codexStubOptions struct { verificationMissing bool legacyInstalled bool legacyRemoveFailure bool + pluginDisabled bool } func installCodexStub(t *testing.T, options codexStubOptions) string { @@ -311,10 +325,11 @@ func installCodexStub(t *testing.T, options codexStubOptions) string { " if [ " + boolShell(options.legacyRemoveFailure) + " = 1 ]; then echo 'remove failure' >&2; exit 1; fi\n" + " echo removed > \"" + legacyPath + "\"; echo '{}'; exit 0 ;;\n" + " \"plugin list --available --json\")\n" + + " enabled=true; if [ " + boolShell(options.pluginDisabled) + " = 1 ]; then enabled=false; fi\n" + " legacy_state=''; [ -f \"" + legacyPath + "\" ] && read -r legacy_state < \"" + legacyPath + "\"\n" + " if [ \"$legacy_state\" = installed ]; then legacy='{\"pluginId\":\"basecamp@37signals\",\"version\":\"0.11.0\",\"installed\":true,\"enabled\":true},'; else legacy=''; fi\n" + " if [ " + boolShell(options.verificationMissing) + " = 1 ]; then echo '{\"installed\":[],\"available\":[{\"pluginId\":\"basecamp-cli@37signals\",\"version\":\"0.7.2\",\"installed\":false,\"enabled\":false}]}'; exit 0; fi\n" + - " if [ -f \"" + statePath + "\" ]; then echo \"{\\\"installed\\\":[$legacy{\\\"pluginId\\\":\\\"basecamp-cli@37signals\\\",\\\"version\\\":\\\"0.7.2\\\",\\\"installed\\\":true,\\\"enabled\\\":true}],\\\"available\\\":[]}\"; else echo \"{\\\"installed\\\":[${legacy%,}],\\\"available\\\":[]}\"; fi; exit 0 ;;\n" + + " if [ -f \"" + statePath + "\" ]; then echo \"{\\\"installed\\\":[$legacy{\\\"pluginId\\\":\\\"basecamp-cli@37signals\\\",\\\"version\\\":\\\"0.7.2\\\",\\\"installed\\\":true,\\\"enabled\\\":$enabled}],\\\"available\\\":[]}\"; else echo \"{\\\"installed\\\":[${legacy%,}],\\\"available\\\":[]}\"; fi; exit 0 ;;\n" + " *) echo 'unexpected command' >&2; exit 1 ;;\n" + "esac\n" require.NoError(t, os.WriteFile(filepath.Join(binDir, "codex"), []byte(script), 0o755)) //nolint:gosec // test executable diff --git a/internal/harness/claude.go b/internal/harness/claude.go index 9c5950fec..3d65280a3 100644 --- a/internal/harness/claude.go +++ b/internal/harness/claude.go @@ -41,8 +41,8 @@ func claudeChecks() []*StatusCheck { const ClaudeMarketplaceSource = "basecamp/claude-plugins" // ClaudePluginName is the plugin identifier to install. It was "basecamp" -// until the 37signals marketplace gave that name to the hosted-connector -// plugin; see ClaudeLegacyPluginKey. +// until that name was set aside for the hosted-connector plugin; see +// ClaudeLegacyPluginKey. const ClaudePluginName = "basecamp-cli" // ClaudeMarketplaceName is the marketplace name as it appears in plugin keys. @@ -52,9 +52,10 @@ const ClaudeMarketplaceName = "37signals" const ClaudeExpectedPluginKey = ClaudePluginName + "@" + ClaudeMarketplaceName // ClaudeLegacyPluginKey is the key this plugin was installed under before the -// rename. During the migration window the 37signals marketplace publishes no -// "basecamp" plugin at all, so an install under this key is always this CLI's -// pre-rename plugin and setup removes it by key, like any stale entry. +// rename. During the migration window the 37signals marketplace lists +// "basecamp" only as a deprecated alias of basecamp-cli with the same source, +// so an install under this key is always this CLI's pre-rename plugin and +// setup removes it by key, like any stale entry. // // The marketplace may later list the hosted Basecamp connector as "basecamp" // again. That step has to wait until this key is no longer treated as stale diff --git a/internal/harness/codex.go b/internal/harness/codex.go index bad2c22f0..42781f64f 100644 --- a/internal/harness/codex.go +++ b/internal/harness/codex.go @@ -18,16 +18,17 @@ const ( // CodexMarketplaceSource is the Git marketplace repository containing Basecamp. CodexMarketplaceSource = "basecamp/claude-plugins" // CodexPluginName is the plugin identifier to install. It was "basecamp" - // until the 37signals marketplace gave that name to the hosted-connector - // plugin; see CodexLegacyPluginKey. + // until that name was set aside for the hosted-connector plugin; see + // CodexLegacyPluginKey. CodexPluginName = "basecamp-cli" // CodexMarketplaceName is the marketplace name published by 37signals. CodexMarketplaceName = "37signals" // CodexExpectedPluginKey is the fully qualified Basecamp plugin ID. CodexExpectedPluginKey = CodexPluginName + "@" + CodexMarketplaceName - // CodexLegacyPluginKey is the pre-rename plugin ID. The marketplace lists - // no "basecamp" plugin during the migration window, so an install under it - // is always this CLI's (see ClaudeLegacyPluginKey). + // CodexLegacyPluginKey is the pre-rename plugin ID. During the migration + // window the marketplace lists "basecamp" only as an alias of basecamp-cli, + // with the same source, so an install under it is always this CLI's (see + // ClaudeLegacyPluginKey). CodexLegacyPluginKey = "basecamp@" + CodexMarketplaceName // codexQueryTimeout bounds how long the Codex probe may run. @@ -325,12 +326,13 @@ func queryCodexPlugin(parent context.Context) (codexPluginState, bool, error) { return codexPluginState{legacyInstalled: legacy}, false, nil } -// CodexLegacyCLIInstalled reports whether this CLI's plugin is still installed -// in Codex under CodexLegacyPluginKey. It reads the plugin list itself, so -// callers that already hold a query result should use its state instead. -func CodexLegacyCLIInstalled(ctx context.Context) bool { - state, _, err := queryCodexPlugin(ctx) - return err == nil && state.legacyInstalled +// CodexLegacyReplaced reports whether this CLI's plugin is still installed in +// Codex under CodexLegacyPluginKey while CodexExpectedPluginKey is installed +// and enabled, so removing the old ID leaves a working plugin behind. A +// disabled replacement (Codex keeps those installed) doesn't count. +func CodexLegacyReplaced(ctx context.Context) bool { + state, found, err := queryCodexPlugin(ctx) + return err == nil && found && state.Installed && state.Enabled && state.legacyInstalled } func codexQueryFailure(name string, err error) *StatusCheck { diff --git a/internal/harness/codex_test.go b/internal/harness/codex_test.go index 43aaf9b3f..38ba92cd9 100644 --- a/internal/harness/codex_test.go +++ b/internal/harness/codex_test.go @@ -212,14 +212,14 @@ func TestCheckCodexPluginLegacyCLIInstall(t *testing.T) { assert.Equal(t, "fail", check.Status) assert.Contains(t, check.Message, "old name basecamp@37signals") assert.Contains(t, check.Hint, "basecamp setup codex") - assert.True(t, CodexLegacyCLIInstalled(context.Background())) + assert.False(t, CodexLegacyReplaced(context.Background()), "no replacement installed yet") } func TestCheckCodexPluginLegacyAvailableOnlyIsNotInstalled(t *testing.T) { stubCodexList(t, `{"installed":[],"available":[{"pluginId":"basecamp@37signals","version":"0.11.0","installed":false,"enabled":false}]}`, nil) assert.Equal(t, "Plugin not installed", CheckCodexPlugin().Message) - assert.False(t, CodexLegacyCLIInstalled(context.Background())) + assert.False(t, CodexLegacyReplaced(context.Background())) } func TestCheckCodexPluginBothInstalledWarns(t *testing.T) { @@ -229,6 +229,7 @@ func TestCheckCodexPluginBothInstalledWarns(t *testing.T) { assert.Equal(t, "warn", check.Status) assert.Contains(t, check.Message, "old basecamp@37signals copy") + assert.True(t, CodexLegacyReplaced(context.Background())) } func TestCheckCodexPluginDisabledMentionsLegacyCopy(t *testing.T) { @@ -238,5 +239,6 @@ func TestCheckCodexPluginDisabledMentionsLegacyCopy(t *testing.T) { assert.Equal(t, "fail", check.Status) assert.Contains(t, check.Message, "disabled") + assert.False(t, CodexLegacyReplaced(context.Background()), "a disabled replacement doesn't replace the old ID") assert.Contains(t, check.Message, "old basecamp@37signals copy") }