diff --git a/libs/aitools/agents/agents.go b/libs/aitools/agents/agents.go index 6ad9ebc12f5..bf49ab8d027 100644 --- a/libs/aitools/agents/agents.go +++ b/libs/aitools/agents/agents.go @@ -21,9 +21,13 @@ type PluginSpec struct { // ID is the plugin identifier that is installed/enabled (e.g. "databricks"). ID string // Source is the argument passed to ` plugin marketplace add` - // (e.g. "databricks/databricks-agent-skills"). Empty marks a built-in - // marketplace that must not be added or de-registered. + // (e.g. "databricks/databricks-agent-skills"). Source string + // Shared marks a marketplace we do not own (e.g. Claude's official + // claude-plugins-official). It is still added when missing — an unregistered + // marketplace can't be refreshed or installed from — but is never de-registered + // on uninstall, since other plugins may rely on it. + Shared bool } // Agent defines a supported coding agent. @@ -141,12 +145,12 @@ const ( databricksPluginID = "databricks" databricksPluginSrc = "databricks/databricks-agent-skills" - // claudeOfficialMarketplace is Claude Code's built-in marketplace - // (anthropics/claude-plugins-official), registered by default. The databricks - // plugin is published there, so Claude installs from it and we never register - // our own marketplace for Claude. An empty PluginSpec.Source marks a built-in - // marketplace that must not be added. - claudeOfficialMarketplace = "claude-plugins-official" + // claudeOfficialMarketplace is Claude Code's official marketplace, where the + // databricks plugin is published. It is not reliably registered locally, so the + // CLI adds it (from claudeOfficialMarketplaceSrc) before refreshing and + // installing, but never de-registers it since it is shared, not ours. + claudeOfficialMarketplace = "claude-plugins-official" + claudeOfficialMarketplaceSrc = "anthropics/claude-plugins-official" ) // databricksPlugin returns the shared plugin descriptor for an agent that @@ -160,13 +164,16 @@ func databricksPlugin() *PluginSpec { } // claudePlugin returns Claude's plugin descriptor. Claude installs the databricks -// plugin from its built-in claude-plugins-official marketplace (Source empty), so -// the CLI doesn't register a separate databricks-agent-skills marketplace for it. +// plugin from Claude's official claude-plugins-official marketplace. The CLI adds +// that marketplace before installing (it is not reliably registered locally, and +// an unregistered marketplace can't be refreshed or installed from) but never +// de-registers it, since it is shared infrastructure rather than ours (Shared). func claudePlugin() *PluginSpec { return &PluginSpec{ Marketplace: claudeOfficialMarketplace, ID: databricksPluginID, - Source: "", + Source: claudeOfficialMarketplaceSrc, + Shared: true, } } diff --git a/libs/aitools/installer/plugin.go b/libs/aitools/installer/plugin.go index 13fc9475e31..ca5c44dc041 100644 --- a/libs/aitools/installer/plugin.go +++ b/libs/aitools/installer/plugin.go @@ -247,9 +247,10 @@ func InstallPluginForAgent(ctx context.Context, agent *agents.Agent, nativeScope // On any uncertainty marketplaceRegistered returns true, keeping us off the // de-register path. // - // An empty Source marks a built-in marketplace (e.g. Claude's - // claude-plugins-official): it is already registered, so we never add or - // de-register it. + // A Shared marketplace (e.g. Claude's official claude-plugins-official) is + // still added here when missing — it is not reliably registered locally, and + // an unregistered marketplace can't be refreshed or installed from — but it is + // never de-registered on uninstall (see UninstallPluginForAgent). installedMarketplace := false if agent.Plugin.Source != "" { alreadyPresent := marketplaceRegistered(ctx, bin, agent.Plugin.Marketplace) @@ -331,9 +332,9 @@ func UninstallPluginForAgent(ctx context.Context, agent *agents.Agent, rec Plugi if _, err := runAgentCmd(ctx, pluginCmdTimeout, prepend(bin, pluginUninstallArgs(agent, rec))); err != nil { return &BlockedError{Agent: agent.Name, Reason: ReasonInstallFailed, Detail: stderrOf(err)} } - // Never de-register a built-in marketplace (empty Source, e.g. Claude's - // claude-plugins-official): it is shared infrastructure we did not add. - if rec.InstalledMarketplace && !keepMarketplace && agent.Plugin.Source != "" { + // Never de-register a Shared marketplace (e.g. Claude's claude-plugins-official): + // it is shared infrastructure we don't own, even if we added it when missing. + if rec.InstalledMarketplace && !keepMarketplace && !agent.Plugin.Shared { if _, err := runAgentCmd(ctx, pluginCmdTimeout, prepend(bin, marketplaceRemoveArgsForRecord(agent, rec))); err != nil { log.Warnf(ctx, "Removed the %s plugin but could not de-register its marketplace (remove it manually if needed): %v", agent.DisplayName, stderrOf(err)) } diff --git a/libs/aitools/installer/plugin_test.go b/libs/aitools/installer/plugin_test.go index 2652918c441..4b536e0e187 100644 --- a/libs/aitools/installer/plugin_test.go +++ b/libs/aitools/installer/plugin_test.go @@ -65,29 +65,23 @@ func TestInstallPluginForAgentClaudeSuccess(t *testing.T) { assert.Contains(t, cmds, "claude plugin install databricks@databricks-agent-skills --scope user") } -func TestInstallPluginForAgentBuiltinMarketplace(t *testing.T) { +func TestInstallPluginForAgentSharedMarketplace(t *testing.T) { stubAgentLookPath(t, true) ctx, stub := process.WithStub(t.Context()) stub.WithCallback(func(*exec.Cmd) error { return nil }) - // An agent whose plugin lives in a built-in marketplace (empty Source) like - // Claude's claude-plugins-official: install from it, never register it. - agent := &agents.Agent{ - Name: agents.NameClaudeCode, - DisplayName: "Claude Code", - Binary: "claude", - Plugin: &agents.PluginSpec{Marketplace: "claude-plugins-official", ID: "databricks", Source: ""}, - } + // Claude installs from its official, Shared marketplace (claude-plugins-official). + // It is not reliably registered locally, so the CLI must add it from its source + // before refreshing and installing. Use the real registry spec. + agent := agents.ByName(agents.NameClaudeCode) rec, err := InstallPluginForAgent(ctx, agent, "user", "main") require.NoError(t, err) assert.Equal(t, "claude-plugins-official", rec.Marketplace) - assert.False(t, rec.InstalledMarketplace, "a built-in marketplace is never registered by us") + assert.True(t, rec.InstalledMarketplace, "an absent shared marketplace is added by us") cmds := stub.Commands() - for _, c := range cmds { - assert.NotContains(t, c, "marketplace add", "must not register a built-in marketplace") - } + assert.Contains(t, cmds, "claude plugin marketplace add anthropics/claude-plugins-official") assert.Contains(t, cmds, "claude plugin marketplace update") assert.Contains(t, cmds, "claude plugin install databricks@claude-plugins-official --scope user") } @@ -309,17 +303,17 @@ func TestUninstallSkillsOptsTargetsPluginAgents(t *testing.T) { assert.Contains(t, state.Plugins, agents.NameCopilot) } -func TestUninstallNeverDeregistersBuiltinMarketplace(t *testing.T) { +func TestUninstallNeverDeregistersSharedMarketplace(t *testing.T) { setupTestHome(t) stubAgentLookPath(t, true) ctx, stub := process.WithStub(t.Context()) stub.WithCallback(func(*exec.Cmd) error { return nil }) - ctx = cmdio.MockDiscard(ctx) + ctx, stderr := cmdio.NewTestContextWithStderr(ctx) dir, err := GlobalSkillsDir(ctx) require.NoError(t, err) - // Claude installs from its built-in claude-plugins-official marketplace; even a - // stale InstalledMarketplace=true must never trigger a de-register. + // Claude's claude-plugins-official is a Shared marketplace; even a stale + // InstalledMarketplace=true must never trigger a de-register. require.NoError(t, SaveState(dir, &InstallState{ SchemaVersion: schemaVersionV2, Plugins: map[string]PluginRecord{ @@ -334,6 +328,9 @@ func TestUninstallNeverDeregistersBuiltinMarketplace(t *testing.T) { for _, c := range cmds { assert.NotContains(t, c, "marketplace remove") } + // The message must not claim to have removed the shared marketplace. + assert.Contains(t, stderr.String(), "removed databricks plugin") + assert.NotContains(t, stderr.String(), "+ marketplace") } func TestUninstallKeepMarketplace(t *testing.T) { diff --git a/libs/aitools/installer/uninstall.go b/libs/aitools/installer/uninstall.go index 82079474990..713ec27f39e 100644 --- a/libs/aitools/installer/uninstall.go +++ b/libs/aitools/installer/uninstall.go @@ -88,8 +88,10 @@ func UninstallSkillsOpts(ctx context.Context, opts UninstallOptions) error { } delete(state.Plugins, name) pluginCount++ + // Mirror the de-register condition in UninstallPluginForAgent: a Shared + // marketplace is never removed, so don't claim we did. note := " + marketplace" - if opts.KeepMarketplace || !rec.InstalledMarketplace { + if opts.KeepMarketplace || !rec.InstalledMarketplace || agent.Plugin.Shared { note = "" } cmdio.LogString(ctx, fmt.Sprintf(" %s removed databricks plugin%s", agent.DisplayName, note))