diff --git a/cmd/aitools/install.go b/cmd/aitools/install.go index c6618a43269..033beb79ad9 100644 --- a/cmd/aitools/install.go +++ b/cmd/aitools/install.go @@ -38,6 +38,21 @@ const ( deliverySkip ) +// String returns the delivery name used in `list --output json`, so the install +// plan and the list output name the same thing the same way. +func (d delivery) String() string { + switch d { + case deliveryPlugin: + return "plugin" + case deliverySkills: + return "skills" + case deliverySkip: + return "skip" + default: + return "unknown" + } +} + // agentPlanItem is the resolved plan for one agent: what we'll do and why. type agentPlanItem struct { agent *agents.Agent diff --git a/cmd/aitools/list.go b/cmd/aitools/list.go index b958413787b..f2ee584a996 100644 --- a/cmd/aitools/list.go +++ b/cmd/aitools/list.go @@ -7,6 +7,7 @@ import ( "fmt" "io" "maps" + "os" "slices" "strings" "text/tabwriter" @@ -75,20 +76,40 @@ type listOutput struct { Agents []agentEntry `json:"agents,omitempty"` } -// agentEntry reports per-agent plugin state for `list`. It mirrors skillEntry: -// Installed maps scope -> the plugin recorded in that scope, so a stale scoped -// install stays visible next to an up-to-date one. Managed says whether the CLI -// installs and tracks the plugin. Up-to-date-ness is derived by comparing each -// Installed version against the top-level release, exactly as the skills view -// does, so there is no precomputed cross-scope status to keep in sync. +// agentEntry is one agent in `list --output json`. The JSON carries an entry for +// every registry agent so the extension sees the full set; the text view still +// lists only agents with a plugin install, so these fields are additive. type agentEntry struct { - Name string `json:"name"` - Managed bool `json:"managed"` - Installed map[string]pluginInfo `json:"installed,omitempty"` + Name string `json:"name"` + DisplayName string `json:"display_name"` + // Managed is whether the CLI can install and track a databricks plugin for it. + Managed bool `json:"managed"` + // Detected is the CLI's presence verdict (Agent.IsPreselected), so consumers get + // the same answer the CLI would act on. + Detected bool `json:"detected"` + // Installed maps CLI scope -> the install found there; always present, empty + // when nothing is installed, matching the skills shape. + Installed map[string]installInfo `json:"installed"` } -// pluginInfo is the per-scope plugin record surfaced in list output. -type pluginInfo struct { +// hasPluginInstall reports whether any scope holds a databricks plugin install +// (as opposed to raw skill files). +func (a agentEntry) hasPluginInstall() bool { + for _, info := range a.Installed { + if info.Delivery == deliveryPlugin.String() { + return true + } + } + return false +} + +// installInfo is one agent's install in one CLI scope: how the databricks tools +// were delivered there, and the version recorded for that delivery. +type installInfo struct { + // Delivery is deliveryPlugin ("plugin") when the CLI installed the databricks + // plugin through the agent's own CLI, or deliverySkills ("skills") when raw + // skill files were installed instead. + Delivery string `json:"delivery"` Version string `json:"version,omitempty"` NativeScope string `json:"native_scope,omitempty"` } @@ -194,36 +215,78 @@ func buildListOutput(ctx context.Context, scope string) (listOutput, error) { if projectState != nil { states[installer.ScopeProject] = projectState } - out.Agents = buildAgentEntries(states) + out.Agents = buildAgentEntries(ctx, states) return out, nil } -// buildAgentEntries reports per-agent plugin state: each plugin agent with a -// recorded install (its version per scope). states maps scope -> install state -// and must contain only non-nil states; the caller filters scopes it did not -// load. Status across scopes is left for the renderer (and JSON consumers) to -// derive from the per-scope versions, so no cross-scope record is merged away here. -func buildAgentEntries(states map[string]*installer.InstallState) []agentEntry { - var entries []agentEntry +// buildAgentEntries reports state for every supported agent in the registry: +// detection (binary on PATH and config dir on disk), whether the CLI can manage +// a databricks plugin for it, and the per-scope install found for it. +// states maps CLI scope -> install state and must contain only non-nil states; +// the caller filters scopes it did not load. Status across scopes is left for +// the renderer (and JSON consumers) to derive from the per-scope versions, so no +// cross-scope record is merged away here. +func buildAgentEntries(ctx context.Context, states map[string]*installer.InstallState) []agentEntry { + entries := make([]agentEntry, 0, len(agents.Registry)) for _, a := range agents.Registry { - if a.Plugin == nil { - continue + entry := agentEntry{ + Name: a.Name, + DisplayName: a.DisplayName, + Managed: a.Plugin != nil, + Detected: a.IsPreselected(ctx), } - installed := map[string]pluginInfo{} + // Always emit an installed map, empty when nothing is installed, so the + // JSON shape matches skillEntry ("installed": {}) rather than omitting it. + entry.Installed = map[string]installInfo{} for scope, st := range states { if rec, ok := st.Plugins[a.Name]; ok { - installed[scope] = pluginInfo{Version: rec.Version, NativeScope: rec.Scope} + entry.Installed[scope] = installInfo{ + Delivery: deliveryPlugin.String(), + Version: rec.Version, + NativeScope: rec.Scope, + } + continue + } + // A skills install produces no plugin record, so fall back to the + // agent's own skills dir on disk. This covers both skills-only agents + // (Plugin == nil) and plugin-capable agents installed with + // --skills-only, which are otherwise indistinguishable here. + if agentHasSkillsInScope(ctx, a, scope) { + entry.Installed[scope] = installInfo{ + Delivery: deliverySkills.String(), + Version: installer.DisplaySkillsVersion(st.Release), + } } } - if len(installed) > 0 { - entries = append(entries, agentEntry{Name: a.Name, Managed: true, Installed: installed}) - } + + entries = append(entries, entry) } return entries } +// agentHasSkillsInScope reports whether databricks skills are present in the +// agent's own skills directory for the given CLI scope. It is the on-disk +// install signal for skills installs, which have no plugin record to consult. +func agentHasSkillsInScope(ctx context.Context, a *agents.Agent, scope string) bool { + if scope == installer.ScopeProject { + if !a.SupportsProjectScope { + return false + } + cwd, err := os.Getwd() + if err != nil { + return false + } + return agents.HasDatabricksSkillsIn(a.ProjectSkillsDir(cwd)) + } + dir, err := a.SkillsDir(ctx) + if err != nil { + return false + } + return agents.HasDatabricksSkillsIn(dir) +} + // loadStateForScope returns the install state for the named scope when the // scope filter allows it. excludeScope is the scope value that means "skip // loading this one" (so passing ScopeProject to the global loader skips @@ -266,13 +329,25 @@ func renderListText(ctx context.Context, out listOutput, scope string) { } } - if len(out.Agents) > 0 { + // The text view keeps its original "Plugin installs:" section: only agents with + // a recorded plugin install are shown here, so an agent whose only install is + // raw skills isn't labelled as a plugin (its skills show in the table below). + // The richer per-agent detection state (every registry agent, skills installs + // included) is JSON-only, for the extension. + var pluginInstalls []agentEntry + for _, a := range out.Agents { + if a.hasPluginInstall() { + pluginInstalls = append(pluginInstalls, a) + } + } + + if len(pluginInstalls) > 0 { cmdio.LogString(ctx, "Plugin installs:") cmdio.LogString(ctx, "") var ab strings.Builder atw := tabwriter.NewWriter(&ab, 0, 4, 2, ' ', 0) fmt.Fprintln(atw, " AGENT\tSTATUS") - for _, a := range out.Agents { + for _, a := range pluginInstalls { fmt.Fprintf(atw, " %s\t%s\n", agentDisplayName(a.Name), agentStatusLabel(a, out.Release)) } atw.Flush() @@ -308,12 +383,14 @@ func renderSkillTable(skills []skillEntry, bothScopes bool) string { // agentStatusLabel renders the text-view status for an agent, collapsing the // per-scope plugin records into a single line. A stale scope (version != // release) is surfaced over an up-to-date one so an outdated install is never -// hidden; project is preferred when every scope matches release. +// hidden; project is preferred when every scope matches release. Skills installs +// are skipped: this line reports the plugin, and their versions would otherwise +// decide the status of an agent that also has a plugin in another scope. func agentStatusLabel(a agentEntry, release string) string { version, upToDate := "", true for _, scope := range []string{installer.ScopeProject, installer.ScopeGlobal} { info, ok := a.Installed[scope] - if !ok { + if !ok || info.Delivery != deliveryPlugin.String() { continue } stale := info.Version != release diff --git a/cmd/aitools/list_test.go b/cmd/aitools/list_test.go index c977ac1b5c8..8fbe37a9047 100644 --- a/cmd/aitools/list_test.go +++ b/cmd/aitools/list_test.go @@ -3,11 +3,14 @@ package aitools import ( "bytes" "encoding/json" + "os" + "path/filepath" "strings" "testing" "github.com/databricks/cli/libs/aitools/installer" "github.com/databricks/cli/libs/cmdio" + "github.com/databricks/cli/libs/env" "github.com/spf13/cobra" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" @@ -110,9 +113,17 @@ func TestRenderListJSONWithAgents(t *testing.T) { Summary: map[string]scopeSummary{installer.ScopeGlobal: {Installed: 0, Total: 0}}, Agents: []agentEntry{ { - Name: "claude-code", - Managed: true, - Installed: map[string]pluginInfo{installer.ScopeGlobal: {Version: "0.2.6"}}, + Name: "claude-code", + DisplayName: "Claude Code", + Managed: true, + Detected: true, + Installed: map[string]installInfo{installer.ScopeGlobal: {Delivery: "plugin", Version: "0.2.6"}}, + }, + { + Name: "cursor", + DisplayName: "Cursor", + Managed: false, + Installed: map[string]installInfo{}, }, }, } @@ -128,16 +139,28 @@ func TestRenderListJSONWithAgents(t *testing.T) { assert.Contains(t, raw, "summary") agentsRaw := raw["agents"].([]any) - require.Len(t, agentsRaw, 1) + require.Len(t, agentsRaw, 2) first := agentsRaw[0].(map[string]any) assert.Equal(t, "claude-code", first["name"]) + assert.Equal(t, "Claude Code", first["display_name"]) assert.Equal(t, true, first["managed"]) + assert.Equal(t, true, first["detected"]) installed := first["installed"].(map[string]any) global := installed["global"].(map[string]any) assert.Equal(t, "0.2.6", global["version"]) + assert.Equal(t, "plugin", global["delivery"]) + + // A not-installed agent emits "installed": {} rather than omitting the key, + // matching the skills shape. + second := agentsRaw[1].(map[string]any) + assert.Contains(t, second, "installed") + assert.Empty(t, second["installed"]) } func TestBuildAgentEntries(t *testing.T) { + // Isolate HOME so the skills-only on-disk detection doesn't pick up the + // developer's real agent skills dirs. + ctx := env.WithUserHomeDir(t.Context(), t.TempDir()) globalState := &installer.InstallState{ Plugins: map[string]installer.PluginRecord{ "claude-code": {Plugin: "databricks", Version: "0.2.6"}, @@ -145,7 +168,7 @@ func TestBuildAgentEntries(t *testing.T) { }, } - entries := buildAgentEntries(map[string]*installer.InstallState{ + entries := buildAgentEntries(ctx, map[string]*installer.InstallState{ installer.ScopeGlobal: globalState, }) byName := map[string]agentEntry{} @@ -155,6 +178,7 @@ func TestBuildAgentEntries(t *testing.T) { require.Contains(t, byName, "claude-code") assert.True(t, byName["claude-code"].Managed) + assert.Equal(t, "Claude Code", byName["claude-code"].DisplayName) assert.Equal(t, "0.2.6", byName["claude-code"].Installed[installer.ScopeGlobal].Version) assert.Equal(t, "databricks plugin · v0.2.6 · up to date", agentStatusLabel(byName["claude-code"], "0.2.6")) @@ -163,8 +187,118 @@ func TestBuildAgentEntries(t *testing.T) { assert.Equal(t, "0.2.5", byName["codex"].Installed[installer.ScopeGlobal].Version) assert.Equal(t, "databricks plugin · v0.2.5 · update available", agentStatusLabel(byName["codex"], "0.2.6")) - // Cursor has no plugin, so it never appears as a plugin agent entry. - assert.NotContains(t, byName, "cursor") + // The JSON output carries an entry for every registry agent, including + // skills-only agents like Cursor and managed agents with no recorded install. + require.Contains(t, byName, "cursor") + assert.False(t, byName["cursor"].Managed) + assert.Empty(t, byName["cursor"].Installed) + + require.Contains(t, byName, "copilot") + assert.True(t, byName["copilot"].Managed) + assert.Empty(t, byName["copilot"].Installed) +} + +func TestBuildAgentEntriesReportsSkillsOnlyAgentFromDisk(t *testing.T) { + // Skills-only agents (Plugin == nil) never get a plugin record; their skills + // are symlinked into the agent's own skills dir. The install must still be + // reported from disk, versioned by the scope's recorded release. + home := t.TempDir() + ctx := env.WithUserHomeDir(t.Context(), home) + // Pin XDG_CONFIG_HOME so OpenCode's config dir resolves under the temp home + // rather than the developer's real ~/.config. + ctx = env.Set(ctx, "XDG_CONFIG_HOME", filepath.Join(home, ".config")) + + // OpenCode is skills-only; its global skills dir is $XDG_CONFIG_HOME/opencode/skills. + // The CLI installs skills there as symlinks to the canonical store, so build + // the symlink to exercise the real on-disk shape. + canonical := filepath.Join(home, ".databricks", "aitools", "skills", "databricks-jobs") + require.NoError(t, os.MkdirAll(canonical, 0o755)) + skillsDir := filepath.Join(home, ".config", "opencode", "skills") + require.NoError(t, os.MkdirAll(skillsDir, 0o755)) + require.NoError(t, os.Symlink(canonical, filepath.Join(skillsDir, "databricks-jobs"))) + + globalState := &installer.InstallState{Release: "0.2.6"} + entries := buildAgentEntries(ctx, map[string]*installer.InstallState{ + installer.ScopeGlobal: globalState, + }) + byName := map[string]agentEntry{} + for _, e := range entries { + byName[e.Name] = e + } + + require.Contains(t, byName, "opencode") + opencode := byName["opencode"] + assert.False(t, opencode.Managed) + assert.Equal(t, "0.2.6", opencode.Installed[installer.ScopeGlobal].Version) + assert.Equal(t, "skills", opencode.Installed[installer.ScopeGlobal].Delivery) + + // A skills-only agent with nothing on disk stays empty. + require.Contains(t, byName, "cursor") + assert.Empty(t, byName["cursor"].Installed) +} + +func TestBuildAgentEntriesReportsSkillsOnlyInstallForPluginAgent(t *testing.T) { + // `install --skills-only` gives a plugin-capable agent skills on disk and no + // plugin record, so the on-disk fallback must fire for it too rather than + // reporting it as not installed. + home := t.TempDir() + ctx := env.WithUserHomeDir(t.Context(), home) + + canonical := filepath.Join(home, ".databricks", "aitools", "skills", "databricks-jobs") + require.NoError(t, os.MkdirAll(canonical, 0o755)) + skillsDir := filepath.Join(home, ".claude", "skills") + require.NoError(t, os.MkdirAll(skillsDir, 0o755)) + require.NoError(t, os.Symlink(canonical, filepath.Join(skillsDir, "databricks-jobs"))) + + entries := buildAgentEntries(ctx, map[string]*installer.InstallState{ + installer.ScopeGlobal: {Release: "0.2.6"}, + }) + byName := map[string]agentEntry{} + for _, e := range entries { + byName[e.Name] = e + } + + require.Contains(t, byName, "claude-code") + cc := byName["claude-code"] + assert.True(t, cc.Managed) + assert.Equal(t, "skills", cc.Installed[installer.ScopeGlobal].Delivery) + assert.Equal(t, "0.2.6", cc.Installed[installer.ScopeGlobal].Version) + + // A skills install is not a plugin install, so the text view's "Plugin + // installs:" section must not claim one. + assert.False(t, cc.hasPluginInstall()) +} + +func TestBuildAgentEntriesPrefersPluginRecordOverSkillsOnDisk(t *testing.T) { + // An agent can have both a plugin record and leftover skills on disk (e.g. a + // --skills-only install later replaced by the plugin). The recorded plugin is + // authoritative for the scope. + home := t.TempDir() + ctx := env.WithUserHomeDir(t.Context(), home) + + canonical := filepath.Join(home, ".databricks", "aitools", "skills", "databricks-jobs") + require.NoError(t, os.MkdirAll(canonical, 0o755)) + skillsDir := filepath.Join(home, ".claude", "skills") + require.NoError(t, os.MkdirAll(skillsDir, 0o755)) + require.NoError(t, os.Symlink(canonical, filepath.Join(skillsDir, "databricks-jobs"))) + + entries := buildAgentEntries(ctx, map[string]*installer.InstallState{ + installer.ScopeGlobal: { + Release: "0.2.6", + Plugins: map[string]installer.PluginRecord{ + "claude-code": {Plugin: "databricks", Version: "0.2.5", Scope: "user"}, + }, + }, + }) + byName := map[string]agentEntry{} + for _, e := range entries { + byName[e.Name] = e + } + + cc := byName["claude-code"] + assert.Equal(t, "plugin", cc.Installed[installer.ScopeGlobal].Delivery) + assert.Equal(t, "0.2.5", cc.Installed[installer.ScopeGlobal].Version) + assert.Equal(t, "user", cc.Installed[installer.ScopeGlobal].NativeScope) } func TestBuildAgentEntriesRecordsPerScopeVersions(t *testing.T) { @@ -177,7 +311,7 @@ func TestBuildAgentEntriesRecordsPerScopeVersions(t *testing.T) { "claude-code": {Plugin: "databricks", Version: "0.2.5"}, }} - entries := buildAgentEntries(map[string]*installer.InstallState{ + entries := buildAgentEntries(t.Context(), map[string]*installer.InstallState{ installer.ScopeGlobal: globalState, installer.ScopeProject: projectState, }) @@ -385,9 +519,16 @@ func TestRenderListTextShowsPluginInstallsBeforeRawSkills(t *testing.T) { }, Agents: []agentEntry{ { - Name: "claude-code", - Managed: true, - Installed: map[string]pluginInfo{installer.ScopeGlobal: {Version: "0.2.6", NativeScope: "user"}}, + Name: "claude-code", + DisplayName: "Claude Code", + Managed: true, + Detected: true, + Installed: map[string]installInfo{installer.ScopeGlobal: {Delivery: "plugin", Version: "0.2.6", NativeScope: "user"}}, + }, + { + Name: "cursor", + DisplayName: "Cursor", + Managed: false, }, }, } @@ -402,6 +543,9 @@ func TestRenderListTextShowsPluginInstallsBeforeRawSkills(t *testing.T) { assert.Less(t, pluginIdx, rawIdx) assert.Contains(t, got, "Claude Code") assert.Contains(t, got, "databricks plugin · v0.2.6 · up to date") + // Skills-only agents (and any without a recorded install) are JSON-only and + // must not appear in the text "Plugin installs:" section. + assert.NotContains(t, got, "Cursor") assert.Contains(t, got, "0/1 raw skill directories installed (global)") } diff --git a/libs/aitools/agents/skills.go b/libs/aitools/agents/skills.go index 522cee6d991..b575f825c2b 100644 --- a/libs/aitools/agents/skills.go +++ b/libs/aitools/agents/skills.go @@ -38,13 +38,20 @@ func HasDatabricksSkillsInstalled(ctx context.Context) bool { } // HasDatabricksSkillsIn checks if dir contains a subdirectory starting with "databricks". +// The CLI installs skills into an agent's skills dir as symlinks to the canonical +// store, and os.ReadDir reports symlinks via Lstat (so IsDir is false for them), so +// entries are resolved with os.Stat to follow the link to its target. func HasDatabricksSkillsIn(dir string) bool { entries, err := os.ReadDir(dir) if err != nil { return false } for _, e := range entries { - if e.IsDir() && strings.HasPrefix(e.Name(), databricksSkillPrefix) { + if !strings.HasPrefix(e.Name(), databricksSkillPrefix) { + continue + } + info, err := os.Stat(filepath.Join(dir, e.Name())) + if err == nil && info.IsDir() { return true } } diff --git a/libs/aitools/agents/skills_test.go b/libs/aitools/agents/skills_test.go index a3366e95ca8..4f58438b8b1 100644 --- a/libs/aitools/agents/skills_test.go +++ b/libs/aitools/agents/skills_test.go @@ -141,6 +141,30 @@ func TestHasDatabricksSkillsInstalledDatabricksAppsCanonical(t *testing.T) { assert.True(t, HasDatabricksSkillsInstalled(t.Context())) } +func TestHasDatabricksSkillsInFollowsSymlinks(t *testing.T) { + // The CLI installs skills into an agent's dir as symlinks to the canonical + // store, so the check must follow the link rather than rely on IsDir (which + // os.ReadDir reports false for a symlink). + tmp := t.TempDir() + target := filepath.Join(tmp, "store", "databricks-jobs") + require.NoError(t, os.MkdirAll(target, 0o755)) + + skillsDir := filepath.Join(tmp, "skills") + require.NoError(t, os.MkdirAll(skillsDir, 0o755)) + require.NoError(t, os.Symlink(target, filepath.Join(skillsDir, "databricks-jobs"))) + + assert.True(t, HasDatabricksSkillsIn(skillsDir)) +} + +func TestHasDatabricksSkillsInIgnoresDanglingSymlink(t *testing.T) { + tmp := t.TempDir() + skillsDir := filepath.Join(tmp, "skills") + require.NoError(t, os.MkdirAll(skillsDir, 0o755)) + require.NoError(t, os.Symlink(filepath.Join(tmp, "missing"), filepath.Join(skillsDir, "databricks-jobs"))) + + assert.False(t, HasDatabricksSkillsIn(skillsDir)) +} + func TestHasDatabricksSkillsInstalledLegacyPath(t *testing.T) { tmpHome := t.TempDir() t.Setenv("HOME", tmpHome)