diff --git a/pkg/cmd/agent.go b/pkg/cmd/agent.go index c2454a46..79023ad8 100644 --- a/pkg/cmd/agent.go +++ b/pkg/cmd/agent.go @@ -60,6 +60,7 @@ type agentSetupCmd struct { skillsCheck func(ctx context.Context, destDir string) (*agentskills.DirStatus, error) skillsLocalDir func() (string, error) skillsGlobalDir func() (string, error) + skillsDirsExist func(localDir, globalDir string) bool // callingAgent returns the AI agent invoking this command (e.g. // "claude_code"), or "" for a human shell. Injectable for tests. @@ -72,7 +73,7 @@ type agentSetupCmd struct { type agentSetupJSON struct { Status string `json:"status"` Clients []agentsetup.Status `json:"clients"` - Skills skillsScopesJSON `json:"skills"` + Skills *skillsScopesJSON `json:"skills,omitempty"` Actions []agentsetup.Plan `json:"actions,omitempty"` Errors []string `json:"errors,omitempty"` } @@ -111,6 +112,7 @@ func newAgentSetupCmd() *agentSetupCmd { }, skillsLocalDir: func() (string, error) { return skillsDirUnder(os.Getwd) }, skillsGlobalDir: func() (string, error) { return skillsDirUnder(os.UserHomeDir) }, + skillsDirsExist: skillsDirsExist, callingAgent: func() string { return useragent.DetectAIAgent(os.Getenv) }, isInteractive: isInteractiveTerminal, } @@ -163,24 +165,14 @@ func (asc *agentSetupCmd) runSetup(cmd *cobra.Command, _ []string) error { detected := detectedStatuses(statuses) - var skills skillsScopes - runErr := spinner.New(). - WithLabel("Checking skills..."). - WithOutput(os.Stderr). - Run(func() error { - var e error - skills, e = asc.checkSkillsScopes(ctx) - return e - }) - if runErr != nil { - return runErr - } - - if asc.jsonOutput { - return asc.writeJSON(out, providers, statuses, skills) - } - - if asc.statusOnly { + if asc.jsonOutput || asc.statusOnly { + view, err := asc.skillsStatusView(ctx, detected) + if err != nil { + return err + } + if asc.jsonOutput { + return asc.writeJSON(out, providers, statuses, view) + } if len(detected) > 0 { printStatusTable(out, detected) fmt.Fprintln(out) @@ -188,10 +180,21 @@ func (asc *agentSetupCmd) runSetup(cmd *cobra.Command, _ []string) error { printNothingDetected(out) fmt.Fprintln(out) } - printSkillsStatusTable(out, skills) + if view.show { + printSkillsStatusTable(out, view.scopes, view.allowInstall) + } return nil } + var skills skillsScopes + if asc.needsInteractiveSkills() { + var err error + skills, err = asc.loadSkillsScopes(ctx) + if err != nil { + return err + } + } + sel, scope, err := asc.resolveSelection(cmd, out, detected, skills) if err != nil { return err @@ -203,6 +206,61 @@ func (asc *agentSetupCmd) runSetup(cmd *cobra.Command, _ []string) error { return asc.install(ctx, out, providers, *sel, scope) } +func (asc *agentSetupCmd) loadSkillsScopes(ctx context.Context) (skillsScopes, error) { + var skills skillsScopes + runErr := spinner.New(). + WithLabel("Checking skills..."). + WithOutput(os.Stderr). + Run(func() error { + var e error + skills, e = asc.checkSkillsScopes(ctx) + return e + }) + return skills, runErr +} + +type skillsStatusView struct { + scopes skillsScopes + show bool + allowInstall bool +} + +func (asc *agentSetupCmd) skillsStatusView(ctx context.Context, detected []agentsetup.Status) (skillsStatusView, error) { + if len(detected) == 0 { + scopes, err := asc.loadSkillsScopes(ctx) + return skillsStatusView{scopes: scopes, show: true, allowInstall: true}, err + } + localDir, err := asc.skillsLocalDir() + if err != nil { + return skillsStatusView{}, fmt.Errorf("resolving local skills directory: %w", err) + } + globalDir, err := asc.skillsGlobalDir() + if err != nil { + return skillsStatusView{}, fmt.Errorf("resolving global skills directory: %w", err) + } + if !asc.skillsDirsExist(localDir, globalDir) { + return skillsStatusView{}, nil + } + scopes, err := asc.loadSkillsScopes(ctx) + if err != nil { + return skillsStatusView{}, err + } + if !skillsScopesHasInstalled(scopes) { + return skillsStatusView{}, nil + } + return skillsStatusView{scopes: scopes, show: true}, nil +} + +func (asc *agentSetupCmd) needsInteractiveSkills() bool { + if asc.yes { + return false + } + if asc.callingAgent() != "" { + return false + } + return asc.isInteractive() +} + // resolveSelection decides which agents and/or skills to install. It uses the // interactive TUI when attached to a terminal (and --yes was not passed), // otherwise it derives the selection from flags. @@ -462,17 +520,51 @@ func (asc *agentSetupCmd) checkSkillsScopes(ctx context.Context) (skillsScopes, } local, err := asc.skillsCheck(ctx, localDir) - if err != nil { + if local == nil { return skillsScopes{}, err } global, err := asc.skillsCheck(ctx, globalDir) - if err != nil { + if global == nil { return skillsScopes{}, err } return skillsScopes{Local: *local, Global: *global}, nil } +func skillsScopeCheckFailed(d agentskills.DirStatus) bool { + return d.Status == agentskills.StatusError +} + +func skillsScopeHasInstalled(d agentskills.DirStatus) bool { + switch d.Status { + case agentskills.StatusCurrent, agentskills.StatusOutOfDate: + return true + case agentskills.StatusError: + return d.InstalledCount > 0 + default: + return false + } +} + +func skillsScopesHasInstalled(scopes skillsScopes) bool { + return skillsScopeHasInstalled(scopes.Local) || skillsScopeHasInstalled(scopes.Global) +} + +func skillsDirsExist(localDir, globalDir string) bool { + for _, dir := range []string{localDir, globalDir} { + entries, err := os.ReadDir(dir) + if err != nil { + continue + } + for _, e := range entries { + if e.IsDir() { + return true + } + } + } + return false +} + func skillsScopeNeedsUpdate(d agentskills.DirStatus) bool { return d.Status == agentskills.StatusOutOfDate } @@ -481,11 +573,10 @@ func skillsScopeNeedsInstall(d agentskills.DirStatus) bool { return d.Status == agentskills.StatusNotInstalled } -func (asc *agentSetupCmd) writeJSON(w io.Writer, providers map[string]agentsetup.Provider, statuses []agentsetup.Status, skills skillsScopes) error { +func (asc *agentSetupCmd) writeJSON(w io.Writer, providers map[string]agentsetup.Provider, statuses []agentsetup.Status, view skillsStatusView) error { result := agentSetupJSON{ Status: aggregateStatus(statuses), Clients: statuses, - Skills: skillsScopesJSON(skills), } for _, status := range statuses { if plan := providers[status.Client].Plan(status, asc.force); plan.Action != agentsetup.ActionNone { @@ -495,16 +586,19 @@ func (asc *agentSetupCmd) writeJSON(w io.Writer, providers map[string]agentsetup result.Errors = append(result.Errors, status.Error) } } - if skillsScopeNeedsUpdate(skills.Local) || skillsScopeNeedsUpdate(skills.Global) { - result.Actions = append(result.Actions, agentsetup.Plan{ - Action: "update_skills", - Command: []string{"stripe", "agent", "setup"}, - }) - } else if skillsScopeNeedsInstall(skills.Local) || skillsScopeNeedsInstall(skills.Global) { - result.Actions = append(result.Actions, agentsetup.Plan{ - Action: "install_skills", - Command: []string{"stripe", "agent", "setup"}, - }) + if view.show { + result.Skills = &skillsScopesJSON{Local: view.scopes.Local, Global: view.scopes.Global} + if skillsScopeNeedsUpdate(view.scopes.Local) || skillsScopeNeedsUpdate(view.scopes.Global) { + result.Actions = append(result.Actions, agentsetup.Plan{ + Action: "update_skills", + Command: []string{"stripe", "agent", "setup"}, + }) + } else if view.allowInstall && (skillsScopeNeedsInstall(view.scopes.Local) || skillsScopeNeedsInstall(view.scopes.Global)) { + result.Actions = append(result.Actions, agentsetup.Plan{ + Action: "install_skills", + Command: []string{"stripe", "agent", "setup"}, + }) + } } enc := json.NewEncoder(w) @@ -641,7 +735,7 @@ func printStatusTable(w io.Writer, statuses []agentsetup.Status) { } // printSkillsStatusTable renders the local and global skills install state. -func printSkillsStatusTable(w io.Writer, skills skillsScopes) { +func printSkillsStatusTable(w io.Writer, skills skillsScopes, allowInstall bool) { color := ansi.Color(w) fmt.Fprintln(w, color.Bold("Stripe skills:").String()) @@ -670,7 +764,7 @@ func printSkillsStatusTable(w io.Writer, skills skillsScopes) { switch { case needsUpdate: fmt.Fprintf(w, "\nRun %s to update your Stripe skills.\n", color.Bold("stripe agent setup").String()) - case needsInstall: + case needsInstall && allowInstall: fmt.Fprintf(w, "\nRun %s to install your Stripe skills.\n", color.Bold("stripe agent setup").String()) } } diff --git a/pkg/cmd/agent_setup_tui.go b/pkg/cmd/agent_setup_tui.go index 9da6cafb..cce0ba04 100644 --- a/pkg/cmd/agent_setup_tui.go +++ b/pkg/cmd/agent_setup_tui.go @@ -281,22 +281,30 @@ func scopeStatusHint(status agentskills.DirStatus) string { return "(out of date)" case agentskills.StatusNotInstalled: return "(not installed)" + case agentskills.StatusError: + return "(unavailable — could not check)" default: return "" } } func preferredSkillsScope(skills skillsScopes) string { - if skillsScopeNeedsUpdate(skills.Local) { + if skillsScopeNeedsUpdate(skills.Local) && !skillsScopeCheckFailed(skills.Local) { return skillsScopeLocal } - if skillsScopeNeedsUpdate(skills.Global) { + if skillsScopeNeedsUpdate(skills.Global) && !skillsScopeCheckFailed(skills.Global) { return skillsScopeGlobal } - if skillsScopeNeedsInstall(skills.Local) { + if skillsScopeNeedsInstall(skills.Local) && !skillsScopeCheckFailed(skills.Local) { return skillsScopeLocal } - if skillsScopeNeedsInstall(skills.Global) { + if skillsScopeNeedsInstall(skills.Global) && !skillsScopeCheckFailed(skills.Global) { + return skillsScopeGlobal + } + if !skillsScopeCheckFailed(skills.Local) { + return skillsScopeLocal + } + if !skillsScopeCheckFailed(skills.Global) { return skillsScopeGlobal } return skillsScopeLocal diff --git a/pkg/cmd/agent_setup_tui_test.go b/pkg/cmd/agent_setup_tui_test.go index cb205d88..9dada5f9 100644 --- a/pkg/cmd/agent_setup_tui_test.go +++ b/pkg/cmd/agent_setup_tui_test.go @@ -189,3 +189,32 @@ func TestScopeModel_PrefersOutOfDateLocalScope(t *testing.T) { require.Contains(t, m.labels[0], "(out of date)") require.Contains(t, m.labels[1], "(up to date)") } + +func TestScopeStatusHintError(t *testing.T) { + require.Equal(t, "(unavailable — could not check)", scopeStatusHint(agentskills.DirStatus{ + Status: agentskills.StatusError, + Error: "fetching skills index: request failed", + })) +} + +func TestScopeModel_PrefersScopeWhenOtherScopeCheckFailed(t *testing.T) { + skills := skillsScopes{ + Local: agentskills.DirStatus{Status: agentskills.StatusNotInstalled}, + Global: agentskills.DirStatus{Status: agentskills.StatusError, Error: "fetching skills index: request failed"}, + } + + m := newScopeModel(skills) + + require.Equal(t, skillsScopeLocal, m.options[m.cursor]) + require.Contains(t, m.labels[0], "(not installed)") + require.Contains(t, m.labels[1], "(unavailable — could not check)") +} + +func TestPreferredSkillsScopeSkipsCheckFailedScope(t *testing.T) { + skills := skillsScopes{ + Local: agentskills.DirStatus{Status: agentskills.StatusError, Error: "fetching skills index: request failed"}, + Global: agentskills.DirStatus{Status: agentskills.StatusNotInstalled}, + } + + require.Equal(t, skillsScopeGlobal, preferredSkillsScope(skills)) +} diff --git a/pkg/cmd/agent_test.go b/pkg/cmd/agent_test.go index f29e8e09..8c712a44 100644 --- a/pkg/cmd/agent_test.go +++ b/pkg/cmd/agent_test.go @@ -5,6 +5,7 @@ import ( "encoding/json" "errors" "fmt" + "os" "path/filepath" "testing" @@ -65,12 +66,10 @@ func TestAgentSetupJSONReportsActionWithoutInstalling(t *testing.T) { require.Len(t, result.Clients, 1) require.True(t, result.Clients[0].Detected) require.False(t, result.Clients[0].Plugin.Installed) - require.Len(t, result.Actions, 2) + require.Len(t, result.Actions, 1) require.Equal(t, agentsetup.ActionInstall, result.Actions[0].Action) require.Equal(t, []string{"claude", "plugin", "install", agentsetup.TargetClaudePlugin}, result.Actions[0].Command) - require.Equal(t, "install_skills", result.Actions[1].Action) - require.Equal(t, agentskills.StatusNotInstalled, result.Skills.Local.Status) - require.Equal(t, agentskills.StatusNotInstalled, result.Skills.Global.Status) + require.Nil(t, result.Skills) } func TestAgentSetupJSONShowsUpgradeHintWhenPluginCommandFails(t *testing.T) { @@ -204,6 +203,32 @@ func TestAgentSetupClientFlagLimitsToOne(t *testing.T) { require.Contains(t, output, "1 installed, 0 updated, 0 skipped, 0 errors") } +func TestAgentSetupClientFlagDoesNotCheckSkills(t *testing.T) { + var installed []string + record := func(_ context.Context, name string, args ...string) error { + installed = append(installed, name) + return nil + } + + claude := agentsetup.NewClaudeProvider(claudeMissingPluginScanner(t), record) + codex := codexMissingProvider(record) + + setup := testAgentSetupCmd() + setup.providers = map[string]agentsetup.Provider{claude.ID(): claude, codex.ID(): codex} + setup.callingAgent = func() string { return "" } + setup.skillsCheck = func(context.Context, string) (*agentskills.DirStatus, error) { + t.Fatal("plugin-only setup should not check skills") + return nil, nil + } + setup.cmd.SetContext(context.Background()) + + output, err := executeCommand(setup.cmd, "--client", "codex") + + require.NoError(t, err) + require.Equal(t, []string{"codex"}, installed) + require.Contains(t, output, "1 installed, 0 updated, 0 skipped, 0 errors") +} + func TestAgentSetupAutoInstallsForCallingAgent(t *testing.T) { var installed []string record := func(_ context.Context, name string, args ...string) error { @@ -229,6 +254,32 @@ func TestAgentSetupAutoInstallsForCallingAgent(t *testing.T) { require.Contains(t, output, "1 installed, 0 updated, 0 skipped, 0 errors") } +func TestAgentSetupCallingAgentDoesNotCheckSkills(t *testing.T) { + var installed []string + record := func(_ context.Context, name string, args ...string) error { + installed = append(installed, name) + return nil + } + + claude := agentsetup.NewClaudeProvider(claudeMissingPluginScanner(t), record) + codex := codexMissingProvider(record) + + setup := testAgentSetupCmd() + setup.providers = map[string]agentsetup.Provider{claude.ID(): claude, codex.ID(): codex} + setup.callingAgent = func() string { return "codex_cli" } + setup.skillsCheck = func(context.Context, string) (*agentskills.DirStatus, error) { + t.Fatal("calling-agent plugin setup should not check skills") + return nil, nil + } + setup.cmd.SetContext(context.Background()) + + output, err := executeCommand(setup.cmd) + + require.NoError(t, err) + require.Equal(t, []string{"codex"}, installed) + require.Contains(t, output, "Detected Codex CLI — setting up its Stripe plugin.") +} + // codexMissingProvider returns a Codex provider that detects the binary, starts // with the Stripe plugin not installed, records install commands via record, and // reports the plugin as installed once the add command has run (mirroring the @@ -411,17 +462,43 @@ func TestAgentSetupAgentNeverUsesInteractivePicker(t *testing.T) { require.Contains(t, output, "1 installed, 0 updated, 0 skipped, 0 errors") } -func TestAgentSetupStatusShowsSkillsForBothScopes(t *testing.T) { +func TestAgentSetupStatusOmitsSkillsWhenAgentsDetected(t *testing.T) { setup := newTestAgentSetupCmd(t, claudeMissingPluginScanner(t), nil) output, err := executeCommand(setup.cmd, "--status") + require.NoError(t, err) + require.Contains(t, output, "Claude Code") + require.NotContains(t, output, "Stripe skills:") +} + +func TestAgentSetupStatusShowsSkillsWhenInstalled(t *testing.T) { + localDir, globalDir := testSkillsDirs(t) + setup := newTestAgentSetupCmdInstalled(t, nil) + setup.skillsCheck = mockSkillsCheckOutOfDate + setup.skillsLocalDir = func() (string, error) { return localDir, nil } + setup.skillsGlobalDir = func() (string, error) { return globalDir, nil } + + output, err := executeCommand(setup.cmd, "--status") + require.NoError(t, err) require.Contains(t, output, "Stripe skills:") - require.Contains(t, output, "local") - require.Contains(t, output, "global") - require.Contains(t, output, "not installed") - require.Contains(t, output, "Run stripe agent setup to install your Stripe skills.") + require.Contains(t, output, "out of date") + require.Contains(t, output, "Run stripe agent setup to update your Stripe skills.") + require.NotContains(t, output, "install your Stripe skills") +} + +func TestAgentSetupStatusDoesNotCheckSkillsWhenNoDirs(t *testing.T) { + setup := newTestAgentSetupCmdInstalled(t, nil) + setup.skillsCheck = func(context.Context, string) (*agentskills.DirStatus, error) { + t.Fatal("skills check should be skipped when no local skill dirs exist") + return nil, nil + } + + output, err := executeCommand(setup.cmd, "--status") + + require.NoError(t, err) + require.NotContains(t, output, "Stripe skills:") } func TestAgentSetupStatusWithNoClientsShowsSkills(t *testing.T) { @@ -438,8 +515,11 @@ func TestAgentSetupStatusWithNoClientsShowsSkills(t *testing.T) { } func TestAgentSetupStatusShowsCTAWhenOutOfDate(t *testing.T) { - setup := newTestAgentSetupCmd(t, claudeMissingPluginScanner(t), nil) + setup := testAgentSetupCmd() + setup.providers = map[string]agentsetup.Provider{} + setup.callingAgent = func() string { return "" } setup.skillsCheck = mockSkillsCheckOutOfDate + setup.cmd.SetContext(context.Background()) output, err := executeCommand(setup.cmd, "--status") @@ -539,6 +619,49 @@ func mockSkillsCheckOutOfDate(_ context.Context, destDir string) (*agentskills.D }, nil } +func mockSkillsCheckError(_ context.Context, destDir string) (*agentskills.DirStatus, error) { + err := fmt.Errorf("fetching skills index: request failed") + return &agentskills.DirStatus{ + Dir: destDir, + Status: agentskills.StatusError, + Error: err.Error(), + }, err +} + +func TestAgentSetupStatusWithSkillsCheckError(t *testing.T) { + setup := testAgentSetupCmd() + setup.providers = map[string]agentsetup.Provider{} + setup.callingAgent = func() string { return "" } + setup.skillsCheck = mockSkillsCheckError + setup.cmd.SetContext(context.Background()) + + output, err := executeCommand(setup.cmd, "--status") + + require.NoError(t, err) + require.Contains(t, output, "Stripe skills:") + require.Contains(t, output, "error: fetching skills index: request failed") +} + +func TestAgentSetupJSONWithSkillsCheckError(t *testing.T) { + setup := testAgentSetupCmd() + setup.providers = map[string]agentsetup.Provider{} + setup.callingAgent = func() string { return "" } + setup.skillsCheck = mockSkillsCheckError + setup.cmd.SetContext(context.Background()) + + output, err := executeCommand(setup.cmd, "--json") + + require.NoError(t, err) + + var result agentSetupJSON + require.NoError(t, json.Unmarshal([]byte(output), &result)) + require.NotNil(t, result.Skills) + require.Equal(t, agentskills.StatusError, result.Skills.Local.Status) + require.Equal(t, agentskills.StatusError, result.Skills.Global.Status) + require.Contains(t, result.Skills.Local.Error, "fetching skills index") + require.Empty(t, result.Errors) +} + func testAgentSetupCmd() *agentSetupCmd { setup := newAgentSetupCmd() setup.skillsCheck = mockSkillsCheckNotInstalled @@ -547,6 +670,13 @@ func testAgentSetupCmd() *agentSetupCmd { return setup } +func testSkillsDirs(t *testing.T) (localDir, globalDir string) { + t.Helper() + localRoot := t.TempDir() + require.NoError(t, os.MkdirAll(filepath.Join(localRoot, ".agents", "skills", "stripe-best-practices"), 0755)) + return filepath.Join(localRoot, ".agents", "skills"), filepath.Join(t.TempDir(), ".agents", "skills") +} + func claudeMissingPluginScanner(t *testing.T) agentsetup.Scanner { t.Helper() return agentsetup.Scanner{