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..f9713f73c 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -44,7 +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`). 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 35b00f8db..94c0abfd7 100644 --- a/README.md +++ b/README.md @@ -292,12 +292,37 @@ 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`, 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`). 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 +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` +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/hooks/hooks.json b/hooks/hooks.json index 91e64a90b..89d7646cf 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 pre-commit-snapshot", + "timeout": 5 + } + ] + } + ], "PreToolUse": [ { "matcher": "Bash", diff --git a/install.md b/install.md index 8d76d03bb..5b6d328ce 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 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 @@ -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/agent_hook.go b/internal/commands/agent_hook.go index db6b9782d..ca6700c57 100644 --- a/internal/commands/agent_hook.go +++ b/internal/commands/agent_hook.go @@ -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 new file mode 100644 index 000000000..a25a70b8f --- /dev/null +++ b/internal/commands/agent_hook_rename.go @@ -0,0 +1,109 @@ +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" + +// 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 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: +// 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..638f62095 100644 --- a/internal/commands/agent_hook_test.go +++ b/internal/commands/agent_hook_test.go @@ -558,3 +558,84 @@ 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, 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") + event, context := hookSpecificOutput(t, output) + assert.Equal(t, "SessionStart", event) + assert.Contains(t, context, "basecamp setup claude") + + assert.Empty(t, runAgentHookPreservingHome(t, sessionStartPayload), "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")) + + 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) { + 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, sessionStartPayload), root) + } +} + +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, input string) string { + t.Helper() + app := appctx.NewApp(config.Default()) + t.Cleanup(app.Close) + cmd := NewAgentHookCmd() + var stdout bytes.Buffer + cmd.SetIn(strings.NewReader(input)) + cmd.SetOut(&stdout) + cmd.SetErr(&bytes.Buffer{}) + 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/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..7b24c7372 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 @@ -100,13 +103,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 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.CodexLegacyReplaced(legacyCtx) + cancelLegacy() + if legacy { + progress("Removing the pre-rename " + harness.CodexLegacyPluginKey + " plugin…") + removeStdout, removeStderr, removeErr := runCodexStep(parent, stderr, codexRemoveTimeout, 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..522d6ef53 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,52 @@ 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", + ) +} + +// 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}) + + envelope := runSetupCodexJSON(t) + + require.NotEmpty(t, envelope.Data.Errors) + assert.Contains(t, envelope.Data.Errors[0], "pre-rename plugin") + assert.Equal(t, []string{"codex plugin remove basecamp@37signals"}, envelope.Data.ManualCommands) +} + +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 +165,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 +200,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 +223,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 +238,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 +282,9 @@ type codexStubOptions struct { pluginAlreadyInstalled bool pluginFailure bool verificationMissing bool + legacyInstalled bool + legacyRemoveFailure bool + pluginDisabled bool } func installCodexStub(t *testing.T, options codexStubOptions) string { @@ -248,6 +296,12 @@ 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)) + } logPath := filepath.Join(home, "codex-calls.log") boolShell := func(value bool) string { if value { @@ -264,12 +318,18 @@ 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" + + " 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\\\":$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/commands/wizard_test.go b/internal/commands/wizard_test.go index 76e3fa19d..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" @@ -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,70 @@ 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 the pre-rename basecamp@37signals plugin +// 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":`+strconv.Quote(cwd)+`}]}}`), 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) + + 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") } // TestSetupClaudeNonInteractiveRefreshesMarketplace verifies the fresh-install @@ -961,7 +1018,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 +1054,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..3d65280a3 100644 --- a/internal/harness/claude.go +++ b/internal/harness/claude.go @@ -40,8 +40,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 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. const ClaudeMarketplaceName = "37signals" @@ -49,6 +51,20 @@ 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. 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 +// here (isStalePluginKey); otherwise setup would uninstall the connector. +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. func DetectClaude() bool { @@ -122,6 +138,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 hasLegacyPluginKey(data) { + 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", @@ -129,6 +153,15 @@ func CheckClaudePlugin() *StatusCheck { } } + if hasLegacyPluginKey(data) { + 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 +369,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 +403,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 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 { @@ -459,11 +495,23 @@ 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. +// 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 { diff --git a/internal/harness/claude_test.go b/internal/harness/claude_test.go index 156c96139..189013f22 100644 --- a/internal/harness/claude_test.go +++ b/internal/harness/claude_test.go @@ -12,7 +12,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 +34,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 +70,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 +85,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 +160,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 +176,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 +187,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 +221,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 +242,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 +252,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 +274,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 +295,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 +318,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 +335,43 @@ func TestCheckClaudePluginVersion_NoFile(t *testing.T) { assert.Equal(t, "pass", check.Status) assert.Contains(t, check.Message, "not tracked") } + +func TestIsStalePluginKey_LegacyName(t *testing.T) { + assert.True(t, isStalePluginKey("basecamp@37signals")) + assert.False(t, isStalePluginKey("basecamp-cli@37signals")) +} + +// 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 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_LegacyNameOnly(t *testing.T) { + home := t.TempDir() + t.Setenv("HOME", home) + 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") +} + +func TestCheckClaudePlugin_BothInstalledWarns(t *testing.T) { + home := t.TempDir() + t.Setenv("HOME", home) + 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, "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 d921ca551..42781f64f 100644 --- a/internal/harness/codex.go +++ b/internal/harness/codex.go @@ -17,12 +17,19 @@ 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 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. 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. 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", @@ -182,10 +201,22 @@ 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", + } + } + 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", } } @@ -270,9 +301,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 { + legacy = true + } + } for _, plugin := range *envelope.Installed { if plugin.PluginID == CodexExpectedPluginKey { + plugin.legacyInstalled = legacy return plugin, true, nil } } @@ -280,11 +318,21 @@ 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 +} + +// 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 c02350650..38ba92cd9 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,44 @@ func boolJSON(value bool) string { } return "false" } + +const codexLegacyInstalledJSON = `{"pluginId":"basecamp@37signals","name":"basecamp","marketplaceName":"37signals","version":"0.11.0","installed":true,"enabled":true}` + +func TestCheckCodexPluginLegacyCLIInstall(t *testing.T) { + 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.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, CodexLegacyReplaced(context.Background())) +} + +func TestCheckCodexPluginBothInstalledWarns(t *testing.T) { + 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") + assert.True(t, CodexLegacyReplaced(context.Background())) +} + +func TestCheckCodexPluginDisabledMentionsLegacyCopy(t *testing.T) { + 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.False(t, CodexLegacyReplaced(context.Background()), "a disabled replacement doesn't replace the old ID") + assert.Contains(t, check.Message, "old basecamp@37signals copy") +} diff --git a/internal/release/manifests_test.go b/internal/release/manifests_test.go index 5cf5a51cd..52c14f726 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 @@ -50,8 +52,8 @@ 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) 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") @@ -141,7 +143,16 @@ 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. + // 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. + 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)