Skip to content

Commit 316deca

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 18d616b commit 316deca

4 files changed

Lines changed: 51 additions & 21 deletions

File tree

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
@@ -92,26 +92,6 @@ func fetchSkillFile(ctx context.Context, ref, skillName, filePath string) ([]byt
9292
return io.ReadAll(resp.Body)
9393
}
9494

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

0 commit comments

Comments
 (0)