From 1119b75651faf7445ef8964d16d19ef55ebce13a Mon Sep 17 00:00:00 2001 From: Nathan Cochran Date: Wed, 7 Oct 2026 11:01:09 -0400 Subject: [PATCH 01/17] Add 'tiger skills install' command --- CLAUDE.md | 2 +- README.md | 24 ++ docs/cli/tiger.md | 1 + docs/cli/tiger_skills.md | 32 ++ docs/cli/tiger_skills_install.md | 66 ++++ docs/development.md | 2 +- internal/cmd/client_helper.go | 353 ++++++++++++++++++++ internal/cmd/client_helper_test.go | 57 ++++ internal/cmd/mcp_install.go | 320 +----------------- internal/cmd/mcp_install_test.go | 54 --- internal/cmd/root.go | 1 + internal/cmd/skills.go | 22 ++ internal/cmd/skills_install.go | 488 ++++++++++++++++++++++++++++ internal/cmd/skills_install_test.go | 428 ++++++++++++++++++++++++ 14 files changed, 1475 insertions(+), 375 deletions(-) create mode 100644 docs/cli/tiger_skills.md create mode 100644 docs/cli/tiger_skills_install.md create mode 100644 internal/cmd/client_helper.go create mode 100644 internal/cmd/client_helper_test.go create mode 100644 internal/cmd/skills.go create mode 100644 internal/cmd/skills_install.go create mode 100644 internal/cmd/skills_install_test.go diff --git a/CLAUDE.md b/CLAUDE.md index 8355e70e..017d567e 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -20,7 +20,7 @@ Use the official product names in all user-facing text (documentation, code comm - **`cmd/tiger/`** - The main CLI binary. Sets up signal handling (SIGINT/SIGTERM cancel the command context), delegates to `cmd.Execute()`, and maps a `common.ExitCodeError` found anywhere in the returned error chain to the process exit code. - **`cmd/generate-docs/`** - Renders the CLI reference in `docs/cli/` from the command tree. Runs under `go generate` (see [CLI Reference Docs](#cli-reference-docs)). - **`internal/`** - All core application logic (non-public Go packages). - - **`internal/cmd/`** - Cobra command implementations for all CLI commands (auth, project, service, db, config, mcp, feedback, version, upgrade, completion). Each command lives in its own file, named to match the command in snake_case (see [Command Architecture](#command-architecture)). `root.go` holds the root command, global flags, and command tree construction; files ending in `_helper.go` contain cross-group helpers rather than commands. + - **`internal/cmd/`** - Cobra command implementations for all CLI commands (auth, project, service, db, config, mcp, skills, feedback, version, upgrade, completion). Each command lives in its own file, named to match the command in snake_case (see [Command Architecture](#command-architecture)). `root.go` holds the root command, global flags, and command tree construction; files ending in `_helper.go` contain cross-group helpers rather than commands. - **`internal/api/`** - API client layer. Includes the OpenAPI-generated REST client (`client.go`, `types.go`), the shared HTTP client (`client_util.go`), and a generated mock of the client interface for testing (`mocks/`). **Never edit the generated files by hand** — see [Code Generation](#code-generation). - **`internal/config/`** - Configuration management. Handles config file loading (via Viper), the `Config` struct and its write helpers, and credential storage (keyring with a file fallback in the config directory). - **`internal/common/`** - Shared business logic used by both CLI commands and MCP tools: the per-invocation `App` (config + API client), database password generation and storage (keyring, pgpass), wait/polling operations and the spinner, connection detail helpers (including read replicas), service readiness checks, query execution, log fetching, and error handling with exit codes. diff --git a/README.md b/README.md index d63d2ec7..1a31067b 100644 --- a/README.md +++ b/README.md @@ -92,6 +92,9 @@ tiger db query -c "SELECT now()" # Install the MCP server tiger mcp install + +# Install agent skills for your coding agent +tiger skills install ``` ## Usage @@ -135,6 +138,8 @@ Tiger CLI provides the following commands: - `start` - Start the MCP server - `list` - List available MCP tools, prompts, and resources (alias: `ls`) - `get` - Get detailed information about a specific MCP capability (aliases: `describe`, `show`) +- `tiger skills` - Agent skills for AI coding agents + - `install` - Install PostgreSQL and TimescaleDB skills for an AI coding agent (alias: `add`) - `tiger feedback` - Submit feedback or a bug report to the Tiger Data team (pass the message as an argument or on stdin) - `tiger version` - Show version information - `tiger upgrade` - Upgrade the Tiger CLI to the latest version (alias: `update`) @@ -247,6 +252,25 @@ To disable the documentation proxy: tiger config set docs_mcp false ``` +## Agent Skills + +Tiger CLI can install the curated PostgreSQL and TimescaleDB skills from +[pg-aiguide](https://github.com/timescale/pg-aiguide) — the same skills the +MCP server proxies — directly into your coding agent: + +```bash +# Interactive installation (prompts for client selection) +tiger skills install + +# Or specify your client directly +tiger skills install claude-code +``` + +Skills are installed for the current user into `~/.agents/skills`, which most +coding agents read. For clients that use their own skills directory (Claude +Code, Kiro CLI, Google Antigravity), each skill is symlinked into that +directory. Re-run the command to update the skills to the latest version. + ## Configuration The CLI stores configuration in `~/.config/tiger/config.yaml` by default, and supports hierarchical configuration through environment variables and command-line flags. diff --git a/docs/cli/tiger.md b/docs/cli/tiger.md index d79573f3..474b6276 100644 --- a/docs/cli/tiger.md +++ b/docs/cli/tiger.md @@ -34,5 +34,6 @@ To get started, run: * [tiger mcp](tiger_mcp.md) - Tiger Model Context Protocol (MCP) server * [tiger project](tiger_project.md) - Manage Tiger Cloud projects * [tiger service](tiger_service.md) - Manage database services +* [tiger skills](tiger_skills.md) - Manage agent skills for AI coding agents * [tiger upgrade](tiger_upgrade.md) - Upgrade the Tiger CLI to the latest version * [tiger version](tiger_version.md) - Show version information diff --git a/docs/cli/tiger_skills.md b/docs/cli/tiger_skills.md new file mode 100644 index 00000000..abaeeba6 --- /dev/null +++ b/docs/cli/tiger_skills.md @@ -0,0 +1,32 @@ +## tiger skills + +Manage agent skills for AI coding agents + +### Synopsis + +Manage agent skills for AI coding agents. + +Skills give AI coding agents curated knowledge and best practices for working +with PostgreSQL, TimescaleDB, and Tiger Cloud. + +### Options + +``` + -h, --help help for skills +``` + +### Options inherited from parent commands + +``` + --analytics enable/disable usage analytics (default true) + --color enable colored output (default true) + --config-dir string config directory (default "~/.config/tiger") + --password-storage string password storage method (keyring, pgpass, none) (default "keyring") + --service-id string service ID + --version-check check for updates on startup (default true) +``` + +### SEE ALSO + +* [tiger](tiger.md) - Tiger CLI - Tiger Cloud Platform command-line interface +* [tiger skills install](tiger_skills_install.md) - Install agent skills for an AI coding agent diff --git a/docs/cli/tiger_skills_install.md b/docs/cli/tiger_skills_install.md new file mode 100644 index 00000000..370b2809 --- /dev/null +++ b/docs/cli/tiger_skills_install.md @@ -0,0 +1,66 @@ +## tiger skills install + +Install agent skills for an AI coding agent + +### Synopsis + +Install agent skills for an AI coding agent. + +Skills are downloaded from https://github.com/timescale/pg-aiguide and installed +for the current user into ~/.agents/skills, which most coding agents read. For +clients that read skills from their own directory, each skill is symlinked +into that directory, so every client shares a single copy. + +Existing skills with the same names are replaced, so re-running the command +updates the installed skills to the latest version. + +Supported Clients: + claude-code Claude Code (~/.claude/skills) + cursor Cursor (~/.agents/skills) + devin Devin (~/.agents/skills) + codex Codex (~/.agents/skills) + gemini Gemini CLI (~/.agents/skills) + vscode VS Code (~/.agents/skills) + antigravity Google Antigravity (~/.gemini/antigravity/skills) + kiro-cli Kiro CLI (~/.kiro/skills) + copilot GitHub Copilot CLI (~/.agents/skills) + +If no client is specified, you'll be prompted to select one interactively. + +``` +tiger skills install [client] [flags] +``` + +### Examples + +``` + # Interactive client selection + tiger skills install + + # Install for Claude Code + tiger skills install claude-code + + # Install for Codex + tiger skills install codex +``` + +### Options + +``` + -h, --help help for install +``` + +### Options inherited from parent commands + +``` + --analytics enable/disable usage analytics (default true) + --color enable colored output (default true) + --config-dir string config directory (default "~/.config/tiger") + --password-storage string password storage method (keyring, pgpass, none) (default "keyring") + --service-id string service ID + --version-check check for updates on startup (default true) +``` + +### SEE ALSO + +* [tiger skills](tiger_skills.md) - Manage agent skills for AI coding agents diff --git a/docs/development.md b/docs/development.md index 228828d6..e5eeeae0 100644 --- a/docs/development.md +++ b/docs/development.md @@ -133,7 +133,7 @@ Tiger CLI is a Go-based command-line interface for managing Tiger resources. The - **Entry Point**: `cmd/tiger/main.go` - Simple main that delegates to cmd.Execute() - **Command Structure**: `internal/cmd/` - Cobra-based command definitions for all - CLI commands (auth, project, service, db, config, mcp, version, upgrade). Each command + CLI commands (auth, project, service, db, config, mcp, skills, version, upgrade). Each command lives in its own file, named to match the command in snake_case (`tiger service create` → `service_create.go`). `root.go` holds the root command, global flags, and `wrapCommands`, which gives every command the same diff --git a/internal/cmd/client_helper.go b/internal/cmd/client_helper.go new file mode 100644 index 00000000..de922ebc --- /dev/null +++ b/internal/cmd/client_helper.go @@ -0,0 +1,353 @@ +package cmd + +import ( + "encoding/json" + "fmt" + "path/filepath" + "sort" + "strconv" + "strings" + + tea "charm.land/bubbletea/v2" + "github.com/spf13/cobra" +) + +// MCPClient represents our internal client types +type MCPClient string + +const ( + ClaudeCode MCPClient = "claude-code" + Cursor MCPClient = "cursor" // Both the IDE and the CLI + Devin MCPClient = "devin" + Codex MCPClient = "codex" + Gemini MCPClient = "gemini" + VSCode MCPClient = "vscode" + Antigravity MCPClient = "antigravity" + KiroCLI MCPClient = "kiro-cli" + Copilot MCPClient = "copilot" // Both the IDE and the CLI +) + +// clientConfig represents a supported client, for both MCP server and skills +// installation +type clientConfig struct { + ClientType MCPClient // Our internal client type + Name string + EditorNames []string // Supported client names for this client + MCPServersPathPrefix string // JSON path prefix for MCP servers config (only for JSON config manipulation clients like Cursor) + ConfigPaths []string // Config file locations - used for backup on all clients, and for JSON manipulation on JSON-config clients + // buildInstallCommand builds the CLI install command for CLI-based clients + // Parameters: serverName (name to register), command (binary path), args (arguments to binary) + buildInstallCommand func(serverName, command string, args []string) ([]string, error) + // skillsDir returns the user-level skills directory for clients that don't + // read skills from ~/.agents/skills, given the home directory and an env + // var lookup. Nil for clients that do. + skillsDir func(home string, getenv func(string) string) string +} + +// BuildInstallCommand constructs the install command with the given parameters +func (c *clientConfig) BuildInstallCommand(serverName, command string, args []string) ([]string, error) { + if c.buildInstallCommand == nil { + return nil, nil + } + return c.buildInstallCommand(serverName, command, args) +} + +// supportedClients defines the clients we support for Tiger MCP installation +// Note: A good place to find the json config location for MCPServersPathPrefix +// is in the supportedClientIntegrations map found in: +// https://github.com/stacklok/toolhive/blob/main/pkg/client/config.go +var supportedClients = []clientConfig{ + { + ClientType: ClaudeCode, + Name: "Claude Code", + EditorNames: []string{"claude-code"}, + ConfigPaths: []string{ + "~/.claude.json", + }, + buildInstallCommand: func(serverName, command string, args []string) ([]string, error) { + return append([]string{"claude", "mcp", "add", "-s", "user", serverName, command}, args...), nil + }, + skillsDir: func(home string, getenv func(string) string) string { + return filepath.Join(envOr(getenv, "CLAUDE_CONFIG_DIR", filepath.Join(home, ".claude")), "skills") + }, + }, + { + ClientType: Cursor, + Name: "Cursor", + EditorNames: []string{"cursor"}, + MCPServersPathPrefix: "/mcpServers", + ConfigPaths: []string{ + "~/.cursor/mcp.json", + }, + }, + { + ClientType: Devin, + Name: "Devin", + EditorNames: []string{"devin"}, + ConfigPaths: []string{ + "~/.config/devin/mcp_config.json", + }, + buildInstallCommand: func(serverName, command string, args []string) ([]string, error) { + return append([]string{"devin", "mcp", "add", "-s", "user", serverName, "--", command}, args...), nil + }, + }, + { + ClientType: Codex, + Name: "Codex", + EditorNames: []string{"codex"}, + ConfigPaths: []string{ + "~/.codex/config.toml", + "$CODEX_HOME/config.toml", + }, + buildInstallCommand: func(serverName, command string, args []string) ([]string, error) { + return append([]string{"codex", "mcp", "add", serverName, command}, args...), nil + }, + }, + { + ClientType: Gemini, + Name: "Gemini CLI", + EditorNames: []string{"gemini", "gemini-cli"}, + ConfigPaths: []string{ + "~/.gemini/settings.json", + }, + buildInstallCommand: func(serverName, command string, args []string) ([]string, error) { + return append([]string{"gemini", "mcp", "add", "-s", "user", serverName, command}, args...), nil + }, + }, + { + ClientType: VSCode, + Name: "VS Code", + EditorNames: []string{"vscode", "code", "vs-code"}, + ConfigPaths: []string{ + "~/.config/Code/User/mcp.json", + "~/Library/Application Support/Code/User/mcp.json", + "~/AppData/Roaming/Code/User/mcp.json", + }, + buildInstallCommand: func(serverName, command string, args []string) ([]string, error) { + j, err := json.Marshal(map[string]any{ + "name": serverName, + "command": command, + "args": args, + }) + if err != nil { + return nil, fmt.Errorf("failed to marshal MCP config: %w", err) + } + return []string{"code", "--add-mcp", string(j)}, nil + }, + }, + { + ClientType: Antigravity, + Name: "Google Antigravity", + EditorNames: []string{"antigravity", "agy"}, + MCPServersPathPrefix: "/mcpServers", + ConfigPaths: []string{ + "~/.gemini/antigravity/mcp_config.json", + }, + skillsDir: func(home string, getenv func(string) string) string { + return filepath.Join(home, ".gemini", "antigravity", "skills") + }, + }, + { + ClientType: KiroCLI, + Name: "Kiro CLI", + EditorNames: []string{"kiro-cli"}, + ConfigPaths: []string{ + "~/.kiro/settings/mcp.json", + }, + buildInstallCommand: func(serverName, command string, args []string) ([]string, error) { + return []string{"kiro-cli", "mcp", "add", "--name", serverName, "--command", command, "--args", strings.Join(args, ",")}, nil + }, + skillsDir: func(home string, getenv func(string) string) string { + return filepath.Join(envOr(getenv, "KIRO_HOME", filepath.Join(home, ".kiro")), "skills") + }, + }, + { + ClientType: Copilot, + Name: "GitHub Copilot CLI", + EditorNames: []string{"copilot", "copilot-cli"}, + ConfigPaths: []string{ + "~/.copilot/mcp-config.json", + }, + buildInstallCommand: func(serverName, command string, args []string) ([]string, error) { + return append([]string{"copilot", "mcp", "add", serverName, "--", command}, args...), nil + }, + }, +} + +// getValidEditorNames returns all valid client names from supportedClients +func getValidEditorNames() []string { + var validNames []string + for _, client := range supportedClients { + validNames = append(validNames, client.EditorNames...) + } + return validNames +} + +// findClientConfig finds the client configuration for a given client name +// This consolidates the logic of mapping client names to client types and finding the config +func findClientConfig(clientName string) (*clientConfig, error) { + normalizedName := strings.ToLower(clientName) + + // Look up in our supported clients config + for i := range supportedClients { + for _, name := range supportedClients[i].EditorNames { + if strings.ToLower(name) == normalizedName { + return &supportedClients[i], nil + } + } + } + + // Build list of supported clients from our config for error message + supportedNames := getValidEditorNames() + + return nil, fmt.Errorf("unsupported client: %s. Supported clients: %s", clientName, strings.Join(supportedNames, ", ")) +} + +// ClientOption represents a client choice for interactive selection +type ClientOption struct { + Name string // Display name + ClientName string // Client name to pass to installMCPForClient +} + +// selectClientInteractively prompts the user to select a client using Bubble Tea +func selectClientInteractively(cmd *cobra.Command, title string) (string, error) { + // Build client options from supportedClients + var options []ClientOption + for _, cfg := range supportedClients { + // Use the first client name as the primary identifier + primaryName := cfg.EditorNames[0] + options = append(options, ClientOption{ + Name: cfg.Name, + ClientName: primaryName, + }) + } + + // Sort options alphabetically by name + sort.Slice(options, func(i, j int) bool { + return options[i].Name < options[j].Name + }) + + model := clientSelectModel{ + title: title, + options: options, + cursor: 0, + } + + program := tea.NewProgram(model, + tea.WithInput(cmd.InOrStdin()), + tea.WithOutput(cmd.ErrOrStderr()), + tea.WithContext(cmd.Context()), + tea.WithoutSignalHandler()) + finalModel, err := program.Run() + if err != nil { + return "", fmt.Errorf("failed to run editor selection: %w", err) + } + + result := finalModel.(clientSelectModel) + if result.selected == "" { + return "", fmt.Errorf("no editor selected") + } + + return result.selected, nil +} + +// clientSelectModel represents the Bubble Tea model for client selection +type clientSelectModel struct { + title string + options []ClientOption + cursor int + selected string + numberBuffer string +} + +func (m clientSelectModel) Init() tea.Cmd { + return nil +} + +func (m clientSelectModel) Update(msg tea.Msg) (tea.Model, tea.Cmd) { + switch msg := msg.(type) { + case tea.KeyPressMsg: + switch msg.String() { + case "ctrl+c", "q", "esc": + return m, tea.Quit + case "up", "k": + // Clear buffer when using arrows + m.numberBuffer = "" + if m.cursor > 0 { + m.cursor-- + } + case "down", "j": + // Clear buffer when using arrows + m.numberBuffer = "" + if m.cursor < len(m.options)-1 { + m.cursor++ + } + case "enter", "space": + m.selected = m.options[m.cursor].ClientName + return m, tea.Quit + case "backspace": + // Handle backspace to remove last character from buffer + if len(m.numberBuffer) > 0 { + m.updateNumberBuffer(m.numberBuffer[:len(m.numberBuffer)-1]) + } + case "0", "1", "2", "3", "4", "5", "6", "7", "8", "9": + // Add digit to buffer and update cursor position + m.updateNumberBuffer(m.numberBuffer + msg.String()) + case "ctrl+w": + // Clear buffer + m.numberBuffer = "" + } + } + return m, nil +} + +// updateNumberBuffer moves the cursor to the editor matching the number buffer +func (m *clientSelectModel) updateNumberBuffer(newBuffer string) { + if newBuffer == "" { + m.numberBuffer = newBuffer + return + } + + // Parse the buffer as a number + num, err := strconv.Atoi(newBuffer) + if err != nil { + return + } + + // Convert from 1-based to 0-based index and validate bounds + index := num - 1 + if index >= 0 && index < len(m.options) { + m.numberBuffer = newBuffer + m.cursor = index + } +} + +func (m clientSelectModel) View() tea.View { + var s strings.Builder + s.WriteString(m.title + "\n\n") + + for i, option := range m.options { + cursor := " " + if m.cursor == i { + cursor = ">" + } + s.WriteString(fmt.Sprintf("%s %d. %s\n", cursor, i+1, option.Name)) + } + + // Show the current number buffer if user is typing + if m.numberBuffer != "" { + s.WriteString(fmt.Sprintf("\nTyping: %s", m.numberBuffer)) + } + + s.WriteString("\nUse ↑/↓ arrows or number keys to navigate, enter to select, q to quit") + return tea.NewView(s.String()) +} + +// envOr returns the value of the environment variable key, or fallback if it +// is unset or empty. +func envOr(getenv func(string) string, key, fallback string) string { + if v := getenv(key); v != "" { + return v + } + return fallback +} diff --git a/internal/cmd/client_helper_test.go b/internal/cmd/client_helper_test.go new file mode 100644 index 00000000..2b67d704 --- /dev/null +++ b/internal/cmd/client_helper_test.go @@ -0,0 +1,57 @@ +package cmd + +import "testing" + +// TestFindClientConfig covers the client-name lookup table: behavior that the +// command-level cases only exercise for cursor. +func TestFindClientConfig(t *testing.T) { + mappings := []struct { + clientName string + expectedType MCPClient + expectedName string + }{ + {"claude-code", ClaudeCode, "Claude Code"}, + {"CLAUDE-CODE", ClaudeCode, "Claude Code"}, + {"cursor", Cursor, "Cursor"}, + {"CURSOR", Cursor, "Cursor"}, + {"devin", Devin, "Devin"}, + {"DEVIN", Devin, "Devin"}, + {"codex", Codex, "Codex"}, + {"CODEX", Codex, "Codex"}, + } + for _, m := range mappings { + t.Run(m.clientName, func(t *testing.T) { + cfg, err := findClientConfig(m.clientName) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if cfg.ClientType != m.expectedType { + t.Errorf("ClientType = %q, want %q", cfg.ClientType, m.expectedType) + } + if cfg.Name != m.expectedName { + t.Errorf("Name = %q, want %q", cfg.Name, m.expectedName) + } + }) + } + + t.Run("every client is installable", func(t *testing.T) { + for _, cfg := range supportedClients { + found, err := findClientConfig(cfg.EditorNames[0]) + if err != nil { + t.Fatalf("findClientConfig(%q) failed: %v", cfg.EditorNames[0], err) + } + if found.Name == "" { + t.Errorf("%s: Name should not be empty", cfg.ClientType) + } + // Every client needs an install mechanism: JSON patching (path + // prefix) or a CLI install command. + if found.MCPServersPathPrefix == "" && found.buildInstallCommand == nil { + t.Errorf("%s: needs MCPServersPathPrefix or buildInstallCommand", cfg.ClientType) + } + // CLI-only clients (no config paths) must have an install command. + if len(found.ConfigPaths) == 0 && found.buildInstallCommand == nil { + t.Errorf("%s: CLI-only clients must have buildInstallCommand", cfg.ClientType) + } + } + }) +} diff --git a/internal/cmd/mcp_install.go b/internal/cmd/mcp_install.go index d790c39b..441df1df 100644 --- a/internal/cmd/mcp_install.go +++ b/internal/cmd/mcp_install.go @@ -8,12 +8,9 @@ import ( "os" "os/exec" "path/filepath" - "sort" - "strconv" "strings" "time" - tea "charm.land/bubbletea/v2" "github.com/spf13/cobra" "github.com/tailscale/hujson" @@ -70,7 +67,7 @@ If no client is specified, you'll be prompted to select one interactively.`, gen return fmt.Errorf("TTY not detected - specify a client as an argument (e.g. 'tiger mcp install claude-code')") } var err error - clientName, err = selectClientInteractively(cmd) + clientName, err = selectClientInteractively(cmd, "Select an MCP client to configure:") if err != nil { return fmt.Errorf("failed to select client: %w", err) } @@ -93,21 +90,6 @@ If no client is specified, you'll be prompted to select one interactively.`, gen return cmd } -// MCPClient represents our internal client types -type MCPClient string - -const ( - ClaudeCode MCPClient = "claude-code" - Cursor MCPClient = "cursor" // Both the IDE and the CLI - Devin MCPClient = "devin" - Codex MCPClient = "codex" - Gemini MCPClient = "gemini" - VSCode MCPClient = "vscode" - Antigravity MCPClient = "antigravity" - KiroCLI MCPClient = "kiro-cli" - Copilot MCPClient = "copilot" // Both the IDE and the CLI -) - // MCPServerConfig represents the MCP server configuration type MCPServerConfig struct { Command string `json:"command"` @@ -130,148 +112,6 @@ type InstallOptions struct { CustomConfigPath string } -// clientConfig represents our own client configuration for Tiger MCP installation -type clientConfig struct { - ClientType MCPClient // Our internal client type - Name string - EditorNames []string // Supported client names for this client - MCPServersPathPrefix string // JSON path prefix for MCP servers config (only for JSON config manipulation clients like Cursor) - ConfigPaths []string // Config file locations - used for backup on all clients, and for JSON manipulation on JSON-config clients - // buildInstallCommand builds the CLI install command for CLI-based clients - // Parameters: serverName (name to register), command (binary path), args (arguments to binary) - buildInstallCommand func(serverName, command string, args []string) ([]string, error) -} - -// BuildInstallCommand constructs the install command with the given parameters -func (c *clientConfig) BuildInstallCommand(serverName, command string, args []string) ([]string, error) { - if c.buildInstallCommand == nil { - return nil, nil - } - return c.buildInstallCommand(serverName, command, args) -} - -// supportedClients defines the clients we support for Tiger MCP installation -// Note: A good place to find the json config location for MCPServersPathPrefix -// is in the supportedClientIntegrations map found in: -// https://github.com/stacklok/toolhive/blob/main/pkg/client/config.go -var supportedClients = []clientConfig{ - { - ClientType: ClaudeCode, - Name: "Claude Code", - EditorNames: []string{"claude-code"}, - ConfigPaths: []string{ - "~/.claude.json", - }, - buildInstallCommand: func(serverName, command string, args []string) ([]string, error) { - return append([]string{"claude", "mcp", "add", "-s", "user", serverName, command}, args...), nil - }, - }, - { - ClientType: Cursor, - Name: "Cursor", - EditorNames: []string{"cursor"}, - MCPServersPathPrefix: "/mcpServers", - ConfigPaths: []string{ - "~/.cursor/mcp.json", - }, - }, - { - ClientType: Devin, - Name: "Devin", - EditorNames: []string{"devin"}, - ConfigPaths: []string{ - "~/.config/devin/mcp_config.json", - }, - buildInstallCommand: func(serverName, command string, args []string) ([]string, error) { - return append([]string{"devin", "mcp", "add", "-s", "user", serverName, "--", command}, args...), nil - }, - }, - { - ClientType: Codex, - Name: "Codex", - EditorNames: []string{"codex"}, - ConfigPaths: []string{ - "~/.codex/config.toml", - "$CODEX_HOME/config.toml", - }, - buildInstallCommand: func(serverName, command string, args []string) ([]string, error) { - return append([]string{"codex", "mcp", "add", serverName, command}, args...), nil - }, - }, - { - ClientType: Gemini, - Name: "Gemini CLI", - EditorNames: []string{"gemini", "gemini-cli"}, - ConfigPaths: []string{ - "~/.gemini/settings.json", - }, - buildInstallCommand: func(serverName, command string, args []string) ([]string, error) { - return append([]string{"gemini", "mcp", "add", "-s", "user", serverName, command}, args...), nil - }, - }, - { - ClientType: VSCode, - Name: "VS Code", - EditorNames: []string{"vscode", "code", "vs-code"}, - ConfigPaths: []string{ - "~/.config/Code/User/mcp.json", - "~/Library/Application Support/Code/User/mcp.json", - "~/AppData/Roaming/Code/User/mcp.json", - }, - buildInstallCommand: func(serverName, command string, args []string) ([]string, error) { - j, err := json.Marshal(map[string]any{ - "name": serverName, - "command": command, - "args": args, - }) - if err != nil { - return nil, fmt.Errorf("failed to marshal MCP config: %w", err) - } - return []string{"code", "--add-mcp", string(j)}, nil - }, - }, - { - ClientType: Antigravity, - Name: "Google Antigravity", - EditorNames: []string{"antigravity", "agy"}, - MCPServersPathPrefix: "/mcpServers", - ConfigPaths: []string{ - "~/.gemini/antigravity/mcp_config.json", - }, - }, - { - ClientType: KiroCLI, - Name: "Kiro CLI", - EditorNames: []string{"kiro-cli"}, - ConfigPaths: []string{ - "~/.kiro/settings/mcp.json", - }, - buildInstallCommand: func(serverName, command string, args []string) ([]string, error) { - return []string{"kiro-cli", "mcp", "add", "--name", serverName, "--command", command, "--args", strings.Join(args, ",")}, nil - }, - }, - { - ClientType: Copilot, - Name: "GitHub Copilot CLI", - EditorNames: []string{"copilot", "copilot-cli"}, - ConfigPaths: []string{ - "~/.copilot/mcp-config.json", - }, - buildInstallCommand: func(serverName, command string, args []string) ([]string, error) { - return append([]string{"copilot", "mcp", "add", serverName, "--", command}, args...), nil - }, - }, -} - -// getValidEditorNames returns all valid client names from supportedClients -func getValidEditorNames() []string { - var validNames []string - for _, client := range supportedClients { - validNames = append(validNames, client.EditorNames...) - } - return validNames -} - // ClientInfo contains information about a supported MCP client. type ClientInfo struct { // Name is the human-readable display name (e.g., "Claude Code", "Cursor") @@ -411,26 +251,6 @@ func installTigerMCPForClient(cmd *cobra.Command, clientName string, createBacku return nil } -// findClientConfig finds the client configuration for a given client name -// This consolidates the logic of mapping client names to client types and finding the config -func findClientConfig(clientName string) (*clientConfig, error) { - normalizedName := strings.ToLower(clientName) - - // Look up in our supported clients config - for i := range supportedClients { - for _, name := range supportedClients[i].EditorNames { - if strings.ToLower(name) == normalizedName { - return &supportedClients[i], nil - } - } - } - - // Build list of supported clients from our config for error message - supportedNames := getValidEditorNames() - - return nil, fmt.Errorf("unsupported client: %s. Supported clients: %s", clientName, strings.Join(supportedNames, ", ")) -} - // generateSupportedEditorsHelp generates the supported clients section for help text func generateSupportedEditorsHelp() string { var result strings.Builder @@ -488,144 +308,6 @@ func defaultGetTigerExecutablePath() (string, error) { return tigerPath, nil } -// ClientOption represents a client choice for interactive selection -type ClientOption struct { - Name string // Display name - ClientName string // Client name to pass to installMCPForClient -} - -// selectClientInteractively prompts the user to select a client using Bubble Tea -func selectClientInteractively(cmd *cobra.Command) (string, error) { - // Build client options from supportedClients - var options []ClientOption - for _, cfg := range supportedClients { - // Use the first client name as the primary identifier - primaryName := cfg.EditorNames[0] - options = append(options, ClientOption{ - Name: cfg.Name, - ClientName: primaryName, - }) - } - - // Sort options alphabetically by name - sort.Slice(options, func(i, j int) bool { - return options[i].Name < options[j].Name - }) - - model := clientSelectModel{ - options: options, - cursor: 0, - } - - program := tea.NewProgram(model, - tea.WithInput(cmd.InOrStdin()), - tea.WithOutput(cmd.ErrOrStderr()), - tea.WithContext(cmd.Context()), - tea.WithoutSignalHandler()) - finalModel, err := program.Run() - if err != nil { - return "", fmt.Errorf("failed to run editor selection: %w", err) - } - - result := finalModel.(clientSelectModel) - if result.selected == "" { - return "", fmt.Errorf("no editor selected") - } - - return result.selected, nil -} - -// clientSelectModel represents the Bubble Tea model for client selection -type clientSelectModel struct { - options []ClientOption - cursor int - selected string - numberBuffer string -} - -func (m clientSelectModel) Init() tea.Cmd { - return nil -} - -func (m clientSelectModel) Update(msg tea.Msg) (tea.Model, tea.Cmd) { - switch msg := msg.(type) { - case tea.KeyPressMsg: - switch msg.String() { - case "ctrl+c", "q", "esc": - return m, tea.Quit - case "up", "k": - // Clear buffer when using arrows - m.numberBuffer = "" - if m.cursor > 0 { - m.cursor-- - } - case "down", "j": - // Clear buffer when using arrows - m.numberBuffer = "" - if m.cursor < len(m.options)-1 { - m.cursor++ - } - case "enter", "space": - m.selected = m.options[m.cursor].ClientName - return m, tea.Quit - case "backspace": - // Handle backspace to remove last character from buffer - if len(m.numberBuffer) > 0 { - m.updateNumberBuffer(m.numberBuffer[:len(m.numberBuffer)-1]) - } - case "0", "1", "2", "3", "4", "5", "6", "7", "8", "9": - // Add digit to buffer and update cursor position - m.updateNumberBuffer(m.numberBuffer + msg.String()) - case "ctrl+w": - // Clear buffer - m.numberBuffer = "" - } - } - return m, nil -} - -// updateNumberBuffer moves the cursor to the editor matching the number buffer -func (m *clientSelectModel) updateNumberBuffer(newBuffer string) { - if newBuffer == "" { - m.numberBuffer = newBuffer - return - } - - // Parse the buffer as a number - num, err := strconv.Atoi(newBuffer) - if err != nil { - return - } - - // Convert from 1-based to 0-based index and validate bounds - index := num - 1 - if index >= 0 && index < len(m.options) { - m.numberBuffer = newBuffer - m.cursor = index - } -} - -func (m clientSelectModel) View() tea.View { - var s strings.Builder - s.WriteString("Select an MCP client to configure:\n\n") - - for i, option := range m.options { - cursor := " " - if m.cursor == i { - cursor = ">" - } - s.WriteString(fmt.Sprintf("%s %d. %s\n", cursor, i+1, option.Name)) - } - - // Show the current number buffer if user is typing - if m.numberBuffer != "" { - s.WriteString(fmt.Sprintf("\nTyping: %s", m.numberBuffer)) - } - - s.WriteString("\nUse ↑/↓ arrows or number keys to navigate, enter to select, q to quit") - return tea.NewView(s.String()) -} - // addMCPServerViaCLI adds an MCP server using a CLI command configured in clientConfig func addMCPServerViaCLI(clientCfg *clientConfig, serverName, command string, args []string) error { if clientCfg.buildInstallCommand == nil { diff --git a/internal/cmd/mcp_install_test.go b/internal/cmd/mcp_install_test.go index 28aef0e9..91b3c9d6 100644 --- a/internal/cmd/mcp_install_test.go +++ b/internal/cmd/mcp_install_test.go @@ -386,60 +386,6 @@ func TestMCPInstallCmd(t *testing.T) { }) } -// TestFindClientConfig covers the client-name lookup table: behavior that the -// command-level cases only exercise for cursor. -func TestFindClientConfig(t *testing.T) { - mappings := []struct { - clientName string - expectedType MCPClient - expectedName string - }{ - {"claude-code", ClaudeCode, "Claude Code"}, - {"CLAUDE-CODE", ClaudeCode, "Claude Code"}, - {"cursor", Cursor, "Cursor"}, - {"CURSOR", Cursor, "Cursor"}, - {"devin", Devin, "Devin"}, - {"DEVIN", Devin, "Devin"}, - {"codex", Codex, "Codex"}, - {"CODEX", Codex, "Codex"}, - } - for _, m := range mappings { - t.Run(m.clientName, func(t *testing.T) { - cfg, err := findClientConfig(m.clientName) - if err != nil { - t.Fatalf("unexpected error: %v", err) - } - if cfg.ClientType != m.expectedType { - t.Errorf("ClientType = %q, want %q", cfg.ClientType, m.expectedType) - } - if cfg.Name != m.expectedName { - t.Errorf("Name = %q, want %q", cfg.Name, m.expectedName) - } - }) - } - - t.Run("every client is installable", func(t *testing.T) { - for _, cfg := range supportedClients { - found, err := findClientConfig(cfg.EditorNames[0]) - if err != nil { - t.Fatalf("findClientConfig(%q) failed: %v", cfg.EditorNames[0], err) - } - if found.Name == "" { - t.Errorf("%s: Name should not be empty", cfg.ClientType) - } - // Every client needs an install mechanism: JSON patching (path - // prefix) or a CLI install command. - if found.MCPServersPathPrefix == "" && found.buildInstallCommand == nil { - t.Errorf("%s: needs MCPServersPathPrefix or buildInstallCommand", cfg.ClientType) - } - // CLI-only clients (no config paths) must have an install command. - if len(found.ConfigPaths) == 0 && found.buildInstallCommand == nil { - t.Errorf("%s: CLI-only clients must have buildInstallCommand", cfg.ClientType) - } - } - }) -} - // TestFindClientConfigFile covers config file discovery, including each // client's per-client fallback path. func TestFindClientConfigFile(t *testing.T) { diff --git a/internal/cmd/root.go b/internal/cmd/root.go index 9e29b839..9180567e 100644 --- a/internal/cmd/root.go +++ b/internal/cmd/root.go @@ -107,6 +107,7 @@ To get started, run: cmd.AddCommand(buildServiceCmd(app)) cmd.AddCommand(buildDbCmd(app)) cmd.AddCommand(buildMCPCmd(app)) + cmd.AddCommand(buildSkillsCmd(app)) cmd.AddCommand(buildFeedbackCmd(app)) wrapCommands(cmd, app) diff --git a/internal/cmd/skills.go b/internal/cmd/skills.go new file mode 100644 index 00000000..7f6cd9cc --- /dev/null +++ b/internal/cmd/skills.go @@ -0,0 +1,22 @@ +package cmd + +import ( + "github.com/spf13/cobra" + + "github.com/timescale/tiger-cli/internal/common" +) + +func buildSkillsCmd(app *common.App) *cobra.Command { + cmd := &cobra.Command{ + Use: "skills", + Short: "Manage agent skills for AI coding agents", + Long: `Manage agent skills for AI coding agents. + +Skills give AI coding agents curated knowledge and best practices for working +with PostgreSQL, TimescaleDB, and Tiger Cloud.`, + } + + cmd.AddCommand(buildSkillsInstallCmd(app)) + + return cmd +} diff --git a/internal/cmd/skills_install.go b/internal/cmd/skills_install.go new file mode 100644 index 00000000..6f3744c3 --- /dev/null +++ b/internal/cmd/skills_install.go @@ -0,0 +1,488 @@ +package cmd + +import ( + "archive/tar" + "compress/gzip" + "context" + "errors" + "fmt" + "io" + "io/fs" + "net/http" + "os" + "path" + "path/filepath" + "regexp" + "slices" + "strconv" + "strings" + "time" + + "github.com/spf13/cobra" + + "github.com/timescale/tiger-cli/internal/api" + "github.com/timescale/tiger-cli/internal/common" + "github.com/timescale/tiger-cli/internal/util" +) + +const ( + skillsRepo = "timescale/pg-aiguide" + skillsRepoRef = "main" + // skillsRepoDir is the directory within the repo holding one subdirectory + // per skill. + skillsRepoDir = "skills" + + // maxSkillsArchiveSize bounds the decompressed archive, guarding against a + // runaway download. + maxSkillsArchiveSize = 256 << 20 + // maxSymlinkHops bounds symlink resolution within the archive, which also + // breaks symlink cycles. + maxSymlinkHops = 40 +) + +// skillsTarballURL is the GitHub REST API endpoint for the skills repo's +// tarball. GitHub answers it with a redirect to a short-lived download URL. +// It's a var so tests can point it at a local server. +var skillsTarballURL = "https://api.github.com/repos/" + skillsRepo + "/tarball/" + skillsRepoRef + +// skillNamePattern matches valid skill names per the Agent Skills +// specification. Anything else is skipped, which also keeps every name safe to +// use as a path component. +var skillNamePattern = regexp.MustCompile(`^[a-z0-9]+(-[a-z0-9]+)*$`) + +func buildSkillsInstallCmd(_ *common.App) *cobra.Command { + cmd := &cobra.Command{ + Use: "install [client]", + Aliases: []string{"add"}, + Short: "Install agent skills for an AI coding agent", + Long: fmt.Sprintf(`Install agent skills for an AI coding agent. + +Skills are downloaded from https://github.com/%s and installed +for the current user into ~/.agents/skills, which most coding agents read. For +clients that read skills from their own directory, each skill is symlinked +into that directory, so every client shares a single copy. + +Existing skills with the same names are replaced, so re-running the command +updates the installed skills to the latest version. + +%s +If no client is specified, you'll be prompted to select one interactively.`, skillsRepo, generateSkillsClientsHelp()), + Example: ` # Interactive client selection + tiger skills install + + # Install for Claude Code + tiger skills install claude-code + + # Install for Codex + tiger skills install codex`, + Args: cobra.MaximumNArgs(1), + ValidArgs: getValidEditorNames(), + SilenceUsage: true, + RunE: func(cmd *cobra.Command, args []string) error { + var clientName string + if len(args) == 0 { + if !util.IsTerminal(cmd.InOrStdin()) || !util.IsTerminal(cmd.ErrOrStderr()) { + return errors.New("TTY not detected - specify a client as an argument (e.g. 'tiger skills install claude-code')") + } + var err error + clientName, err = selectClientInteractively(cmd, "Select a client to install skills for:") + if err != nil { + return fmt.Errorf("failed to select client: %w", err) + } + } else { + clientName = args[0] + } + + clientCfg, err := findClientConfig(clientName) + if err != nil { + return err + } + + home, err := os.UserHomeDir() + if err != nil { + return fmt.Errorf("failed to determine home directory: %w", err) + } + skillsDir := filepath.Join(home, ".agents", "skills") + var linkDir string + if clientCfg.skillsDir != nil { + linkDir = clientCfg.skillsDir(home, os.Getenv) + } + + skills, err := fetchSkills(cmd.Context()) + if err != nil { + return err + } + + result, err := installSkills(skills, skillsDir, linkDir) + if err != nil { + return err + } + + cmd.Printf("Installed %d skills to %s:\n", len(skills), skillsDir) + for _, s := range skills { + cmd.Printf(" %s\n", s.name) + } + switch { + case result.copied: + cmd.Printf("Copied skills into %s (symlinks are not supported here).\n", linkDir) + case result.linked: + cmd.Printf("Linked skills into %s.\n", linkDir) + } + cmd.Printf("\nRestart %s to load the new skills.\n", clientCfg.Name) + return nil + }, + } + + return cmd +} + +// generateSkillsClientsHelp generates the supported clients section of the +// help text, noting where each client's skills land. Env var overrides are +// ignored so the text doesn't depend on the environment it's generated in. +func generateSkillsClientsHelp() string { + noEnv := func(string) string { return "" } + var b strings.Builder + b.WriteString("Supported Clients:\n") + for _, cfg := range supportedClients { + dir := "~/.agents/skills" + if cfg.skillsDir != nil { + dir = cfg.skillsDir("~", noEnv) + } + fmt.Fprintf(&b, " %-24s %s (%s)\n", cfg.EditorNames[0], cfg.Name, dir) + } + return b.String() +} + +// skill is one skill directory, with its contents held in memory. +type skill struct { + name string + files []skillFile +} + +// skillFile is a regular file within a skill, with symlinks already resolved. +type skillFile struct { + path string // slash-separated, relative to the skill directory + executable bool + data []byte +} + +// fetchSkills downloads the skills repo's tarball from GitHub and extracts the +// skills from it. +func fetchSkills(ctx context.Context) ([]skill, error) { + req, err := http.NewRequestWithContext(ctx, http.MethodGet, skillsTarballURL, nil) + if err != nil { + return nil, fmt.Errorf("failed to create request: %w", err) + } + req.Header.Set("Accept", "application/vnd.github+json") + req.Header.Set("X-GitHub-Api-Version", "2022-11-28") + + resp, err := api.HTTPClient.Do(req) + if err != nil { + return nil, fmt.Errorf("failed to download skills: %w", err) + } + defer resp.Body.Close() + + if resp.StatusCode != http.StatusOK { + return nil, skillsDownloadError(resp) + } + + skills, err := extractSkills(resp.Body) + if err != nil { + return nil, fmt.Errorf("failed to read skills archive: %w", err) + } + if len(skills) == 0 { + return nil, fmt.Errorf("no skills found in %s", skillsRepo) + } + return skills, nil +} + +// skillsDownloadError describes a failed tarball request, calling out GitHub's +// rate limit for unauthenticated requests. +func skillsDownloadError(resp *http.Response) error { + if (resp.StatusCode == http.StatusForbidden || resp.StatusCode == http.StatusTooManyRequests) && + resp.Header.Get("X-RateLimit-Remaining") == "0" { + if reset, err := strconv.ParseInt(resp.Header.Get("X-RateLimit-Reset"), 10, 64); err == nil { + return fmt.Errorf("failed to download skills: GitHub API rate limit exceeded. Try again after %s", + time.Unix(reset, 0).Local().Format("15:04 MST")) + } + return errors.New("failed to download skills: GitHub API rate limit exceeded. Try again later") + } + return fmt.Errorf("failed to download skills: unexpected status from GitHub: %s", resp.Status) +} + +// extractSkills reads a gzipped tarball of the skills repo and returns its +// skills, sorted by name. A skill is a subdirectory of skillsRepoDir holding a +// SKILL.md. Symlinks within the repo are resolved to the files they point at, +// since skills share files that way and an installed skill must stand alone. +func extractSkills(r io.Reader) ([]skill, error) { + gz, err := gzip.NewReader(r) + if err != nil { + return nil, err + } + defer gz.Close() + + a := &skillsArchive{ + files: map[string]skillFile{}, + links: map[string]string{}, + } + tr := tar.NewReader(io.LimitReader(gz, maxSkillsArchiveSize)) + for { + hdr, err := tr.Next() + if errors.Is(err, io.EOF) { + break + } + if err != nil { + return nil, err + } + + // GitHub tarballs nest everything under a single top-level directory + // named after the repo and commit. Entries outside it (the pax global + // header) have no slash and are skipped. + _, name, ok := strings.Cut(hdr.Name, "/") + name = strings.TrimSuffix(name, "/") + if !ok || !fs.ValidPath(name) || name == "." { + continue + } + + switch hdr.Typeflag { + case tar.TypeReg: + data, err := io.ReadAll(tr) + if err != nil { + return nil, err + } + a.files[name] = skillFile{ + executable: hdr.Mode&0o111 != 0, + data: data, + } + case tar.TypeSymlink: + // Absolute targets point outside the repo, and relative ones may + // climb out of it; either way they can't be resolved. + if path.IsAbs(hdr.Linkname) { + continue + } + a.links[name] = path.Join(path.Dir(name), hdr.Linkname) + } + } + + var skills []skill + for _, name := range a.children(skillsRepoDir) { + if !skillNamePattern.MatchString(name) { + continue + } + s := skill{name: name} + a.walk(path.Join(skillsRepoDir, name), "", 0, func(f skillFile) { + s.files = append(s.files, f) + }) + if !slices.ContainsFunc(s.files, func(f skillFile) bool { return f.path == "SKILL.md" }) { + continue + } + slices.SortFunc(s.files, func(a, b skillFile) int { return strings.Compare(a.path, b.path) }) + skills = append(skills, s) + } + return skills, nil +} + +// skillsArchive indexes the regular files and symlinks of an extracted +// tarball by their slash-separated path within the repo. +type skillsArchive struct { + files map[string]skillFile + links map[string]string // symlink path -> target path +} + +// walk emits every regular file reachable at src, which may be a file, a +// symlink, or a directory, with paths rebased from src onto dst. Dangling +// symlinks, and those nested more than maxSymlinkHops deep, are skipped. +func (a *skillsArchive) walk(src, dst string, hops int, emit func(skillFile)) { + if f, ok := a.files[src]; ok { + if dst != "" { + f.path = dst + emit(f) + } + return + } + if target, ok := a.links[src]; ok { + if hops < maxSymlinkHops { + a.walk(target, dst, hops+1, emit) + } + return + } + for _, child := range a.children(src) { + a.walk(path.Join(src, child), path.Join(dst, child), hops, emit) + } +} + +// children returns the sorted names of the entries directly inside dir. +func (a *skillsArchive) children(dir string) []string { + prefix := dir + "/" + seen := map[string]bool{} + var names []string + add := func(p string) { + rest, ok := strings.CutPrefix(p, prefix) + if !ok { + return + } + child, _, _ := strings.Cut(rest, "/") + if !seen[child] { + seen[child] = true + names = append(names, child) + } + } + for p := range a.files { + add(p) + } + for p := range a.links { + add(p) + } + slices.Sort(names) + return names +} + +// installSkillsResult reports how skills reached the client's own skills +// directory, if it has one. +type installSkillsResult struct { + linked bool // symlinked into the client's directory + copied bool // copied into the client's directory, as symlinks failed +} + +// installSkills writes each skill into skillsDir, replacing any existing copy, +// then symlinks it into linkDir if set. Linking is skipped when linkDir +// resolves to skillsDir itself (e.g. the user has symlinked one to the other). +func installSkills(skills []skill, skillsDir, linkDir string) (installSkillsResult, error) { + var result installSkillsResult + if err := os.MkdirAll(skillsDir, 0o755); err != nil { + return result, fmt.Errorf("failed to create skills directory: %w", err) + } + for _, s := range skills { + if err := writeSkill(filepath.Join(skillsDir, s.name), s); err != nil { + return result, fmt.Errorf("failed to install skill %s: %w", s.name, err) + } + } + + if linkDir == "" { + return result, nil + } + if err := os.MkdirAll(linkDir, 0o755); err != nil { + return result, fmt.Errorf("failed to create skills directory: %w", err) + } + same, err := sameDir(skillsDir, linkDir) + if err != nil { + return result, err + } + if same { + return result, nil + } + for _, s := range skills { + target := filepath.Join(skillsDir, s.name) + link := filepath.Join(linkDir, s.name) + err := linkSkill(target, link) + if errors.Is(err, errSymlinkUnsupported) { + err = writeSkill(link, s) + result.copied = true + } else { + result.linked = true + } + if err != nil { + return result, fmt.Errorf("failed to link skill %s: %w", s.name, err) + } + } + return result, nil +} + +// errSymlinkUnsupported reports that a symlink couldn't be created (e.g. on +// Windows without Developer Mode), so the caller should fall back to a copy. +var errSymlinkUnsupported = errors.New("symlinks not supported") + +// writeSkill writes s to dir, replacing whatever is there. The new copy is +// built in a temporary directory beside dir and swapped in, so a failure +// partway through leaves the existing copy intact. +func writeSkill(dir string, s skill) error { + tmp, err := os.MkdirTemp(filepath.Dir(dir), ".tiger-"+s.name+"-") + if err != nil { + return err + } + defer os.RemoveAll(tmp) + + for _, f := range s.files { + p := filepath.Join(tmp, filepath.FromSlash(f.path)) + if err := os.MkdirAll(filepath.Dir(p), 0o755); err != nil { + return err + } + mode := fs.FileMode(0o644) + if f.executable { + mode = 0o755 + } + if err := os.WriteFile(p, f.data, mode); err != nil { + return err + } + } + // MkdirTemp creates the directory 0700; skills are ordinary user files. + if err := os.Chmod(tmp, 0o755); err != nil { + return err + } + return replacePath(tmp, dir) +} + +// linkSkill points link at target, replacing whatever is at link unless it's +// already the right symlink. The symlink is created under a temporary name +// and swapped in, so link is never left missing. +func linkSkill(target, link string) error { + if current, err := os.Readlink(link); err == nil && current == target { + return nil + } + + tmp, err := os.MkdirTemp(filepath.Dir(link), ".tiger-"+filepath.Base(link)+"-") + if err != nil { + return err + } + defer os.RemoveAll(tmp) + + tmpLink := filepath.Join(tmp, "link") + if err := os.Symlink(target, tmpLink); err != nil { + return fmt.Errorf("%w: %w", errSymlinkUnsupported, err) + } + return replacePath(tmpLink, link) +} + +// replacePath moves src to dst, replacing any file, directory, or symlink at +// dst. A rename can't replace a non-empty directory, so the existing entry is +// first moved aside, and restored if the swap fails. Symlinks are moved and +// removed as links, never followed. +func replacePath(src, dst string) error { + if _, err := os.Lstat(dst); errors.Is(err, fs.ErrNotExist) { + return os.Rename(src, dst) + } else if err != nil { + return err + } + + aside, err := os.MkdirTemp(filepath.Dir(dst), ".tiger-old-") + if err != nil { + return err + } + defer os.RemoveAll(aside) + + old := filepath.Join(aside, "old") + if err := os.Rename(dst, old); err != nil { + return err + } + if err := os.Rename(src, dst); err != nil { + if restoreErr := os.Rename(old, dst); restoreErr != nil { + return fmt.Errorf("%w (and failed to restore %s: %w)", err, dst, restoreErr) + } + return err + } + return nil +} + +// sameDir reports whether a and b resolve to the same directory once symlinks +// are followed. +func sameDir(a, b string) (bool, error) { + ra, err := filepath.EvalSymlinks(a) + if err != nil { + return false, err + } + rb, err := filepath.EvalSymlinks(b) + if err != nil { + return false, err + } + return ra == rb, nil +} diff --git a/internal/cmd/skills_install_test.go b/internal/cmd/skills_install_test.go new file mode 100644 index 00000000..a35230cb --- /dev/null +++ b/internal/cmd/skills_install_test.go @@ -0,0 +1,428 @@ +package cmd + +import ( + "archive/tar" + "bytes" + "compress/gzip" + "fmt" + "io/fs" + "maps" + "net/http" + "net/http/httptest" + "os" + "path/filepath" + "strconv" + "strings" + "testing" + "time" + + "github.com/google/go-cmp/cmp" +) + +// tarEntry is one entry of a test tarball, named relative to the repo root. +type tarEntry struct { + name string + body string + link string // symlink target, if set + mode int64 // file mode; 0o644 if unset +} + +// skillsTarball builds a gzipped tarball shaped like GitHub's: a pax global +// header followed by everything nested under a top-level directory named after +// the repo and commit. +func skillsTarball(t *testing.T, entries ...tarEntry) []byte { + t.Helper() + var buf bytes.Buffer + gz := gzip.NewWriter(&buf) + tw := tar.NewWriter(gz) + headers := []*tar.Header{ + {Name: "pax_global_header", Typeflag: tar.TypeXGlobalHeader, PAXRecords: map[string]string{"comment": "abc123"}}, + {Name: "timescale-pg-aiguide-abc123/", Typeflag: tar.TypeDir, Mode: 0o755}, + } + for _, e := range headers { + if err := tw.WriteHeader(e); err != nil { + t.Fatal(err) + } + } + for _, e := range entries { + hdr := &tar.Header{Name: "timescale-pg-aiguide-abc123/" + e.name, Mode: e.mode} + if hdr.Mode == 0 { + hdr.Mode = 0o644 + } + switch { + case e.link != "": + hdr.Typeflag = tar.TypeSymlink + hdr.Linkname = e.link + case e.name[len(e.name)-1] == '/': + hdr.Typeflag = tar.TypeDir + hdr.Mode = 0o755 + default: + hdr.Typeflag = tar.TypeReg + hdr.Size = int64(len(e.body)) + } + if err := tw.WriteHeader(hdr); err != nil { + t.Fatal(err) + } + if hdr.Typeflag == tar.TypeReg { + if _, err := tw.Write([]byte(e.body)); err != nil { + t.Fatal(err) + } + } + } + if err := tw.Close(); err != nil { + t.Fatal(err) + } + if err := gz.Close(); err != nil { + t.Fatal(err) + } + return buf.Bytes() +} + +// withSkillsServer points the skills download at a local server standing in +// for GitHub. The server expects the tarball API request, then answers it +// with handle. +func withSkillsServer(handle http.HandlerFunc) runOption { + return withSkillsServerDownload(handle, nil) +} + +// withSkillsTarball serves body as the repo tarball, via a redirect to a +// separate download URL as GitHub does. +func withSkillsTarball(body []byte) runOption { + return withSkillsServerDownload(func(w http.ResponseWriter, r *http.Request) { + http.Redirect(w, r, "/download", http.StatusFound) + }, body) +} + +// withSkillsServerDownload is withSkillsServer, also serving download at +// /download if set. +func withSkillsServerDownload(handle http.HandlerFunc, download []byte) runOption { + return withSetup(func(t *testing.T) { + mux := http.NewServeMux() + if download != nil { + mux.HandleFunc("/download", func(w http.ResponseWriter, r *http.Request) { + w.Write(download) + }) + } + mux.HandleFunc("/repos/timescale/pg-aiguide/tarball/main", func(w http.ResponseWriter, r *http.Request) { + got := map[string]string{ + "method": r.Method, + "Accept": r.Header.Get("Accept"), + "X-GitHub-Api-Version": r.Header.Get("X-GitHub-Api-Version"), + } + want := map[string]string{ + "method": http.MethodGet, + "Accept": "application/vnd.github+json", + "X-GitHub-Api-Version": "2022-11-28", + } + if diff := cmp.Diff(want, got); diff != "" { + t.Errorf("tarball request mismatch (-want +got):\n%s", diff) + } + handle(w, r) + }) + srv := httptest.NewServer(mux) + t.Cleanup(srv.Close) + + original := skillsTarballURL + skillsTarballURL = srv.URL + "/repos/timescale/pg-aiguide/tarball/main" + t.Cleanup(func() { skillsTarballURL = original }) + }) +} + +// readTree returns every file and symlink under root, keyed by slash-separated +// path. Files map to their content (prefixed with "[executable] " if so) and +// symlinks to "-> target". Symlinks are not followed. +func readTree(t *testing.T, root string) map[string]string { + t.Helper() + tree := map[string]string{} + err := filepath.WalkDir(root, func(p string, d fs.DirEntry, err error) error { + if err != nil { + return err + } + rel, err := filepath.Rel(root, p) + if err != nil { + return err + } + rel = filepath.ToSlash(rel) + switch { + case d.Type()&fs.ModeSymlink != 0: + target, err := os.Readlink(p) + if err != nil { + return err + } + tree[rel] = "-> " + target + case d.Type().IsRegular(): + data, err := os.ReadFile(p) + if err != nil { + return err + } + info, err := d.Info() + if err != nil { + return err + } + if info.Mode().Perm()&0o111 != 0 { + tree[rel] = "[executable] " + string(data) + } else { + tree[rel] = string(data) + } + } + return nil + }) + if err != nil { + t.Fatalf("failed to read tree %s: %v", root, err) + } + return tree +} + +// checkTree asserts the exact files and symlinks under root (see readTree). +func checkTree(root string, want map[string]string) checkFunc { + return func(t *testing.T, _ cmdResult) { + t.Helper() + if diff := cmp.Diff(want, readTree(t, root)); diff != "" { + t.Errorf("tree %s mismatch (-want +got):\n%s", root, diff) + } + } +} + +// writeTree creates the given files and symlinks under root (see readTree for +// the format, minus executable bits). +func writeTree(t *testing.T, root string, tree map[string]string) { + t.Helper() + for rel, content := range tree { + p := filepath.Join(root, filepath.FromSlash(rel)) + if err := os.MkdirAll(filepath.Dir(p), 0o755); err != nil { + t.Fatal(err) + } + if target, ok := strings.CutPrefix(content, "-> "); ok { + if err := os.Symlink(target, p); err != nil { + t.Fatal(err) + } + continue + } + if err := os.WriteFile(p, []byte(content), 0o644); err != nil { + t.Fatal(err) + } + } +} + +// skillsInstallOutput is the exact stdout of a successful install of the +// fixture skills. linkLine is the line describing the client directory, if +// any. +func skillsInstallOutput(home, linkLine, clientName string) string { + return fmt.Sprintf("Installed 2 skills to %s:\n alpha\n beta\n%s\nRestart %s to load the new skills.\n", + filepath.Join(home, ".agents", "skills"), linkLine, clientName) +} + +func TestSkillsInstallCmd(t *testing.T) { + // The fixture repo exercises skill discovery and symlink resolution: + // file and directory symlinks between skills, one that climbs out of + // skills/ but stays in the repo, and absolute, dangling, and looping ones + // (skipped), plus entries that aren't valid skills. + tarball := skillsTarball(t, + tarEntry{name: "README.md", body: "repo readme"}, + tarEntry{name: "skills/"}, + tarEntry{name: "skills/README.md", body: "not a skill"}, + tarEntry{name: "skills/alpha/"}, + tarEntry{name: "skills/alpha/SKILL.md", body: "alpha skill"}, + tarEntry{name: "skills/alpha/references/guide.md", body: "alpha guide"}, + tarEntry{name: "skills/alpha/scripts/run.sh", body: "#!/bin/sh", mode: 0o755}, + tarEntry{name: "skills/beta/SKILL.md", body: "beta skill"}, + tarEntry{name: "skills/beta/references/alpha-guide.md", link: "../../alpha/references/guide.md"}, + tarEntry{name: "skills/beta/references/alpha-refs", link: "../../alpha/references"}, + tarEntry{name: "skills/beta/references/readme.md", link: "../../../README.md"}, + tarEntry{name: "skills/beta/references/absolute.md", link: "/etc/passwd"}, + tarEntry{name: "skills/beta/references/dangling.md", link: "missing.md"}, + tarEntry{name: "skills/beta/references/loop.md", link: "loop.md"}, + tarEntry{name: "skills/Bad_Name/SKILL.md", body: "invalid name"}, + tarEntry{name: "skills/no-skill-md/README.md", body: "no SKILL.md"}, + ) + // installedSkills is the tree the fixture installs under ~/.agents/skills. + installedSkills := map[string]string{ + ".agents/skills/alpha/SKILL.md": "alpha skill", + ".agents/skills/alpha/references/guide.md": "alpha guide", + ".agents/skills/alpha/scripts/run.sh": "[executable] #!/bin/sh", + ".agents/skills/beta/SKILL.md": "beta skill", + ".agents/skills/beta/references/alpha-guide.md": "alpha guide", + ".agents/skills/beta/references/alpha-refs/guide.md": "alpha guide", + ".agents/skills/beta/references/readme.md": "repo readme", + } + // withLinks returns installedSkills plus symlinks to each skill from the + // client directory linkDir (relative to home). + withLinks := func(home, linkDir string) map[string]string { + tree := map[string]string{ + linkDir + "/alpha": "-> " + filepath.Join(home, ".agents", "skills", "alpha"), + linkDir + "/beta": "-> " + filepath.Join(home, ".agents", "skills", "beta"), + } + maps.Copy(tree, installedSkills) + return tree + } + + codexHome := t.TempDir() + claudeHome := t.TempDir() + claudeConfigHome := t.TempDir() + claudeConfigDir := filepath.Join(claudeConfigHome, "claude-config") + kiroHome := t.TempDir() + antigravityHome := t.TempDir() + aliasHome := t.TempDir() + + // reinstallHome holds a previous install to be replaced: a stale skill + // directory, a client entry that's a real directory, a client symlink + // pointing elsewhere, and an unrelated skill that must be left alone. + reinstallHome := t.TempDir() + writeTree(t, reinstallHome, map[string]string{ + ".agents/skills/alpha/SKILL.md": "old alpha", + ".agents/skills/alpha/stale.md": "stale", + ".agents/skills/other/SKILL.md": "unrelated skill", + ".claude/skills/alpha": "-> /somewhere/else/alpha", + ".claude/skills/beta/SKILL.md": "old beta copy", + }) + reinstallTree := withLinks(reinstallHome, ".claude/skills") + reinstallTree[".agents/skills/other/SKILL.md"] = "unrelated skill" + + // sharedHome has ~/.claude/skills symlinked to ~/.agents/skills, so the + // skills are already visible to Claude Code and no per-skill links are made. + sharedHome := t.TempDir() + writeTree(t, sharedHome, map[string]string{ + ".claude/skills": "-> " + filepath.Join(sharedHome, ".agents", "skills"), + }) + sharedTree := map[string]string{".claude/skills": "-> " + filepath.Join(sharedHome, ".agents", "skills")} + maps.Copy(sharedTree, installedSkills) + + rateLimitReset := time.Date(2030, 1, 1, 15, 4, 0, 0, time.UTC).Unix() + + runCmdTests(t, []cmdTest{ + { + name: "too many arguments", + args: []string{"skills", "install", "codex", "cursor"}, + wantErr: "accepts at most 1 arg(s), received 2", + }, + { + name: "no client and no TTY", + args: []string{"skills", "install"}, + wantErr: "TTY not detected - specify a client as an argument (e.g. 'tiger skills install claude-code')", + }, + { + name: "unsupported client", + args: []string{"skills", "install", "bogus"}, + wantErr: "unsupported client: bogus. Supported clients: claude-code, cursor, devin, codex, gemini, gemini-cli, vscode, code, vs-code, antigravity, agy, kiro-cli, copilot, copilot-cli", + }, + { + name: "download fails", + args: []string{"skills", "install", "codex"}, + opts: []runOption{ + withEnv("HOME", t.TempDir()), + withSkillsServer(func(w http.ResponseWriter, r *http.Request) { + w.WriteHeader(http.StatusInternalServerError) + }), + }, + wantErr: "failed to download skills: unexpected status from GitHub: 500 Internal Server Error", + }, + { + name: "rate limited", + args: []string{"skills", "install", "codex"}, + opts: []runOption{ + withEnv("HOME", t.TempDir()), + withSkillsServer(func(w http.ResponseWriter, r *http.Request) { + w.Header().Set("X-RateLimit-Remaining", "0") + w.Header().Set("X-RateLimit-Reset", strconv.FormatInt(rateLimitReset, 10)) + w.WriteHeader(http.StatusForbidden) + }), + }, + wantErr: "failed to download skills: GitHub API rate limit exceeded. Try again after 15:04 UTC", + }, + { + name: "invalid archive", + args: []string{"skills", "install", "codex"}, + opts: []runOption{ + withEnv("HOME", t.TempDir()), + withSkillsServer(func(w http.ResponseWriter, r *http.Request) { + w.Write([]byte("not a tarball")) + }), + }, + wantErr: "failed to read skills archive: gzip: invalid header", + }, + { + name: "no skills in archive", + args: []string{"skills", "install", "codex"}, + opts: []runOption{ + withEnv("HOME", t.TempDir()), + withSkillsTarball(skillsTarball(t, tarEntry{name: "README.md", body: "repo readme"})), + }, + wantErr: "no skills found in timescale/pg-aiguide", + }, + { + name: "installs to ~/.agents/skills", + args: []string{"skills", "install", "codex"}, + opts: []runOption{withEnv("HOME", codexHome), withSkillsTarball(tarball)}, + wantStdout: skillsInstallOutput(codexHome, "", "Codex"), + checks: []checkFunc{checkTree(codexHome, installedSkills)}, + }, + { + name: "symlinks into claude code's skills directory", + args: []string{"skills", "install", "claude-code"}, + opts: []runOption{withEnv("HOME", claudeHome), withSkillsTarball(tarball)}, + wantStdout: skillsInstallOutput(claudeHome, + fmt.Sprintf("Linked skills into %s.\n", filepath.Join(claudeHome, ".claude", "skills")), "Claude Code"), + checks: []checkFunc{checkTree(claudeHome, withLinks(claudeHome, ".claude/skills"))}, + }, + { + name: "respects CLAUDE_CONFIG_DIR", + args: []string{"skills", "install", "claude-code"}, + opts: []runOption{ + withEnv("HOME", claudeConfigHome), + withEnv("CLAUDE_CONFIG_DIR", claudeConfigDir), + withSkillsTarball(tarball), + }, + wantStdout: skillsInstallOutput(claudeConfigHome, + fmt.Sprintf("Linked skills into %s.\n", filepath.Join(claudeConfigDir, "skills")), "Claude Code"), + checks: []checkFunc{checkTree(claudeConfigHome, withLinks(claudeConfigHome, "claude-config/skills"))}, + }, + { + name: "symlinks into kiro's skills directory", + args: []string{"skills", "install", "kiro-cli"}, + opts: []runOption{withEnv("HOME", kiroHome), withSkillsTarball(tarball)}, + wantStdout: skillsInstallOutput(kiroHome, + fmt.Sprintf("Linked skills into %s.\n", filepath.Join(kiroHome, ".kiro", "skills")), "Kiro CLI"), + checks: []checkFunc{checkTree(kiroHome, withLinks(kiroHome, ".kiro/skills"))}, + }, + { + name: "symlinks into antigravity's skills directory", + args: []string{"skills", "install", "antigravity"}, + opts: []runOption{withEnv("HOME", antigravityHome), withSkillsTarball(tarball)}, + wantStdout: skillsInstallOutput(antigravityHome, + fmt.Sprintf("Linked skills into %s.\n", filepath.Join(antigravityHome, ".gemini", "antigravity", "skills")), "Google Antigravity"), + checks: []checkFunc{checkTree(antigravityHome, withLinks(antigravityHome, ".gemini/antigravity/skills"))}, + }, + { + name: "reinstall replaces existing skills and links", + args: []string{"skills", "install", "claude-code"}, + opts: []runOption{withEnv("HOME", reinstallHome), withSkillsTarball(tarball)}, + wantStdout: skillsInstallOutput(reinstallHome, + fmt.Sprintf("Linked skills into %s.\n", filepath.Join(reinstallHome, ".claude", "skills")), "Claude Code"), + checks: []checkFunc{ + checkTree(reinstallHome, reinstallTree), + // Running again over a current install leaves it unchanged. + func(t *testing.T, _ cmdResult) { + again := runCommand(t, []string{"skills", "install", "claude-code"}, nil) + if again.err != nil { + t.Fatalf("second install failed: %v", again.err) + } + }, + checkTree(reinstallHome, reinstallTree), + }, + }, + { + name: "client directory already shared with ~/.agents/skills", + args: []string{"skills", "install", "claude-code"}, + opts: []runOption{withEnv("HOME", sharedHome), withSkillsTarball(tarball)}, + wantStdout: skillsInstallOutput(sharedHome, "", "Claude Code"), + checks: []checkFunc{checkTree(sharedHome, sharedTree)}, + }, + { + name: "add alias and case-insensitive client name", + args: []string{"skills", "add", "CODEX"}, + opts: []runOption{withEnv("HOME", aliasHome), withSkillsTarball(tarball)}, + wantStdout: skillsInstallOutput(aliasHome, "", "Codex"), + checks: []checkFunc{checkTree(aliasHome, installedSkills)}, + }, + }) +} From 3e5ee32b9cb26721b567d1af2a8ff5e591224cb7 Mon Sep 17 00:00:00 2001 From: Nathan Cochran Date: Wed, 7 Oct 2026 15:18:10 -0400 Subject: [PATCH 02/17] Minor cleanup, improvements --- README.md | 11 +- docs/cli/tiger_skills_install.md | 7 +- internal/cmd/client_helper.go | 87 +++++----- internal/cmd/client_helper_test.go | 11 +- internal/cmd/mcp_install.go | 22 +-- internal/cmd/mcp_install_test.go | 12 +- internal/cmd/skills_install.go | 252 +++++++++++----------------- internal/cmd/skills_install_test.go | 154 ++++++++--------- 8 files changed, 238 insertions(+), 318 deletions(-) diff --git a/README.md b/README.md index 1a31067b..9f7800d8 100644 --- a/README.md +++ b/README.md @@ -254,9 +254,8 @@ tiger config set docs_mcp false ## Agent Skills -Tiger CLI can install the curated PostgreSQL and TimescaleDB skills from -[pg-aiguide](https://github.com/timescale/pg-aiguide) — the same skills the -MCP server proxies — directly into your coding agent: +Tiger CLI can install curated PostgreSQL and TimescaleDB skills directly into +your coding agent: ```bash # Interactive installation (prompts for client selection) @@ -267,9 +266,9 @@ tiger skills install claude-code ``` Skills are installed for the current user into `~/.agents/skills`, which most -coding agents read. For clients that use their own skills directory (Claude -Code, Kiro CLI, Google Antigravity), each skill is symlinked into that -directory. Re-run the command to update the skills to the latest version. +coding agents read, or into the client's own skills directory for clients that +don't (Claude Code, Kiro CLI, Google Antigravity). Re-run the command to update +the skills to the latest version. ## Configuration diff --git a/docs/cli/tiger_skills_install.md b/docs/cli/tiger_skills_install.md index 370b2809..b50b674f 100644 --- a/docs/cli/tiger_skills_install.md +++ b/docs/cli/tiger_skills_install.md @@ -6,10 +6,9 @@ Install agent skills for an AI coding agent Install agent skills for an AI coding agent. -Skills are downloaded from https://github.com/timescale/pg-aiguide and installed -for the current user into ~/.agents/skills, which most coding agents read. For -clients that read skills from their own directory, each skill is symlinked -into that directory, so every client shares a single copy. +Skills are installed for the current user into ~/.agents/skills, which most +coding agents read, or into the client's own skills directory for clients that +don't. Existing skills with the same names are replaced, so re-running the command updates the installed skills to the latest version. diff --git a/internal/cmd/client_helper.go b/internal/cmd/client_helper.go index de922ebc..2c92246d 100644 --- a/internal/cmd/client_helper.go +++ b/internal/cmd/client_helper.go @@ -3,7 +3,6 @@ package cmd import ( "encoding/json" "fmt" - "path/filepath" "sort" "strconv" "strings" @@ -34,22 +33,25 @@ type clientConfig struct { Name string EditorNames []string // Supported client names for this client MCPServersPathPrefix string // JSON path prefix for MCP servers config (only for JSON config manipulation clients like Cursor) - ConfigPaths []string // Config file locations - used for backup on all clients, and for JSON manipulation on JSON-config clients - // buildInstallCommand builds the CLI install command for CLI-based clients + MCPConfigPaths []string // Config file locations - used for backup on all clients, and for JSON manipulation on JSON-config clients + // buildMCPInstallCommand builds the CLI install command for CLI-based clients // Parameters: serverName (name to register), command (binary path), args (arguments to binary) - buildInstallCommand func(serverName, command string, args []string) ([]string, error) - // skillsDir returns the user-level skills directory for clients that don't - // read skills from ~/.agents/skills, given the home directory and an env - // var lookup. Nil for clients that do. - skillsDir func(home string, getenv func(string) string) string + buildMCPInstallCommand func(serverName, command string, args []string) ([]string, error) + // SkillsDir is the user-level directory skills are installed into. + SkillsDir string + // SkillsDirEnv overrides SkillsDir for clients whose skills directory can + // be relocated by an env var. It's a path containing env var references + // (e.g. "${CLAUDE_CONFIG_DIR}/skills"), used only when every variable it + // references is set and non-empty. + SkillsDirEnv string } -// BuildInstallCommand constructs the install command with the given parameters -func (c *clientConfig) BuildInstallCommand(serverName, command string, args []string) ([]string, error) { - if c.buildInstallCommand == nil { +// BuildMCPInstallCommand constructs the install command with the given parameters +func (c *clientConfig) BuildMCPInstallCommand(serverName, command string, args []string) ([]string, error) { + if c.buildMCPInstallCommand == nil { return nil, nil } - return c.buildInstallCommand(serverName, command, args) + return c.buildMCPInstallCommand(serverName, command, args) } // supportedClients defines the clients we support for Tiger MCP installation @@ -61,69 +63,72 @@ var supportedClients = []clientConfig{ ClientType: ClaudeCode, Name: "Claude Code", EditorNames: []string{"claude-code"}, - ConfigPaths: []string{ + MCPConfigPaths: []string{ "~/.claude.json", }, - buildInstallCommand: func(serverName, command string, args []string) ([]string, error) { + buildMCPInstallCommand: func(serverName, command string, args []string) ([]string, error) { return append([]string{"claude", "mcp", "add", "-s", "user", serverName, command}, args...), nil }, - skillsDir: func(home string, getenv func(string) string) string { - return filepath.Join(envOr(getenv, "CLAUDE_CONFIG_DIR", filepath.Join(home, ".claude")), "skills") - }, + SkillsDir: "~/.claude/skills", + SkillsDirEnv: "${CLAUDE_CONFIG_DIR}/skills", }, { ClientType: Cursor, Name: "Cursor", EditorNames: []string{"cursor"}, MCPServersPathPrefix: "/mcpServers", - ConfigPaths: []string{ + MCPConfigPaths: []string{ "~/.cursor/mcp.json", }, + SkillsDir: "~/.agents/skills", }, { ClientType: Devin, Name: "Devin", EditorNames: []string{"devin"}, - ConfigPaths: []string{ + MCPConfigPaths: []string{ "~/.config/devin/mcp_config.json", }, - buildInstallCommand: func(serverName, command string, args []string) ([]string, error) { + buildMCPInstallCommand: func(serverName, command string, args []string) ([]string, error) { return append([]string{"devin", "mcp", "add", "-s", "user", serverName, "--", command}, args...), nil }, + SkillsDir: "~/.agents/skills", }, { ClientType: Codex, Name: "Codex", EditorNames: []string{"codex"}, - ConfigPaths: []string{ + MCPConfigPaths: []string{ "~/.codex/config.toml", "$CODEX_HOME/config.toml", }, - buildInstallCommand: func(serverName, command string, args []string) ([]string, error) { + buildMCPInstallCommand: func(serverName, command string, args []string) ([]string, error) { return append([]string{"codex", "mcp", "add", serverName, command}, args...), nil }, + SkillsDir: "~/.agents/skills", }, { ClientType: Gemini, Name: "Gemini CLI", EditorNames: []string{"gemini", "gemini-cli"}, - ConfigPaths: []string{ + MCPConfigPaths: []string{ "~/.gemini/settings.json", }, - buildInstallCommand: func(serverName, command string, args []string) ([]string, error) { + buildMCPInstallCommand: func(serverName, command string, args []string) ([]string, error) { return append([]string{"gemini", "mcp", "add", "-s", "user", serverName, command}, args...), nil }, + SkillsDir: "~/.agents/skills", }, { ClientType: VSCode, Name: "VS Code", EditorNames: []string{"vscode", "code", "vs-code"}, - ConfigPaths: []string{ + MCPConfigPaths: []string{ "~/.config/Code/User/mcp.json", "~/Library/Application Support/Code/User/mcp.json", "~/AppData/Roaming/Code/User/mcp.json", }, - buildInstallCommand: func(serverName, command string, args []string) ([]string, error) { + buildMCPInstallCommand: func(serverName, command string, args []string) ([]string, error) { j, err := json.Marshal(map[string]any{ "name": serverName, "command": command, @@ -134,43 +139,42 @@ var supportedClients = []clientConfig{ } return []string{"code", "--add-mcp", string(j)}, nil }, + SkillsDir: "~/.agents/skills", }, { ClientType: Antigravity, Name: "Google Antigravity", EditorNames: []string{"antigravity", "agy"}, MCPServersPathPrefix: "/mcpServers", - ConfigPaths: []string{ + MCPConfigPaths: []string{ "~/.gemini/antigravity/mcp_config.json", }, - skillsDir: func(home string, getenv func(string) string) string { - return filepath.Join(home, ".gemini", "antigravity", "skills") - }, + SkillsDir: "~/.gemini/antigravity/skills", }, { ClientType: KiroCLI, Name: "Kiro CLI", EditorNames: []string{"kiro-cli"}, - ConfigPaths: []string{ + MCPConfigPaths: []string{ "~/.kiro/settings/mcp.json", }, - buildInstallCommand: func(serverName, command string, args []string) ([]string, error) { + buildMCPInstallCommand: func(serverName, command string, args []string) ([]string, error) { return []string{"kiro-cli", "mcp", "add", "--name", serverName, "--command", command, "--args", strings.Join(args, ",")}, nil }, - skillsDir: func(home string, getenv func(string) string) string { - return filepath.Join(envOr(getenv, "KIRO_HOME", filepath.Join(home, ".kiro")), "skills") - }, + SkillsDir: "~/.kiro/skills", + SkillsDirEnv: "${KIRO_HOME}/skills", }, { ClientType: Copilot, Name: "GitHub Copilot CLI", EditorNames: []string{"copilot", "copilot-cli"}, - ConfigPaths: []string{ + MCPConfigPaths: []string{ "~/.copilot/mcp-config.json", }, - buildInstallCommand: func(serverName, command string, args []string) ([]string, error) { + buildMCPInstallCommand: func(serverName, command string, args []string) ([]string, error) { return append([]string{"copilot", "mcp", "add", serverName, "--", command}, args...), nil }, + SkillsDir: "~/.agents/skills", }, } @@ -342,12 +346,3 @@ func (m clientSelectModel) View() tea.View { s.WriteString("\nUse ↑/↓ arrows or number keys to navigate, enter to select, q to quit") return tea.NewView(s.String()) } - -// envOr returns the value of the environment variable key, or fallback if it -// is unset or empty. -func envOr(getenv func(string) string, key, fallback string) string { - if v := getenv(key); v != "" { - return v - } - return fallback -} diff --git a/internal/cmd/client_helper_test.go b/internal/cmd/client_helper_test.go index 2b67d704..d1dc00b4 100644 --- a/internal/cmd/client_helper_test.go +++ b/internal/cmd/client_helper_test.go @@ -45,12 +45,15 @@ func TestFindClientConfig(t *testing.T) { } // Every client needs an install mechanism: JSON patching (path // prefix) or a CLI install command. - if found.MCPServersPathPrefix == "" && found.buildInstallCommand == nil { - t.Errorf("%s: needs MCPServersPathPrefix or buildInstallCommand", cfg.ClientType) + if found.MCPServersPathPrefix == "" && found.buildMCPInstallCommand == nil { + t.Errorf("%s: needs MCPServersPathPrefix or buildMCPInstallCommand", cfg.ClientType) } // CLI-only clients (no config paths) must have an install command. - if len(found.ConfigPaths) == 0 && found.buildInstallCommand == nil { - t.Errorf("%s: CLI-only clients must have buildInstallCommand", cfg.ClientType) + if len(found.MCPConfigPaths) == 0 && found.buildMCPInstallCommand == nil { + t.Errorf("%s: CLI-only clients must have buildMCPInstallCommand", cfg.ClientType) + } + if found.SkillsDir == "" { + t.Errorf("%s: needs SkillsDir", cfg.ClientType) } } }) diff --git a/internal/cmd/mcp_install.go b/internal/cmd/mcp_install.go index 441df1df..a7b941c6 100644 --- a/internal/cmd/mcp_install.go +++ b/internal/cmd/mcp_install.go @@ -162,17 +162,17 @@ func InstallMCPForClient(opts InstallOptions) error { if opts.CustomConfigPath != "" { // Expand custom config path for ~ and environment variables, then use it directly configPath = util.ExpandPath(opts.CustomConfigPath) - } else if len(clientCfg.ConfigPaths) > 0 { + } else if len(clientCfg.MCPConfigPaths) > 0 { // Use manual config path discovery for clients with configured paths - configPath, err = findClientConfigFile(clientCfg.ConfigPaths) + configPath, err = findClientConfigFile(clientCfg.MCPConfigPaths) if err != nil { return fmt.Errorf("failed to find configuration for %s: %w", opts.ClientName, err) } - } else if clientCfg.buildInstallCommand == nil { - // Client has neither ConfigPaths nor buildInstallCommand - return fmt.Errorf("client %s has no ConfigPaths or buildInstallCommand defined", opts.ClientName) + } else if clientCfg.buildMCPInstallCommand == nil { + // Client has neither MCPConfigPaths nor buildMCPInstallCommand + return fmt.Errorf("client %s has no MCPConfigPaths or buildMCPInstallCommand defined", opts.ClientName) } - // else: CLI-only client - configPath remains empty, will use buildInstallCommand + // else: CLI-only client - configPath remains empty, will use buildMCPInstallCommand // Create backup if requested and we have a config file if opts.CreateBackup && configPath != "" { @@ -183,7 +183,7 @@ func InstallMCPForClient(opts InstallOptions) error { } // Add MCP server to configuration - if clientCfg.buildInstallCommand != nil { + if clientCfg.buildMCPInstallCommand != nil { // Use CLI approach when install command builder is configured if err := addMCPServerViaCLI(clientCfg, opts.ServerName, opts.Command, opts.Args); err != nil { return fmt.Errorf("failed to add MCP server configuration: %w", err) @@ -224,8 +224,8 @@ func installTigerMCPForClient(cmd *cobra.Command, clientName string, createBacku configPath := customConfigPath if configPath == "" { clientCfg, _ := findClientConfig(clientName) - if clientCfg != nil && len(clientCfg.ConfigPaths) > 0 { - configPath, _ = findClientConfigFile(clientCfg.ConfigPaths) + if clientCfg != nil && len(clientCfg.MCPConfigPaths) > 0 { + configPath, _ = findClientConfigFile(clientCfg.MCPConfigPaths) } } @@ -310,12 +310,12 @@ func defaultGetTigerExecutablePath() (string, error) { // addMCPServerViaCLI adds an MCP server using a CLI command configured in clientConfig func addMCPServerViaCLI(clientCfg *clientConfig, serverName, command string, args []string) error { - if clientCfg.buildInstallCommand == nil { + if clientCfg.buildMCPInstallCommand == nil { return fmt.Errorf("no install command configured for client %s", clientCfg.Name) } // Build the install command with the provided parameters - installCommand, err := clientCfg.BuildInstallCommand(serverName, command, args) + installCommand, err := clientCfg.BuildMCPInstallCommand(serverName, command, args) if err != nil { return fmt.Errorf("failed to build install command: %w", err) } diff --git a/internal/cmd/mcp_install_test.go b/internal/cmd/mcp_install_test.go index 91b3c9d6..83506ad7 100644 --- a/internal/cmd/mcp_install_test.go +++ b/internal/cmd/mcp_install_test.go @@ -454,15 +454,15 @@ func TestFindClientConfigFile(t *testing.T) { t.Run("per-client fallback paths", func(t *testing.T) { t.Setenv("HOME", t.TempDir()) for _, cfg := range supportedClients { - if len(cfg.ConfigPaths) == 0 { + if len(cfg.MCPConfigPaths) == 0 { continue } - got, err := findClientConfigFile(cfg.ConfigPaths) + got, err := findClientConfigFile(cfg.MCPConfigPaths) if err != nil { t.Errorf("%s: unexpected error: %v", cfg.Name, err) continue } - if want := util.ExpandPath(cfg.ConfigPaths[0]); got != want { + if want := util.ExpandPath(cfg.MCPConfigPaths[0]); got != want { t.Errorf("%s: fallback = %q, want %q", cfg.Name, got, want) } } @@ -485,7 +485,7 @@ func TestAddMCPServerViaCLI(t *testing.T) { cfg := &clientConfig{ ClientType: "test-client", Name: "Test Client", - buildInstallCommand: func(serverName, command string, args []string) ([]string, error) { + buildMCPInstallCommand: func(serverName, command string, args []string) ([]string, error) { return []string{"nonexistent-command-12345", "arg1", "arg2"}, nil }, } @@ -512,7 +512,7 @@ func TestAddMCPServerViaCLI(t *testing.T) { Copilot: {"copilot", "mcp", "add", "tiger", "--", "/path/to/tiger", "mcp", "start"}, } for _, cfg := range supportedClients { - if cfg.buildInstallCommand == nil { + if cfg.buildMCPInstallCommand == nil { continue } wantCmd, ok := want[cfg.ClientType] @@ -520,7 +520,7 @@ func TestAddMCPServerViaCLI(t *testing.T) { t.Errorf("%s: CLI-based client missing from expected command table", cfg.ClientType) continue } - got, err := cfg.BuildInstallCommand("tiger", "/path/to/tiger", []string{"mcp", "start"}) + got, err := cfg.BuildMCPInstallCommand("tiger", "/path/to/tiger", []string{"mcp", "start"}) if err != nil { t.Errorf("%s: unexpected error: %v", cfg.ClientType, err) continue diff --git a/internal/cmd/skills_install.go b/internal/cmd/skills_install.go index 6f3744c3..e8cdf495 100644 --- a/internal/cmd/skills_install.go +++ b/internal/cmd/skills_install.go @@ -12,7 +12,6 @@ import ( "os" "path" "path/filepath" - "regexp" "slices" "strconv" "strings" @@ -31,13 +30,6 @@ const ( // skillsRepoDir is the directory within the repo holding one subdirectory // per skill. skillsRepoDir = "skills" - - // maxSkillsArchiveSize bounds the decompressed archive, guarding against a - // runaway download. - maxSkillsArchiveSize = 256 << 20 - // maxSymlinkHops bounds symlink resolution within the archive, which also - // breaks symlink cycles. - maxSymlinkHops = 40 ) // skillsTarballURL is the GitHub REST API endpoint for the skills repo's @@ -45,11 +37,6 @@ const ( // It's a var so tests can point it at a local server. var skillsTarballURL = "https://api.github.com/repos/" + skillsRepo + "/tarball/" + skillsRepoRef -// skillNamePattern matches valid skill names per the Agent Skills -// specification. Anything else is skipped, which also keeps every name safe to -// use as a path component. -var skillNamePattern = regexp.MustCompile(`^[a-z0-9]+(-[a-z0-9]+)*$`) - func buildSkillsInstallCmd(_ *common.App) *cobra.Command { cmd := &cobra.Command{ Use: "install [client]", @@ -57,16 +44,15 @@ func buildSkillsInstallCmd(_ *common.App) *cobra.Command { Short: "Install agent skills for an AI coding agent", Long: fmt.Sprintf(`Install agent skills for an AI coding agent. -Skills are downloaded from https://github.com/%s and installed -for the current user into ~/.agents/skills, which most coding agents read. For -clients that read skills from their own directory, each skill is symlinked -into that directory, so every client shares a single copy. +Skills are installed for the current user into ~/.agents/skills, which most +coding agents read, or into the client's own skills directory for clients that +don't. Existing skills with the same names are replaced, so re-running the command updates the installed skills to the latest version. %s -If no client is specified, you'll be prompted to select one interactively.`, skillsRepo, generateSkillsClientsHelp()), +If no client is specified, you'll be prompted to select one interactively.`, generateSkillsClientsHelp()), Example: ` # Interactive client selection tiger skills install @@ -98,14 +84,9 @@ If no client is specified, you'll be prompted to select one interactively.`, ski return err } - home, err := os.UserHomeDir() + skillsDir, err := resolveSkillsDir(clientCfg) if err != nil { - return fmt.Errorf("failed to determine home directory: %w", err) - } - skillsDir := filepath.Join(home, ".agents", "skills") - var linkDir string - if clientCfg.skillsDir != nil { - linkDir = clientCfg.skillsDir(home, os.Getenv) + return err } skills, err := fetchSkills(cmd.Context()) @@ -113,8 +94,7 @@ If no client is specified, you'll be prompted to select one interactively.`, ski return err } - result, err := installSkills(skills, skillsDir, linkDir) - if err != nil { + if err := installSkills(skills, skillsDir); err != nil { return err } @@ -122,12 +102,6 @@ If no client is specified, you'll be prompted to select one interactively.`, ski for _, s := range skills { cmd.Printf(" %s\n", s.name) } - switch { - case result.copied: - cmd.Printf("Copied skills into %s (symlinks are not supported here).\n", linkDir) - case result.linked: - cmd.Printf("Linked skills into %s.\n", linkDir) - } cmd.Printf("\nRestart %s to load the new skills.\n", clientCfg.Name) return nil }, @@ -137,22 +111,47 @@ If no client is specified, you'll be prompted to select one interactively.`, ski } // generateSkillsClientsHelp generates the supported clients section of the -// help text, noting where each client's skills land. Env var overrides are -// ignored so the text doesn't depend on the environment it's generated in. +// help text, noting where each client's skills land. func generateSkillsClientsHelp() string { - noEnv := func(string) string { return "" } var b strings.Builder b.WriteString("Supported Clients:\n") for _, cfg := range supportedClients { - dir := "~/.agents/skills" - if cfg.skillsDir != nil { - dir = cfg.skillsDir("~", noEnv) - } - fmt.Fprintf(&b, " %-24s %s (%s)\n", cfg.EditorNames[0], cfg.Name, dir) + fmt.Fprintf(&b, " %-24s %s (%s)\n", cfg.EditorNames[0], cfg.Name, cfg.SkillsDir) } return b.String() } +// resolveSkillsDir returns the absolute directory to install skills into for +// the given client. +func resolveSkillsDir(cfg *clientConfig) (string, error) { + var dir string + if expanded, ok := expandEnvStrict(cfg.SkillsDirEnv); cfg.SkillsDirEnv != "" && ok { + dir = filepath.Clean(expanded) + } else { + dir = util.ExpandPath(cfg.SkillsDir) + } + // ExpandPath leaves a ~ in place if the home directory is unknown, which + // would otherwise install relative to the working directory. + if !filepath.IsAbs(dir) { + return "", fmt.Errorf("failed to determine skills directory: %s is not an absolute path", dir) + } + return dir, nil +} + +// expandEnvStrict expands env var references in s, reporting false if any +// referenced variable is unset or empty. +func expandEnvStrict(s string) (string, bool) { + ok := true + expanded := os.Expand(s, func(name string) string { + v := os.Getenv(name) + if v == "" { + ok = false + } + return v + }) + return expanded, ok +} + // skill is one skill directory, with its contents held in memory. type skill struct { name string @@ -190,8 +189,11 @@ func fetchSkills(ctx context.Context) ([]skill, error) { if err != nil { return nil, fmt.Errorf("failed to read skills archive: %w", err) } + if err := resp.Body.Close(); err != nil { + return nil, fmt.Errorf("failed to read skills archive: %w", err) + } if len(skills) == 0 { - return nil, fmt.Errorf("no skills found in %s", skillsRepo) + return nil, errors.New("no skills found") } return skills, nil } @@ -212,8 +214,9 @@ func skillsDownloadError(resp *http.Response) error { // extractSkills reads a gzipped tarball of the skills repo and returns its // skills, sorted by name. A skill is a subdirectory of skillsRepoDir holding a -// SKILL.md. Symlinks within the repo are resolved to the files they point at, -// since skills share files that way and an installed skill must stand alone. +// SKILL.md. Symlinks within skillsRepoDir are resolved to the files they point +// at, since skills share files that way and an installed skill must stand +// alone; anything else in the repo is ignored. func extractSkills(r io.Reader) ([]skill, error) { gz, err := gzip.NewReader(r) if err != nil { @@ -221,11 +224,11 @@ func extractSkills(r io.Reader) ([]skill, error) { } defer gz.Close() - a := &skillsArchive{ - files: map[string]skillFile{}, + a := &repoArchive{ + files: map[string]archiveFile{}, links: map[string]string{}, } - tr := tar.NewReader(io.LimitReader(gz, maxSkillsArchiveSize)) + tr := tar.NewReader(gz) for { hdr, err := tr.Next() if errors.Is(err, io.EOF) { @@ -236,11 +239,10 @@ func extractSkills(r io.Reader) ([]skill, error) { } // GitHub tarballs nest everything under a single top-level directory - // named after the repo and commit. Entries outside it (the pax global - // header) have no slash and are skipped. - _, name, ok := strings.Cut(hdr.Name, "/") - name = strings.TrimSuffix(name, "/") - if !ok || !fs.ValidPath(name) || name == "." { + // named after the repo and commit. ValidPath keeps names safe to + // join onto the install directory later. + _, name, _ := strings.Cut(hdr.Name, "/") + if !strings.HasPrefix(name, skillsRepoDir+"/") || !fs.ValidPath(name) { continue } @@ -250,13 +252,12 @@ func extractSkills(r io.Reader) ([]skill, error) { if err != nil { return nil, err } - a.files[name] = skillFile{ + a.files[name] = archiveFile{ executable: hdr.Mode&0o111 != 0, data: data, } case tar.TypeSymlink: - // Absolute targets point outside the repo, and relative ones may - // climb out of it; either way they can't be resolved. + // An absolute target can't refer to a file in the repo. if path.IsAbs(hdr.Linkname) { continue } @@ -264,13 +265,14 @@ func extractSkills(r io.Reader) ([]skill, error) { } } + if err := gz.Close(); err != nil { + return nil, err + } + var skills []skill for _, name := range a.children(skillsRepoDir) { - if !skillNamePattern.MatchString(name) { - continue - } s := skill{name: name} - a.walk(path.Join(skillsRepoDir, name), "", 0, func(f skillFile) { + a.walk(path.Join(skillsRepoDir, name), "", map[string]bool{}, func(f skillFile) { s.files = append(s.files, f) }) if !slices.ContainsFunc(s.files, func(f skillFile) bool { return f.path == "SKILL.md" }) { @@ -282,37 +284,49 @@ func extractSkills(r io.Reader) ([]skill, error) { return skills, nil } -// skillsArchive indexes the regular files and symlinks of an extracted -// tarball by their slash-separated path within the repo. -type skillsArchive struct { - files map[string]skillFile +// repoArchive indexes the regular files and symlinks under skillsRepoDir in +// the repo's tarball by their slash-separated path within the repo. +type repoArchive struct { + files map[string]archiveFile links map[string]string // symlink path -> target path } +// archiveFile is a regular file in the repo's tarball. +type archiveFile struct { + executable bool + data []byte +} + // walk emits every regular file reachable at src, which may be a file, a -// symlink, or a directory, with paths rebased from src onto dst. Dangling -// symlinks, and those nested more than maxSymlinkHops deep, are skipped. -func (a *skillsArchive) walk(src, dst string, hops int, emit func(skillFile)) { +// symlink, or a directory, with paths rebased from src onto dst. following +// holds the symlinks being resolved on the way to src, so a symlink that leads +// back to one of them (a cycle) is skipped, as are dangling symlinks. +func (a *repoArchive) walk(src, dst string, following map[string]bool, emit func(skillFile)) { if f, ok := a.files[src]; ok { if dst != "" { - f.path = dst - emit(f) + emit(skillFile{ + path: dst, + executable: f.executable, + data: f.data, + }) } return } if target, ok := a.links[src]; ok { - if hops < maxSymlinkHops { - a.walk(target, dst, hops+1, emit) + if !following[src] { + following[src] = true + a.walk(target, dst, following, emit) + delete(following, src) } return } for _, child := range a.children(src) { - a.walk(path.Join(src, child), path.Join(dst, child), hops, emit) + a.walk(path.Join(src, child), path.Join(dst, child), following, emit) } } // children returns the sorted names of the entries directly inside dir. -func (a *skillsArchive) children(dir string) []string { +func (a *repoArchive) children(dir string) []string { prefix := dir + "/" seen := map[string]bool{} var names []string @@ -337,61 +351,19 @@ func (a *skillsArchive) children(dir string) []string { return names } -// installSkillsResult reports how skills reached the client's own skills -// directory, if it has one. -type installSkillsResult struct { - linked bool // symlinked into the client's directory - copied bool // copied into the client's directory, as symlinks failed -} - -// installSkills writes each skill into skillsDir, replacing any existing copy, -// then symlinks it into linkDir if set. Linking is skipped when linkDir -// resolves to skillsDir itself (e.g. the user has symlinked one to the other). -func installSkills(skills []skill, skillsDir, linkDir string) (installSkillsResult, error) { - var result installSkillsResult +// installSkills writes each skill into skillsDir, replacing any existing copy. +func installSkills(skills []skill, skillsDir string) error { if err := os.MkdirAll(skillsDir, 0o755); err != nil { - return result, fmt.Errorf("failed to create skills directory: %w", err) + return fmt.Errorf("failed to create skills directory: %w", err) } for _, s := range skills { if err := writeSkill(filepath.Join(skillsDir, s.name), s); err != nil { - return result, fmt.Errorf("failed to install skill %s: %w", s.name, err) + return fmt.Errorf("failed to install skill %s: %w", s.name, err) } } - - if linkDir == "" { - return result, nil - } - if err := os.MkdirAll(linkDir, 0o755); err != nil { - return result, fmt.Errorf("failed to create skills directory: %w", err) - } - same, err := sameDir(skillsDir, linkDir) - if err != nil { - return result, err - } - if same { - return result, nil - } - for _, s := range skills { - target := filepath.Join(skillsDir, s.name) - link := filepath.Join(linkDir, s.name) - err := linkSkill(target, link) - if errors.Is(err, errSymlinkUnsupported) { - err = writeSkill(link, s) - result.copied = true - } else { - result.linked = true - } - if err != nil { - return result, fmt.Errorf("failed to link skill %s: %w", s.name, err) - } - } - return result, nil + return nil } -// errSymlinkUnsupported reports that a symlink couldn't be created (e.g. on -// Windows without Developer Mode), so the caller should fall back to a copy. -var errSymlinkUnsupported = errors.New("symlinks not supported") - // writeSkill writes s to dir, replacing whatever is there. The new copy is // built in a temporary directory beside dir and swapped in, so a failure // partway through leaves the existing copy intact. @@ -419,34 +391,16 @@ func writeSkill(dir string, s skill) error { if err := os.Chmod(tmp, 0o755); err != nil { return err } - return replacePath(tmp, dir) -} - -// linkSkill points link at target, replacing whatever is at link unless it's -// already the right symlink. The symlink is created under a temporary name -// and swapped in, so link is never left missing. -func linkSkill(target, link string) error { - if current, err := os.Readlink(link); err == nil && current == target { - return nil - } - - tmp, err := os.MkdirTemp(filepath.Dir(link), ".tiger-"+filepath.Base(link)+"-") - if err != nil { + if err := replacePath(tmp, dir); err != nil { return err } - defer os.RemoveAll(tmp) - - tmpLink := filepath.Join(tmp, "link") - if err := os.Symlink(target, tmpLink); err != nil { - return fmt.Errorf("%w: %w", errSymlinkUnsupported, err) - } - return replacePath(tmpLink, link) + return os.RemoveAll(tmp) } // replacePath moves src to dst, replacing any file, directory, or symlink at // dst. A rename can't replace a non-empty directory, so the existing entry is -// first moved aside, and restored if the swap fails. Symlinks are moved and -// removed as links, never followed. +// first moved aside, and restored if the swap fails. A symlink at dst is +// removed as a link, never followed. func replacePath(src, dst string) error { if _, err := os.Lstat(dst); errors.Is(err, fs.ErrNotExist) { return os.Rename(src, dst) @@ -470,19 +424,5 @@ func replacePath(src, dst string) error { } return err } - return nil -} - -// sameDir reports whether a and b resolve to the same directory once symlinks -// are followed. -func sameDir(a, b string) (bool, error) { - ra, err := filepath.EvalSymlinks(a) - if err != nil { - return false, err - } - rb, err := filepath.EvalSymlinks(b) - if err != nil { - return false, err - } - return ra == rb, nil + return os.RemoveAll(aside) } diff --git a/internal/cmd/skills_install_test.go b/internal/cmd/skills_install_test.go index a35230cb..33b07236 100644 --- a/internal/cmd/skills_install_test.go +++ b/internal/cmd/skills_install_test.go @@ -6,7 +6,6 @@ import ( "compress/gzip" "fmt" "io/fs" - "maps" "net/http" "net/http/httptest" "os" @@ -205,18 +204,16 @@ func writeTree(t *testing.T, root string, tree map[string]string) { } // skillsInstallOutput is the exact stdout of a successful install of the -// fixture skills. linkLine is the line describing the client directory, if -// any. -func skillsInstallOutput(home, linkLine, clientName string) string { - return fmt.Sprintf("Installed 2 skills to %s:\n alpha\n beta\n%s\nRestart %s to load the new skills.\n", - filepath.Join(home, ".agents", "skills"), linkLine, clientName) +// fixture skills into dir. +func skillsInstallOutput(dir, clientName string) string { + return fmt.Sprintf("Installed 2 skills to %s:\n alpha\n beta\n\nRestart %s to load the new skills.\n", dir, clientName) } func TestSkillsInstallCmd(t *testing.T) { // The fixture repo exercises skill discovery and symlink resolution: - // file and directory symlinks between skills, one that climbs out of - // skills/ but stays in the repo, and absolute, dangling, and looping ones - // (skipped), plus entries that aren't valid skills. + // file and directory symlinks between skills (resolved), and ones that + // leave skills/, are absolute, dangle, or loop (skipped), plus entries + // that aren't skills. tarball := skillsTarball(t, tarEntry{name: "README.md", body: "repo readme"}, tarEntry{name: "skills/"}, @@ -232,60 +229,48 @@ func TestSkillsInstallCmd(t *testing.T) { tarEntry{name: "skills/beta/references/absolute.md", link: "/etc/passwd"}, tarEntry{name: "skills/beta/references/dangling.md", link: "missing.md"}, tarEntry{name: "skills/beta/references/loop.md", link: "loop.md"}, - tarEntry{name: "skills/Bad_Name/SKILL.md", body: "invalid name"}, + tarEntry{name: "skills/beta/references/loop-dir", link: "."}, tarEntry{name: "skills/no-skill-md/README.md", body: "no SKILL.md"}, ) - // installedSkills is the tree the fixture installs under ~/.agents/skills. - installedSkills := map[string]string{ - ".agents/skills/alpha/SKILL.md": "alpha skill", - ".agents/skills/alpha/references/guide.md": "alpha guide", - ".agents/skills/alpha/scripts/run.sh": "[executable] #!/bin/sh", - ".agents/skills/beta/SKILL.md": "beta skill", - ".agents/skills/beta/references/alpha-guide.md": "alpha guide", - ".agents/skills/beta/references/alpha-refs/guide.md": "alpha guide", - ".agents/skills/beta/references/readme.md": "repo readme", - } - // withLinks returns installedSkills plus symlinks to each skill from the - // client directory linkDir (relative to home). - withLinks := func(home, linkDir string) map[string]string { - tree := map[string]string{ - linkDir + "/alpha": "-> " + filepath.Join(home, ".agents", "skills", "alpha"), - linkDir + "/beta": "-> " + filepath.Join(home, ".agents", "skills", "beta"), + // installedSkills is the tree the fixture installs into dir (relative to + // the home directory). + installedSkills := func(dir string) map[string]string { + return map[string]string{ + dir + "/alpha/SKILL.md": "alpha skill", + dir + "/alpha/references/guide.md": "alpha guide", + dir + "/alpha/scripts/run.sh": "[executable] #!/bin/sh", + dir + "/beta/SKILL.md": "beta skill", + dir + "/beta/references/alpha-guide.md": "alpha guide", + dir + "/beta/references/alpha-refs/guide.md": "alpha guide", + // A directory symlink cycle is followed once, then cut off. + dir + "/beta/references/loop-dir/alpha-guide.md": "alpha guide", + dir + "/beta/references/loop-dir/alpha-refs/guide.md": "alpha guide", } - maps.Copy(tree, installedSkills) - return tree } codexHome := t.TempDir() claudeHome := t.TempDir() claudeConfigHome := t.TempDir() + claudeEmptyEnvHome := t.TempDir() claudeConfigDir := filepath.Join(claudeConfigHome, "claude-config") kiroHome := t.TempDir() antigravityHome := t.TempDir() aliasHome := t.TempDir() - // reinstallHome holds a previous install to be replaced: a stale skill - // directory, a client entry that's a real directory, a client symlink - // pointing elsewhere, and an unrelated skill that must be left alone. + // reinstallHome holds a previous install to be replaced: a skill with a + // stale file, a skill that's a symlink to elsewhere (replaced as a link, + // leaving its target alone), and an unrelated skill that must be kept. reinstallHome := t.TempDir() writeTree(t, reinstallHome, map[string]string{ - ".agents/skills/alpha/SKILL.md": "old alpha", - ".agents/skills/alpha/stale.md": "stale", - ".agents/skills/other/SKILL.md": "unrelated skill", - ".claude/skills/alpha": "-> /somewhere/else/alpha", - ".claude/skills/beta/SKILL.md": "old beta copy", - }) - reinstallTree := withLinks(reinstallHome, ".claude/skills") - reinstallTree[".agents/skills/other/SKILL.md"] = "unrelated skill" - - // sharedHome has ~/.claude/skills symlinked to ~/.agents/skills, so the - // skills are already visible to Claude Code and no per-skill links are made. - sharedHome := t.TempDir() - writeTree(t, sharedHome, map[string]string{ - ".claude/skills": "-> " + filepath.Join(sharedHome, ".agents", "skills"), + ".claude/skills/alpha": "-> " + filepath.Join(reinstallHome, "elsewhere"), + ".claude/skills/beta/SKILL.md": "old beta", + ".claude/skills/beta/stale.md": "stale", + ".claude/skills/other/SKILL.md": "unrelated skill", + "elsewhere/SKILL.md": "symlink target", }) - sharedTree := map[string]string{".claude/skills": "-> " + filepath.Join(sharedHome, ".agents", "skills")} - maps.Copy(sharedTree, installedSkills) + reinstallTree := installedSkills(".claude/skills") + reinstallTree[".claude/skills/other/SKILL.md"] = "unrelated skill" + reinstallTree["elsewhere/SKILL.md"] = "symlink target" rateLimitReset := time.Date(2030, 1, 1, 15, 4, 0, 0, time.UTC).Unix() @@ -347,22 +332,21 @@ func TestSkillsInstallCmd(t *testing.T) { withEnv("HOME", t.TempDir()), withSkillsTarball(skillsTarball(t, tarEntry{name: "README.md", body: "repo readme"})), }, - wantErr: "no skills found in timescale/pg-aiguide", + wantErr: "no skills found", }, { name: "installs to ~/.agents/skills", args: []string{"skills", "install", "codex"}, opts: []runOption{withEnv("HOME", codexHome), withSkillsTarball(tarball)}, - wantStdout: skillsInstallOutput(codexHome, "", "Codex"), - checks: []checkFunc{checkTree(codexHome, installedSkills)}, + wantStdout: skillsInstallOutput(filepath.Join(codexHome, ".agents", "skills"), "Codex"), + checks: []checkFunc{checkTree(codexHome, installedSkills(".agents/skills"))}, }, { - name: "symlinks into claude code's skills directory", - args: []string{"skills", "install", "claude-code"}, - opts: []runOption{withEnv("HOME", claudeHome), withSkillsTarball(tarball)}, - wantStdout: skillsInstallOutput(claudeHome, - fmt.Sprintf("Linked skills into %s.\n", filepath.Join(claudeHome, ".claude", "skills")), "Claude Code"), - checks: []checkFunc{checkTree(claudeHome, withLinks(claudeHome, ".claude/skills"))}, + name: "installs to claude code's skills directory", + args: []string{"skills", "install", "claude-code"}, + opts: []runOption{withEnv("HOME", claudeHome), withSkillsTarball(tarball)}, + wantStdout: skillsInstallOutput(filepath.Join(claudeHome, ".claude", "skills"), "Claude Code"), + checks: []checkFunc{checkTree(claudeHome, installedSkills(".claude/skills"))}, }, { name: "respects CLAUDE_CONFIG_DIR", @@ -372,32 +356,39 @@ func TestSkillsInstallCmd(t *testing.T) { withEnv("CLAUDE_CONFIG_DIR", claudeConfigDir), withSkillsTarball(tarball), }, - wantStdout: skillsInstallOutput(claudeConfigHome, - fmt.Sprintf("Linked skills into %s.\n", filepath.Join(claudeConfigDir, "skills")), "Claude Code"), - checks: []checkFunc{checkTree(claudeConfigHome, withLinks(claudeConfigHome, "claude-config/skills"))}, + wantStdout: skillsInstallOutput(filepath.Join(claudeConfigDir, "skills"), "Claude Code"), + checks: []checkFunc{checkTree(claudeConfigHome, installedSkills("claude-config/skills"))}, + }, + { + name: "empty CLAUDE_CONFIG_DIR counts as unset", + args: []string{"skills", "install", "claude-code"}, + opts: []runOption{ + withEnv("HOME", claudeEmptyEnvHome), + withEnv("CLAUDE_CONFIG_DIR", ""), + withSkillsTarball(tarball), + }, + wantStdout: skillsInstallOutput(filepath.Join(claudeEmptyEnvHome, ".claude", "skills"), "Claude Code"), + checks: []checkFunc{checkTree(claudeEmptyEnvHome, installedSkills(".claude/skills"))}, }, { - name: "symlinks into kiro's skills directory", - args: []string{"skills", "install", "kiro-cli"}, - opts: []runOption{withEnv("HOME", kiroHome), withSkillsTarball(tarball)}, - wantStdout: skillsInstallOutput(kiroHome, - fmt.Sprintf("Linked skills into %s.\n", filepath.Join(kiroHome, ".kiro", "skills")), "Kiro CLI"), - checks: []checkFunc{checkTree(kiroHome, withLinks(kiroHome, ".kiro/skills"))}, + name: "installs to kiro's skills directory", + args: []string{"skills", "install", "kiro-cli"}, + opts: []runOption{withEnv("HOME", kiroHome), withSkillsTarball(tarball)}, + wantStdout: skillsInstallOutput(filepath.Join(kiroHome, ".kiro", "skills"), "Kiro CLI"), + checks: []checkFunc{checkTree(kiroHome, installedSkills(".kiro/skills"))}, }, { - name: "symlinks into antigravity's skills directory", - args: []string{"skills", "install", "antigravity"}, - opts: []runOption{withEnv("HOME", antigravityHome), withSkillsTarball(tarball)}, - wantStdout: skillsInstallOutput(antigravityHome, - fmt.Sprintf("Linked skills into %s.\n", filepath.Join(antigravityHome, ".gemini", "antigravity", "skills")), "Google Antigravity"), - checks: []checkFunc{checkTree(antigravityHome, withLinks(antigravityHome, ".gemini/antigravity/skills"))}, + name: "installs to antigravity's skills directory", + args: []string{"skills", "install", "antigravity"}, + opts: []runOption{withEnv("HOME", antigravityHome), withSkillsTarball(tarball)}, + wantStdout: skillsInstallOutput(filepath.Join(antigravityHome, ".gemini", "antigravity", "skills"), "Google Antigravity"), + checks: []checkFunc{checkTree(antigravityHome, installedSkills(".gemini/antigravity/skills"))}, }, { - name: "reinstall replaces existing skills and links", - args: []string{"skills", "install", "claude-code"}, - opts: []runOption{withEnv("HOME", reinstallHome), withSkillsTarball(tarball)}, - wantStdout: skillsInstallOutput(reinstallHome, - fmt.Sprintf("Linked skills into %s.\n", filepath.Join(reinstallHome, ".claude", "skills")), "Claude Code"), + name: "reinstall replaces existing skills", + args: []string{"skills", "install", "claude-code"}, + opts: []runOption{withEnv("HOME", reinstallHome), withSkillsTarball(tarball)}, + wantStdout: skillsInstallOutput(filepath.Join(reinstallHome, ".claude", "skills"), "Claude Code"), checks: []checkFunc{ checkTree(reinstallHome, reinstallTree), // Running again over a current install leaves it unchanged. @@ -410,19 +401,12 @@ func TestSkillsInstallCmd(t *testing.T) { checkTree(reinstallHome, reinstallTree), }, }, - { - name: "client directory already shared with ~/.agents/skills", - args: []string{"skills", "install", "claude-code"}, - opts: []runOption{withEnv("HOME", sharedHome), withSkillsTarball(tarball)}, - wantStdout: skillsInstallOutput(sharedHome, "", "Claude Code"), - checks: []checkFunc{checkTree(sharedHome, sharedTree)}, - }, { name: "add alias and case-insensitive client name", args: []string{"skills", "add", "CODEX"}, opts: []runOption{withEnv("HOME", aliasHome), withSkillsTarball(tarball)}, - wantStdout: skillsInstallOutput(aliasHome, "", "Codex"), - checks: []checkFunc{checkTree(aliasHome, installedSkills)}, + wantStdout: skillsInstallOutput(filepath.Join(aliasHome, ".agents", "skills"), "Codex"), + checks: []checkFunc{checkTree(aliasHome, installedSkills(".agents/skills"))}, }, }) } From be84f0f5a48e67a2b7bb793442a8273ad97a3cbe Mon Sep 17 00:00:00 2001 From: Nathan Cochran Date: Wed, 7 Oct 2026 16:40:12 -0400 Subject: [PATCH 03/17] Add logic to detect collisions and clean up old skills --- README.md | 8 +- docs/cli/tiger_skills_install.md | 12 ++- internal/cmd/skills_install.go | 157 +++++++++++++++++++++++++--- internal/cmd/skills_install_test.go | 103 +++++++++++++++--- 4 files changed, 241 insertions(+), 39 deletions(-) diff --git a/README.md b/README.md index 9f7800d8..98fb0640 100644 --- a/README.md +++ b/README.md @@ -267,8 +267,12 @@ tiger skills install claude-code Skills are installed for the current user into `~/.agents/skills`, which most coding agents read, or into the client's own skills directory for clients that -don't (Claude Code, Kiro CLI, Google Antigravity). Re-run the command to update -the skills to the latest version. +don't (Claude Code, Kiro CLI, Google Antigravity). Use `--skills-dir` to install +somewhere else. + +Re-run the command to update the skills to the latest version; skills that are +no longer available are removed. Existing skills that weren't installed by +Tiger CLI are never replaced unless you pass `--force`. ## Configuration diff --git a/docs/cli/tiger_skills_install.md b/docs/cli/tiger_skills_install.md index b50b674f..225a165f 100644 --- a/docs/cli/tiger_skills_install.md +++ b/docs/cli/tiger_skills_install.md @@ -10,8 +10,9 @@ Skills are installed for the current user into ~/.agents/skills, which most coding agents read, or into the client's own skills directory for clients that don't. -Existing skills with the same names are replaced, so re-running the command -updates the installed skills to the latest version. +Re-running the command updates the installed skills to the latest version and +removes any that are no longer available. Existing skills that weren't +installed by Tiger CLI are never replaced unless --force is given. Supported Clients: claude-code Claude Code (~/.claude/skills) @@ -41,12 +42,17 @@ tiger skills install [client] [flags] # Install for Codex tiger skills install codex + + # Install into a custom skills directory + tiger skills install claude-code --skills-dir ~/my-skills ``` ### Options ``` - -h, --help help for install + --force Replace existing skills that weren't installed by Tiger CLI + -h, --help help for install + --skills-dir string Custom skills directory to install into (overrides the client's default) ``` ### Options inherited from parent commands diff --git a/internal/cmd/skills_install.go b/internal/cmd/skills_install.go index e8cdf495..7569d6ef 100644 --- a/internal/cmd/skills_install.go +++ b/internal/cmd/skills_install.go @@ -30,6 +30,16 @@ const ( // skillsRepoDir is the directory within the repo holding one subdirectory // per skill. skillsRepoDir = "skills" + + // skillMarkerFile is written into every skill Tiger CLI installs, marking + // it as one a later install may replace or remove. + skillMarkerFile = ".tiger-cli" + skillMarkerContent = "This skill is managed by Tiger CLI. Running 'tiger skills install' updates it\n" + + "to the latest version, or removes it if it's no longer available.\n" + + // skillsTempPrefix starts the name of every temporary directory an install + // creates in the skills directory. + skillsTempPrefix = ".tiger-" ) // skillsTarballURL is the GitHub REST API endpoint for the skills repo's @@ -38,6 +48,9 @@ const ( var skillsTarballURL = "https://api.github.com/repos/" + skillsRepo + "/tarball/" + skillsRepoRef func buildSkillsInstallCmd(_ *common.App) *cobra.Command { + var force bool + var skillsDirFlag string + cmd := &cobra.Command{ Use: "install [client]", Aliases: []string{"add"}, @@ -48,8 +61,9 @@ Skills are installed for the current user into ~/.agents/skills, which most coding agents read, or into the client's own skills directory for clients that don't. -Existing skills with the same names are replaced, so re-running the command -updates the installed skills to the latest version. +Re-running the command updates the installed skills to the latest version and +removes any that are no longer available. Existing skills that weren't +installed by Tiger CLI are never replaced unless --force is given. %s If no client is specified, you'll be prompted to select one interactively.`, generateSkillsClientsHelp()), @@ -60,7 +74,10 @@ If no client is specified, you'll be prompted to select one interactively.`, gen tiger skills install claude-code # Install for Codex - tiger skills install codex`, + tiger skills install codex + + # Install into a custom skills directory + tiger skills install claude-code --skills-dir ~/my-skills`, Args: cobra.MaximumNArgs(1), ValidArgs: getValidEditorNames(), SilenceUsage: true, @@ -84,7 +101,7 @@ If no client is specified, you'll be prompted to select one interactively.`, gen return err } - skillsDir, err := resolveSkillsDir(clientCfg) + skillsDir, err := resolveSkillsDir(clientCfg, skillsDirFlag) if err != nil { return err } @@ -94,7 +111,8 @@ If no client is specified, you'll be prompted to select one interactively.`, gen return err } - if err := installSkills(skills, skillsDir); err != nil { + removed, err := installSkills(skills, skillsDir, force) + if err != nil { return err } @@ -102,11 +120,18 @@ If no client is specified, you'll be prompted to select one interactively.`, gen for _, s := range skills { cmd.Printf(" %s\n", s.name) } + for _, name := range removed { + cmd.Printf("Removed %s (no longer available).\n", name) + } cmd.Printf("\nRestart %s to load the new skills.\n", clientCfg.Name) return nil }, } + cmd.Flags().BoolVar(&force, "force", false, "Replace existing skills that weren't installed by Tiger CLI") + cmd.Flags().StringVar(&skillsDirFlag, "skills-dir", "", "Custom skills directory to install into (overrides the client's default)") + registerFlagCompletion(cmd, "skills-dir", dirCompletion) + return cmd } @@ -122,9 +147,16 @@ func generateSkillsClientsHelp() string { } // resolveSkillsDir returns the absolute directory to install skills into for -// the given client. -func resolveSkillsDir(cfg *clientConfig) (string, error) { +// the given client, or custom (relative to the working directory) if set. +func resolveSkillsDir(cfg *clientConfig, custom string) (string, error) { var dir string + if custom != "" { + abs, err := filepath.Abs(util.ExpandPath(custom)) + if err != nil { + return "", fmt.Errorf("failed to resolve skills directory: %w", err) + } + return abs, nil + } if expanded, ok := expandEnvStrict(cfg.SkillsDirEnv); cfg.SkillsDirEnv != "" && ok { dir = filepath.Clean(expanded) } else { @@ -351,24 +383,112 @@ func (a *repoArchive) children(dir string) []string { return names } -// installSkills writes each skill into skillsDir, replacing any existing copy. -func installSkills(skills []skill, skillsDir string) error { +// installSkills writes each skill into skillsDir, replacing any existing copy, +// then removes skills Tiger CLI installed earlier that are no longer among +// them, returning their names. Unless force is set, it refuses up front to +// replace anything Tiger CLI didn't install. +func installSkills(skills []skill, skillsDir string, force bool) ([]string, error) { + if !force { + var conflicts []string + for _, s := range skills { + exists, owned, err := skillOwnership(filepath.Join(skillsDir, s.name)) + if err != nil { + return nil, fmt.Errorf("failed to check existing skill %s: %w", s.name, err) + } + if exists && !owned { + conflicts = append(conflicts, s.name) + } + } + if len(conflicts) > 0 { + return nil, fmt.Errorf("skills already exist in %s and weren't installed by Tiger CLI: %s. Use --force to replace them", + skillsDir, strings.Join(conflicts, ", ")) + } + } + if err := os.MkdirAll(skillsDir, 0o755); err != nil { - return fmt.Errorf("failed to create skills directory: %w", err) + return nil, fmt.Errorf("failed to create skills directory: %w", err) } for _, s := range skills { if err := writeSkill(filepath.Join(skillsDir, s.name), s); err != nil { - return fmt.Errorf("failed to install skill %s: %w", s.name, err) + return nil, fmt.Errorf("failed to install skill %s: %w", s.name, err) } } - return nil + + removed, err := removeStaleSkills(skills, skillsDir) + if err != nil { + return nil, fmt.Errorf("failed to remove skills that are no longer available: %w", err) + } + return removed, nil } -// writeSkill writes s to dir, replacing whatever is there. The new copy is -// built in a temporary directory beside dir and swapped in, so a failure -// partway through leaves the existing copy intact. +// removeStaleSkills removes the skills in skillsDir that Tiger CLI installed +// but that aren't among skills, returning their names. It also quietly removes +// temporary directories left behind by an install that was killed partway +// through. +func removeStaleSkills(skills []skill, skillsDir string) ([]string, error) { + current := make(map[string]bool, len(skills)) + for _, s := range skills { + current[s.name] = true + } + + entries, err := os.ReadDir(skillsDir) + if err != nil { + return nil, err + } + var removed []string + for _, e := range entries { + if !e.IsDir() || current[e.Name()] { + continue + } + dir := filepath.Join(skillsDir, e.Name()) + if strings.HasPrefix(e.Name(), skillsTempPrefix) { + if err := os.RemoveAll(dir); err != nil { + return nil, err + } + continue + } + _, owned, err := skillOwnership(dir) + if err != nil { + return nil, err + } + if !owned { + continue + } + if err := os.RemoveAll(dir); err != nil { + return nil, err + } + removed = append(removed, e.Name()) + } + return removed, nil +} + +// skillOwnership reports whether anything exists at dir, and whether it's a +// skill Tiger CLI installed: a directory (not a symlink, since Tiger CLI never +// creates those) holding the marker file. +func skillOwnership(dir string) (exists, owned bool, err error) { + info, err := os.Lstat(dir) + if errors.Is(err, fs.ErrNotExist) { + return false, false, nil + } else if err != nil { + return false, false, err + } + if !info.IsDir() { + return true, false, nil + } + info, err = os.Lstat(filepath.Join(dir, skillMarkerFile)) + if errors.Is(err, fs.ErrNotExist) { + return true, false, nil + } else if err != nil { + return true, false, err + } + return true, info.Mode().IsRegular(), nil +} + +// writeSkill writes s to dir along with the marker file, replacing whatever +// is there. The new copy is built in a temporary directory beside dir and +// swapped in, so a failure partway through leaves the existing copy intact. func writeSkill(dir string, s skill) error { - tmp, err := os.MkdirTemp(filepath.Dir(dir), ".tiger-"+s.name+"-") + tmp, err := os.MkdirTemp(filepath.Dir(dir), skillsTempPrefix+s.name+"-") if err != nil { return err } @@ -387,6 +507,9 @@ func writeSkill(dir string, s skill) error { return err } } + if err := os.WriteFile(filepath.Join(tmp, skillMarkerFile), []byte(skillMarkerContent), 0o644); err != nil { + return err + } // MkdirTemp creates the directory 0700; skills are ordinary user files. if err := os.Chmod(tmp, 0o755); err != nil { return err @@ -408,7 +531,7 @@ func replacePath(src, dst string) error { return err } - aside, err := os.MkdirTemp(filepath.Dir(dst), ".tiger-old-") + aside, err := os.MkdirTemp(filepath.Dir(dst), skillsTempPrefix+"old-") if err != nil { return err } diff --git a/internal/cmd/skills_install_test.go b/internal/cmd/skills_install_test.go index 33b07236..c3e14324 100644 --- a/internal/cmd/skills_install_test.go +++ b/internal/cmd/skills_install_test.go @@ -204,9 +204,11 @@ func writeTree(t *testing.T, root string, tree map[string]string) { } // skillsInstallOutput is the exact stdout of a successful install of the -// fixture skills into dir. -func skillsInstallOutput(dir, clientName string) string { - return fmt.Sprintf("Installed 2 skills to %s:\n alpha\n beta\n\nRestart %s to load the new skills.\n", dir, clientName) +// fixture skills into dir, with removed holding any lines reporting removed +// skills. +func skillsInstallOutput(dir, clientName string, removed ...string) string { + return fmt.Sprintf("Installed 2 skills to %s:\n alpha\n beta\n%s\nRestart %s to load the new skills.\n", + dir, strings.Join(removed, ""), clientName) } func TestSkillsInstallCmd(t *testing.T) { @@ -236,9 +238,11 @@ func TestSkillsInstallCmd(t *testing.T) { // the home directory). installedSkills := func(dir string) map[string]string { return map[string]string{ + dir + "/alpha/.tiger-cli": skillMarkerContent, dir + "/alpha/SKILL.md": "alpha skill", dir + "/alpha/references/guide.md": "alpha guide", dir + "/alpha/scripts/run.sh": "[executable] #!/bin/sh", + dir + "/beta/.tiger-cli": skillMarkerContent, dir + "/beta/SKILL.md": "beta skill", dir + "/beta/references/alpha-guide.md": "alpha guide", dir + "/beta/references/alpha-refs/guide.md": "alpha guide", @@ -257,20 +261,62 @@ func TestSkillsInstallCmd(t *testing.T) { antigravityHome := t.TempDir() aliasHome := t.TempDir() - // reinstallHome holds a previous install to be replaced: a skill with a - // stale file, a skill that's a symlink to elsewhere (replaced as a link, - // leaving its target alone), and an unrelated skill that must be kept. + // reinstallHome holds a previous install: one of our skills with a stale + // file (replaced), one no longer available (removed), skills of the + // user's, hidden or not, and a symlink of the user's to a skill of ours + // (kept), and both kinds of temporary directory left by an install that + // was killed (quietly removed). reinstallHome := t.TempDir() writeTree(t, reinstallHome, map[string]string{ - ".claude/skills/alpha": "-> " + filepath.Join(reinstallHome, "elsewhere"), - ".claude/skills/beta/SKILL.md": "old beta", - ".claude/skills/beta/stale.md": "stale", - ".claude/skills/other/SKILL.md": "unrelated skill", - "elsewhere/SKILL.md": "symlink target", + ".claude/skills/alpha/.tiger-cli": skillMarkerContent, + ".claude/skills/alpha/SKILL.md": "old alpha", + ".claude/skills/alpha/stale.md": "stale", + ".claude/skills/retired/.tiger-cli": skillMarkerContent, + ".claude/skills/retired/SKILL.md": "retired skill", + ".claude/skills/other/SKILL.md": "user's skill", + ".claude/skills/.tiger-beta-123/.tiger-cli": skillMarkerContent, + ".claude/skills/.tiger-beta-123/SKILL.md": "leftover", + ".claude/skills/.tiger-old-456/old/.tiger-cli": skillMarkerContent, + ".claude/skills/.tiger-old-456/old/SKILL.md": "leftover", + ".claude/skills/.hidden/SKILL.md": "user's hidden entry", + ".claude/skills/linked": "-> " + filepath.Join(reinstallHome, "marked"), + "marked/.tiger-cli": skillMarkerContent, + "marked/SKILL.md": "marked skill", }) reinstallTree := installedSkills(".claude/skills") - reinstallTree[".claude/skills/other/SKILL.md"] = "unrelated skill" - reinstallTree["elsewhere/SKILL.md"] = "symlink target" + reinstallTree[".claude/skills/other/SKILL.md"] = "user's skill" + reinstallTree[".claude/skills/.hidden/SKILL.md"] = "user's hidden entry" + reinstallTree[".claude/skills/linked"] = "-> " + filepath.Join(reinstallHome, "marked") + reinstallTree["marked/.tiger-cli"] = skillMarkerContent + reinstallTree["marked/SKILL.md"] = "marked skill" + + // conflictTree holds a symlink and a directory of the user's under the + // names of skills being installed, which aren't replaced without --force. + // The symlink counts as the user's even though it points at one of our + // skills, since Tiger CLI never creates symlinks. + conflictHome := t.TempDir() + conflictTree := map[string]string{ + ".agents/skills/alpha": "-> " + filepath.Join(conflictHome, "marked"), + ".agents/skills/beta/SKILL.md": "user's beta", + "marked/.tiger-cli": skillMarkerContent, + "marked/SKILL.md": "marked skill", + } + writeTree(t, conflictHome, conflictTree) + + // forceHome holds a symlink and a directory of the user's under the names + // of skills being installed. --force replaces both; the symlink is + // replaced as a link, leaving its target alone. + forceHome := t.TempDir() + writeTree(t, forceHome, map[string]string{ + ".agents/skills/alpha": "-> " + filepath.Join(forceHome, "elsewhere"), + ".agents/skills/beta/SKILL.md": "user's beta", + ".agents/skills/beta/stale.md": "stale", + "elsewhere/SKILL.md": "symlink target", + }) + forceTree := installedSkills(".agents/skills") + forceTree["elsewhere/SKILL.md"] = "symlink target" + + customHome := t.TempDir() rateLimitReset := time.Date(2030, 1, 1, 15, 4, 0, 0, time.UTC).Unix() @@ -334,6 +380,13 @@ func TestSkillsInstallCmd(t *testing.T) { }, wantErr: "no skills found", }, + { + name: "existing skills not installed by Tiger CLI", + args: []string{"skills", "install", "codex"}, + opts: []runOption{withEnv("HOME", conflictHome), withSkillsTarball(tarball)}, + wantErr: fmt.Sprintf("skills already exist in %s and weren't installed by Tiger CLI: alpha, beta. Use --force to replace them", filepath.Join(conflictHome, ".agents", "skills")), + checks: []checkFunc{checkTree(conflictHome, conflictTree)}, + }, { name: "installs to ~/.agents/skills", args: []string{"skills", "install", "codex"}, @@ -385,10 +438,11 @@ func TestSkillsInstallCmd(t *testing.T) { checks: []checkFunc{checkTree(antigravityHome, installedSkills(".gemini/antigravity/skills"))}, }, { - name: "reinstall replaces existing skills", - args: []string{"skills", "install", "claude-code"}, - opts: []runOption{withEnv("HOME", reinstallHome), withSkillsTarball(tarball)}, - wantStdout: skillsInstallOutput(filepath.Join(reinstallHome, ".claude", "skills"), "Claude Code"), + name: "reinstall replaces our skills and removes ones no longer available", + args: []string{"skills", "install", "claude-code"}, + opts: []runOption{withEnv("HOME", reinstallHome), withSkillsTarball(tarball)}, + wantStdout: skillsInstallOutput(filepath.Join(reinstallHome, ".claude", "skills"), "Claude Code", + "Removed retired (no longer available).\n"), checks: []checkFunc{ checkTree(reinstallHome, reinstallTree), // Running again over a current install leaves it unchanged. @@ -397,10 +451,25 @@ func TestSkillsInstallCmd(t *testing.T) { if again.err != nil { t.Fatalf("second install failed: %v", again.err) } + assertOutput(t, again.stdout, skillsInstallOutput(filepath.Join(reinstallHome, ".claude", "skills"), "Claude Code")) }, checkTree(reinstallHome, reinstallTree), }, }, + { + name: "--force replaces skills not installed by Tiger CLI", + args: []string{"skills", "install", "codex", "--force"}, + opts: []runOption{withEnv("HOME", forceHome), withSkillsTarball(tarball)}, + wantStdout: skillsInstallOutput(filepath.Join(forceHome, ".agents", "skills"), "Codex"), + checks: []checkFunc{checkTree(forceHome, forceTree)}, + }, + { + name: "--skills-dir overrides the client's directory", + args: []string{"skills", "install", "claude-code", "--skills-dir", "~/custom"}, + opts: []runOption{withEnv("HOME", customHome), withSkillsTarball(tarball)}, + wantStdout: skillsInstallOutput(filepath.Join(customHome, "custom"), "Claude Code"), + checks: []checkFunc{checkTree(customHome, installedSkills("custom"))}, + }, { name: "add alias and case-insensitive client name", args: []string{"skills", "add", "CODEX"}, From 5a9774f0c7f8d8a44e77809d0508cf5dab77cac7 Mon Sep 17 00:00:00 2001 From: Nathan Cochran Date: Wed, 7 Oct 2026 18:13:06 -0400 Subject: [PATCH 04/17] Checkbox picker, universal option, support multiple clients --- README.md | 27 +- docs/cli/tiger_skills.md | 2 +- docs/cli/tiger_skills_install.md | 52 +-- internal/cmd/client_helper.go | 13 +- internal/cmd/client_helper_test.go | 3 - internal/cmd/skills_install.go | 476 +++++++++++++++++++++++----- internal/cmd/skills_install_test.go | 363 ++++++++++++++++----- 7 files changed, 717 insertions(+), 219 deletions(-) diff --git a/README.md b/README.md index 98fb0640..cd971d87 100644 --- a/README.md +++ b/README.md @@ -139,7 +139,7 @@ Tiger CLI provides the following commands: - `list` - List available MCP tools, prompts, and resources (alias: `ls`) - `get` - Get detailed information about a specific MCP capability (aliases: `describe`, `show`) - `tiger skills` - Agent skills for AI coding agents - - `install` - Install PostgreSQL and TimescaleDB skills for an AI coding agent (alias: `add`) + - `install` - Install PostgreSQL and TimescaleDB skills for AI coding agents (alias: `add`) - `tiger feedback` - Submit feedback or a bug report to the Tiger Data team (pass the message as an argument or on stdin) - `tiger version` - Show version information - `tiger upgrade` - Upgrade the Tiger CLI to the latest version (alias: `update`) @@ -255,24 +255,29 @@ tiger config set docs_mcp false ## Agent Skills Tiger CLI can install curated PostgreSQL and TimescaleDB skills directly into -your coding agent: +your coding agents: ```bash -# Interactive installation (prompts for client selection) +# Interactive installation (prompts for install locations) tiger skills install -# Or specify your client directly -tiger skills install claude-code +# Or name the locations directly +tiger skills install universal claude-code ``` -Skills are installed for the current user into `~/.agents/skills`, which most -coding agents read, or into the client's own skills directory for clients that -don't (Claude Code, Kiro CLI, Google Antigravity). Use `--skills-dir` to install -somewhere else. +Skills are installed for the current user. The `universal` location, +`~/.agents/skills`, is read by most coding agents (Cursor, Codex, Gemini CLI, +VS Code, GitHub Copilot CLI, Devin). Claude Code, Kiro CLI, and Google +Antigravity read their own skills directories, so they're separate locations +(`claude-code`, `kiro-cli`, `antigravity`). Naming a client that reads +`~/.agents/skills`, such as `cursor`, selects `universal`. Use `--skills-dir` +to install into a directory of your choice instead. Re-run the command to update the skills to the latest version; skills that are -no longer available are removed. Existing skills that weren't installed by -Tiger CLI are never replaced unless you pass `--force`. +no longer available are removed. With no arguments, the locations you've +installed to before are selected by default, and `--no-prompt` installs to them +without prompting. Existing skills that weren't installed by Tiger CLI are +never replaced unless you pass `--force`. ## Configuration diff --git a/docs/cli/tiger_skills.md b/docs/cli/tiger_skills.md index abaeeba6..31fbfeb7 100644 --- a/docs/cli/tiger_skills.md +++ b/docs/cli/tiger_skills.md @@ -29,4 +29,4 @@ with PostgreSQL, TimescaleDB, and Tiger Cloud. ### SEE ALSO * [tiger](tiger.md) - Tiger CLI - Tiger Cloud Platform command-line interface -* [tiger skills install](tiger_skills_install.md) - Install agent skills for an AI coding agent +* [tiger skills install](tiger_skills_install.md) - Install agent skills for AI coding agents diff --git a/docs/cli/tiger_skills_install.md b/docs/cli/tiger_skills_install.md index 225a165f..29c9c7ef 100644 --- a/docs/cli/tiger_skills_install.md +++ b/docs/cli/tiger_skills_install.md @@ -1,50 +1,49 @@ ## tiger skills install -Install agent skills for an AI coding agent +Install agent skills for AI coding agents ### Synopsis -Install agent skills for an AI coding agent. +Install agent skills for AI coding agents. -Skills are installed for the current user into ~/.agents/skills, which most -coding agents read, or into the client's own skills directory for clients that -don't. +Skills are installed for the current user into one or more install locations: +the universal ~/.agents/skills directory, which most coding agents read, and +the skills directories of clients that don't read it. Each argument selects a +location, either by name or by the name of a client that reads it. + +Install locations: + universal ~/.agents/skills (Cursor, Devin, Codex, Gemini CLI, VS Code, GitHub Copilot CLI) + claude-code ~/.claude/skills (Claude Code) + antigravity ~/.gemini/config/skills (Google Antigravity) + kiro-cli ~/.kiro/skills (Kiro CLI) + +With no arguments, you're prompted to select locations interactively. The +locations Tiger CLI has installed skills to before are selected by default, or +universal if there are none; use --no-prompt (or run without a terminal) to +install to them without prompting. Re-running the command updates the installed skills to the latest version and removes any that are no longer available. Existing skills that weren't installed by Tiger CLI are never replaced unless --force is given. -Supported Clients: - claude-code Claude Code (~/.claude/skills) - cursor Cursor (~/.agents/skills) - devin Devin (~/.agents/skills) - codex Codex (~/.agents/skills) - gemini Gemini CLI (~/.agents/skills) - vscode VS Code (~/.agents/skills) - antigravity Google Antigravity (~/.gemini/antigravity/skills) - kiro-cli Kiro CLI (~/.kiro/skills) - copilot GitHub Copilot CLI (~/.agents/skills) - -If no client is specified, you'll be prompted to select one interactively. - ``` -tiger skills install [client] [flags] +tiger skills install [client...] [flags] ``` ### Examples ``` - # Interactive client selection + # Interactive selection tiger skills install - # Install for Claude Code - tiger skills install claude-code + # Update the skills wherever they were installed before + tiger skills install --no-prompt - # Install for Codex - tiger skills install codex + # Install to ~/.agents/skills and for Claude Code + tiger skills install universal claude-code # Install into a custom skills directory - tiger skills install claude-code --skills-dir ~/my-skills + tiger skills install --skills-dir ~/my-skills ``` ### Options @@ -52,7 +51,8 @@ tiger skills install [client] [flags] ``` --force Replace existing skills that weren't installed by Tiger CLI -h, --help help for install - --skills-dir string Custom skills directory to install into (overrides the client's default) + --no-prompt Install to the default locations without prompting + --skills-dir string Install into this skills directory only (not remembered for later installs) ``` ### Options inherited from parent commands diff --git a/internal/cmd/client_helper.go b/internal/cmd/client_helper.go index 2c92246d..0c29a959 100644 --- a/internal/cmd/client_helper.go +++ b/internal/cmd/client_helper.go @@ -37,7 +37,8 @@ type clientConfig struct { // buildMCPInstallCommand builds the CLI install command for CLI-based clients // Parameters: serverName (name to register), command (binary path), args (arguments to binary) buildMCPInstallCommand func(serverName, command string, args []string) ([]string, error) - // SkillsDir is the user-level directory skills are installed into. + // SkillsDir is the user-level skills directory of a client that doesn't + // read the universal ~/.agents/skills. Empty for clients that do. SkillsDir string // SkillsDirEnv overrides SkillsDir for clients whose skills directory can // be relocated by an env var. It's a path containing env var references @@ -80,7 +81,6 @@ var supportedClients = []clientConfig{ MCPConfigPaths: []string{ "~/.cursor/mcp.json", }, - SkillsDir: "~/.agents/skills", }, { ClientType: Devin, @@ -92,7 +92,6 @@ var supportedClients = []clientConfig{ buildMCPInstallCommand: func(serverName, command string, args []string) ([]string, error) { return append([]string{"devin", "mcp", "add", "-s", "user", serverName, "--", command}, args...), nil }, - SkillsDir: "~/.agents/skills", }, { ClientType: Codex, @@ -105,7 +104,6 @@ var supportedClients = []clientConfig{ buildMCPInstallCommand: func(serverName, command string, args []string) ([]string, error) { return append([]string{"codex", "mcp", "add", serverName, command}, args...), nil }, - SkillsDir: "~/.agents/skills", }, { ClientType: Gemini, @@ -117,7 +115,6 @@ var supportedClients = []clientConfig{ buildMCPInstallCommand: func(serverName, command string, args []string) ([]string, error) { return append([]string{"gemini", "mcp", "add", "-s", "user", serverName, command}, args...), nil }, - SkillsDir: "~/.agents/skills", }, { ClientType: VSCode, @@ -139,7 +136,6 @@ var supportedClients = []clientConfig{ } return []string{"code", "--add-mcp", string(j)}, nil }, - SkillsDir: "~/.agents/skills", }, { ClientType: Antigravity, @@ -147,9 +143,9 @@ var supportedClients = []clientConfig{ EditorNames: []string{"antigravity", "agy"}, MCPServersPathPrefix: "/mcpServers", MCPConfigPaths: []string{ - "~/.gemini/antigravity/mcp_config.json", + "~/.gemini/config/mcp_config.json", }, - SkillsDir: "~/.gemini/antigravity/skills", + SkillsDir: "~/.gemini/config/skills", }, { ClientType: KiroCLI, @@ -174,7 +170,6 @@ var supportedClients = []clientConfig{ buildMCPInstallCommand: func(serverName, command string, args []string) ([]string, error) { return append([]string{"copilot", "mcp", "add", serverName, "--", command}, args...), nil }, - SkillsDir: "~/.agents/skills", }, } diff --git a/internal/cmd/client_helper_test.go b/internal/cmd/client_helper_test.go index d1dc00b4..74035f7f 100644 --- a/internal/cmd/client_helper_test.go +++ b/internal/cmd/client_helper_test.go @@ -52,9 +52,6 @@ func TestFindClientConfig(t *testing.T) { if len(found.MCPConfigPaths) == 0 && found.buildMCPInstallCommand == nil { t.Errorf("%s: CLI-only clients must have buildMCPInstallCommand", cfg.ClientType) } - if found.SkillsDir == "" { - t.Errorf("%s: needs SkillsDir", cfg.ClientType) - } } }) } diff --git a/internal/cmd/skills_install.go b/internal/cmd/skills_install.go index 7569d6ef..f3489bb6 100644 --- a/internal/cmd/skills_install.go +++ b/internal/cmd/skills_install.go @@ -17,6 +17,8 @@ import ( "strings" "time" + tea "charm.land/bubbletea/v2" + "github.com/fatih/color" "github.com/spf13/cobra" "github.com/timescale/tiger-cli/internal/api" @@ -49,125 +51,252 @@ var skillsTarballURL = "https://api.github.com/repos/" + skillsRepo + "/tarball/ func buildSkillsInstallCmd(_ *common.App) *cobra.Command { var force bool + var noPrompt bool var skillsDirFlag string cmd := &cobra.Command{ - Use: "install [client]", + Use: "install [client...]", Aliases: []string{"add"}, - Short: "Install agent skills for an AI coding agent", - Long: fmt.Sprintf(`Install agent skills for an AI coding agent. + Short: "Install agent skills for AI coding agents", + Long: fmt.Sprintf(`Install agent skills for AI coding agents. -Skills are installed for the current user into ~/.agents/skills, which most -coding agents read, or into the client's own skills directory for clients that -don't. +Skills are installed for the current user into one or more install locations: +the universal ~/.agents/skills directory, which most coding agents read, and +the skills directories of clients that don't read it. Each argument selects a +location, either by name or by the name of a client that reads it. + +%s +With no arguments, you're prompted to select locations interactively. The +locations Tiger CLI has installed skills to before are selected by default, or +universal if there are none; use --no-prompt (or run without a terminal) to +install to them without prompting. Re-running the command updates the installed skills to the latest version and removes any that are no longer available. Existing skills that weren't -installed by Tiger CLI are never replaced unless --force is given. - -%s -If no client is specified, you'll be prompted to select one interactively.`, generateSkillsClientsHelp()), - Example: ` # Interactive client selection +installed by Tiger CLI are never replaced unless --force is given.`, generateSkillsTargetsHelp()), + Example: ` # Interactive selection tiger skills install - # Install for Claude Code - tiger skills install claude-code + # Update the skills wherever they were installed before + tiger skills install --no-prompt - # Install for Codex - tiger skills install codex + # Install to ~/.agents/skills and for Claude Code + tiger skills install universal claude-code # Install into a custom skills directory - tiger skills install claude-code --skills-dir ~/my-skills`, - Args: cobra.MaximumNArgs(1), - ValidArgs: getValidEditorNames(), + tiger skills install --skills-dir ~/my-skills`, + ValidArgs: skillsTargetNames(), SilenceUsage: true, RunE: func(cmd *cobra.Command, args []string) error { - var clientName string - if len(args) == 0 { - if !util.IsTerminal(cmd.InOrStdin()) || !util.IsTerminal(cmd.ErrOrStderr()) { - return errors.New("TTY not detected - specify a client as an argument (e.g. 'tiger skills install claude-code')") + if skillsDirFlag != "" && len(args) > 0 { + return errors.New("--skills-dir can't be combined with client arguments") + } + + var dirs []string + switch { + case skillsDirFlag != "": + dir, err := filepath.Abs(util.ExpandPath(skillsDirFlag)) + if err != nil { + return fmt.Errorf("failed to resolve skills directory: %w", err) } - var err error - clientName, err = selectClientInteractively(cmd, "Select a client to install skills for:") + dirs = []string{dir} + case len(args) > 0: + targets, err := findSkillsTargets(args) if err != nil { - return fmt.Errorf("failed to select client: %w", err) + return err + } + if dirs, err = resolveSkillsTargetDirs(targets); err != nil { + return err + } + default: + targets := skillsTargets() + targetDirs, err := resolveSkillsTargetDirs(targets) + if err != nil { + return err + } + selected, err := defaultSkillsTargets(targetDirs) + if err != nil { + return err + } + if !noPrompt && util.IsTerminal(cmd.InOrStdin()) && util.IsTerminal(cmd.ErrOrStderr()) { + if selected, err = selectSkillsTargets(cmd, targets, selected); err != nil { + return err + } + } + for i, dir := range targetDirs { + if selected[i] { + dirs = append(dirs, dir) + } + } + if len(dirs) == 0 { + return errors.New("no install locations selected") } - } else { - clientName = args[0] - } - - clientCfg, err := findClientConfig(clientName) - if err != nil { - return err } - skillsDir, err := resolveSkillsDir(clientCfg, skillsDirFlag) + skills, err := fetchSkills(cmd.Context()) if err != nil { return err } - skills, err := fetchSkills(cmd.Context()) - if err != nil { - return err + if !force { + if err := checkSkillConflicts(skills, dirs); err != nil { + return err + } } - removed, err := installSkills(skills, skillsDir, force) - if err != nil { - return err + removed := make([][]string, len(dirs)) + for i, dir := range dirs { + if removed[i], err = installSkills(skills, dir); err != nil { + return err + } } - cmd.Printf("Installed %d skills to %s:\n", len(skills), skillsDir) + cmd.Printf("Installed %d skills:\n", len(skills)) for _, s := range skills { cmd.Printf(" %s\n", s.name) } - for _, name := range removed { - cmd.Printf("Removed %s (no longer available).\n", name) + cmd.Printf("\nInstalled to:\n") + for _, dir := range dirs { + cmd.Printf(" %s\n", dir) } - cmd.Printf("\nRestart %s to load the new skills.\n", clientCfg.Name) + for i, dir := range dirs { + if len(removed[i]) == 0 { + continue + } + cmd.Printf("\nRemoved from %s (no longer available):\n", dir) + for _, name := range removed[i] { + cmd.Printf(" %s\n", name) + } + } + cmd.Printf("\nRestart your coding agents to load the new skills.\n") return nil }, } cmd.Flags().BoolVar(&force, "force", false, "Replace existing skills that weren't installed by Tiger CLI") - cmd.Flags().StringVar(&skillsDirFlag, "skills-dir", "", "Custom skills directory to install into (overrides the client's default)") + cmd.Flags().BoolVar(&noPrompt, "no-prompt", false, "Install to the default locations without prompting") + cmd.Flags().StringVar(&skillsDirFlag, "skills-dir", "", "Install into this skills directory only (not remembered for later installs)") registerFlagCompletion(cmd, "skills-dir", dirCompletion) return cmd } -// generateSkillsClientsHelp generates the supported clients section of the -// help text, noting where each client's skills land. -func generateSkillsClientsHelp() string { +// skillsTarget is a location skills can be installed to: the universal +// directory most clients read, or the skills directory of a client that +// doesn't read it. +type skillsTarget struct { + label string // display name + names []string // names that select it on the command line; the first is canonical + readers []string // display names of the clients that read it + dir string // see clientConfig.SkillsDir + dirEnv string // see clientConfig.SkillsDirEnv +} + +// skillsTargets returns every install location: universal first, then each +// client with a skills directory of its own, in supportedClients order. +func skillsTargets() []skillsTarget { + targets := []skillsTarget{{ + label: "Universal", + names: []string{"universal"}, + dir: "~/.agents/skills", + }} + for _, c := range supportedClients { + if c.SkillsDir == "" { + targets[0].names = append(targets[0].names, c.EditorNames...) + targets[0].readers = append(targets[0].readers, c.Name) + continue + } + targets = append(targets, skillsTarget{ + label: c.Name, + names: c.EditorNames, + readers: []string{c.Name}, + dir: c.SkillsDir, + dirEnv: c.SkillsDirEnv, + }) + } + return targets +} + +// skillsTargetNames returns every name that selects an install location. +func skillsTargetNames() []string { + var names []string + for _, t := range skillsTargets() { + names = append(names, t.names...) + } + return names +} + +// findSkillsTargets returns the install locations the given names select, in +// skillsTargets order and without duplicates. Names match case-insensitively. +func findSkillsTargets(names []string) ([]skillsTarget, error) { + targets := skillsTargets() + selected := make([]bool, len(targets)) + for _, name := range names { + i := slices.IndexFunc(targets, func(t skillsTarget) bool { + return slices.ContainsFunc(t.names, func(n string) bool { return strings.EqualFold(n, name) }) + }) + if i < 0 { + return nil, fmt.Errorf("unsupported client: %s. Supported clients: %s", name, strings.Join(skillsTargetNames(), ", ")) + } + selected[i] = true + } + var result []skillsTarget + for i, t := range targets { + if selected[i] { + result = append(result, t) + } + } + return result, nil +} + +// generateSkillsTargetsHelp generates the install locations section of the +// help text. +func generateSkillsTargetsHelp() string { var b strings.Builder - b.WriteString("Supported Clients:\n") - for _, cfg := range supportedClients { - fmt.Fprintf(&b, " %-24s %s (%s)\n", cfg.EditorNames[0], cfg.Name, cfg.SkillsDir) + b.WriteString("Install locations:\n") + for _, t := range skillsTargets() { + fmt.Fprintf(&b, " %-24s %s (%s)\n", t.names[0], t.dir, strings.Join(t.readers, ", ")) } return b.String() } -// resolveSkillsDir returns the absolute directory to install skills into for -// the given client, or custom (relative to the working directory) if set. -func resolveSkillsDir(cfg *clientConfig, custom string) (string, error) { - var dir string - if custom != "" { - abs, err := filepath.Abs(util.ExpandPath(custom)) - if err != nil { - return "", fmt.Errorf("failed to resolve skills directory: %w", err) +// resolveSkillsTargetDirs returns the absolute directory of each target. +func resolveSkillsTargetDirs(targets []skillsTarget) ([]string, error) { + dirs := make([]string, len(targets)) + for i, t := range targets { + var dir string + if expanded, ok := expandEnvStrict(t.dirEnv); t.dirEnv != "" && ok { + dir = filepath.Clean(expanded) + } else { + dir = util.ExpandPath(t.dir) + } + // ExpandPath leaves a ~ in place if the home directory is unknown, + // which would otherwise install relative to the working directory. + if !filepath.IsAbs(dir) { + return nil, fmt.Errorf("failed to determine skills directory: %s is not an absolute path", dir) } - return abs, nil + dirs[i] = dir } - if expanded, ok := expandEnvStrict(cfg.SkillsDirEnv); cfg.SkillsDirEnv != "" && ok { - dir = filepath.Clean(expanded) - } else { - dir = util.ExpandPath(cfg.SkillsDir) + return dirs, nil +} + +// defaultSkillsTargets reports which of dirs are selected by default: those +// holding skills Tiger CLI installed, or the first (universal) if none do. +func defaultSkillsTargets(dirs []string) ([]bool, error) { + selected := make([]bool, len(dirs)) + found := false + for i, dir := range dirs { + has, err := hasInstalledSkills(dir) + if err != nil { + return nil, fmt.Errorf("failed to check for installed skills: %w", err) + } + selected[i] = has + found = found || has } - // ExpandPath leaves a ~ in place if the home directory is unknown, which - // would otherwise install relative to the working directory. - if !filepath.IsAbs(dir) { - return "", fmt.Errorf("failed to determine skills directory: %s is not an absolute path", dir) + if !found { + selected[0] = true } - return dir, nil + return selected, nil } // expandEnvStrict expands env var references in s, reporting false if any @@ -383,28 +512,33 @@ func (a *repoArchive) children(dir string) []string { return names } -// installSkills writes each skill into skillsDir, replacing any existing copy, -// then removes skills Tiger CLI installed earlier that are no longer among -// them, returning their names. Unless force is set, it refuses up front to -// replace anything Tiger CLI didn't install. -func installSkills(skills []skill, skillsDir string, force bool) ([]string, error) { - if !force { - var conflicts []string +// checkSkillConflicts returns an error naming every place in dirs where a +// skill would replace something Tiger CLI didn't install. +func checkSkillConflicts(skills []skill, dirs []string) error { + var conflicts []string + for _, dir := range dirs { for _, s := range skills { - exists, owned, err := skillOwnership(filepath.Join(skillsDir, s.name)) + p := filepath.Join(dir, s.name) + exists, owned, err := skillOwnership(p) if err != nil { - return nil, fmt.Errorf("failed to check existing skill %s: %w", s.name, err) + return fmt.Errorf("failed to check existing skill %s: %w", p, err) } if exists && !owned { - conflicts = append(conflicts, s.name) + conflicts = append(conflicts, p) } } - if len(conflicts) > 0 { - return nil, fmt.Errorf("skills already exist in %s and weren't installed by Tiger CLI: %s. Use --force to replace them", - skillsDir, strings.Join(conflicts, ", ")) - } } + if len(conflicts) > 0 { + return fmt.Errorf("skills already exist and weren't installed by Tiger CLI: %s. Use --force to replace them", + strings.Join(conflicts, ", ")) + } + return nil +} +// installSkills writes each skill into skillsDir, replacing any existing copy, +// then removes skills Tiger CLI installed earlier that are no longer among +// them, returning their names. +func installSkills(skills []skill, skillsDir string) ([]string, error) { if err := os.MkdirAll(skillsDir, 0o755); err != nil { return nil, fmt.Errorf("failed to create skills directory: %w", err) } @@ -421,6 +555,30 @@ func installSkills(skills []skill, skillsDir string, force bool) ([]string, erro return removed, nil } +// hasInstalledSkills reports whether skillsDir holds any skill Tiger CLI +// installed. +func hasInstalledSkills(skillsDir string) (bool, error) { + entries, err := os.ReadDir(skillsDir) + if errors.Is(err, fs.ErrNotExist) { + return false, nil + } else if err != nil { + return false, err + } + for _, e := range entries { + if strings.HasPrefix(e.Name(), skillsTempPrefix) { + continue + } + _, owned, err := skillOwnership(filepath.Join(skillsDir, e.Name())) + if err != nil { + return false, err + } + if owned { + return true, nil + } + } + return false, nil +} + // removeStaleSkills removes the skills in skillsDir that Tiger CLI installed // but that aren't among skills, returning their names. It also quietly removes // temporary directories left behind by an install that was killed partway @@ -549,3 +707,157 @@ func replacePath(src, dst string) error { } return os.RemoveAll(aside) } + +// skillsPickerItem is one install location offered by the picker. +type skillsPickerItem struct { + label string + dir string // as displayed + selected bool +} + +// selectSkillsTargets prompts the user to choose install locations, starting +// from the given selection, and returns the chosen selection. It's a var so +// command tests can stub the interactive picker. +var selectSkillsTargets = func(cmd *cobra.Command, targets []skillsTarget, selected []bool) ([]bool, error) { + dirs, err := resolveSkillsTargetDirs(targets) + if err != nil { + return nil, err + } + items := make([]skillsPickerItem, len(targets)) + for i, t := range targets { + items[i] = skillsPickerItem{ + label: t.label, + dir: displayPath(dirs[i]), + selected: selected[i], + } + } + + model := skillsPickerModel{ + header: fmt.Sprintf("Select where to install agent skills. %s installs to %s,\nwhich %s read.", + targets[0].label, items[0].dir, joinWithAnd(targets[0].readers)), + items: items, + } + program := tea.NewProgram(model, + tea.WithInput(cmd.InOrStdin()), + tea.WithOutput(cmd.ErrOrStderr()), + tea.WithContext(cmd.Context()), + tea.WithoutSignalHandler()) + final, err := program.Run() + if err != nil { + return nil, fmt.Errorf("failed to run install location selection: %w", err) + } + + result := final.(skillsPickerModel) + if !result.confirmed { + return nil, errors.New("installation cancelled") + } + chosen := make([]bool, len(result.items)) + for i, item := range result.items { + chosen[i] = item.selected + } + return chosen, nil +} + +// Colors for the install location picker, taken from the terminal theme. +// Bright colors and bold are avoided because some themes render them as greys, +// and dim text uses the faint attribute so it suits light and dark themes +// alike. They honor color.NoColor, which the command lifecycle sets from the +// color config option. +var ( + skillsPickerCursorColor = color.New(color.FgBlue) + skillsPickerCheckColor = color.New(color.FgGreen) + skillsPickerDimColor = color.New(color.Faint) +) + +// skillsPickerModel is the Bubble Tea model for the install location picker. +type skillsPickerModel struct { + header string + items []skillsPickerItem + cursor int + confirmed bool +} + +func (m skillsPickerModel) Init() tea.Cmd { + return nil +} + +func (m skillsPickerModel) Update(msg tea.Msg) (tea.Model, tea.Cmd) { + key, ok := msg.(tea.KeyPressMsg) + if !ok { + return m, nil + } + switch k := key.String(); k { + case "ctrl+c", "q", "esc": + return m, tea.Quit + case "enter": + m.confirmed = true + return m, tea.Quit + case "up", "k": + if m.cursor > 0 { + m.cursor-- + } + case "down", "j": + if m.cursor < len(m.items)-1 { + m.cursor++ + } + case "space": + m.items[m.cursor].selected = !m.items[m.cursor].selected + case "1", "2", "3", "4", "5", "6", "7", "8", "9": + if i := int(k[0] - '1'); i < len(m.items) { + m.cursor = i + m.items[i].selected = !m.items[i].selected + } + } + return m, nil +} + +func (m skillsPickerModel) View() tea.View { + if m.confirmed { + return tea.NewView("") + } + width := 0 + for _, item := range m.items { + width = max(width, len(item.label)) + } + + var b strings.Builder + b.WriteString(m.header + "\n\n") + for i, item := range m.items { + cursor := " " + if m.cursor == i { + cursor = skillsPickerCursorColor.Sprint(">") + } + check := " " + if item.selected { + check = skillsPickerCheckColor.Sprint("✓") + } + fmt.Fprintf(&b, "%s [%s] %d. %-*s %s\n", cursor, check, i+1, width, item.label, skillsPickerDimColor.Sprint(item.dir)) + } + b.WriteString("\n" + skillsPickerDimColor.Sprint("Use ↑/↓ to navigate, space or number keys to toggle, enter to confirm, q to quit")) + return tea.NewView(b.String()) +} + +// displayPath abbreviates the home directory in path to ~. +func displayPath(path string) string { + home, err := os.UserHomeDir() + if err != nil { + return path + } + if rel, ok := strings.CutPrefix(path, home+string(filepath.Separator)); ok { + return filepath.Join("~", rel) + } + return path +} + +// joinWithAnd joins items into an English list ("a, b, and c"). +func joinWithAnd(items []string) string { + switch len(items) { + case 0: + return "" + case 1: + return items[0] + case 2: + return items[0] + " and " + items[1] + } + return strings.Join(items[:len(items)-1], ", ") + ", and " + items[len(items)-1] +} diff --git a/internal/cmd/skills_install_test.go b/internal/cmd/skills_install_test.go index c3e14324..9c155f40 100644 --- a/internal/cmd/skills_install_test.go +++ b/internal/cmd/skills_install_test.go @@ -6,6 +6,7 @@ import ( "compress/gzip" "fmt" "io/fs" + "maps" "net/http" "net/http/httptest" "os" @@ -15,7 +16,9 @@ import ( "testing" "time" + tea "charm.land/bubbletea/v2" "github.com/google/go-cmp/cmp" + "github.com/spf13/cobra" ) // tarEntry is one entry of a test tarball, named relative to the repo root. @@ -204,11 +207,50 @@ func writeTree(t *testing.T, root string, tree map[string]string) { } // skillsInstallOutput is the exact stdout of a successful install of the -// fixture skills into dir, with removed holding any lines reporting removed -// skills. -func skillsInstallOutput(dir, clientName string, removed ...string) string { - return fmt.Sprintf("Installed 2 skills to %s:\n alpha\n beta\n%s\nRestart %s to load the new skills.\n", - dir, strings.Join(removed, ""), clientName) +// fixture skills into dirs, with removed holding any sections reporting +// removed skills. +func skillsInstallOutput(dirs []string, removed ...string) string { + var b strings.Builder + b.WriteString("Installed 2 skills:\n alpha\n beta\n\nInstalled to:\n") + for _, dir := range dirs { + b.WriteString(" " + dir + "\n") + } + for _, r := range removed { + b.WriteString("\n" + r) + } + b.WriteString("\nRestart your coding agents to load the new skills.\n") + return b.String() +} + +// skillsTargetLabels are the install locations, in the order the picker +// offers them. +var skillsTargetLabels = []string{"Universal", "Claude Code", "Google Antigravity", "Kiro CLI"} + +// withSelectSkillsTargets stubs the interactive install location picker: it +// expects to be offered the locations with want preselected, and answers with +// choose. A nil want expects the picker not to be shown at all. +func withSelectSkillsTargets(want, choose []bool) runOption { + return withSetup(func(t *testing.T) { + original := selectSkillsTargets + selectSkillsTargets = func(_ *cobra.Command, targets []skillsTarget, selected []bool) ([]bool, error) { + if want == nil { + t.Error("install location picker shown, want none") + return selected, nil + } + labels := make([]string, len(targets)) + for i, target := range targets { + labels[i] = target.label + } + if diff := cmp.Diff(skillsTargetLabels, labels); diff != "" { + t.Errorf("picker locations mismatch (-want +got):\n%s", diff) + } + if diff := cmp.Diff(want, selected); diff != "" { + t.Errorf("picker preselection mismatch (-want +got):\n%s", diff) + } + return choose, nil + } + t.Cleanup(func() { selectSkillsTargets = original }) + }) } func TestSkillsInstallCmd(t *testing.T) { @@ -252,14 +294,39 @@ func TestSkillsInstallCmd(t *testing.T) { } } - codexHome := t.TempDir() - claudeHome := t.TempDir() - claudeConfigHome := t.TempDir() - claudeEmptyEnvHome := t.TempDir() + // home creates a home directory seeded with tree (see writeTree). + home := func(tree map[string]string) string { + dir := t.TempDir() + writeTree(t, dir, tree) + return dir + } + // withTrees merges trees into one. + withTrees := func(trees ...map[string]string) map[string]string { + merged := map[string]string{} + for _, tree := range trees { + maps.Copy(merged, tree) + } + return merged + } + + universalHome := home(nil) + claudeHome := home(nil) + multiHome := home(nil) + claudeConfigHome := home(nil) claudeConfigDir := filepath.Join(claudeConfigHome, "claude-config") - kiroHome := t.TempDir() - antigravityHome := t.TempDir() - aliasHome := t.TempDir() + claudeEmptyEnvHome := home(nil) + antigravityHome := home(nil) + aliasHome := home(nil) + firstDefaultHome := home(nil) + customHome := home(nil) + + // existingClaude holds an earlier install for Claude Code only, so it's + // what the defaults select; existingBoth adds one to the universal + // directory. + existingClaude := installedSkills(".claude/skills") + existingClaudeHome := home(existingClaude) + existingBothHome := home(withTrees(installedSkills(".agents/skills"), existingClaude)) + pickerHome := home(withTrees(installedSkills(".agents/skills"), existingClaude)) // reinstallHome holds a previous install: one of our skills with a stale // file (replaced), one no longer available (removed), skills of the @@ -283,64 +350,71 @@ func TestSkillsInstallCmd(t *testing.T) { "marked/.tiger-cli": skillMarkerContent, "marked/SKILL.md": "marked skill", }) - reinstallTree := installedSkills(".claude/skills") - reinstallTree[".claude/skills/other/SKILL.md"] = "user's skill" - reinstallTree[".claude/skills/.hidden/SKILL.md"] = "user's hidden entry" - reinstallTree[".claude/skills/linked"] = "-> " + filepath.Join(reinstallHome, "marked") - reinstallTree["marked/.tiger-cli"] = skillMarkerContent - reinstallTree["marked/SKILL.md"] = "marked skill" + reinstallTree := withTrees(installedSkills(".claude/skills"), map[string]string{ + ".claude/skills/other/SKILL.md": "user's skill", + ".claude/skills/.hidden/SKILL.md": "user's hidden entry", + ".claude/skills/linked": "-> " + filepath.Join(reinstallHome, "marked"), + "marked/.tiger-cli": skillMarkerContent, + "marked/SKILL.md": "marked skill", + }) - // conflictTree holds a symlink and a directory of the user's under the - // names of skills being installed, which aren't replaced without --force. - // The symlink counts as the user's even though it points at one of our - // skills, since Tiger CLI never creates symlinks. + // conflictTree holds things of the user's under the names of skills being + // installed, in both of the locations being installed to, which aren't + // replaced without --force. The symlink counts as the user's even though + // it points at one of our skills, since Tiger CLI never creates symlinks. conflictHome := t.TempDir() conflictTree := map[string]string{ - ".agents/skills/alpha": "-> " + filepath.Join(conflictHome, "marked"), - ".agents/skills/beta/SKILL.md": "user's beta", - "marked/.tiger-cli": skillMarkerContent, - "marked/SKILL.md": "marked skill", + ".agents/skills/alpha": "-> " + filepath.Join(conflictHome, "marked"), + ".agents/skills/beta/.tiger-cli": skillMarkerContent, + ".agents/skills/beta/SKILL.md": "our beta", + ".claude/skills/beta/SKILL.md": "user's beta", + "marked/.tiger-cli": skillMarkerContent, + "marked/SKILL.md": "marked skill", } writeTree(t, conflictHome, conflictTree) // forceHome holds a symlink and a directory of the user's under the names // of skills being installed. --force replaces both; the symlink is // replaced as a link, leaving its target alone. - forceHome := t.TempDir() - writeTree(t, forceHome, map[string]string{ - ".agents/skills/alpha": "-> " + filepath.Join(forceHome, "elsewhere"), + forceHome := home(map[string]string{ + ".agents/skills/alpha": "-> " + filepath.Join("..", "..", "elsewhere"), ".agents/skills/beta/SKILL.md": "user's beta", ".agents/skills/beta/stale.md": "stale", "elsewhere/SKILL.md": "symlink target", }) - forceTree := installedSkills(".agents/skills") - forceTree["elsewhere/SKILL.md"] = "symlink target" + forceTree := withTrees(installedSkills(".agents/skills"), map[string]string{"elsewhere/SKILL.md": "symlink target"}) - customHome := t.TempDir() + universalDir := func(home string) string { return filepath.Join(home, ".agents", "skills") } + claudeDir := func(home string) string { return filepath.Join(home, ".claude", "skills") } rateLimitReset := time.Date(2030, 1, 1, 15, 4, 0, 0, time.UTC).Unix() runCmdTests(t, []cmdTest{ { - name: "too many arguments", - args: []string{"skills", "install", "codex", "cursor"}, - wantErr: "accepts at most 1 arg(s), received 2", + name: "--skills-dir with client arguments", + args: []string{"skills", "install", "claude-code", "--skills-dir", "/tmp/skills"}, + wantErr: "--skills-dir can't be combined with client arguments", }, { - name: "no client and no TTY", - args: []string{"skills", "install"}, - wantErr: "TTY not detected - specify a client as an argument (e.g. 'tiger skills install claude-code')", + name: "unsupported client", + args: []string{"skills", "install", "claude-code", "bogus"}, + wantErr: "unsupported client: bogus. Supported clients: universal, cursor, devin, codex, gemini, gemini-cli, vscode, code, vs-code, copilot, copilot-cli, claude-code, antigravity, agy, kiro-cli", }, { - name: "unsupported client", - args: []string{"skills", "install", "bogus"}, - wantErr: "unsupported client: bogus. Supported clients: claude-code, cursor, devin, codex, gemini, gemini-cli, vscode, code, vs-code, antigravity, agy, kiro-cli, copilot, copilot-cli", + name: "nothing selected in the picker", + args: []string{"skills", "install"}, + opts: []runOption{ + withEnv("HOME", home(nil)), + withIsTerminal(true), + withSelectSkillsTargets([]bool{true, false, false, false}, []bool{false, false, false, false}), + }, + wantErr: "no install locations selected", }, { name: "download fails", - args: []string{"skills", "install", "codex"}, + args: []string{"skills", "install", "universal"}, opts: []runOption{ - withEnv("HOME", t.TempDir()), + withEnv("HOME", home(nil)), withSkillsServer(func(w http.ResponseWriter, r *http.Request) { w.WriteHeader(http.StatusInternalServerError) }), @@ -349,9 +423,9 @@ func TestSkillsInstallCmd(t *testing.T) { }, { name: "rate limited", - args: []string{"skills", "install", "codex"}, + args: []string{"skills", "install", "universal"}, opts: []runOption{ - withEnv("HOME", t.TempDir()), + withEnv("HOME", home(nil)), withSkillsServer(func(w http.ResponseWriter, r *http.Request) { w.Header().Set("X-RateLimit-Remaining", "0") w.Header().Set("X-RateLimit-Reset", strconv.FormatInt(rateLimitReset, 10)) @@ -362,9 +436,9 @@ func TestSkillsInstallCmd(t *testing.T) { }, { name: "invalid archive", - args: []string{"skills", "install", "codex"}, + args: []string{"skills", "install", "universal"}, opts: []runOption{ - withEnv("HOME", t.TempDir()), + withEnv("HOME", home(nil)), withSkillsServer(func(w http.ResponseWriter, r *http.Request) { w.Write([]byte("not a tarball")) }), @@ -373,34 +447,52 @@ func TestSkillsInstallCmd(t *testing.T) { }, { name: "no skills in archive", - args: []string{"skills", "install", "codex"}, + args: []string{"skills", "install", "universal"}, opts: []runOption{ - withEnv("HOME", t.TempDir()), + withEnv("HOME", home(nil)), withSkillsTarball(skillsTarball(t, tarEntry{name: "README.md", body: "repo readme"})), }, wantErr: "no skills found", }, { - name: "existing skills not installed by Tiger CLI", - args: []string{"skills", "install", "codex"}, - opts: []runOption{withEnv("HOME", conflictHome), withSkillsTarball(tarball)}, - wantErr: fmt.Sprintf("skills already exist in %s and weren't installed by Tiger CLI: alpha, beta. Use --force to replace them", filepath.Join(conflictHome, ".agents", "skills")), - checks: []checkFunc{checkTree(conflictHome, conflictTree)}, + name: "existing skills not installed by Tiger CLI", + args: []string{"skills", "install", "universal", "claude-code"}, + opts: []runOption{withEnv("HOME", conflictHome), withSkillsTarball(tarball)}, + wantErr: fmt.Sprintf("skills already exist and weren't installed by Tiger CLI: %s, %s. Use --force to replace them", + filepath.Join(universalDir(conflictHome), "alpha"), filepath.Join(claudeDir(conflictHome), "beta")), + checks: []checkFunc{checkTree(conflictHome, conflictTree)}, }, { name: "installs to ~/.agents/skills", - args: []string{"skills", "install", "codex"}, - opts: []runOption{withEnv("HOME", codexHome), withSkillsTarball(tarball)}, - wantStdout: skillsInstallOutput(filepath.Join(codexHome, ".agents", "skills"), "Codex"), - checks: []checkFunc{checkTree(codexHome, installedSkills(".agents/skills"))}, + args: []string{"skills", "install", "universal"}, + opts: []runOption{withEnv("HOME", universalHome), withSkillsTarball(tarball)}, + wantStdout: skillsInstallOutput([]string{universalDir(universalHome)}), + checks: []checkFunc{checkTree(universalHome, installedSkills(".agents/skills"))}, }, { - name: "installs to claude code's skills directory", + // Arguments select exactly the locations named, so the universal + // directory isn't written. + name: "installs to claude code's skills directory only", args: []string{"skills", "install", "claude-code"}, opts: []runOption{withEnv("HOME", claudeHome), withSkillsTarball(tarball)}, - wantStdout: skillsInstallOutput(filepath.Join(claudeHome, ".claude", "skills"), "Claude Code"), + wantStdout: skillsInstallOutput([]string{claudeDir(claudeHome)}), checks: []checkFunc{checkTree(claudeHome, installedSkills(".claude/skills"))}, }, + { + name: "installs to several locations", + args: []string{"skills", "install", "kiro-cli", "claude-code", "universal"}, + opts: []runOption{withEnv("HOME", multiHome), withSkillsTarball(tarball)}, + wantStdout: skillsInstallOutput([]string{ + universalDir(multiHome), + claudeDir(multiHome), + filepath.Join(multiHome, ".kiro", "skills"), + }), + checks: []checkFunc{checkTree(multiHome, withTrees( + installedSkills(".agents/skills"), + installedSkills(".claude/skills"), + installedSkills(".kiro/skills"), + ))}, + }, { name: "respects CLAUDE_CONFIG_DIR", args: []string{"skills", "install", "claude-code"}, @@ -409,7 +501,7 @@ func TestSkillsInstallCmd(t *testing.T) { withEnv("CLAUDE_CONFIG_DIR", claudeConfigDir), withSkillsTarball(tarball), }, - wantStdout: skillsInstallOutput(filepath.Join(claudeConfigDir, "skills"), "Claude Code"), + wantStdout: skillsInstallOutput([]string{filepath.Join(claudeConfigDir, "skills")}), checks: []checkFunc{checkTree(claudeConfigHome, installedSkills("claude-config/skills"))}, }, { @@ -420,29 +512,79 @@ func TestSkillsInstallCmd(t *testing.T) { withEnv("CLAUDE_CONFIG_DIR", ""), withSkillsTarball(tarball), }, - wantStdout: skillsInstallOutput(filepath.Join(claudeEmptyEnvHome, ".claude", "skills"), "Claude Code"), + wantStdout: skillsInstallOutput([]string{claudeDir(claudeEmptyEnvHome)}), checks: []checkFunc{checkTree(claudeEmptyEnvHome, installedSkills(".claude/skills"))}, }, - { - name: "installs to kiro's skills directory", - args: []string{"skills", "install", "kiro-cli"}, - opts: []runOption{withEnv("HOME", kiroHome), withSkillsTarball(tarball)}, - wantStdout: skillsInstallOutput(filepath.Join(kiroHome, ".kiro", "skills"), "Kiro CLI"), - checks: []checkFunc{checkTree(kiroHome, installedSkills(".kiro/skills"))}, - }, { name: "installs to antigravity's skills directory", args: []string{"skills", "install", "antigravity"}, opts: []runOption{withEnv("HOME", antigravityHome), withSkillsTarball(tarball)}, - wantStdout: skillsInstallOutput(filepath.Join(antigravityHome, ".gemini", "antigravity", "skills"), "Google Antigravity"), - checks: []checkFunc{checkTree(antigravityHome, installedSkills(".gemini/antigravity/skills"))}, + wantStdout: skillsInstallOutput([]string{filepath.Join(antigravityHome, ".gemini", "config", "skills")}), + checks: []checkFunc{checkTree(antigravityHome, installedSkills(".gemini/config/skills"))}, + }, + { + // Client names that read the universal directory select it, once. + name: "add alias and case-insensitive client names", + args: []string{"skills", "add", "CURSOR", "codex"}, + opts: []runOption{withEnv("HOME", aliasHome), withSkillsTarball(tarball)}, + wantStdout: skillsInstallOutput([]string{universalDir(aliasHome)}), + checks: []checkFunc{checkTree(aliasHome, installedSkills(".agents/skills"))}, + }, + { + name: "defaults to universal without a terminal", + args: []string{"skills", "install"}, + opts: []runOption{withEnv("HOME", firstDefaultHome), withSkillsTarball(tarball)}, + wantStdout: skillsInstallOutput([]string{universalDir(firstDefaultHome)}), + checks: []checkFunc{checkTree(firstDefaultHome, installedSkills(".agents/skills"))}, + }, + { + // An earlier install for Claude Code only is what the defaults + // select, so the universal directory isn't added. + name: "defaults to earlier installs without a terminal", + args: []string{"skills", "install"}, + opts: []runOption{withEnv("HOME", existingClaudeHome), withSkillsTarball(tarball)}, + wantStdout: skillsInstallOutput([]string{claudeDir(existingClaudeHome)}), + checks: []checkFunc{checkTree(existingClaudeHome, existingClaude)}, + }, + { + name: "--no-prompt installs to the defaults without the picker", + args: []string{"skills", "install", "--no-prompt"}, + opts: []runOption{ + withEnv("HOME", existingBothHome), + withIsTerminal(true), + withSelectSkillsTargets(nil, nil), + withSkillsTarball(tarball), + }, + wantStdout: skillsInstallOutput([]string{universalDir(existingBothHome), claudeDir(existingBothHome)}), + checks: []checkFunc{checkTree(existingBothHome, withTrees( + installedSkills(".agents/skills"), + existingClaude, + ))}, + }, + { + // The picker starts from the earlier installs; what's chosen is + // installed, and an unchosen earlier install is left as it is. + name: "installs to the locations chosen in the picker", + args: []string{"skills", "install"}, + opts: []runOption{ + withEnv("HOME", pickerHome), + withIsTerminal(true), + withSelectSkillsTargets([]bool{true, true, false, false}, []bool{false, true, false, true}), + withSkillsTarball(tarball), + }, + wantStdout: skillsInstallOutput([]string{claudeDir(pickerHome), filepath.Join(pickerHome, ".kiro", "skills")}), + checks: []checkFunc{checkTree(pickerHome, withTrees( + installedSkills(".agents/skills"), + existingClaude, + installedSkills(".kiro/skills"), + ))}, }, { name: "reinstall replaces our skills and removes ones no longer available", args: []string{"skills", "install", "claude-code"}, opts: []runOption{withEnv("HOME", reinstallHome), withSkillsTarball(tarball)}, - wantStdout: skillsInstallOutput(filepath.Join(reinstallHome, ".claude", "skills"), "Claude Code", - "Removed retired (no longer available).\n"), + wantStdout: skillsInstallOutput([]string{claudeDir(reinstallHome)}, + fmt.Sprintf("Removed from %s (no longer available):\n retired\n", claudeDir(reinstallHome))), checks: []checkFunc{ checkTree(reinstallHome, reinstallTree), // Running again over a current install leaves it unchanged. @@ -451,31 +593,78 @@ func TestSkillsInstallCmd(t *testing.T) { if again.err != nil { t.Fatalf("second install failed: %v", again.err) } - assertOutput(t, again.stdout, skillsInstallOutput(filepath.Join(reinstallHome, ".claude", "skills"), "Claude Code")) + assertOutput(t, again.stdout, skillsInstallOutput([]string{claudeDir(reinstallHome)})) }, checkTree(reinstallHome, reinstallTree), }, }, { name: "--force replaces skills not installed by Tiger CLI", - args: []string{"skills", "install", "codex", "--force"}, + args: []string{"skills", "install", "universal", "--force"}, opts: []runOption{withEnv("HOME", forceHome), withSkillsTarball(tarball)}, - wantStdout: skillsInstallOutput(filepath.Join(forceHome, ".agents", "skills"), "Codex"), + wantStdout: skillsInstallOutput([]string{universalDir(forceHome)}), checks: []checkFunc{checkTree(forceHome, forceTree)}, }, { - name: "--skills-dir overrides the client's directory", - args: []string{"skills", "install", "claude-code", "--skills-dir", "~/custom"}, - opts: []runOption{withEnv("HOME", customHome), withSkillsTarball(tarball)}, - wantStdout: skillsInstallOutput(filepath.Join(customHome, "custom"), "Claude Code"), + name: "--skills-dir installs there only", + args: []string{"skills", "install", "--skills-dir", "~/custom"}, + opts: []runOption{ + withEnv("HOME", customHome), + withIsTerminal(true), + withSelectSkillsTargets(nil, nil), + withSkillsTarball(tarball), + }, + wantStdout: skillsInstallOutput([]string{filepath.Join(customHome, "custom")}), checks: []checkFunc{checkTree(customHome, installedSkills("custom"))}, }, - { - name: "add alias and case-insensitive client name", - args: []string{"skills", "add", "CODEX"}, - opts: []runOption{withEnv("HOME", aliasHome), withSkillsTarball(tarball)}, - wantStdout: skillsInstallOutput(filepath.Join(aliasHome, ".agents", "skills"), "Codex"), - checks: []checkFunc{checkTree(aliasHome, installedSkills(".agents/skills"))}, - }, }) } + +// TestSkillsPickerModel checks the picker's keys: toggling, moving, and that +// only enter confirms. Helper-level because the picker is a Bubble Tea model +// that needs a real TTY to run through the command; the command tests stub it +// via withSelectSkillsTargets. +func TestSkillsPickerModel(t *testing.T) { + // Ctrl+C is {Code: 'c', Mod: tea.ModCtrl}; the raw control byte {Code: 3} + // stringifies to "\x03" and would match nothing. + cases := []struct { + name string + keys []tea.KeyPressMsg + wantSelected []bool + wantConfirmed bool + }{ + {"enter keeps the preselection", []tea.KeyPressMsg{{Code: tea.KeyEnter}}, []bool{true, false, false}, true}, + {"space toggles the current row", []tea.KeyPressMsg{{Code: tea.KeySpace}, {Code: tea.KeyEnter}}, []bool{false, false, false}, true}, + {"down then space toggles the next row", []tea.KeyPressMsg{{Code: tea.KeyDown}, {Code: tea.KeySpace}, {Code: tea.KeyEnter}}, []bool{true, true, false}, true}, + {"number keys toggle their row", []tea.KeyPressMsg{{Code: '3'}, {Code: '1'}, {Code: tea.KeyEnter}}, []bool{false, false, true}, true}, + {"number keys past the end do nothing", []tea.KeyPressMsg{{Code: '4'}, {Code: tea.KeyEnter}}, []bool{true, false, false}, true}, + {"cursor can't run past the end", []tea.KeyPressMsg{{Code: tea.KeyDown}, {Code: tea.KeyDown}, {Code: tea.KeyDown}, {Code: tea.KeySpace}, {Code: tea.KeyEnter}}, []bool{true, false, true}, true}, + {"q cancels", []tea.KeyPressMsg{{Code: tea.KeySpace}, {Code: 'q'}}, []bool{false, false, false}, false}, + {"esc cancels", []tea.KeyPressMsg{{Code: tea.KeyEsc}}, []bool{true, false, false}, false}, + {"ctrl+c cancels", []tea.KeyPressMsg{{Code: 'c', Mod: tea.ModCtrl}}, []bool{true, false, false}, false}, + } + + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + var m tea.Model = skillsPickerModel{items: []skillsPickerItem{ + {label: "Universal", dir: "~/.agents/skills", selected: true}, + {label: "Claude Code", dir: "~/.claude/skills"}, + {label: "Kiro CLI", dir: "~/.kiro/skills"}, + }} + for _, key := range tc.keys { + m, _ = m.Update(key) + } + result := m.(skillsPickerModel) + selected := make([]bool, len(result.items)) + for i, item := range result.items { + selected[i] = item.selected + } + if diff := cmp.Diff(tc.wantSelected, selected); diff != "" { + t.Errorf("selection mismatch (-want +got):\n%s", diff) + } + if result.confirmed != tc.wantConfirmed { + t.Errorf("confirmed = %v, want %v", result.confirmed, tc.wantConfirmed) + } + }) + } +} From e696bb1442077960396e9ae148e16adbb705792d Mon Sep 17 00:00:00 2001 From: Nathan Cochran Date: Wed, 7 Oct 2026 18:23:32 -0400 Subject: [PATCH 05/17] Clean up --- go.mod | 2 +- internal/cmd/client_helper.go | 174 ----------------------------- internal/cmd/client_helper_test.go | 57 ---------- internal/cmd/mcp_install.go | 172 +++++++++++++++++++++++++++- internal/cmd/mcp_install_test.go | 54 +++++++++ internal/cmd/skills_install.go | 5 +- 6 files changed, 229 insertions(+), 235 deletions(-) delete mode 100644 internal/cmd/client_helper_test.go diff --git a/go.mod b/go.mod index 67aa2ccc..8f2122b5 100644 --- a/go.mod +++ b/go.mod @@ -14,6 +14,7 @@ require ( charm.land/bubbletea/v2 v2.0.9 github.com/Masterminds/semver/v3 v3.5.0 github.com/charmbracelet/colorprofile v0.4.3 + github.com/charmbracelet/x/ansi v0.11.8 github.com/cli/safeexec v1.0.1 github.com/docker/go-units v0.5.0 github.com/fatih/color v1.19.0 @@ -41,7 +42,6 @@ require ( github.com/apapsch/go-jsonmerge/v2 v2.0.0 // indirect github.com/cespare/xxhash/v2 v2.3.0 // indirect github.com/charmbracelet/ultraviolet v0.0.0-20260903151058-ae99b731b8c5 // indirect - github.com/charmbracelet/x/ansi v0.11.8 // indirect github.com/charmbracelet/x/term v0.2.2 // indirect github.com/charmbracelet/x/termios v0.1.1 // indirect github.com/charmbracelet/x/windows v0.2.2 // indirect diff --git a/internal/cmd/client_helper.go b/internal/cmd/client_helper.go index 0c29a959..c6eb93e2 100644 --- a/internal/cmd/client_helper.go +++ b/internal/cmd/client_helper.go @@ -3,12 +3,7 @@ package cmd import ( "encoding/json" "fmt" - "sort" - "strconv" "strings" - - tea "charm.land/bubbletea/v2" - "github.com/spf13/cobra" ) // MCPClient represents our internal client types @@ -172,172 +167,3 @@ var supportedClients = []clientConfig{ }, }, } - -// getValidEditorNames returns all valid client names from supportedClients -func getValidEditorNames() []string { - var validNames []string - for _, client := range supportedClients { - validNames = append(validNames, client.EditorNames...) - } - return validNames -} - -// findClientConfig finds the client configuration for a given client name -// This consolidates the logic of mapping client names to client types and finding the config -func findClientConfig(clientName string) (*clientConfig, error) { - normalizedName := strings.ToLower(clientName) - - // Look up in our supported clients config - for i := range supportedClients { - for _, name := range supportedClients[i].EditorNames { - if strings.ToLower(name) == normalizedName { - return &supportedClients[i], nil - } - } - } - - // Build list of supported clients from our config for error message - supportedNames := getValidEditorNames() - - return nil, fmt.Errorf("unsupported client: %s. Supported clients: %s", clientName, strings.Join(supportedNames, ", ")) -} - -// ClientOption represents a client choice for interactive selection -type ClientOption struct { - Name string // Display name - ClientName string // Client name to pass to installMCPForClient -} - -// selectClientInteractively prompts the user to select a client using Bubble Tea -func selectClientInteractively(cmd *cobra.Command, title string) (string, error) { - // Build client options from supportedClients - var options []ClientOption - for _, cfg := range supportedClients { - // Use the first client name as the primary identifier - primaryName := cfg.EditorNames[0] - options = append(options, ClientOption{ - Name: cfg.Name, - ClientName: primaryName, - }) - } - - // Sort options alphabetically by name - sort.Slice(options, func(i, j int) bool { - return options[i].Name < options[j].Name - }) - - model := clientSelectModel{ - title: title, - options: options, - cursor: 0, - } - - program := tea.NewProgram(model, - tea.WithInput(cmd.InOrStdin()), - tea.WithOutput(cmd.ErrOrStderr()), - tea.WithContext(cmd.Context()), - tea.WithoutSignalHandler()) - finalModel, err := program.Run() - if err != nil { - return "", fmt.Errorf("failed to run editor selection: %w", err) - } - - result := finalModel.(clientSelectModel) - if result.selected == "" { - return "", fmt.Errorf("no editor selected") - } - - return result.selected, nil -} - -// clientSelectModel represents the Bubble Tea model for client selection -type clientSelectModel struct { - title string - options []ClientOption - cursor int - selected string - numberBuffer string -} - -func (m clientSelectModel) Init() tea.Cmd { - return nil -} - -func (m clientSelectModel) Update(msg tea.Msg) (tea.Model, tea.Cmd) { - switch msg := msg.(type) { - case tea.KeyPressMsg: - switch msg.String() { - case "ctrl+c", "q", "esc": - return m, tea.Quit - case "up", "k": - // Clear buffer when using arrows - m.numberBuffer = "" - if m.cursor > 0 { - m.cursor-- - } - case "down", "j": - // Clear buffer when using arrows - m.numberBuffer = "" - if m.cursor < len(m.options)-1 { - m.cursor++ - } - case "enter", "space": - m.selected = m.options[m.cursor].ClientName - return m, tea.Quit - case "backspace": - // Handle backspace to remove last character from buffer - if len(m.numberBuffer) > 0 { - m.updateNumberBuffer(m.numberBuffer[:len(m.numberBuffer)-1]) - } - case "0", "1", "2", "3", "4", "5", "6", "7", "8", "9": - // Add digit to buffer and update cursor position - m.updateNumberBuffer(m.numberBuffer + msg.String()) - case "ctrl+w": - // Clear buffer - m.numberBuffer = "" - } - } - return m, nil -} - -// updateNumberBuffer moves the cursor to the editor matching the number buffer -func (m *clientSelectModel) updateNumberBuffer(newBuffer string) { - if newBuffer == "" { - m.numberBuffer = newBuffer - return - } - - // Parse the buffer as a number - num, err := strconv.Atoi(newBuffer) - if err != nil { - return - } - - // Convert from 1-based to 0-based index and validate bounds - index := num - 1 - if index >= 0 && index < len(m.options) { - m.numberBuffer = newBuffer - m.cursor = index - } -} - -func (m clientSelectModel) View() tea.View { - var s strings.Builder - s.WriteString(m.title + "\n\n") - - for i, option := range m.options { - cursor := " " - if m.cursor == i { - cursor = ">" - } - s.WriteString(fmt.Sprintf("%s %d. %s\n", cursor, i+1, option.Name)) - } - - // Show the current number buffer if user is typing - if m.numberBuffer != "" { - s.WriteString(fmt.Sprintf("\nTyping: %s", m.numberBuffer)) - } - - s.WriteString("\nUse ↑/↓ arrows or number keys to navigate, enter to select, q to quit") - return tea.NewView(s.String()) -} diff --git a/internal/cmd/client_helper_test.go b/internal/cmd/client_helper_test.go deleted file mode 100644 index 74035f7f..00000000 --- a/internal/cmd/client_helper_test.go +++ /dev/null @@ -1,57 +0,0 @@ -package cmd - -import "testing" - -// TestFindClientConfig covers the client-name lookup table: behavior that the -// command-level cases only exercise for cursor. -func TestFindClientConfig(t *testing.T) { - mappings := []struct { - clientName string - expectedType MCPClient - expectedName string - }{ - {"claude-code", ClaudeCode, "Claude Code"}, - {"CLAUDE-CODE", ClaudeCode, "Claude Code"}, - {"cursor", Cursor, "Cursor"}, - {"CURSOR", Cursor, "Cursor"}, - {"devin", Devin, "Devin"}, - {"DEVIN", Devin, "Devin"}, - {"codex", Codex, "Codex"}, - {"CODEX", Codex, "Codex"}, - } - for _, m := range mappings { - t.Run(m.clientName, func(t *testing.T) { - cfg, err := findClientConfig(m.clientName) - if err != nil { - t.Fatalf("unexpected error: %v", err) - } - if cfg.ClientType != m.expectedType { - t.Errorf("ClientType = %q, want %q", cfg.ClientType, m.expectedType) - } - if cfg.Name != m.expectedName { - t.Errorf("Name = %q, want %q", cfg.Name, m.expectedName) - } - }) - } - - t.Run("every client is installable", func(t *testing.T) { - for _, cfg := range supportedClients { - found, err := findClientConfig(cfg.EditorNames[0]) - if err != nil { - t.Fatalf("findClientConfig(%q) failed: %v", cfg.EditorNames[0], err) - } - if found.Name == "" { - t.Errorf("%s: Name should not be empty", cfg.ClientType) - } - // Every client needs an install mechanism: JSON patching (path - // prefix) or a CLI install command. - if found.MCPServersPathPrefix == "" && found.buildMCPInstallCommand == nil { - t.Errorf("%s: needs MCPServersPathPrefix or buildMCPInstallCommand", cfg.ClientType) - } - // CLI-only clients (no config paths) must have an install command. - if len(found.MCPConfigPaths) == 0 && found.buildMCPInstallCommand == nil { - t.Errorf("%s: CLI-only clients must have buildMCPInstallCommand", cfg.ClientType) - } - } - }) -} diff --git a/internal/cmd/mcp_install.go b/internal/cmd/mcp_install.go index a7b941c6..ab3b688b 100644 --- a/internal/cmd/mcp_install.go +++ b/internal/cmd/mcp_install.go @@ -8,9 +8,12 @@ import ( "os" "os/exec" "path/filepath" + "sort" + "strconv" "strings" "time" + tea "charm.land/bubbletea/v2" "github.com/spf13/cobra" "github.com/tailscale/hujson" @@ -67,7 +70,7 @@ If no client is specified, you'll be prompted to select one interactively.`, gen return fmt.Errorf("TTY not detected - specify a client as an argument (e.g. 'tiger mcp install claude-code')") } var err error - clientName, err = selectClientInteractively(cmd, "Select an MCP client to configure:") + clientName, err = selectClientInteractively(cmd) if err != nil { return fmt.Errorf("failed to select client: %w", err) } @@ -112,6 +115,15 @@ type InstallOptions struct { CustomConfigPath string } +// getValidEditorNames returns all valid client names from supportedClients +func getValidEditorNames() []string { + var validNames []string + for _, client := range supportedClients { + validNames = append(validNames, client.EditorNames...) + } + return validNames +} + // ClientInfo contains information about a supported MCP client. type ClientInfo struct { // Name is the human-readable display name (e.g., "Claude Code", "Cursor") @@ -251,6 +263,26 @@ func installTigerMCPForClient(cmd *cobra.Command, clientName string, createBacku return nil } +// findClientConfig finds the client configuration for a given client name +// This consolidates the logic of mapping client names to client types and finding the config +func findClientConfig(clientName string) (*clientConfig, error) { + normalizedName := strings.ToLower(clientName) + + // Look up in our supported clients config + for i := range supportedClients { + for _, name := range supportedClients[i].EditorNames { + if strings.ToLower(name) == normalizedName { + return &supportedClients[i], nil + } + } + } + + // Build list of supported clients from our config for error message + supportedNames := getValidEditorNames() + + return nil, fmt.Errorf("unsupported client: %s. Supported clients: %s", clientName, strings.Join(supportedNames, ", ")) +} + // generateSupportedEditorsHelp generates the supported clients section for help text func generateSupportedEditorsHelp() string { var result strings.Builder @@ -308,6 +340,144 @@ func defaultGetTigerExecutablePath() (string, error) { return tigerPath, nil } +// ClientOption represents a client choice for interactive selection +type ClientOption struct { + Name string // Display name + ClientName string // Client name to pass to installMCPForClient +} + +// selectClientInteractively prompts the user to select a client using Bubble Tea +func selectClientInteractively(cmd *cobra.Command) (string, error) { + // Build client options from supportedClients + var options []ClientOption + for _, cfg := range supportedClients { + // Use the first client name as the primary identifier + primaryName := cfg.EditorNames[0] + options = append(options, ClientOption{ + Name: cfg.Name, + ClientName: primaryName, + }) + } + + // Sort options alphabetically by name + sort.Slice(options, func(i, j int) bool { + return options[i].Name < options[j].Name + }) + + model := clientSelectModel{ + options: options, + cursor: 0, + } + + program := tea.NewProgram(model, + tea.WithInput(cmd.InOrStdin()), + tea.WithOutput(cmd.ErrOrStderr()), + tea.WithContext(cmd.Context()), + tea.WithoutSignalHandler()) + finalModel, err := program.Run() + if err != nil { + return "", fmt.Errorf("failed to run editor selection: %w", err) + } + + result := finalModel.(clientSelectModel) + if result.selected == "" { + return "", fmt.Errorf("no editor selected") + } + + return result.selected, nil +} + +// clientSelectModel represents the Bubble Tea model for client selection +type clientSelectModel struct { + options []ClientOption + cursor int + selected string + numberBuffer string +} + +func (m clientSelectModel) Init() tea.Cmd { + return nil +} + +func (m clientSelectModel) Update(msg tea.Msg) (tea.Model, tea.Cmd) { + switch msg := msg.(type) { + case tea.KeyPressMsg: + switch msg.String() { + case "ctrl+c", "q", "esc": + return m, tea.Quit + case "up", "k": + // Clear buffer when using arrows + m.numberBuffer = "" + if m.cursor > 0 { + m.cursor-- + } + case "down", "j": + // Clear buffer when using arrows + m.numberBuffer = "" + if m.cursor < len(m.options)-1 { + m.cursor++ + } + case "enter", "space": + m.selected = m.options[m.cursor].ClientName + return m, tea.Quit + case "backspace": + // Handle backspace to remove last character from buffer + if len(m.numberBuffer) > 0 { + m.updateNumberBuffer(m.numberBuffer[:len(m.numberBuffer)-1]) + } + case "0", "1", "2", "3", "4", "5", "6", "7", "8", "9": + // Add digit to buffer and update cursor position + m.updateNumberBuffer(m.numberBuffer + msg.String()) + case "ctrl+w": + // Clear buffer + m.numberBuffer = "" + } + } + return m, nil +} + +// updateNumberBuffer moves the cursor to the editor matching the number buffer +func (m *clientSelectModel) updateNumberBuffer(newBuffer string) { + if newBuffer == "" { + m.numberBuffer = newBuffer + return + } + + // Parse the buffer as a number + num, err := strconv.Atoi(newBuffer) + if err != nil { + return + } + + // Convert from 1-based to 0-based index and validate bounds + index := num - 1 + if index >= 0 && index < len(m.options) { + m.numberBuffer = newBuffer + m.cursor = index + } +} + +func (m clientSelectModel) View() tea.View { + var s strings.Builder + s.WriteString("Select an MCP client to configure:\n\n") + + for i, option := range m.options { + cursor := " " + if m.cursor == i { + cursor = ">" + } + s.WriteString(fmt.Sprintf("%s %d. %s\n", cursor, i+1, option.Name)) + } + + // Show the current number buffer if user is typing + if m.numberBuffer != "" { + s.WriteString(fmt.Sprintf("\nTyping: %s", m.numberBuffer)) + } + + s.WriteString("\nUse ↑/↓ arrows or number keys to navigate, enter to select, q to quit") + return tea.NewView(s.String()) +} + // addMCPServerViaCLI adds an MCP server using a CLI command configured in clientConfig func addMCPServerViaCLI(clientCfg *clientConfig, serverName, command string, args []string) error { if clientCfg.buildMCPInstallCommand == nil { diff --git a/internal/cmd/mcp_install_test.go b/internal/cmd/mcp_install_test.go index 83506ad7..58f50bdf 100644 --- a/internal/cmd/mcp_install_test.go +++ b/internal/cmd/mcp_install_test.go @@ -386,6 +386,60 @@ func TestMCPInstallCmd(t *testing.T) { }) } +// TestFindClientConfig covers the client-name lookup table: behavior that the +// command-level cases only exercise for cursor. +func TestFindClientConfig(t *testing.T) { + mappings := []struct { + clientName string + expectedType MCPClient + expectedName string + }{ + {"claude-code", ClaudeCode, "Claude Code"}, + {"CLAUDE-CODE", ClaudeCode, "Claude Code"}, + {"cursor", Cursor, "Cursor"}, + {"CURSOR", Cursor, "Cursor"}, + {"devin", Devin, "Devin"}, + {"DEVIN", Devin, "Devin"}, + {"codex", Codex, "Codex"}, + {"CODEX", Codex, "Codex"}, + } + for _, m := range mappings { + t.Run(m.clientName, func(t *testing.T) { + cfg, err := findClientConfig(m.clientName) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if cfg.ClientType != m.expectedType { + t.Errorf("ClientType = %q, want %q", cfg.ClientType, m.expectedType) + } + if cfg.Name != m.expectedName { + t.Errorf("Name = %q, want %q", cfg.Name, m.expectedName) + } + }) + } + + t.Run("every client is installable", func(t *testing.T) { + for _, cfg := range supportedClients { + found, err := findClientConfig(cfg.EditorNames[0]) + if err != nil { + t.Fatalf("findClientConfig(%q) failed: %v", cfg.EditorNames[0], err) + } + if found.Name == "" { + t.Errorf("%s: Name should not be empty", cfg.ClientType) + } + // Every client needs an install mechanism: JSON patching (path + // prefix) or a CLI install command. + if found.MCPServersPathPrefix == "" && found.buildMCPInstallCommand == nil { + t.Errorf("%s: needs MCPServersPathPrefix or buildMCPInstallCommand", cfg.ClientType) + } + // CLI-only clients (no config paths) must have an install command. + if len(found.MCPConfigPaths) == 0 && found.buildMCPInstallCommand == nil { + t.Errorf("%s: CLI-only clients must have buildMCPInstallCommand", cfg.ClientType) + } + } + }) +} + // TestFindClientConfigFile covers config file discovery, including each // client's per-client fallback path. func TestFindClientConfigFile(t *testing.T) { diff --git a/internal/cmd/skills_install.go b/internal/cmd/skills_install.go index f3489bb6..c2e59642 100644 --- a/internal/cmd/skills_install.go +++ b/internal/cmd/skills_install.go @@ -18,6 +18,7 @@ import ( "time" tea "charm.land/bubbletea/v2" + "github.com/charmbracelet/x/ansi" "github.com/fatih/color" "github.com/spf13/cobra" @@ -733,8 +734,8 @@ var selectSkillsTargets = func(cmd *cobra.Command, targets []skillsTarget, selec } model := skillsPickerModel{ - header: fmt.Sprintf("Select where to install agent skills. %s installs to %s,\nwhich %s read.", - targets[0].label, items[0].dir, joinWithAnd(targets[0].readers)), + header: ansi.Wordwrap(fmt.Sprintf("Select where to install agent skills. %s installs to %s, which %s read.", + targets[0].label, items[0].dir, joinWithAnd(append(slices.Clone(targets[0].readers), "many other agents"))), 80, ""), items: items, } program := tea.NewProgram(model, From cc896f058271999dbcd7b65a960167f459b652e7 Mon Sep 17 00:00:00 2001 From: Nathan Cochran Date: Wed, 7 Oct 2026 18:29:45 -0400 Subject: [PATCH 06/17] Remove --no-prompt flag --- README.md | 6 +-- docs/cli/tiger_skills_install.md | 10 +---- internal/cmd/skills_install.go | 29 ++++++--------- internal/cmd/skills_install_test.go | 57 +++++++++++++++-------------- 4 files changed, 46 insertions(+), 56 deletions(-) diff --git a/README.md b/README.md index cd971d87..9cad8e1f 100644 --- a/README.md +++ b/README.md @@ -275,9 +275,9 @@ to install into a directory of your choice instead. Re-run the command to update the skills to the latest version; skills that are no longer available are removed. With no arguments, the locations you've -installed to before are selected by default, and `--no-prompt` installs to them -without prompting. Existing skills that weren't installed by Tiger CLI are -never replaced unless you pass `--force`. +installed to before are selected in the picker to start with. Existing skills +that weren't installed by Tiger CLI are never replaced unless you pass +`--force`. ## Configuration diff --git a/docs/cli/tiger_skills_install.md b/docs/cli/tiger_skills_install.md index 29c9c7ef..d3067edd 100644 --- a/docs/cli/tiger_skills_install.md +++ b/docs/cli/tiger_skills_install.md @@ -17,10 +17,8 @@ Install locations: antigravity ~/.gemini/config/skills (Google Antigravity) kiro-cli ~/.kiro/skills (Kiro CLI) -With no arguments, you're prompted to select locations interactively. The -locations Tiger CLI has installed skills to before are selected by default, or -universal if there are none; use --no-prompt (or run without a terminal) to -install to them without prompting. +With no arguments, you're prompted to select locations interactively, starting +from the locations Tiger CLI has installed skills to before. Re-running the command updates the installed skills to the latest version and removes any that are no longer available. Existing skills that weren't @@ -36,9 +34,6 @@ tiger skills install [client...] [flags] # Interactive selection tiger skills install - # Update the skills wherever they were installed before - tiger skills install --no-prompt - # Install to ~/.agents/skills and for Claude Code tiger skills install universal claude-code @@ -51,7 +46,6 @@ tiger skills install [client...] [flags] ``` --force Replace existing skills that weren't installed by Tiger CLI -h, --help help for install - --no-prompt Install to the default locations without prompting --skills-dir string Install into this skills directory only (not remembered for later installs) ``` diff --git a/internal/cmd/skills_install.go b/internal/cmd/skills_install.go index c2e59642..1d1d9f27 100644 --- a/internal/cmd/skills_install.go +++ b/internal/cmd/skills_install.go @@ -52,7 +52,6 @@ var skillsTarballURL = "https://api.github.com/repos/" + skillsRepo + "/tarball/ func buildSkillsInstallCmd(_ *common.App) *cobra.Command { var force bool - var noPrompt bool var skillsDirFlag string cmd := &cobra.Command{ @@ -67,10 +66,8 @@ the skills directories of clients that don't read it. Each argument selects a location, either by name or by the name of a client that reads it. %s -With no arguments, you're prompted to select locations interactively. The -locations Tiger CLI has installed skills to before are selected by default, or -universal if there are none; use --no-prompt (or run without a terminal) to -install to them without prompting. +With no arguments, you're prompted to select locations interactively, starting +from the locations Tiger CLI has installed skills to before. Re-running the command updates the installed skills to the latest version and removes any that are no longer available. Existing skills that weren't @@ -78,9 +75,6 @@ installed by Tiger CLI are never replaced unless --force is given.`, generateSki Example: ` # Interactive selection tiger skills install - # Update the skills wherever they were installed before - tiger skills install --no-prompt - # Install to ~/.agents/skills and for Claude Code tiger skills install universal claude-code @@ -115,14 +109,15 @@ installed by Tiger CLI are never replaced unless --force is given.`, generateSki if err != nil { return err } - selected, err := defaultSkillsTargets(targetDirs) + selected, err := preselectSkillsTargets(targetDirs) if err != nil { return err } - if !noPrompt && util.IsTerminal(cmd.InOrStdin()) && util.IsTerminal(cmd.ErrOrStderr()) { - if selected, err = selectSkillsTargets(cmd, targets, selected); err != nil { - return err - } + if !util.IsTerminal(cmd.InOrStdin()) || !util.IsTerminal(cmd.ErrOrStderr()) { + return errors.New("TTY not detected - specify install locations as arguments (e.g. 'tiger skills install universal')") + } + if selected, err = selectSkillsTargets(cmd, targets, selected); err != nil { + return err } for i, dir := range targetDirs { if selected[i] { @@ -175,7 +170,6 @@ installed by Tiger CLI are never replaced unless --force is given.`, generateSki } cmd.Flags().BoolVar(&force, "force", false, "Replace existing skills that weren't installed by Tiger CLI") - cmd.Flags().BoolVar(&noPrompt, "no-prompt", false, "Install to the default locations without prompting") cmd.Flags().StringVar(&skillsDirFlag, "skills-dir", "", "Install into this skills directory only (not remembered for later installs)") registerFlagCompletion(cmd, "skills-dir", dirCompletion) @@ -281,9 +275,10 @@ func resolveSkillsTargetDirs(targets []skillsTarget) ([]string, error) { return dirs, nil } -// defaultSkillsTargets reports which of dirs are selected by default: those -// holding skills Tiger CLI installed, or the first (universal) if none do. -func defaultSkillsTargets(dirs []string) ([]bool, error) { +// preselectSkillsTargets reports which of dirs the picker starts with +// selected: those holding skills Tiger CLI installed, or the first (universal) +// if none do. +func preselectSkillsTargets(dirs []string) ([]bool, error) { selected := make([]bool, len(dirs)) found := false for i, dir := range dirs { diff --git a/internal/cmd/skills_install_test.go b/internal/cmd/skills_install_test.go index 9c155f40..2501f97e 100644 --- a/internal/cmd/skills_install_test.go +++ b/internal/cmd/skills_install_test.go @@ -317,15 +317,13 @@ func TestSkillsInstallCmd(t *testing.T) { claudeEmptyEnvHome := home(nil) antigravityHome := home(nil) aliasHome := home(nil) - firstDefaultHome := home(nil) + universalPickerHome := home(nil) customHome := home(nil) - // existingClaude holds an earlier install for Claude Code only, so it's - // what the defaults select; existingBoth adds one to the universal - // directory. + // existingClaude is an earlier install for Claude Code, which the picker + // starts with selected. existingClaude := installedSkills(".claude/skills") existingClaudeHome := home(existingClaude) - existingBothHome := home(withTrees(installedSkills(".agents/skills"), existingClaude)) pickerHome := home(withTrees(installedSkills(".agents/skills"), existingClaude)) // reinstallHome holds a previous install: one of our skills with a stale @@ -400,6 +398,12 @@ func TestSkillsInstallCmd(t *testing.T) { args: []string{"skills", "install", "claude-code", "bogus"}, wantErr: "unsupported client: bogus. Supported clients: universal, cursor, devin, codex, gemini, gemini-cli, vscode, code, vs-code, copilot, copilot-cli, claude-code, antigravity, agy, kiro-cli", }, + { + name: "no arguments and no TTY", + args: []string{"skills", "install"}, + opts: []runOption{withEnv("HOME", home(nil))}, + wantErr: "TTY not detected - specify install locations as arguments (e.g. 'tiger skills install universal')", + }, { name: "nothing selected in the picker", args: []string{"skills", "install"}, @@ -531,35 +535,32 @@ func TestSkillsInstallCmd(t *testing.T) { checks: []checkFunc{checkTree(aliasHome, installedSkills(".agents/skills"))}, }, { - name: "defaults to universal without a terminal", - args: []string{"skills", "install"}, - opts: []runOption{withEnv("HOME", firstDefaultHome), withSkillsTarball(tarball)}, - wantStdout: skillsInstallOutput([]string{universalDir(firstDefaultHome)}), - checks: []checkFunc{checkTree(firstDefaultHome, installedSkills(".agents/skills"))}, - }, - { - // An earlier install for Claude Code only is what the defaults - // select, so the universal directory isn't added. - name: "defaults to earlier installs without a terminal", - args: []string{"skills", "install"}, - opts: []runOption{withEnv("HOME", existingClaudeHome), withSkillsTarball(tarball)}, - wantStdout: skillsInstallOutput([]string{claudeDir(existingClaudeHome)}), - checks: []checkFunc{checkTree(existingClaudeHome, existingClaude)}, + // Without other installs, the picker starts with universal + // selected. + name: "installs to universal from the picker", + args: []string{"skills", "install"}, + opts: []runOption{ + withEnv("HOME", universalPickerHome), + withIsTerminal(true), + withSelectSkillsTargets([]bool{true, false, false, false}, []bool{true, false, false, false}), + withSkillsTarball(tarball), + }, + wantStdout: skillsInstallOutput([]string{universalDir(universalPickerHome)}), + checks: []checkFunc{checkTree(universalPickerHome, installedSkills(".agents/skills"))}, }, { - name: "--no-prompt installs to the defaults without the picker", - args: []string{"skills", "install", "--no-prompt"}, + // An earlier install for Claude Code only is what the picker + // starts with, so universal isn't selected. + name: "picker starts from earlier installs", + args: []string{"skills", "install"}, opts: []runOption{ - withEnv("HOME", existingBothHome), + withEnv("HOME", existingClaudeHome), withIsTerminal(true), - withSelectSkillsTargets(nil, nil), + withSelectSkillsTargets([]bool{false, true, false, false}, []bool{false, true, false, false}), withSkillsTarball(tarball), }, - wantStdout: skillsInstallOutput([]string{universalDir(existingBothHome), claudeDir(existingBothHome)}), - checks: []checkFunc{checkTree(existingBothHome, withTrees( - installedSkills(".agents/skills"), - existingClaude, - ))}, + wantStdout: skillsInstallOutput([]string{claudeDir(existingClaudeHome)}), + checks: []checkFunc{checkTree(existingClaudeHome, existingClaude)}, }, { // The picker starts from the earlier installs; what's chosen is From f3fd80d090a85667608f16cf222e5552a4a5a8c0 Mon Sep 17 00:00:00 2001 From: Nathan Cochran Date: Wed, 7 Oct 2026 18:42:46 -0400 Subject: [PATCH 07/17] Rearrange code in skills_install.go --- internal/cmd/skills_install.go | 606 ++++++++++++++++----------------- 1 file changed, 303 insertions(+), 303 deletions(-) diff --git a/internal/cmd/skills_install.go b/internal/cmd/skills_install.go index 1d1d9f27..e2c2b60d 100644 --- a/internal/cmd/skills_install.go +++ b/internal/cmd/skills_install.go @@ -50,6 +50,17 @@ const ( // It's a var so tests can point it at a local server. var skillsTarballURL = "https://api.github.com/repos/" + skillsRepo + "/tarball/" + skillsRepoRef +// Colors for the install location picker, taken from the terminal theme. +// Bright colors and bold are avoided because some themes render them as greys, +// and dim text uses the faint attribute so it suits light and dark themes +// alike. They honor color.NoColor, which the command lifecycle sets from the +// color config option. +var ( + skillsPickerCursorColor = color.New(color.FgBlue) + skillsPickerCheckColor = color.New(color.FgGreen) + skillsPickerDimColor = color.New(color.Faint) +) + func buildSkillsInstallCmd(_ *common.App) *cobra.Command { var force bool var skillsDirFlag string @@ -187,6 +198,49 @@ type skillsTarget struct { dirEnv string // see clientConfig.SkillsDirEnv } +// generateSkillsTargetsHelp generates the install locations section of the +// help text. +func generateSkillsTargetsHelp() string { + var b strings.Builder + b.WriteString("Install locations:\n") + for _, t := range skillsTargets() { + fmt.Fprintf(&b, " %-24s %s (%s)\n", t.names[0], t.dir, strings.Join(t.readers, ", ")) + } + return b.String() +} + +// findSkillsTargets returns the install locations the given names select, in +// skillsTargets order and without duplicates. Names match case-insensitively. +func findSkillsTargets(names []string) ([]skillsTarget, error) { + targets := skillsTargets() + selected := make([]bool, len(targets)) + for _, name := range names { + i := slices.IndexFunc(targets, func(t skillsTarget) bool { + return slices.ContainsFunc(t.names, func(n string) bool { return strings.EqualFold(n, name) }) + }) + if i < 0 { + return nil, fmt.Errorf("unsupported client: %s. Supported clients: %s", name, strings.Join(skillsTargetNames(), ", ")) + } + selected[i] = true + } + var result []skillsTarget + for i, t := range targets { + if selected[i] { + result = append(result, t) + } + } + return result, nil +} + +// skillsTargetNames returns every name that selects an install location. +func skillsTargetNames() []string { + var names []string + for _, t := range skillsTargets() { + names = append(names, t.names...) + } + return names +} + // skillsTargets returns every install location: universal first, then each // client with a skills directory of its own, in supportedClients order. func skillsTargets() []skillsTarget { @@ -212,47 +266,98 @@ func skillsTargets() []skillsTarget { return targets } -// skillsTargetNames returns every name that selects an install location. -func skillsTargetNames() []string { - var names []string - for _, t := range skillsTargets() { - names = append(names, t.names...) +// preselectSkillsTargets reports which of dirs the picker starts with +// selected: those holding skills Tiger CLI installed, or the first (universal) +// if none do. +func preselectSkillsTargets(dirs []string) ([]bool, error) { + selected := make([]bool, len(dirs)) + found := false + for i, dir := range dirs { + has, err := hasInstalledSkills(dir) + if err != nil { + return nil, fmt.Errorf("failed to check for installed skills: %w", err) + } + selected[i] = has + found = found || has } - return names + if !found { + selected[0] = true + } + return selected, nil } -// findSkillsTargets returns the install locations the given names select, in -// skillsTargets order and without duplicates. Names match case-insensitively. -func findSkillsTargets(names []string) ([]skillsTarget, error) { - targets := skillsTargets() - selected := make([]bool, len(targets)) - for _, name := range names { - i := slices.IndexFunc(targets, func(t skillsTarget) bool { - return slices.ContainsFunc(t.names, func(n string) bool { return strings.EqualFold(n, name) }) - }) - if i < 0 { - return nil, fmt.Errorf("unsupported client: %s. Supported clients: %s", name, strings.Join(skillsTargetNames(), ", ")) +// hasInstalledSkills reports whether skillsDir holds any skill Tiger CLI +// installed. +func hasInstalledSkills(skillsDir string) (bool, error) { + entries, err := os.ReadDir(skillsDir) + if errors.Is(err, fs.ErrNotExist) { + return false, nil + } else if err != nil { + return false, err + } + for _, e := range entries { + if strings.HasPrefix(e.Name(), skillsTempPrefix) { + continue + } + _, owned, err := skillOwnership(filepath.Join(skillsDir, e.Name())) + if err != nil { + return false, err + } + if owned { + return true, nil } - selected[i] = true } - var result []skillsTarget + return false, nil +} + +// skillsPickerItem is one install location offered by the picker. +type skillsPickerItem struct { + label string + dir string // as displayed + selected bool +} + +// selectSkillsTargets prompts the user to choose install locations, starting +// from the given selection, and returns the chosen selection. It's a var so +// command tests can stub the interactive picker. +var selectSkillsTargets = func(cmd *cobra.Command, targets []skillsTarget, selected []bool) ([]bool, error) { + dirs, err := resolveSkillsTargetDirs(targets) + if err != nil { + return nil, err + } + items := make([]skillsPickerItem, len(targets)) for i, t := range targets { - if selected[i] { - result = append(result, t) + items[i] = skillsPickerItem{ + label: t.label, + dir: displayPath(dirs[i]), + selected: selected[i], } } - return result, nil -} -// generateSkillsTargetsHelp generates the install locations section of the -// help text. -func generateSkillsTargetsHelp() string { - var b strings.Builder - b.WriteString("Install locations:\n") - for _, t := range skillsTargets() { - fmt.Fprintf(&b, " %-24s %s (%s)\n", t.names[0], t.dir, strings.Join(t.readers, ", ")) + model := skillsPickerModel{ + header: ansi.Wordwrap(fmt.Sprintf("Select where to install agent skills. %s installs to %s, which %s read.", + targets[0].label, items[0].dir, joinWithAnd(append(slices.Clone(targets[0].readers), "many other agents"))), 80, ""), + items: items, } - return b.String() + program := tea.NewProgram(model, + tea.WithInput(cmd.InOrStdin()), + tea.WithOutput(cmd.ErrOrStderr()), + tea.WithContext(cmd.Context()), + tea.WithoutSignalHandler()) + final, err := program.Run() + if err != nil { + return nil, fmt.Errorf("failed to run install location selection: %w", err) + } + + result := final.(skillsPickerModel) + if !result.confirmed { + return nil, errors.New("installation cancelled") + } + chosen := make([]bool, len(result.items)) + for i, item := range result.items { + chosen[i] = item.selected + } + return chosen, nil } // resolveSkillsTargetDirs returns the absolute directory of each target. @@ -275,26 +380,6 @@ func resolveSkillsTargetDirs(targets []skillsTarget) ([]string, error) { return dirs, nil } -// preselectSkillsTargets reports which of dirs the picker starts with -// selected: those holding skills Tiger CLI installed, or the first (universal) -// if none do. -func preselectSkillsTargets(dirs []string) ([]bool, error) { - selected := make([]bool, len(dirs)) - found := false - for i, dir := range dirs { - has, err := hasInstalledSkills(dir) - if err != nil { - return nil, fmt.Errorf("failed to check for installed skills: %w", err) - } - selected[i] = has - found = found || has - } - if !found { - selected[0] = true - } - return selected, nil -} - // expandEnvStrict expands env var references in s, reporting false if any // referenced variable is unset or empty. func expandEnvStrict(s string) (string, bool) { @@ -309,42 +394,135 @@ func expandEnvStrict(s string) (string, bool) { return expanded, ok } -// skill is one skill directory, with its contents held in memory. -type skill struct { - name string - files []skillFile +// skillsPickerModel is the Bubble Tea model for the install location picker. +type skillsPickerModel struct { + header string + items []skillsPickerItem + cursor int + confirmed bool } -// skillFile is a regular file within a skill, with symlinks already resolved. -type skillFile struct { - path string // slash-separated, relative to the skill directory - executable bool - data []byte +func (m skillsPickerModel) Init() tea.Cmd { + return nil } -// fetchSkills downloads the skills repo's tarball from GitHub and extracts the -// skills from it. -func fetchSkills(ctx context.Context) ([]skill, error) { - req, err := http.NewRequestWithContext(ctx, http.MethodGet, skillsTarballURL, nil) - if err != nil { - return nil, fmt.Errorf("failed to create request: %w", err) +func (m skillsPickerModel) Update(msg tea.Msg) (tea.Model, tea.Cmd) { + key, ok := msg.(tea.KeyPressMsg) + if !ok { + return m, nil } - req.Header.Set("Accept", "application/vnd.github+json") - req.Header.Set("X-GitHub-Api-Version", "2022-11-28") - - resp, err := api.HTTPClient.Do(req) - if err != nil { - return nil, fmt.Errorf("failed to download skills: %w", err) + switch k := key.String(); k { + case "ctrl+c", "q", "esc": + return m, tea.Quit + case "enter": + m.confirmed = true + return m, tea.Quit + case "up", "k": + if m.cursor > 0 { + m.cursor-- + } + case "down", "j": + if m.cursor < len(m.items)-1 { + m.cursor++ + } + case "space": + m.items[m.cursor].selected = !m.items[m.cursor].selected + case "1", "2", "3", "4", "5", "6", "7", "8", "9": + if i := int(k[0] - '1'); i < len(m.items) { + m.cursor = i + m.items[i].selected = !m.items[i].selected + } } - defer resp.Body.Close() + return m, nil +} - if resp.StatusCode != http.StatusOK { - return nil, skillsDownloadError(resp) +func (m skillsPickerModel) View() tea.View { + if m.confirmed { + return tea.NewView("") } - - skills, err := extractSkills(resp.Body) - if err != nil { - return nil, fmt.Errorf("failed to read skills archive: %w", err) + width := 0 + for _, item := range m.items { + width = max(width, len(item.label)) + } + + var b strings.Builder + b.WriteString(m.header + "\n\n") + for i, item := range m.items { + cursor := " " + if m.cursor == i { + cursor = skillsPickerCursorColor.Sprint(">") + } + check := " " + if item.selected { + check = skillsPickerCheckColor.Sprint("✓") + } + fmt.Fprintf(&b, "%s [%s] %d. %-*s %s\n", cursor, check, i+1, width, item.label, skillsPickerDimColor.Sprint(item.dir)) + } + b.WriteString("\n" + skillsPickerDimColor.Sprint("Use ↑/↓ to navigate, space or number keys to toggle, enter to confirm, q to quit")) + return tea.NewView(b.String()) +} + +// displayPath abbreviates the home directory in path to ~. +func displayPath(path string) string { + home, err := os.UserHomeDir() + if err != nil { + return path + } + if rel, ok := strings.CutPrefix(path, home+string(filepath.Separator)); ok { + return filepath.Join("~", rel) + } + return path +} + +// joinWithAnd joins items into an English list ("a, b, and c"). +func joinWithAnd(items []string) string { + switch len(items) { + case 0: + return "" + case 1: + return items[0] + case 2: + return items[0] + " and " + items[1] + } + return strings.Join(items[:len(items)-1], ", ") + ", and " + items[len(items)-1] +} + +// skill is one skill directory, with its contents held in memory. +type skill struct { + name string + files []skillFile +} + +// skillFile is a regular file within a skill, with symlinks already resolved. +type skillFile struct { + path string // slash-separated, relative to the skill directory + executable bool + data []byte +} + +// fetchSkills downloads the skills repo's tarball from GitHub and extracts the +// skills from it. +func fetchSkills(ctx context.Context) ([]skill, error) { + req, err := http.NewRequestWithContext(ctx, http.MethodGet, skillsTarballURL, nil) + if err != nil { + return nil, fmt.Errorf("failed to create request: %w", err) + } + req.Header.Set("Accept", "application/vnd.github+json") + req.Header.Set("X-GitHub-Api-Version", "2022-11-28") + + resp, err := api.HTTPClient.Do(req) + if err != nil { + return nil, fmt.Errorf("failed to download skills: %w", err) + } + defer resp.Body.Close() + + if resp.StatusCode != http.StatusOK { + return nil, skillsDownloadError(resp) + } + + skills, err := extractSkills(resp.Body) + if err != nil { + return nil, fmt.Errorf("failed to read skills archive: %w", err) } if err := resp.Body.Close(); err != nil { return nil, fmt.Errorf("failed to read skills archive: %w", err) @@ -551,93 +729,6 @@ func installSkills(skills []skill, skillsDir string) ([]string, error) { return removed, nil } -// hasInstalledSkills reports whether skillsDir holds any skill Tiger CLI -// installed. -func hasInstalledSkills(skillsDir string) (bool, error) { - entries, err := os.ReadDir(skillsDir) - if errors.Is(err, fs.ErrNotExist) { - return false, nil - } else if err != nil { - return false, err - } - for _, e := range entries { - if strings.HasPrefix(e.Name(), skillsTempPrefix) { - continue - } - _, owned, err := skillOwnership(filepath.Join(skillsDir, e.Name())) - if err != nil { - return false, err - } - if owned { - return true, nil - } - } - return false, nil -} - -// removeStaleSkills removes the skills in skillsDir that Tiger CLI installed -// but that aren't among skills, returning their names. It also quietly removes -// temporary directories left behind by an install that was killed partway -// through. -func removeStaleSkills(skills []skill, skillsDir string) ([]string, error) { - current := make(map[string]bool, len(skills)) - for _, s := range skills { - current[s.name] = true - } - - entries, err := os.ReadDir(skillsDir) - if err != nil { - return nil, err - } - var removed []string - for _, e := range entries { - if !e.IsDir() || current[e.Name()] { - continue - } - dir := filepath.Join(skillsDir, e.Name()) - if strings.HasPrefix(e.Name(), skillsTempPrefix) { - if err := os.RemoveAll(dir); err != nil { - return nil, err - } - continue - } - _, owned, err := skillOwnership(dir) - if err != nil { - return nil, err - } - if !owned { - continue - } - if err := os.RemoveAll(dir); err != nil { - return nil, err - } - removed = append(removed, e.Name()) - } - return removed, nil -} - -// skillOwnership reports whether anything exists at dir, and whether it's a -// skill Tiger CLI installed: a directory (not a symlink, since Tiger CLI never -// creates those) holding the marker file. -func skillOwnership(dir string) (exists, owned bool, err error) { - info, err := os.Lstat(dir) - if errors.Is(err, fs.ErrNotExist) { - return false, false, nil - } else if err != nil { - return false, false, err - } - if !info.IsDir() { - return true, false, nil - } - info, err = os.Lstat(filepath.Join(dir, skillMarkerFile)) - if errors.Is(err, fs.ErrNotExist) { - return true, false, nil - } else if err != nil { - return true, false, err - } - return true, info.Mode().IsRegular(), nil -} - // writeSkill writes s to dir along with the marker file, replacing whatever // is there. The new copy is built in a temporary directory beside dir and // swapped in, so a failure partway through leaves the existing copy intact. @@ -704,156 +795,65 @@ func replacePath(src, dst string) error { return os.RemoveAll(aside) } -// skillsPickerItem is one install location offered by the picker. -type skillsPickerItem struct { - label string - dir string // as displayed - selected bool -} - -// selectSkillsTargets prompts the user to choose install locations, starting -// from the given selection, and returns the chosen selection. It's a var so -// command tests can stub the interactive picker. -var selectSkillsTargets = func(cmd *cobra.Command, targets []skillsTarget, selected []bool) ([]bool, error) { - dirs, err := resolveSkillsTargetDirs(targets) - if err != nil { - return nil, err - } - items := make([]skillsPickerItem, len(targets)) - for i, t := range targets { - items[i] = skillsPickerItem{ - label: t.label, - dir: displayPath(dirs[i]), - selected: selected[i], - } +// removeStaleSkills removes the skills in skillsDir that Tiger CLI installed +// but that aren't among skills, returning their names. It also quietly removes +// temporary directories left behind by an install that was killed partway +// through. +func removeStaleSkills(skills []skill, skillsDir string) ([]string, error) { + current := make(map[string]bool, len(skills)) + for _, s := range skills { + current[s.name] = true } - model := skillsPickerModel{ - header: ansi.Wordwrap(fmt.Sprintf("Select where to install agent skills. %s installs to %s, which %s read.", - targets[0].label, items[0].dir, joinWithAnd(append(slices.Clone(targets[0].readers), "many other agents"))), 80, ""), - items: items, - } - program := tea.NewProgram(model, - tea.WithInput(cmd.InOrStdin()), - tea.WithOutput(cmd.ErrOrStderr()), - tea.WithContext(cmd.Context()), - tea.WithoutSignalHandler()) - final, err := program.Run() + entries, err := os.ReadDir(skillsDir) if err != nil { - return nil, fmt.Errorf("failed to run install location selection: %w", err) - } - - result := final.(skillsPickerModel) - if !result.confirmed { - return nil, errors.New("installation cancelled") - } - chosen := make([]bool, len(result.items)) - for i, item := range result.items { - chosen[i] = item.selected - } - return chosen, nil -} - -// Colors for the install location picker, taken from the terminal theme. -// Bright colors and bold are avoided because some themes render them as greys, -// and dim text uses the faint attribute so it suits light and dark themes -// alike. They honor color.NoColor, which the command lifecycle sets from the -// color config option. -var ( - skillsPickerCursorColor = color.New(color.FgBlue) - skillsPickerCheckColor = color.New(color.FgGreen) - skillsPickerDimColor = color.New(color.Faint) -) - -// skillsPickerModel is the Bubble Tea model for the install location picker. -type skillsPickerModel struct { - header string - items []skillsPickerItem - cursor int - confirmed bool -} - -func (m skillsPickerModel) Init() tea.Cmd { - return nil -} - -func (m skillsPickerModel) Update(msg tea.Msg) (tea.Model, tea.Cmd) { - key, ok := msg.(tea.KeyPressMsg) - if !ok { - return m, nil + return nil, err } - switch k := key.String(); k { - case "ctrl+c", "q", "esc": - return m, tea.Quit - case "enter": - m.confirmed = true - return m, tea.Quit - case "up", "k": - if m.cursor > 0 { - m.cursor-- + var removed []string + for _, e := range entries { + if !e.IsDir() || current[e.Name()] { + continue } - case "down", "j": - if m.cursor < len(m.items)-1 { - m.cursor++ + dir := filepath.Join(skillsDir, e.Name()) + if strings.HasPrefix(e.Name(), skillsTempPrefix) { + if err := os.RemoveAll(dir); err != nil { + return nil, err + } + continue } - case "space": - m.items[m.cursor].selected = !m.items[m.cursor].selected - case "1", "2", "3", "4", "5", "6", "7", "8", "9": - if i := int(k[0] - '1'); i < len(m.items) { - m.cursor = i - m.items[i].selected = !m.items[i].selected + _, owned, err := skillOwnership(dir) + if err != nil { + return nil, err } - } - return m, nil -} - -func (m skillsPickerModel) View() tea.View { - if m.confirmed { - return tea.NewView("") - } - width := 0 - for _, item := range m.items { - width = max(width, len(item.label)) - } - - var b strings.Builder - b.WriteString(m.header + "\n\n") - for i, item := range m.items { - cursor := " " - if m.cursor == i { - cursor = skillsPickerCursorColor.Sprint(">") + if !owned { + continue } - check := " " - if item.selected { - check = skillsPickerCheckColor.Sprint("✓") + if err := os.RemoveAll(dir); err != nil { + return nil, err } - fmt.Fprintf(&b, "%s [%s] %d. %-*s %s\n", cursor, check, i+1, width, item.label, skillsPickerDimColor.Sprint(item.dir)) + removed = append(removed, e.Name()) } - b.WriteString("\n" + skillsPickerDimColor.Sprint("Use ↑/↓ to navigate, space or number keys to toggle, enter to confirm, q to quit")) - return tea.NewView(b.String()) + return removed, nil } -// displayPath abbreviates the home directory in path to ~. -func displayPath(path string) string { - home, err := os.UserHomeDir() - if err != nil { - return path +// skillOwnership reports whether anything exists at dir, and whether it's a +// skill Tiger CLI installed: a directory (not a symlink, since Tiger CLI never +// creates those) holding the marker file. +func skillOwnership(dir string) (exists, owned bool, err error) { + info, err := os.Lstat(dir) + if errors.Is(err, fs.ErrNotExist) { + return false, false, nil + } else if err != nil { + return false, false, err } - if rel, ok := strings.CutPrefix(path, home+string(filepath.Separator)); ok { - return filepath.Join("~", rel) + if !info.IsDir() { + return true, false, nil } - return path -} - -// joinWithAnd joins items into an English list ("a, b, and c"). -func joinWithAnd(items []string) string { - switch len(items) { - case 0: - return "" - case 1: - return items[0] - case 2: - return items[0] + " and " + items[1] + info, err = os.Lstat(filepath.Join(dir, skillMarkerFile)) + if errors.Is(err, fs.ErrNotExist) { + return true, false, nil + } else if err != nil { + return true, false, err } - return strings.Join(items[:len(items)-1], ", ") + ", and " + items[len(items)-1] + return true, info.Mode().IsRegular(), nil } From 7d4f8fd2b82585eb7788daa11c92475e4510e014 Mon Sep 17 00:00:00 2001 From: Nathan Cochran Date: Thu, 8 Oct 2026 12:21:36 -0400 Subject: [PATCH 08/17] Move TTY gate up --- internal/cmd/skills_install.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/internal/cmd/skills_install.go b/internal/cmd/skills_install.go index e2c2b60d..d6c2518a 100644 --- a/internal/cmd/skills_install.go +++ b/internal/cmd/skills_install.go @@ -115,6 +115,9 @@ installed by Tiger CLI are never replaced unless --force is given.`, generateSki return err } default: + if !util.IsTerminal(cmd.InOrStdin()) || !util.IsTerminal(cmd.ErrOrStderr()) { + return errors.New("TTY not detected - specify install locations as arguments (e.g. 'tiger skills install universal')") + } targets := skillsTargets() targetDirs, err := resolveSkillsTargetDirs(targets) if err != nil { @@ -124,9 +127,6 @@ installed by Tiger CLI are never replaced unless --force is given.`, generateSki if err != nil { return err } - if !util.IsTerminal(cmd.InOrStdin()) || !util.IsTerminal(cmd.ErrOrStderr()) { - return errors.New("TTY not detected - specify install locations as arguments (e.g. 'tiger skills install universal')") - } if selected, err = selectSkillsTargets(cmd, targets, selected); err != nil { return err } From c37cd74ad019aec420ebb96f2687aa636de5940e Mon Sep 17 00:00:00 2001 From: Nathan Cochran Date: Thu, 8 Oct 2026 12:32:09 -0400 Subject: [PATCH 09/17] Don't fail command entirely if we fail to pre-select a menu option --- internal/cmd/skills_install.go | 39 +++++++++++------------------ internal/cmd/skills_install_test.go | 27 ++++++++++++++++++++ 2 files changed, 41 insertions(+), 25 deletions(-) diff --git a/internal/cmd/skills_install.go b/internal/cmd/skills_install.go index d6c2518a..56abf4cf 100644 --- a/internal/cmd/skills_install.go +++ b/internal/cmd/skills_install.go @@ -123,10 +123,7 @@ installed by Tiger CLI are never replaced unless --force is given.`, generateSki if err != nil { return err } - selected, err := preselectSkillsTargets(targetDirs) - if err != nil { - return err - } + selected := preselectSkillsTargets(targetDirs) if selected, err = selectSkillsTargets(cmd, targets, selected); err != nil { return err } @@ -269,45 +266,37 @@ func skillsTargets() []skillsTarget { // preselectSkillsTargets reports which of dirs the picker starts with // selected: those holding skills Tiger CLI installed, or the first (universal) // if none do. -func preselectSkillsTargets(dirs []string) ([]bool, error) { +func preselectSkillsTargets(dirs []string) []bool { selected := make([]bool, len(dirs)) found := false for i, dir := range dirs { - has, err := hasInstalledSkills(dir) - if err != nil { - return nil, fmt.Errorf("failed to check for installed skills: %w", err) - } - selected[i] = has - found = found || has + selected[i] = hasInstalledSkills(dir) + found = found || selected[i] } if !found { selected[0] = true } - return selected, nil + return selected } // hasInstalledSkills reports whether skillsDir holds any skill Tiger CLI -// installed. -func hasInstalledSkills(skillsDir string) (bool, error) { +// installed. Anything that can't be inspected counts as not installed: this +// only decides what the picker starts with, and installing to skillsDir would +// report the underlying problem. +func hasInstalledSkills(skillsDir string) bool { entries, err := os.ReadDir(skillsDir) - if errors.Is(err, fs.ErrNotExist) { - return false, nil - } else if err != nil { - return false, err + if err != nil { + return false } for _, e := range entries { if strings.HasPrefix(e.Name(), skillsTempPrefix) { continue } - _, owned, err := skillOwnership(filepath.Join(skillsDir, e.Name())) - if err != nil { - return false, err - } - if owned { - return true, nil + if _, owned, err := skillOwnership(filepath.Join(skillsDir, e.Name())); err == nil && owned { + return true } } - return false, nil + return false } // skillsPickerItem is one install location offered by the picker. diff --git a/internal/cmd/skills_install_test.go b/internal/cmd/skills_install_test.go index 2501f97e..b0cfab79 100644 --- a/internal/cmd/skills_install_test.go +++ b/internal/cmd/skills_install_test.go @@ -325,6 +325,7 @@ func TestSkillsInstallCmd(t *testing.T) { existingClaude := installedSkills(".claude/skills") existingClaudeHome := home(existingClaude) pickerHome := home(withTrees(installedSkills(".agents/skills"), existingClaude)) + unreadableHome := home(existingClaude) // reinstallHome holds a previous install: one of our skills with a stale // file (replaced), one no longer available (removed), skills of the @@ -562,6 +563,32 @@ func TestSkillsInstallCmd(t *testing.T) { wantStdout: skillsInstallOutput([]string{claudeDir(existingClaudeHome)}), checks: []checkFunc{checkTree(existingClaudeHome, existingClaude)}, }, + { + // A location that can't be inspected starts unselected, even + // though it holds an earlier install. The directory is made + // unreadable by the setup hook (and restored, so t.TempDir can + // clean up); root ignores file permissions, hence the skip. + name: "picker leaves locations it can't inspect unselected", + args: []string{"skills", "install"}, + opts: []runOption{ + withEnv("HOME", unreadableHome), + withIsTerminal(true), + withSetup(func(t *testing.T) { + if os.Geteuid() == 0 { + t.Skip("cannot test permission errors as root user") + } + dir := claudeDir(unreadableHome) + if err := os.Chmod(dir, 0o000); err != nil { + t.Fatalf("failed to chmod dir: %v", err) + } + t.Cleanup(func() { os.Chmod(dir, 0o755) }) + }), + withSelectSkillsTargets([]bool{true, false, false, false}, []bool{true, false, false, false}), + withSkillsTarball(tarball), + }, + wantStdout: skillsInstallOutput([]string{universalDir(unreadableHome)}), + checks: []checkFunc{checkTree(filepath.Join(unreadableHome, ".agents"), installedSkills("skills"))}, + }, { // The picker starts from the earlier installs; what's chosen is // installed, and an unchosen earlier install is left as it is. From c35382feca3f9f30c0a5ef5e76f8c5f478557072 Mon Sep 17 00:00:00 2001 From: Nathan Cochran Date: Thu, 8 Oct 2026 12:42:33 -0400 Subject: [PATCH 10/17] Make tiger skills install resilient to bad env vars --- internal/cmd/client_helper.go | 2 +- internal/cmd/skills_install.go | 15 +++++++++------ internal/cmd/skills_install_test.go | 12 ++++++++++++ 3 files changed, 22 insertions(+), 7 deletions(-) diff --git a/internal/cmd/client_helper.go b/internal/cmd/client_helper.go index c6eb93e2..fb8ecc58 100644 --- a/internal/cmd/client_helper.go +++ b/internal/cmd/client_helper.go @@ -38,7 +38,7 @@ type clientConfig struct { // SkillsDirEnv overrides SkillsDir for clients whose skills directory can // be relocated by an env var. It's a path containing env var references // (e.g. "${CLAUDE_CONFIG_DIR}/skills"), used only when every variable it - // references is set and non-empty. + // references is set and non-empty, and it expands to an absolute path. SkillsDirEnv string } diff --git a/internal/cmd/skills_install.go b/internal/cmd/skills_install.go index 56abf4cf..72ec2389 100644 --- a/internal/cmd/skills_install.go +++ b/internal/cmd/skills_install.go @@ -349,18 +349,21 @@ var selectSkillsTargets = func(cmd *cobra.Command, targets []skillsTarget, selec return chosen, nil } -// resolveSkillsTargetDirs returns the absolute directory of each target. +// resolveSkillsTargetDirs returns the absolute directory of each target. An +// env var override that doesn't give an absolute path is ignored, since +// there's no telling what a relative one is relative to. func resolveSkillsTargetDirs(targets []skillsTarget) ([]string, error) { dirs := make([]string, len(targets)) for i, t := range targets { - var dir string - if expanded, ok := expandEnvStrict(t.dirEnv); t.dirEnv != "" && ok { - dir = filepath.Clean(expanded) - } else { - dir = util.ExpandPath(t.dir) + if t.dirEnv != "" { + if expanded, ok := expandEnvStrict(t.dirEnv); ok && filepath.IsAbs(expanded) { + dirs[i] = filepath.Clean(expanded) + continue + } } // ExpandPath leaves a ~ in place if the home directory is unknown, // which would otherwise install relative to the working directory. + dir := util.ExpandPath(t.dir) if !filepath.IsAbs(dir) { return nil, fmt.Errorf("failed to determine skills directory: %s is not an absolute path", dir) } diff --git a/internal/cmd/skills_install_test.go b/internal/cmd/skills_install_test.go index b0cfab79..ccbcfb50 100644 --- a/internal/cmd/skills_install_test.go +++ b/internal/cmd/skills_install_test.go @@ -315,6 +315,7 @@ func TestSkillsInstallCmd(t *testing.T) { claudeConfigHome := home(nil) claudeConfigDir := filepath.Join(claudeConfigHome, "claude-config") claudeEmptyEnvHome := home(nil) + claudeRelativeEnvHome := home(nil) antigravityHome := home(nil) aliasHome := home(nil) universalPickerHome := home(nil) @@ -520,6 +521,17 @@ func TestSkillsInstallCmd(t *testing.T) { wantStdout: skillsInstallOutput([]string{claudeDir(claudeEmptyEnvHome)}), checks: []checkFunc{checkTree(claudeEmptyEnvHome, installedSkills(".claude/skills"))}, }, + { + name: "relative CLAUDE_CONFIG_DIR is ignored", + args: []string{"skills", "install", "claude-code"}, + opts: []runOption{ + withEnv("HOME", claudeRelativeEnvHome), + withEnv("CLAUDE_CONFIG_DIR", "claude-config"), + withSkillsTarball(tarball), + }, + wantStdout: skillsInstallOutput([]string{claudeDir(claudeRelativeEnvHome)}), + checks: []checkFunc{checkTree(claudeRelativeEnvHome, installedSkills(".claude/skills"))}, + }, { name: "installs to antigravity's skills directory", args: []string{"skills", "install", "antigravity"}, From d31cd9d3def0e72b886d19e369bb3a3d558e08c2 Mon Sep 17 00:00:00 2001 From: Nathan Cochran Date: Thu, 8 Oct 2026 13:42:16 -0400 Subject: [PATCH 11/17] Clean up code, de-duplicate function calls --- internal/cmd/skills_install.go | 216 +++++++++++++++------------- internal/cmd/skills_install_test.go | 83 +++++++---- 2 files changed, 169 insertions(+), 130 deletions(-) diff --git a/internal/cmd/skills_install.go b/internal/cmd/skills_install.go index 72ec2389..6d759cfe 100644 --- a/internal/cmd/skills_install.go +++ b/internal/cmd/skills_install.go @@ -99,42 +99,17 @@ installed by Tiger CLI are never replaced unless --force is given.`, generateSki } var dirs []string + var err error switch { case skillsDirFlag != "": - dir, err := filepath.Abs(util.ExpandPath(skillsDirFlag)) - if err != nil { - return fmt.Errorf("failed to resolve skills directory: %w", err) - } - dirs = []string{dir} + dirs, err = skillsDirsFromFlag(skillsDirFlag) case len(args) > 0: - targets, err := findSkillsTargets(args) - if err != nil { - return err - } - if dirs, err = resolveSkillsTargetDirs(targets); err != nil { - return err - } + dirs, err = skillsDirsFromArgs(args) default: - if !util.IsTerminal(cmd.InOrStdin()) || !util.IsTerminal(cmd.ErrOrStderr()) { - return errors.New("TTY not detected - specify install locations as arguments (e.g. 'tiger skills install universal')") - } - targets := skillsTargets() - targetDirs, err := resolveSkillsTargetDirs(targets) - if err != nil { - return err - } - selected := preselectSkillsTargets(targetDirs) - if selected, err = selectSkillsTargets(cmd, targets, selected); err != nil { - return err - } - for i, dir := range targetDirs { - if selected[i] { - dirs = append(dirs, dir) - } - } - if len(dirs) == 0 { - return errors.New("no install locations selected") - } + dirs, err = skillsDirsFromPicker(cmd) + } + if err != nil { + return err } skills, err := fetchSkills(cmd.Context()) @@ -184,6 +159,16 @@ installed by Tiger CLI are never replaced unless --force is given.`, generateSki return cmd } +// skillsDirsFromFlag returns the directory given by --skills-dir, made +// absolute (relative to the working directory). +func skillsDirsFromFlag(path string) ([]string, error) { + dir, err := filepath.Abs(util.ExpandPath(path)) + if err != nil { + return nil, fmt.Errorf("failed to resolve skills directory: %w", err) + } + return []string{dir}, nil +} + // skillsTarget is a location skills can be installed to: the universal // directory most clients read, or the skills directory of a client that // doesn't read it. @@ -206,9 +191,10 @@ func generateSkillsTargetsHelp() string { return b.String() } -// findSkillsTargets returns the install locations the given names select, in -// skillsTargets order and without duplicates. Names match case-insensitively. -func findSkillsTargets(names []string) ([]skillsTarget, error) { +// skillsDirsFromArgs returns the directories of the install locations the +// given names select, in skillsTargets order and without duplicates. Names +// match case-insensitively. +func skillsDirsFromArgs(names []string) ([]string, error) { targets := skillsTargets() selected := make([]bool, len(targets)) for _, name := range names { @@ -226,7 +212,7 @@ func findSkillsTargets(names []string) ([]skillsTarget, error) { result = append(result, t) } } - return result, nil + return resolveSkillsTargetDirs(result) } // skillsTargetNames returns every name that selects an install location. @@ -238,6 +224,53 @@ func skillsTargetNames() []string { return names } +// skillsPickerItem is one install location offered by the picker. +type skillsPickerItem struct { + label string + dir string // as displayed + selected bool +} + +// skillsDirsFromPicker prompts the user to choose install locations, starting +// from the ones preselectSkillsDirs picks, and returns the chosen +// directories. It fails without a terminal to prompt on. +func skillsDirsFromPicker(cmd *cobra.Command) ([]string, error) { + if !util.IsTerminal(cmd.InOrStdin()) || !util.IsTerminal(cmd.ErrOrStderr()) { + return nil, errors.New("TTY not detected - specify install locations as arguments (e.g. 'tiger skills install universal')") + } + targets := skillsTargets() + dirs, err := resolveSkillsTargetDirs(targets) + if err != nil { + return nil, err + } + selected := preselectSkillsDirs(dirs) + items := make([]skillsPickerItem, len(targets)) + for i, t := range targets { + items[i] = skillsPickerItem{ + label: t.label, + dir: displayPath(dirs[i]), + selected: selected[i], + } + } + header := ansi.Wordwrap(fmt.Sprintf("Select where to install agent skills. %s installs to %s, which %s read.", + targets[0].label, items[0].dir, joinWithAnd(append(slices.Clone(targets[0].readers), "many other agents"))), 80, "") + + chosen, err := runSkillsPicker(cmd, header, items) + if err != nil { + return nil, err + } + var result []string + for i, dir := range dirs { + if chosen[i] { + result = append(result, dir) + } + } + if len(result) == 0 { + return nil, errors.New("no install locations selected") + } + return result, nil +} + // skillsTargets returns every install location: universal first, then each // client with a skills directory of its own, in supportedClients order. func skillsTargets() []skillsTarget { @@ -263,10 +296,47 @@ func skillsTargets() []skillsTarget { return targets } -// preselectSkillsTargets reports which of dirs the picker starts with +// resolveSkillsTargetDirs returns the absolute directory of each target. An +// env var override that doesn't give an absolute path is ignored, since +// there's no telling what a relative one is relative to. +func resolveSkillsTargetDirs(targets []skillsTarget) ([]string, error) { + dirs := make([]string, len(targets)) + for i, t := range targets { + if t.dirEnv != "" { + if expanded, ok := expandEnvStrict(t.dirEnv); ok && filepath.IsAbs(expanded) { + dirs[i] = filepath.Clean(expanded) + continue + } + } + // ExpandPath leaves a ~ in place if the home directory is unknown, + // which would otherwise install relative to the working directory. + dir := util.ExpandPath(t.dir) + if !filepath.IsAbs(dir) { + return nil, fmt.Errorf("failed to determine skills directory: %s is not an absolute path", dir) + } + dirs[i] = dir + } + return dirs, nil +} + +// expandEnvStrict expands env var references in s, reporting false if any +// referenced variable is unset or empty. +func expandEnvStrict(s string) (string, bool) { + ok := true + expanded := os.Expand(s, func(name string) string { + v := os.Getenv(name) + if v == "" { + ok = false + } + return v + }) + return expanded, ok +} + +// preselectSkillsDirs reports which of dirs the picker starts with // selected: those holding skills Tiger CLI installed, or the first (universal) // if none do. -func preselectSkillsTargets(dirs []string) []bool { +func preselectSkillsDirs(dirs []string) []bool { selected := make([]bool, len(dirs)) found := false for i, dir := range dirs { @@ -299,34 +369,13 @@ func hasInstalledSkills(skillsDir string) bool { return false } -// skillsPickerItem is one install location offered by the picker. -type skillsPickerItem struct { - label string - dir string // as displayed - selected bool -} - -// selectSkillsTargets prompts the user to choose install locations, starting -// from the given selection, and returns the chosen selection. It's a var so -// command tests can stub the interactive picker. -var selectSkillsTargets = func(cmd *cobra.Command, targets []skillsTarget, selected []bool) ([]bool, error) { - dirs, err := resolveSkillsTargetDirs(targets) - if err != nil { - return nil, err - } - items := make([]skillsPickerItem, len(targets)) - for i, t := range targets { - items[i] = skillsPickerItem{ - label: t.label, - dir: displayPath(dirs[i]), - selected: selected[i], - } - } - +// runSkillsPicker runs the interactive install location picker over items and +// returns which of them the user chose. It's a var so command tests can stub +// the interactive part. +var runSkillsPicker = func(cmd *cobra.Command, header string, items []skillsPickerItem) ([]bool, error) { model := skillsPickerModel{ - header: ansi.Wordwrap(fmt.Sprintf("Select where to install agent skills. %s installs to %s, which %s read.", - targets[0].label, items[0].dir, joinWithAnd(append(slices.Clone(targets[0].readers), "many other agents"))), 80, ""), - items: items, + header: header, + items: items, } program := tea.NewProgram(model, tea.WithInput(cmd.InOrStdin()), @@ -349,43 +398,6 @@ var selectSkillsTargets = func(cmd *cobra.Command, targets []skillsTarget, selec return chosen, nil } -// resolveSkillsTargetDirs returns the absolute directory of each target. An -// env var override that doesn't give an absolute path is ignored, since -// there's no telling what a relative one is relative to. -func resolveSkillsTargetDirs(targets []skillsTarget) ([]string, error) { - dirs := make([]string, len(targets)) - for i, t := range targets { - if t.dirEnv != "" { - if expanded, ok := expandEnvStrict(t.dirEnv); ok && filepath.IsAbs(expanded) { - dirs[i] = filepath.Clean(expanded) - continue - } - } - // ExpandPath leaves a ~ in place if the home directory is unknown, - // which would otherwise install relative to the working directory. - dir := util.ExpandPath(t.dir) - if !filepath.IsAbs(dir) { - return nil, fmt.Errorf("failed to determine skills directory: %s is not an absolute path", dir) - } - dirs[i] = dir - } - return dirs, nil -} - -// expandEnvStrict expands env var references in s, reporting false if any -// referenced variable is unset or empty. -func expandEnvStrict(s string) (string, bool) { - ok := true - expanded := os.Expand(s, func(name string) string { - v := os.Getenv(name) - if v == "" { - ok = false - } - return v - }) - return expanded, ok -} - // skillsPickerModel is the Bubble Tea model for the install location picker. type skillsPickerModel struct { header string diff --git a/internal/cmd/skills_install_test.go b/internal/cmd/skills_install_test.go index ccbcfb50..fd6a58ac 100644 --- a/internal/cmd/skills_install_test.go +++ b/internal/cmd/skills_install_test.go @@ -4,6 +4,7 @@ import ( "archive/tar" "bytes" "compress/gzip" + "errors" "fmt" "io/fs" "maps" @@ -11,6 +12,7 @@ import ( "net/http/httptest" "os" "path/filepath" + "slices" "strconv" "strings" "testing" @@ -222,37 +224,41 @@ func skillsInstallOutput(dirs []string, removed ...string) string { return b.String() } -// skillsTargetLabels are the install locations, in the order the picker -// offers them. -var skillsTargetLabels = []string{"Universal", "Claude Code", "Google Antigravity", "Kiro CLI"} - -// withSelectSkillsTargets stubs the interactive install location picker: it -// expects to be offered the locations with want preselected, and answers with -// choose. A nil want expects the picker not to be shown at all. -func withSelectSkillsTargets(want, choose []bool) runOption { +// withRunSkillsPicker stubs the interactive install location picker: it +// expects to be offered exactly want, and answers with choose. A nil want +// expects the picker not to be shown at all. +func withRunSkillsPicker(want []skillsPickerItem, choose []bool) runOption { return withSetup(func(t *testing.T) { - original := selectSkillsTargets - selectSkillsTargets = func(_ *cobra.Command, targets []skillsTarget, selected []bool) ([]bool, error) { + original := runSkillsPicker + runSkillsPicker = func(_ *cobra.Command, _ string, items []skillsPickerItem) ([]bool, error) { if want == nil { t.Error("install location picker shown, want none") - return selected, nil - } - labels := make([]string, len(targets)) - for i, target := range targets { - labels[i] = target.label + return nil, errors.New("picker shown") } - if diff := cmp.Diff(skillsTargetLabels, labels); diff != "" { - t.Errorf("picker locations mismatch (-want +got):\n%s", diff) - } - if diff := cmp.Diff(want, selected); diff != "" { - t.Errorf("picker preselection mismatch (-want +got):\n%s", diff) + if diff := cmp.Diff(want, items, cmp.AllowUnexported(skillsPickerItem{})); diff != "" { + t.Errorf("picker items mismatch (-want +got):\n%s", diff) } return choose, nil } - t.Cleanup(func() { selectSkillsTargets = original }) + t.Cleanup(func() { runSkillsPicker = original }) }) } +// pickerItems returns the items the picker offers with the default install +// locations (the home directory shown as ~), selecting those in selected. +func pickerItems(selected ...string) []skillsPickerItem { + items := []skillsPickerItem{ + {label: "Universal", dir: "~/.agents/skills"}, + {label: "Claude Code", dir: "~/.claude/skills"}, + {label: "Google Antigravity", dir: "~/.gemini/config/skills"}, + {label: "Kiro CLI", dir: "~/.kiro/skills"}, + } + for i := range items { + items[i].selected = slices.Contains(selected, items[i].label) + } + return items +} + func TestSkillsInstallCmd(t *testing.T) { // The fixture repo exercises skill discovery and symlink resolution: // file and directory symlinks between skills (resolved), and ones that @@ -316,6 +322,7 @@ func TestSkillsInstallCmd(t *testing.T) { claudeConfigDir := filepath.Join(claudeConfigHome, "claude-config") claudeEmptyEnvHome := home(nil) claudeRelativeEnvHome := home(nil) + claudePickerEnvHome := home(nil) antigravityHome := home(nil) aliasHome := home(nil) universalPickerHome := home(nil) @@ -412,7 +419,7 @@ func TestSkillsInstallCmd(t *testing.T) { opts: []runOption{ withEnv("HOME", home(nil)), withIsTerminal(true), - withSelectSkillsTargets([]bool{true, false, false, false}, []bool{false, false, false, false}), + withRunSkillsPicker(pickerItems("Universal"), []bool{false, false, false, false}), }, wantErr: "no install locations selected", }, @@ -555,7 +562,7 @@ func TestSkillsInstallCmd(t *testing.T) { opts: []runOption{ withEnv("HOME", universalPickerHome), withIsTerminal(true), - withSelectSkillsTargets([]bool{true, false, false, false}, []bool{true, false, false, false}), + withRunSkillsPicker(pickerItems("Universal"), []bool{true, false, false, false}), withSkillsTarball(tarball), }, wantStdout: skillsInstallOutput([]string{universalDir(universalPickerHome)}), @@ -569,7 +576,7 @@ func TestSkillsInstallCmd(t *testing.T) { opts: []runOption{ withEnv("HOME", existingClaudeHome), withIsTerminal(true), - withSelectSkillsTargets([]bool{false, true, false, false}, []bool{false, true, false, false}), + withRunSkillsPicker(pickerItems("Claude Code"), []bool{false, true, false, false}), withSkillsTarball(tarball), }, wantStdout: skillsInstallOutput([]string{claudeDir(existingClaudeHome)}), @@ -595,12 +602,32 @@ func TestSkillsInstallCmd(t *testing.T) { } t.Cleanup(func() { os.Chmod(dir, 0o755) }) }), - withSelectSkillsTargets([]bool{true, false, false, false}, []bool{true, false, false, false}), + withRunSkillsPicker(pickerItems("Universal"), []bool{true, false, false, false}), withSkillsTarball(tarball), }, wantStdout: skillsInstallOutput([]string{universalDir(unreadableHome)}), checks: []checkFunc{checkTree(filepath.Join(unreadableHome, ".agents"), installedSkills("skills"))}, }, + { + // The picker shows where each location actually is, including an + // env var override. + name: "picker shows CLAUDE_CONFIG_DIR", + args: []string{"skills", "install"}, + opts: []runOption{ + withEnv("HOME", claudePickerEnvHome), + withEnv("CLAUDE_CONFIG_DIR", filepath.Join(claudePickerEnvHome, "claude-config")), + withIsTerminal(true), + withRunSkillsPicker([]skillsPickerItem{ + {label: "Universal", dir: "~/.agents/skills", selected: true}, + {label: "Claude Code", dir: "~/claude-config/skills"}, + {label: "Google Antigravity", dir: "~/.gemini/config/skills"}, + {label: "Kiro CLI", dir: "~/.kiro/skills"}, + }, []bool{false, true, false, false}), + withSkillsTarball(tarball), + }, + wantStdout: skillsInstallOutput([]string{filepath.Join(claudePickerEnvHome, "claude-config", "skills")}), + checks: []checkFunc{checkTree(claudePickerEnvHome, installedSkills("claude-config/skills"))}, + }, { // The picker starts from the earlier installs; what's chosen is // installed, and an unchosen earlier install is left as it is. @@ -609,7 +636,7 @@ func TestSkillsInstallCmd(t *testing.T) { opts: []runOption{ withEnv("HOME", pickerHome), withIsTerminal(true), - withSelectSkillsTargets([]bool{true, true, false, false}, []bool{false, true, false, true}), + withRunSkillsPicker(pickerItems("Universal", "Claude Code"), []bool{false, true, false, true}), withSkillsTarball(tarball), }, wantStdout: skillsInstallOutput([]string{claudeDir(pickerHome), filepath.Join(pickerHome, ".kiro", "skills")}), @@ -651,7 +678,7 @@ func TestSkillsInstallCmd(t *testing.T) { opts: []runOption{ withEnv("HOME", customHome), withIsTerminal(true), - withSelectSkillsTargets(nil, nil), + withRunSkillsPicker(nil, nil), withSkillsTarball(tarball), }, wantStdout: skillsInstallOutput([]string{filepath.Join(customHome, "custom")}), @@ -663,7 +690,7 @@ func TestSkillsInstallCmd(t *testing.T) { // TestSkillsPickerModel checks the picker's keys: toggling, moving, and that // only enter confirms. Helper-level because the picker is a Bubble Tea model // that needs a real TTY to run through the command; the command tests stub it -// via withSelectSkillsTargets. +// via withRunSkillsPicker. func TestSkillsPickerModel(t *testing.T) { // Ctrl+C is {Code: 'c', Mod: tea.ModCtrl}; the raw control byte {Code: 3} // stringifies to "\x03" and would match nothing. From 13b928ab37aa7af1e25a00b2fb40c01b0bb9e74f Mon Sep 17 00:00:00 2001 From: Nathan Cochran Date: Thu, 8 Oct 2026 13:55:04 -0400 Subject: [PATCH 12/17] More minor cleanup and function call de-duplication --- internal/cmd/skills_install.go | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/internal/cmd/skills_install.go b/internal/cmd/skills_install.go index 6d759cfe..8977a73c 100644 --- a/internal/cmd/skills_install.go +++ b/internal/cmd/skills_install.go @@ -91,7 +91,7 @@ installed by Tiger CLI are never replaced unless --force is given.`, generateSki # Install into a custom skills directory tiger skills install --skills-dir ~/my-skills`, - ValidArgs: skillsTargetNames(), + ValidArgs: skillsTargetNames(skillsTargets()), SilenceUsage: true, RunE: func(cmd *cobra.Command, args []string) error { if skillsDirFlag != "" && len(args) > 0 { @@ -202,7 +202,7 @@ func skillsDirsFromArgs(names []string) ([]string, error) { return slices.ContainsFunc(t.names, func(n string) bool { return strings.EqualFold(n, name) }) }) if i < 0 { - return nil, fmt.Errorf("unsupported client: %s. Supported clients: %s", name, strings.Join(skillsTargetNames(), ", ")) + return nil, fmt.Errorf("unsupported client: %s. Supported clients: %s", name, strings.Join(skillsTargetNames(targets), ", ")) } selected[i] = true } @@ -215,10 +215,10 @@ func skillsDirsFromArgs(names []string) ([]string, error) { return resolveSkillsTargetDirs(result) } -// skillsTargetNames returns every name that selects an install location. -func skillsTargetNames() []string { +// skillsTargetNames returns every name that selects one of targets. +func skillsTargetNames(targets []skillsTarget) []string { var names []string - for _, t := range skillsTargets() { + for _, t := range targets { names = append(names, t.names...) } return names @@ -617,7 +617,6 @@ func extractSkills(r io.Reader) ([]skill, error) { if !slices.ContainsFunc(s.files, func(f skillFile) bool { return f.path == "SKILL.md" }) { continue } - slices.SortFunc(s.files, func(a, b skillFile) int { return strings.Compare(a.path, b.path) }) skills = append(skills, s) } return skills, nil From 44ca47a852836d7f2c78f802dd95c224a73ed55c Mon Sep 17 00:00:00 2001 From: Nathan Cochran Date: Thu, 8 Oct 2026 15:05:13 -0400 Subject: [PATCH 13/17] Update CLAUDE.md --- CLAUDE.md | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index 017d567e..dc433c73 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -157,7 +157,7 @@ Helper functions inside `internal/cmd` take the `*cobra.Command` and print throu - Commands with structured output take `-o`/`--output` with `json`, `yaml`, and `table` (the default); some commands support extras (`env`, `bare`). Register the flag with the validating flag types in `flag_helper.go` (`new(outputFlag)` and friends, with no bound variable) and read the value from `cfg.Output` — `output` is a config value. - Serialize with `util.SerializeToJSON` and `util.SerializeToYAML`. **Don't add `yaml:` struct tags to output types** — `SerializeToYAML` encodes to JSON first and converts, so only `json:` tags matter and the two formats stay consistent (including for generated types that only carry `json:` tags). -- Colored output uses `fatih/color`; the lifecycle sets `color.NoColor` from `cfg.Color`, so commands need no per-command wiring. +- Colored output uses `fatih/color`; the lifecycle sets `color.NoColor` from `cfg.Color`, so commands need no per-command wiring. Stick to the eight standard ANSI colors (`color.FgBlue`, `color.FgGreen`, …), without their bright variants or bold, and de-emphasize text with `color.Faint` rather than a grey, so output follows the user's terminal theme (some themes render bright colors and bold as greys). The `tiger skills install` picker is the model. ### Tone @@ -169,7 +169,7 @@ Warnings and errors follow the same standard. A non-fatal failure prints to stde Read user input through the helpers in `internal/util/read.go` — `util.ReadLine` (one trimmed line), `util.ReadPassword` (no echo, saves and restores terminal state), and `util.ReadAll` (everything, for piped input) — never by driving `bufio`/`term` directly. All three select on `ctx.Done()`, so Ctrl-C unblocks a waiting prompt instead of hanging. -**Every read that prompts the user in real time must be gated on `util.IsTerminal(cmd.InOrStdin()) && util.IsTerminal(cmd.ErrOrStderr())`** — both streams, so the user can both see the question and answer it — and must return an error naming that command's non-interactive alternative when either check fails (e.g. "use --confirm to skip the prompt", or "use --new-password, --auto-generate, or TIGER_NEW_PASSWORD"). Put the gate immediately before the prompt in the command body, not inside a shared helper, so the error can name the right flag or env var. A prompt that merely *offers* something skips silently instead of erroring, since nothing is being refused — `offerProdProtection` in `auth_login.go` is the one example. The gate is about *prompts*, not reading stdin as such — a read whose whole point is piped input (`util.ReadAll` on a here-doc or a `|`) must not be gated. +**Every read that prompts the user in real time must be gated on `util.IsTerminal(cmd.InOrStdin()) && util.IsTerminal(cmd.ErrOrStderr())`** — both streams, so the user can both see the question and answer it — and must return an error naming that command's non-interactive alternative when either check fails (e.g. "use --confirm to skip the prompt", or "use --new-password, --auto-generate, or TIGER_NEW_PASSWORD"). Put the gate just before the prompt, in the command body or a helper used only by that command — never in a helper shared across commands — so the error can name the right flag or env var. A prompt that merely *offers* something skips silently instead of erroring, since nothing is being refused — `offerProdProtection` in `auth_login.go` is the one example. The gate is about *prompts*, not reading stdin as such — a read whose whole point is piped input (`util.ReadAll` on a here-doc or a `|`) must not be gated. `util.IsTerminal` and `util.ReadPassword` are `var`s so tests can replace them (the `withIsTerminal` and `withReadPassword` options in `main_test.go`). `ReadPassword` needs stdin to be a real `*os.File`, so password prompts can't be driven with `cmd.SetIn`; `ReadLine`/`ReadAll` take any `io.Reader` and work fine with it. @@ -189,6 +189,7 @@ Interactive menus and spinners use BubbleTea v2 (`charm.land/bubbletea/v2`). Alw - Error strings start with a **lowercase** letter, so they read correctly when a caller wraps them (`fmt.Errorf("failed to get service: %w", err)`). A leading proper noun or initialism keeps its capitalization (`API key validation failed`); if that reads awkwardly, reword so the identifier isn't first (`missing required option: ClientName`). Log messages are the opposite — they start with a capital letter. - Use `errors.New` for static error strings; use `fmt.Errorf` only when you need format verbs. - Handle every error — never ignore one or assign it to `_` (at minimum, print a warning). +- Cleanup that can fail (closing a reader, removing a temporary directory) is called explicitly on the success path, with its error returned, and also deferred as best-effort cleanup for the error paths, where a more important error is already being returned. This relies on the cleanup being safe to call twice, so use it only for idempotent operations. `writeSkill` in `skills_install.go` is the model. - Return errors from `RunE`; never call `os.Exit` in a command. For specific exit codes, wrap with `common.ExitWithCode`, and map API errors to exit codes with `common.ExitWithErrorFromStatusCode`. The exit codes are defined in `internal/common/errors.go`, and `main.go` honors a `common.ExitCodeError` anywhere in the error chain. - After checking an API response's status code, nil-check the parsed body (`resp.JSON200`, etc.) before using it — a nil body indicates a bug and should return `errors.New("empty response from API")`. - Operations that connect to a service should call `common.CheckServiceReady` first and translate `common.ErrPaused`/`common.ErrNotReady` into actionable messages. From 4475b64a381804f02434062212ed6a7965f8f69f Mon Sep 17 00:00:00 2001 From: Nathan Cochran Date: Thu, 8 Oct 2026 15:22:50 -0400 Subject: [PATCH 14/17] Fix small file cleanup bug --- internal/cmd/skills_install.go | 15 +++++++++++---- internal/cmd/skills_install_test.go | 28 ++++++++++++++-------------- 2 files changed, 25 insertions(+), 18 deletions(-) diff --git a/internal/cmd/skills_install.go b/internal/cmd/skills_install.go index 8977a73c..c4ac013c 100644 --- a/internal/cmd/skills_install.go +++ b/internal/cmd/skills_install.go @@ -736,7 +736,7 @@ func installSkills(skills []skill, skillsDir string) ([]string, error) { // is there. The new copy is built in a temporary directory beside dir and // swapped in, so a failure partway through leaves the existing copy intact. func writeSkill(dir string, s skill) error { - tmp, err := os.MkdirTemp(filepath.Dir(dir), skillsTempPrefix+s.name+"-") + tmp, err := os.MkdirTemp(filepath.Dir(dir), skillsTempPrefix+"new-"+s.name+"-") if err != nil { return err } @@ -770,7 +770,8 @@ func writeSkill(dir string, s skill) error { // replacePath moves src to dst, replacing any file, directory, or symlink at // dst. A rename can't replace a non-empty directory, so the existing entry is -// first moved aside, and restored if the swap fails. A symlink at dst is +// first moved aside, and restored if the swap fails. If restoring fails too, +// it's left where it was moved and the error says where. A symlink at dst is // removed as a link, never followed. func replacePath(src, dst string) error { if _, err := os.Lstat(dst); errors.Is(err, fs.ErrNotExist) { @@ -783,7 +784,12 @@ func replacePath(src, dst string) error { if err != nil { return err } - defer os.RemoveAll(aside) + keepAside := false + defer func() { + if !keepAside { + os.RemoveAll(aside) + } + }() old := filepath.Join(aside, "old") if err := os.Rename(dst, old); err != nil { @@ -791,7 +797,8 @@ func replacePath(src, dst string) error { } if err := os.Rename(src, dst); err != nil { if restoreErr := os.Rename(old, dst); restoreErr != nil { - return fmt.Errorf("%w (and failed to restore %s: %w)", err, dst, restoreErr) + keepAside = true + return fmt.Errorf("%w (and failed to restore %s, so the previous copy was left at %s: %w)", err, dst, old, restoreErr) } return err } diff --git a/internal/cmd/skills_install_test.go b/internal/cmd/skills_install_test.go index fd6a58ac..97374e59 100644 --- a/internal/cmd/skills_install_test.go +++ b/internal/cmd/skills_install_test.go @@ -342,20 +342,20 @@ func TestSkillsInstallCmd(t *testing.T) { // was killed (quietly removed). reinstallHome := t.TempDir() writeTree(t, reinstallHome, map[string]string{ - ".claude/skills/alpha/.tiger-cli": skillMarkerContent, - ".claude/skills/alpha/SKILL.md": "old alpha", - ".claude/skills/alpha/stale.md": "stale", - ".claude/skills/retired/.tiger-cli": skillMarkerContent, - ".claude/skills/retired/SKILL.md": "retired skill", - ".claude/skills/other/SKILL.md": "user's skill", - ".claude/skills/.tiger-beta-123/.tiger-cli": skillMarkerContent, - ".claude/skills/.tiger-beta-123/SKILL.md": "leftover", - ".claude/skills/.tiger-old-456/old/.tiger-cli": skillMarkerContent, - ".claude/skills/.tiger-old-456/old/SKILL.md": "leftover", - ".claude/skills/.hidden/SKILL.md": "user's hidden entry", - ".claude/skills/linked": "-> " + filepath.Join(reinstallHome, "marked"), - "marked/.tiger-cli": skillMarkerContent, - "marked/SKILL.md": "marked skill", + ".claude/skills/alpha/.tiger-cli": skillMarkerContent, + ".claude/skills/alpha/SKILL.md": "old alpha", + ".claude/skills/alpha/stale.md": "stale", + ".claude/skills/retired/.tiger-cli": skillMarkerContent, + ".claude/skills/retired/SKILL.md": "retired skill", + ".claude/skills/other/SKILL.md": "user's skill", + ".claude/skills/.tiger-new-beta-123/.tiger-cli": skillMarkerContent, + ".claude/skills/.tiger-new-beta-123/SKILL.md": "leftover", + ".claude/skills/.tiger-old-456/old/.tiger-cli": skillMarkerContent, + ".claude/skills/.tiger-old-456/old/SKILL.md": "leftover", + ".claude/skills/.hidden/SKILL.md": "user's hidden entry", + ".claude/skills/linked": "-> " + filepath.Join(reinstallHome, "marked"), + "marked/.tiger-cli": skillMarkerContent, + "marked/SKILL.md": "marked skill", }) reinstallTree := withTrees(installedSkills(".claude/skills"), map[string]string{ ".claude/skills/other/SKILL.md": "user's skill", From 3fdd3872fee2d4e5ef43986aa198fa82b2f06eeb Mon Sep 17 00:00:00 2001 From: Nathan Cochran Date: Thu, 8 Oct 2026 16:09:10 -0400 Subject: [PATCH 15/17] Update bubbletea, fix menu clearing bug --- go.mod | 8 ++++---- go.sum | 16 ++++++++-------- internal/cmd/skills_install.go | 5 ++++- internal/cmd/skills_install_test.go | 3 +++ 4 files changed, 19 insertions(+), 13 deletions(-) diff --git a/go.mod b/go.mod index 8f2122b5..d2f70942 100644 --- a/go.mod +++ b/go.mod @@ -11,10 +11,10 @@ tool ( ) require ( - charm.land/bubbletea/v2 v2.0.9 + charm.land/bubbletea/v2 v2.1.0 github.com/Masterminds/semver/v3 v3.5.0 github.com/charmbracelet/colorprofile v0.4.3 - github.com/charmbracelet/x/ansi v0.11.8 + github.com/charmbracelet/x/ansi v0.11.9 github.com/cli/safeexec v1.0.1 github.com/docker/go-units v0.5.0 github.com/fatih/color v1.19.0 @@ -41,7 +41,7 @@ require ( github.com/BurntSushi/toml v1.6.0 // indirect github.com/apapsch/go-jsonmerge/v2 v2.0.0 // indirect github.com/cespare/xxhash/v2 v2.3.0 // indirect - github.com/charmbracelet/ultraviolet v0.0.0-20260903151058-ae99b731b8c5 // indirect + github.com/charmbracelet/ultraviolet v0.0.0-20261008173134-6b8d4baf91b4 // indirect github.com/charmbracelet/x/term v0.2.2 // indirect github.com/charmbracelet/x/termios v0.1.1 // indirect github.com/charmbracelet/x/windows v0.2.2 // indirect @@ -64,7 +64,7 @@ require ( github.com/lucasb-eyer/go-colorful v1.4.1 // indirect github.com/mattn/go-colorable v0.1.15 // indirect github.com/mattn/go-isatty v0.0.24 // indirect - github.com/mattn/go-runewidth v0.0.29 // indirect + github.com/mattn/go-runewidth v0.0.30 // indirect github.com/muesli/cancelreader v0.2.2 // indirect github.com/oapi-codegen/oapi-codegen/v2 v2.8.0 // indirect github.com/oasdiff/yaml v0.1.1 // indirect diff --git a/go.sum b/go.sum index c7893f98..f38ffe3d 100644 --- a/go.sum +++ b/go.sum @@ -1,5 +1,5 @@ -charm.land/bubbletea/v2 v2.0.9 h1:DpJCMWKgzQK8SJv4zbKKFHAI10ymWy/evClPFk0k0f8= -charm.land/bubbletea/v2 v2.0.9/go.mod h1:2SkdgoTXluXJHOUwAoRlRXF/28vklb1rFl6GcgV1/ss= +charm.land/bubbletea/v2 v2.1.0 h1:A9fj9h5LFrvQhQHFNggp8K8rvY6CUfgn9dMbb6p2HIw= +charm.land/bubbletea/v2 v2.1.0/go.mod h1:xWHeyxIX9/wiFU2kgPW04ojOZyTzrWf+8lGjUCaftiU= github.com/BurntSushi/toml v1.6.0 h1:dRaEfpa2VI55EwlIW72hMRHdWouJeRF7TPYhI+AUQjk= github.com/BurntSushi/toml v1.6.0/go.mod h1:ukJfTF/6rtPPRCnwkur4qwRxa8vTRFBF0uk2lLoLwho= github.com/Masterminds/semver/v3 v3.5.0 h1:kQceYJfbupGfZOKZQg0kou0DgAKhzDg2NZPAwZ/2OOE= @@ -14,10 +14,10 @@ github.com/cespare/xxhash/v2 v2.3.0 h1:UL815xU9SqsFlibzuggzjXhog7bL6oX9BbNZnL2UF github.com/cespare/xxhash/v2 v2.3.0/go.mod h1:VGX0DQ3Q6kWi7AoAeZDth3/j3BFtOZR5XLFGgcrjCOs= github.com/charmbracelet/colorprofile v0.4.3 h1:QPa1IWkYI+AOB+fE+mg/5/4HRMZcaXex9t5KX76i20Q= github.com/charmbracelet/colorprofile v0.4.3/go.mod h1:/zT4BhpD5aGFpqQQqw7a+VtHCzu+zrQtt1zhMt9mR4Q= -github.com/charmbracelet/ultraviolet v0.0.0-20260903151058-ae99b731b8c5 h1:omidyaLW3GwtN5N35bPZWCxygynfhMRO2dTJ+OCcjLg= -github.com/charmbracelet/ultraviolet v0.0.0-20260903151058-ae99b731b8c5/go.mod h1:nAw0d9PhFp1qdzi2xhQU5YOu5sVpDIHWlaW2Uz/bCro= -github.com/charmbracelet/x/ansi v0.11.8 h1:JMFwp0CgDC2+jcOB162HH5k7I3FVbgFSMMYg7dSPBQQ= -github.com/charmbracelet/x/ansi v0.11.8/go.mod h1:ZNN+3mXny/516oTQPLMPIBeSINvNJJQ8uQXDgbeJxY0= +github.com/charmbracelet/ultraviolet v0.0.0-20261008173134-6b8d4baf91b4 h1:hISLzUbwcvJbGoNR6A3L/TpguRRnACBmZOee8LcHLJw= +github.com/charmbracelet/ultraviolet v0.0.0-20261008173134-6b8d4baf91b4/go.mod h1:aJDVk+WOVM1K04rdc/71PMPoUGfjxjhvXfP3Pjz1Zio= +github.com/charmbracelet/x/ansi v0.11.9 h1:kIrqLBxtTVbLyyHO2Evoi93EjQo1mylWmpNEsfPxXAM= +github.com/charmbracelet/x/ansi v0.11.9/go.mod h1:DG/hpOnaI2QJ/KMMqLL2h0KVhluuJLazf8973P+Vy5w= github.com/charmbracelet/x/exp/golden v0.0.0-20241212170349-ad4b7ae0f25f h1:UytXHv0UxnsDFmL/7Z9Q5SBYPwSuRLXHbwx+6LycZ2w= github.com/charmbracelet/x/exp/golden v0.0.0-20241212170349-ad4b7ae0f25f/go.mod h1:wDlXFlCrmJ8J+swcL/MnGUuYnqgQdW9rhSD61oNMb6U= github.com/charmbracelet/x/term v0.2.2 h1:xVRT/S2ZcKdhhOuSP4t5cLi5o+JxklsoEObBSgfgZRk= @@ -123,8 +123,8 @@ github.com/mattn/go-colorable v0.1.15 h1:+u9SLTRGnXv73cEsnsmoZBom+dMU88B2M0aDcWy github.com/mattn/go-colorable v0.1.15/go.mod h1:6LmQG8QLFO4G5z1gPvYEzlUgJ2wF+stgPZH1UqBm1s8= github.com/mattn/go-isatty v0.0.24 h1:tGZZoVgT/KiqK1c8ocVLeDS8BSWMRd47J3Lbz7vsReI= github.com/mattn/go-isatty v0.0.24/go.mod h1:nMCL3Zebbrt45jsMDgnfIwz6ydEQApk5oEI3HqDio6A= -github.com/mattn/go-runewidth v0.0.29 h1:3oGF3R/S2N9DQ3ptftzVIvg2eicmojCzlwBEmqEPDfQ= -github.com/mattn/go-runewidth v0.0.29/go.mod h1:3qAiGCV4Koz/yuveO58qUefmUTRm8r0IGEXZ9jeHp/8= +github.com/mattn/go-runewidth v0.0.30 h1:+KUuiDA4fF0R1p5FeueHefjDm+GIM+kWfFnDjybOPgk= +github.com/mattn/go-runewidth v0.0.30/go.mod h1:3qAiGCV4Koz/yuveO58qUefmUTRm8r0IGEXZ9jeHp/8= github.com/modelcontextprotocol/go-sdk v1.8.0 h1:KIvahhYqwtbeniWVPs3TcXEA7b8jEtwfBpOTAI+Urx4= github.com/modelcontextprotocol/go-sdk v1.8.0/go.mod h1:dL7u98E/zjJTGzEq+j30jQ8K2k1mb6LeAH4inEcSGts= github.com/muesli/cancelreader v0.2.2 h1:3I4Kt4BQjOR54NavqnDogx/MIoWBFa0StPA8ELUXHmA= diff --git a/internal/cmd/skills_install.go b/internal/cmd/skills_install.go index c4ac013c..26dffb2d 100644 --- a/internal/cmd/skills_install.go +++ b/internal/cmd/skills_install.go @@ -404,6 +404,7 @@ type skillsPickerModel struct { items []skillsPickerItem cursor int confirmed bool + done bool // set on quitting, however that happens, to clear the picker } func (m skillsPickerModel) Init() tea.Cmd { @@ -417,9 +418,11 @@ func (m skillsPickerModel) Update(msg tea.Msg) (tea.Model, tea.Cmd) { } switch k := key.String(); k { case "ctrl+c", "q", "esc": + m.done = true return m, tea.Quit case "enter": m.confirmed = true + m.done = true return m, tea.Quit case "up", "k": if m.cursor > 0 { @@ -441,7 +444,7 @@ func (m skillsPickerModel) Update(msg tea.Msg) (tea.Model, tea.Cmd) { } func (m skillsPickerModel) View() tea.View { - if m.confirmed { + if m.done { return tea.NewView("") } width := 0 diff --git a/internal/cmd/skills_install_test.go b/internal/cmd/skills_install_test.go index 97374e59..ae2a07e2 100644 --- a/internal/cmd/skills_install_test.go +++ b/internal/cmd/skills_install_test.go @@ -732,6 +732,9 @@ func TestSkillsPickerModel(t *testing.T) { if result.confirmed != tc.wantConfirmed { t.Errorf("confirmed = %v, want %v", result.confirmed, tc.wantConfirmed) } + if !result.done { + t.Error("done = false, want true (every case ends by quitting)") + } }) } } From 23bc8206264144a66a7d032ea0ae813fc3d28f49 Mon Sep 17 00:00:00 2001 From: Nathan Cochran Date: Thu, 8 Oct 2026 16:37:52 -0400 Subject: [PATCH 16/17] Reorder functions --- internal/cmd/skills_install.go | 148 ++++++++++++++++----------------- 1 file changed, 74 insertions(+), 74 deletions(-) diff --git a/internal/cmd/skills_install.go b/internal/cmd/skills_install.go index 26dffb2d..647d1a14 100644 --- a/internal/cmd/skills_install.go +++ b/internal/cmd/skills_install.go @@ -159,14 +159,15 @@ installed by Tiger CLI are never replaced unless --force is given.`, generateSki return cmd } -// skillsDirsFromFlag returns the directory given by --skills-dir, made -// absolute (relative to the working directory). -func skillsDirsFromFlag(path string) ([]string, error) { - dir, err := filepath.Abs(util.ExpandPath(path)) - if err != nil { - return nil, fmt.Errorf("failed to resolve skills directory: %w", err) +// generateSkillsTargetsHelp generates the install locations section of the +// help text. +func generateSkillsTargetsHelp() string { + var b strings.Builder + b.WriteString("Install locations:\n") + for _, t := range skillsTargets() { + fmt.Fprintf(&b, " %-24s %s (%s)\n", t.names[0], t.dir, strings.Join(t.readers, ", ")) } - return []string{dir}, nil + return b.String() } // skillsTarget is a location skills can be installed to: the universal @@ -180,15 +181,48 @@ type skillsTarget struct { dirEnv string // see clientConfig.SkillsDirEnv } -// generateSkillsTargetsHelp generates the install locations section of the -// help text. -func generateSkillsTargetsHelp() string { - var b strings.Builder - b.WriteString("Install locations:\n") - for _, t := range skillsTargets() { - fmt.Fprintf(&b, " %-24s %s (%s)\n", t.names[0], t.dir, strings.Join(t.readers, ", ")) +// skillsTargets returns every install location: universal first, then each +// client with a skills directory of its own, in supportedClients order. +func skillsTargets() []skillsTarget { + targets := []skillsTarget{{ + label: "Universal", + names: []string{"universal"}, + dir: "~/.agents/skills", + }} + for _, c := range supportedClients { + if c.SkillsDir == "" { + targets[0].names = append(targets[0].names, c.EditorNames...) + targets[0].readers = append(targets[0].readers, c.Name) + continue + } + targets = append(targets, skillsTarget{ + label: c.Name, + names: c.EditorNames, + readers: []string{c.Name}, + dir: c.SkillsDir, + dirEnv: c.SkillsDirEnv, + }) } - return b.String() + return targets +} + +// skillsTargetNames returns every name that selects one of targets. +func skillsTargetNames(targets []skillsTarget) []string { + var names []string + for _, t := range targets { + names = append(names, t.names...) + } + return names +} + +// skillsDirsFromFlag returns the directory given by --skills-dir, made +// absolute (relative to the working directory). +func skillsDirsFromFlag(path string) ([]string, error) { + dir, err := filepath.Abs(util.ExpandPath(path)) + if err != nil { + return nil, fmt.Errorf("failed to resolve skills directory: %w", err) + } + return []string{dir}, nil } // skillsDirsFromArgs returns the directories of the install locations the @@ -215,15 +249,6 @@ func skillsDirsFromArgs(names []string) ([]string, error) { return resolveSkillsTargetDirs(result) } -// skillsTargetNames returns every name that selects one of targets. -func skillsTargetNames(targets []skillsTarget) []string { - var names []string - for _, t := range targets { - names = append(names, t.names...) - } - return names -} - // skillsPickerItem is one install location offered by the picker. type skillsPickerItem struct { label string @@ -271,31 +296,6 @@ func skillsDirsFromPicker(cmd *cobra.Command) ([]string, error) { return result, nil } -// skillsTargets returns every install location: universal first, then each -// client with a skills directory of its own, in supportedClients order. -func skillsTargets() []skillsTarget { - targets := []skillsTarget{{ - label: "Universal", - names: []string{"universal"}, - dir: "~/.agents/skills", - }} - for _, c := range supportedClients { - if c.SkillsDir == "" { - targets[0].names = append(targets[0].names, c.EditorNames...) - targets[0].readers = append(targets[0].readers, c.Name) - continue - } - targets = append(targets, skillsTarget{ - label: c.Name, - names: c.EditorNames, - readers: []string{c.Name}, - dir: c.SkillsDir, - dirEnv: c.SkillsDirEnv, - }) - } - return targets -} - // resolveSkillsTargetDirs returns the absolute directory of each target. An // env var override that doesn't give an absolute path is ignored, since // there's no telling what a relative one is relative to. @@ -369,6 +369,31 @@ func hasInstalledSkills(skillsDir string) bool { return false } +// displayPath abbreviates the home directory in path to ~. +func displayPath(path string) string { + home, err := os.UserHomeDir() + if err != nil { + return path + } + if rel, ok := strings.CutPrefix(path, home+string(filepath.Separator)); ok { + return filepath.Join("~", rel) + } + return path +} + +// joinWithAnd joins items into an English list ("a, b, and c"). +func joinWithAnd(items []string) string { + switch len(items) { + case 0: + return "" + case 1: + return items[0] + case 2: + return items[0] + " and " + items[1] + } + return strings.Join(items[:len(items)-1], ", ") + ", and " + items[len(items)-1] +} + // runSkillsPicker runs the interactive install location picker over items and // returns which of them the user chose. It's a var so command tests can stub // the interactive part. @@ -469,31 +494,6 @@ func (m skillsPickerModel) View() tea.View { return tea.NewView(b.String()) } -// displayPath abbreviates the home directory in path to ~. -func displayPath(path string) string { - home, err := os.UserHomeDir() - if err != nil { - return path - } - if rel, ok := strings.CutPrefix(path, home+string(filepath.Separator)); ok { - return filepath.Join("~", rel) - } - return path -} - -// joinWithAnd joins items into an English list ("a, b, and c"). -func joinWithAnd(items []string) string { - switch len(items) { - case 0: - return "" - case 1: - return items[0] - case 2: - return items[0] + " and " + items[1] - } - return strings.Join(items[:len(items)-1], ", ") + ", and " + items[len(items)-1] -} - // skill is one skill directory, with its contents held in memory. type skill struct { name string From 4047c8507a6329139bf7f5aa8e955026cd2a3f94 Mon Sep 17 00:00:00 2001 From: Nathan Cochran Date: Tue, 13 Oct 2026 14:09:04 -0400 Subject: [PATCH 17/17] Switch color library --- go.mod | 2 +- internal/cmd/skills_install.go | 31 +++++++++++++++---------------- 2 files changed, 16 insertions(+), 17 deletions(-) diff --git a/go.mod b/go.mod index 947c2910..ce4834be 100644 --- a/go.mod +++ b/go.mod @@ -18,7 +18,6 @@ require ( github.com/charmbracelet/x/ansi v0.11.9 github.com/cli/safeexec v1.0.1 github.com/docker/go-units v0.5.0 - github.com/fatih/color v1.19.0 github.com/google/go-cmp v0.7.0 github.com/google/jsonschema-go v0.4.3 github.com/jackc/pgx/v5 v5.10.0 @@ -51,6 +50,7 @@ require ( github.com/cpuguy83/go-md2man/v2 v2.0.6 // indirect github.com/danieljoos/wincred v1.2.3 // indirect github.com/dprotaso/go-yit v0.0.0-20220510233725-9ba8df137936 // indirect + github.com/fatih/color v1.19.0 // indirect github.com/fsnotify/fsnotify v1.10.1 // indirect github.com/getkin/kin-openapi v0.149.0 // indirect github.com/go-openapi/jsonpointer v1.0.1 // indirect diff --git a/internal/cmd/skills_install.go b/internal/cmd/skills_install.go index 647d1a14..dccf9588 100644 --- a/internal/cmd/skills_install.go +++ b/internal/cmd/skills_install.go @@ -18,8 +18,8 @@ import ( "time" tea "charm.land/bubbletea/v2" + lipgloss "charm.land/lipgloss/v2" "github.com/charmbracelet/x/ansi" - "github.com/fatih/color" "github.com/spf13/cobra" "github.com/timescale/tiger-cli/internal/api" @@ -50,17 +50,6 @@ const ( // It's a var so tests can point it at a local server. var skillsTarballURL = "https://api.github.com/repos/" + skillsRepo + "/tarball/" + skillsRepoRef -// Colors for the install location picker, taken from the terminal theme. -// Bright colors and bold are avoided because some themes render them as greys, -// and dim text uses the faint attribute so it suits light and dark themes -// alike. They honor color.NoColor, which the command lifecycle sets from the -// color config option. -var ( - skillsPickerCursorColor = color.New(color.FgBlue) - skillsPickerCheckColor = color.New(color.FgGreen) - skillsPickerDimColor = color.New(color.Faint) -) - func buildSkillsInstallCmd(_ *common.App) *cobra.Command { var force bool var skillsDirFlag string @@ -468,6 +457,16 @@ func (m skillsPickerModel) Update(msg tea.Msg) (tea.Model, tea.Cmd) { return m, nil } +// Styles for the install location picker, taken from the terminal theme. +// Bright colors and bold are avoided because some themes render them as greys, +// and dim text uses the faint attribute so it suits light and dark themes +// alike. +var ( + skillsPickerCursorStyle = lipgloss.NewStyle().Foreground(lipgloss.Blue) + skillsPickerCheckStyle = lipgloss.NewStyle().Foreground(lipgloss.Green) + skillsPickerDimStyle = lipgloss.NewStyle().Faint(true) +) + func (m skillsPickerModel) View() tea.View { if m.done { return tea.NewView("") @@ -482,15 +481,15 @@ func (m skillsPickerModel) View() tea.View { for i, item := range m.items { cursor := " " if m.cursor == i { - cursor = skillsPickerCursorColor.Sprint(">") + cursor = skillsPickerCursorStyle.Render(">") } check := " " if item.selected { - check = skillsPickerCheckColor.Sprint("✓") + check = skillsPickerCheckStyle.Render("✓") } - fmt.Fprintf(&b, "%s [%s] %d. %-*s %s\n", cursor, check, i+1, width, item.label, skillsPickerDimColor.Sprint(item.dir)) + fmt.Fprintf(&b, "%s [%s] %d. %-*s %s\n", cursor, check, i+1, width, item.label, skillsPickerDimStyle.Render(item.dir)) } - b.WriteString("\n" + skillsPickerDimColor.Sprint("Use ↑/↓ to navigate, space or number keys to toggle, enter to confirm, q to quit")) + b.WriteString("\n" + skillsPickerDimStyle.Render("Use ↑/↓ to navigate, space or number keys to toggle, enter to confirm, q to quit")) return tea.NewView(b.String()) }