Skip to content

Commit f2fba9a

Browse files
committed
Fix PR4 review findings: skills install compat, dead code, yellow warning
- Always call SetArgs in skills install backward-compat alias (fixes Execute path inheriting parent args) - Add cobra.MaximumNArgs(1) to reject extra positional args - Remove dead installer.ListSkills function (replaced by list command) - Restore yellow color for "No agents detected" warning in install.go - Add Execute-path tests for skills install alias Co-authored-by: Isaac
1 parent edafa6b commit f2fba9a

File tree

4 files changed

+51
-21
lines changed

4 files changed

+51
-21
lines changed

experimental/aitools/cmd/install.go

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ import (
99
"github.com/databricks/cli/experimental/aitools/lib/agents"
1010
"github.com/databricks/cli/experimental/aitools/lib/installer"
1111
"github.com/databricks/cli/libs/cmdio"
12+
"github.com/fatih/color"
1213
"github.com/spf13/cobra"
1314
)
1415

@@ -111,7 +112,7 @@ func resolveAgentNames(ctx context.Context, names string) ([]*agents.Agent, erro
111112

112113
// printNoAgentsMessage prints the "no agents detected" message.
113114
func printNoAgentsMessage(ctx context.Context) {
114-
cmdio.LogString(ctx, "No supported coding agents detected.")
115+
cmdio.LogString(ctx, color.YellowString("No supported coding agents detected."))
115116
cmdio.LogString(ctx, "")
116117
cmdio.LogString(ctx, "Supported agents: Claude Code, Cursor, Codex CLI, OpenCode, GitHub Copilot, Antigravity")
117118
cmdio.LogString(ctx, "Please install at least one coding agent first.")

experimental/aitools/cmd/install_test.go

Lines changed: 46 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -278,6 +278,52 @@ func TestSkillsInstallForwardsSkillName(t *testing.T) {
278278
assert.Equal(t, []string{"databricks"}, (*calls)[0].opts.SpecificSkills)
279279
}
280280

281+
func TestSkillsInstallExecuteNoArgs(t *testing.T) {
282+
setupTestAgents(t)
283+
calls := setupInstallMock(t)
284+
285+
ctx := cmdio.MockDiscard(t.Context())
286+
cmd := newSkillsInstallCmd()
287+
cmd.SetContext(ctx)
288+
cmd.SetArgs([]string{})
289+
290+
err := cmd.Execute()
291+
require.NoError(t, err)
292+
293+
require.Len(t, *calls, 1)
294+
assert.Len(t, (*calls)[0].agents, 2)
295+
assert.Nil(t, (*calls)[0].opts.SpecificSkills)
296+
}
297+
298+
func TestSkillsInstallExecuteWithSkillName(t *testing.T) {
299+
setupTestAgents(t)
300+
calls := setupInstallMock(t)
301+
302+
ctx := cmdio.MockDiscard(t.Context())
303+
cmd := newSkillsInstallCmd()
304+
cmd.SetContext(ctx)
305+
cmd.SetArgs([]string{"databricks"})
306+
307+
err := cmd.Execute()
308+
require.NoError(t, err)
309+
310+
require.Len(t, *calls, 1)
311+
assert.Equal(t, []string{"databricks"}, (*calls)[0].opts.SpecificSkills)
312+
}
313+
314+
func TestSkillsInstallExecuteRejectsTwoArgs(t *testing.T) {
315+
ctx := cmdio.MockDiscard(t.Context())
316+
cmd := newSkillsInstallCmd()
317+
cmd.SetContext(ctx)
318+
cmd.SetArgs([]string{"a", "b"})
319+
cmd.SilenceErrors = true
320+
cmd.SilenceUsage = true
321+
322+
err := cmd.Execute()
323+
require.Error(t, err)
324+
assert.Contains(t, err.Error(), "accepts at most 1 arg")
325+
}
326+
281327
func TestResolveAgentNamesValid(t *testing.T) {
282328
ctx := t.Context()
283329
result, err := resolveAgentNames(ctx, "claude-code,cursor")

experimental/aitools/cmd/skills.go

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -75,13 +75,16 @@ func newSkillsInstallCmd() *cobra.Command {
7575
return &cobra.Command{
7676
Use: "install [skill-name]",
7777
Short: "Install Databricks skills for detected coding agents",
78+
Args: cobra.MaximumNArgs(1),
7879
RunE: func(cmd *cobra.Command, args []string) error {
7980
// Delegate to the flat install command's logic.
8081
installCmd := newInstallCmd()
8182
installCmd.SetContext(cmd.Context())
8283
if len(args) > 0 {
8384
// Pass the skill name as a --skills flag.
8485
installCmd.SetArgs([]string{"--skills", args[0]})
86+
} else {
87+
installCmd.SetArgs([]string{})
8588
}
8689
return installCmd.Execute()
8790
},

experimental/aitools/lib/installer/installer.go

Lines changed: 0 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -91,26 +91,6 @@ func fetchSkillFile(ctx context.Context, ref, skillName, filePath string) ([]byt
9191
return io.ReadAll(resp.Body)
9292
}
9393

94-
// ListSkills fetches and prints available skills.
95-
func ListSkills(ctx context.Context) error {
96-
manifest, err := FetchManifest(ctx)
97-
if err != nil {
98-
return err
99-
}
100-
101-
cmdio.LogString(ctx, "Available skills:")
102-
cmdio.LogString(ctx, "")
103-
104-
for name, meta := range manifest.Skills {
105-
cmdio.LogString(ctx, fmt.Sprintf(" %s (v%s)", name, meta.Version))
106-
}
107-
108-
cmdio.LogString(ctx, "")
109-
cmdio.LogString(ctx, "Install all with: databricks experimental aitools skills install")
110-
cmdio.LogString(ctx, "Install one with: databricks experimental aitools skills install <skill-name>")
111-
return nil
112-
}
113-
11494
// InstallSkillsForAgents fetches the manifest and installs skills for the given agents.
11595
// This is the core installation function. Callers are responsible for agent detection,
11696
// prompting, and printing the "Installing..." header.

0 commit comments

Comments
 (0)