diff --git a/.claude/skills/cross-platform-development b/.claude/skills/cross-platform-development new file mode 120000 index 0000000..805ba02 --- /dev/null +++ b/.claude/skills/cross-platform-development @@ -0,0 +1 @@ +../../skills/cross-platform-development \ No newline at end of file diff --git a/.claude/skills/implementing-command-patterns b/.claude/skills/implementing-command-patterns new file mode 120000 index 0000000..c0388db --- /dev/null +++ b/.claude/skills/implementing-command-patterns @@ -0,0 +1 @@ +../../skills/implementing-command-patterns \ No newline at end of file diff --git a/.claude/skills/testing-best-practices b/.claude/skills/testing-best-practices new file mode 120000 index 0000000..a5adfac --- /dev/null +++ b/.claude/skills/testing-best-practices @@ -0,0 +1 @@ +../../skills/testing-best-practices \ No newline at end of file diff --git a/.claude/skills/testing-commands b/.claude/skills/testing-commands new file mode 120000 index 0000000..05da890 --- /dev/null +++ b/.claude/skills/testing-commands @@ -0,0 +1 @@ +../../skills/testing-commands \ No newline at end of file diff --git a/.claude/skills/working-with-config-system b/.claude/skills/working-with-config-system new file mode 120000 index 0000000..bc498a4 --- /dev/null +++ b/.claude/skills/working-with-config-system @@ -0,0 +1 @@ +../../skills/working-with-config-system \ No newline at end of file diff --git a/.claude/skills/working-with-instances-manager b/.claude/skills/working-with-instances-manager new file mode 120000 index 0000000..d8c4195 --- /dev/null +++ b/.claude/skills/working-with-instances-manager @@ -0,0 +1 @@ +../../skills/working-with-instances-manager \ No newline at end of file diff --git a/.claude/skills/working-with-podman-runtime-config b/.claude/skills/working-with-podman-runtime-config new file mode 120000 index 0000000..a6dace8 --- /dev/null +++ b/.claude/skills/working-with-podman-runtime-config @@ -0,0 +1 @@ +../../skills/working-with-podman-runtime-config \ No newline at end of file diff --git a/.claude/skills/working-with-runtime-system b/.claude/skills/working-with-runtime-system new file mode 120000 index 0000000..76ffcf1 --- /dev/null +++ b/.claude/skills/working-with-runtime-system @@ -0,0 +1 @@ +../../skills/working-with-runtime-system \ No newline at end of file diff --git a/.claude/skills/working-with-steplogger b/.claude/skills/working-with-steplogger new file mode 120000 index 0000000..ecdd21a --- /dev/null +++ b/.claude/skills/working-with-steplogger @@ -0,0 +1 @@ +../../skills/working-with-steplogger \ No newline at end of file diff --git a/AGENTS.md b/AGENTS.md index c85a761..9d5ef83 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -144,23 +144,6 @@ When designing JSON storage structures for persistent data, use **nested objects } ``` -**Avoid (flat structure with snake_case or camelCase):** -```json -{ - "id": "...", - "source_dir": "/Users/user/project", // Don't use snake_case - "config_dir": "/Users/user/project/.kortex" -} -``` - -```json -{ - "id": "...", - "sourceDir": "/Users/user/project", // Don't use camelCase - "configDir": "/Users/user/project/.kortex" -} -``` - **Benefits:** - **Better organization** - Related fields are grouped together - **Clarity** - Field relationships are explicit through nesting @@ -168,24 +151,6 @@ When designing JSON storage structures for persistent data, use **nested objects - **No naming conflicts** - Avoids debates about snake_case vs camelCase - **Self-documenting** - Structure communicates intent -**Implementation:** -- Create nested structs with `json` tags -- Use lowercase field names in JSON (Go convention for exported fields + json tags) -- Group related fields under descriptive parent keys - -**Example:** -```go -type InstancePaths struct { - Source string `json:"source"` - Configuration string `json:"configuration"` -} - -type InstanceData struct { - ID string `json:"id"` - Paths InstancePaths `json:"paths"` -} -``` - ### Runtime System The runtime system provides a pluggable architecture for managing workspaces on different container/VM platforms (Podman, MicroVM, Kubernetes, etc.). @@ -193,713 +158,51 @@ The runtime system provides a pluggable architecture for managing workspaces on **Key Components:** - **Runtime Interface** (`pkg/runtime/runtime.go`): Contract all runtimes must implement - **Registry** (`pkg/runtime/registry.go`): Manages runtime registration and discovery -- **Runtime Implementations** (`pkg/runtime//`): Platform-specific packages (e.g., `fake`) +- **Runtime Implementations** (`pkg/runtime//`): Platform-specific packages (e.g., `fake`, `podman`) - **Centralized Registration** (`pkg/runtimesetup/register.go`): Automatically registers all available runtimes -**Adding a New Runtime:** - -Use the `/add-runtime` skill which provides step-by-step instructions for creating a new runtime implementation. The `fake` runtime in `pkg/runtime/fake/` serves as a reference implementation. - -**Runtime Registration in Commands:** - -Commands use `runtimesetup.RegisterAll()` to automatically register all available runtimes: - -```go -import "github.com/kortex-hub/kortex-cli/pkg/runtimesetup" - -// In command preRun -manager, err := instances.NewManager(storageDir) -if err != nil { - return err -} - -// Register all available runtimes -if err := runtimesetup.RegisterAll(manager); err != nil { - return err -} -``` - -This automatically registers all runtimes from `pkg/runtimesetup/register.go` that report as available (e.g., only registers Podman if `podman` CLI is installed). - -**Optional Runtime Interfaces:** - -Some runtimes may implement additional optional interfaces to provide extended functionality: +**Optional Interfaces:** +- **StorageAware**: Enables runtimes to persist data in a dedicated storage directory +- **Terminal**: Enables interactive terminal sessions with running instances -**Terminal Interface** (`runtime.Terminal`): +**For detailed runtime implementation guidance, use:** `/working-with-runtime-system` -Runtimes implementing this interface enable interactive terminal sessions for connecting to running instances. This is used by the `terminal` command. - -```go -type Terminal interface { - // Terminal starts an interactive terminal session inside a running instance. - // The command is executed with stdin/stdout/stderr connected directly to the user's terminal. - Terminal(ctx context.Context, instanceID string, command []string) error -} -``` - -Example implementation (Podman runtime): -```go -func (p *podmanRuntime) Terminal(ctx context.Context, instanceID string, command []string) error { - if instanceID == "" { - return fmt.Errorf("%w: instance ID is required", runtime.ErrInvalidParams) - } - if len(command) == 0 { - return fmt.Errorf("%w: command is required", runtime.ErrInvalidParams) - } - - // Build podman exec -it - args := []string{"exec", "-it", instanceID} - args = append(args, command...) - - return p.executor.RunInteractive(ctx, args...) -} -``` - -The Terminal interface follows the same pattern as `StorageAware` - it's optional, and runtimes that don't support interactive sessions simply don't implement it. The instances manager checks for Terminal support at runtime using type assertion. +**To add a new runtime, use:** `/add-runtime` ### StepLogger System -The StepLogger system provides user-facing progress feedback during runtime operations. It displays operational steps with spinners and completion messages in text mode, improving the user experience for long-running operations. - -**Key Components:** -- **StepLogger Interface** (`pkg/steplogger/steplogger.go`): Contract for logging operational steps -- **TextLogger** (`pkg/steplogger/text.go`): Implementation with spinner animations for text output -- **NoOpLogger** (`pkg/steplogger/noop.go`): Silent implementation for JSON mode and tests -- **Context Integration** (`pkg/steplogger/context.go`): Attach/retrieve loggers from context +The StepLogger system provides user-facing progress feedback during runtime operations with spinners and completion messages. -**Injecting StepLogger into Context:** +**Key Points:** +- Commands inject StepLogger into context based on output mode (text with spinners vs JSON silent) +- Runtime methods retrieve logger from context and report progress steps +- Automatic behavior: animated spinners in text mode, silent in JSON mode -Commands are responsible for creating and injecting the appropriate StepLogger into the context before calling runtime methods: +**For detailed StepLogger integration guidance, use:** `/working-with-steplogger` -```go -func (c *myCmd) run(cmd *cobra.Command, args []string) error { - // Create appropriate logger based on output mode - var logger steplogger.StepLogger - if c.output == "json" { - // No step logging in JSON mode - logger = steplogger.NewNoOpLogger() - } else { - // Use text logger with spinners for text output - logger = steplogger.NewTextLogger(cmd.ErrOrStderr()) - } - defer logger.Complete() - - // Attach logger to context - ctx := steplogger.WithLogger(cmd.Context(), logger) - - // Pass context to runtime methods - info, err := runtime.Create(ctx, params) - if err != nil { - return err - } - - return nil -} -``` - -**Logger Selection Rules:** -- **JSON mode** (`--output json`): Use `steplogger.NewNoOpLogger()` - completely silent, no output -- **Text mode** (default): Use `steplogger.NewTextLogger(cmd.ErrOrStderr())` - displays spinners and messages to stderr - -**Important:** -- Always call `defer logger.Complete()` immediately after creating the logger -- Attach the logger to context using `steplogger.WithLogger(cmd.Context(), logger)` -- Pass the context (not `cmd.Context()`) to all runtime methods -- Output to stderr (`cmd.ErrOrStderr()`) so it doesn't interfere with stdout JSON output - -**Reference:** See `pkg/cmd/init.go`, `pkg/cmd/workspace_start.go`, `pkg/cmd/workspace_stop.go`, and `pkg/cmd/workspace_remove.go` for complete examples. - -**Using StepLogger in Runtime Methods:** - -All runtime methods that accept a `context.Context` should use the StepLogger for user feedback. - -**Note for Runtime Implementers:** You don't need to create or inject the StepLogger - it's already in the context. Simply retrieve it using `steplogger.FromContext(ctx)` and use it as shown below: - -```go -func (r *myRuntime) Create(ctx context.Context, params runtime.CreateParams) (runtime.RuntimeInfo, error) { - logger := steplogger.FromContext(ctx) - defer logger.Complete() - - // Step 1: Create resources - logger.Start("Creating workspace directory", "Workspace directory created") - if err := r.createDirectory(params.Name); err != nil { - logger.Fail(err) - return runtime.RuntimeInfo{}, err - } - - // Step 2: Build image - logger.Start("Building container image", "Container image built") - if err := r.buildImage(ctx, params.Name); err != nil { - logger.Fail(err) - return runtime.RuntimeInfo{}, err - } - - // Step 3: Create instance - logger.Start("Creating instance", "Instance created") - info, err := r.createInstance(ctx, params) - if err != nil { - logger.Fail(err) - return runtime.RuntimeInfo{}, err - } - - return info, nil -} -``` - -**StepLogger Methods:** - -- **`Start(inProgress, completed string)`** - Begin a new step with progress and completion messages - - Automatically completes the previous step if one exists - - `inProgress`: Message shown while the step is running (e.g., "Building container image") - - `completed`: Message shown when the step completes (e.g., "Container image built") - -- **`Complete()`** - Mark the current step as successfully completed - - Typically called with `defer` at the start of the method to complete the last step - -- **`Fail(err error)`** - Mark the current step as failed - - Displays the error message to the user - - Should be called before returning the error - -**Best Practices:** - -1. **Always call `Complete()` with defer** at the start of the method -2. **Use descriptive messages** that inform users about what's happening -3. **Call `Fail()` before returning errors** to show which step failed -4. **Retrieve logger from context** using `steplogger.FromContext(ctx)` -5. **Don't worry about JSON mode** - the NoOpLogger is automatically used when `--output json` is set - -**Example Pattern for All Runtime Methods:** - -```go -// Create -func (r *myRuntime) Create(ctx context.Context, params runtime.CreateParams) (runtime.RuntimeInfo, error) { - logger := steplogger.FromContext(ctx) - defer logger.Complete() - - logger.Start("Creating resource", "Resource created") - // ... implementation ... -} - -// Start -func (r *myRuntime) Start(ctx context.Context, id string) (runtime.RuntimeInfo, error) { - logger := steplogger.FromContext(ctx) - defer logger.Complete() - - logger.Start("Starting instance", "Instance started") - // ... implementation ... -} - -// Stop -func (r *myRuntime) Stop(ctx context.Context, id string) error { - logger := steplogger.FromContext(ctx) - defer logger.Complete() - - logger.Start("Stopping instance", "Instance stopped") - // ... implementation ... -} - -// Remove -func (r *myRuntime) Remove(ctx context.Context, id string) error { - logger := steplogger.FromContext(ctx) - defer logger.Complete() - - logger.Start("Removing instance", "Instance removed") - // ... implementation ... -} -``` - -**Automatic Behavior:** - -- **Text Mode**: Displays animated spinners during operations and completion checkmarks -- **JSON Mode**: Silent - no output to avoid polluting JSON responses -- **Testing**: Use `steplogger.NewNoOpLogger()` or don't attach a logger (defaults to NoOp) - -**Reference Implementation:** - -See `pkg/runtime/podman/` for complete examples: -- `create.go` - Multi-step Create operation with 4 steps -- `start.go` - Start operation with verification step -- `stop.go` - Simple single-step Stop operation -- `remove.go` - Remove operation with state checking - -**Testing StepLogger:** - -Create a fake step logger to verify step behavior in tests: - -```go -// Create fake logger -type fakeStepLogger struct { - startCalls []stepCall - failCalls []error - completeCalls int -} - -type stepCall struct { - inProgress string - completed string -} - -func (f *fakeStepLogger) Start(inProgress, completed string) { - f.startCalls = append(f.startCalls, stepCall{inProgress, completed}) -} +### Config System -func (f *fakeStepLogger) Fail(err error) { - f.failCalls = append(f.failCalls, err) -} +The config system manages workspace configuration for **injecting environment variables and mounting directories** into workspaces (different from runtime-specific configuration). -func (f *fakeStepLogger) Complete() { - f.completeCalls++ -} +**Multi-Level Configuration:** +- **Workspace-level** (`.kortex/workspace.json`) - Project configuration, set via `--workspace-configuration` flag +- **Project-specific** (`~/.kortex-cli/config/projects.json`) - User's custom config for specific projects +- **Global** (empty string `""` key in `projects.json`) - Settings applied to all projects +- **Agent-specific** (`~/.kortex-cli/config/agents.json`) - Per-agent overrides -// Use in tests -func TestCreate_StepLogger(t *testing.T) { - fakeLogger := &fakeStepLogger{} - ctx := steplogger.WithLogger(context.Background(), fakeLogger) +**Configuration Precedence:** Agent > Project > Global > Workspace (highest to lowest) - _, err := runtime.Create(ctx, params) - - // Verify step calls - if len(fakeLogger.startCalls) != 3 { - t.Errorf("Expected 3 Start() calls, got %d", len(fakeLogger.startCalls)) - } - if fakeLogger.completeCalls != 1 { - t.Errorf("Expected 1 Complete() call, got %d", fakeLogger.completeCalls) - } -} -``` - -**Reference:** See `pkg/runtime/podman/steplogger_test.go` and step logger tests in `create_test.go`, `start_test.go`, `stop_test.go`, `remove_test.go`. +**For detailed configuration guidance, use:** `/working-with-config-system` ### Podman Runtime Configuration -The Podman runtime supports configurable image and agent settings through JSON files stored in the runtime's storage directory. This allows customization of the base image, installed packages, sudo permissions, and agent setup. - -**Key Components:** -- **Config Interface** (`pkg/runtime/podman/config/config.go`): Interface for managing Podman runtime configuration -- **ImageConfig** (`pkg/runtime/podman/config/types.go`): Base image configuration (Fedora version, packages, sudo binaries, custom RUN commands) -- **AgentConfig** (`pkg/runtime/podman/config/types.go`): Agent-specific configuration (packages, RUN commands, terminal command) -- **Defaults** (`pkg/runtime/podman/config/defaults.go`): Default configurations for image and Claude agent - -**Configuration Storage:** - -Configuration files are stored in the runtime's storage directory: -```text -/runtimes/podman/config/ -├── image.json # Base image configuration -└── claude.json # Agent-specific configuration (e.g., for Claude Code) -``` +The Podman runtime supports runtime-specific configuration for **building and configuring containers** (base image, packages, sudo permissions, agent setup). **Configuration Files:** +- `/runtimes/podman/config/image.json` - Base image configuration +- `/runtimes/podman/config/claude.json` - Agent-specific configuration -**image.json** - Base image configuration: -```json -{ - "version": "latest", - "packages": ["which", "procps-ng", "wget2", "@development-tools", "jq", "gh", "golang", "golangci-lint", "python3", "python3-pip"], - "sudo": ["/usr/bin/dnf", "/bin/nice", "/bin/kill", "/usr/bin/kill", "/usr/bin/killall"], - "run_commands": [] -} -``` - -Fields: -- `version` (required) - Fedora version tag (e.g., "latest", "40", "41") -- `packages` (optional) - DNF packages to install -- `sudo` (optional) - Absolute paths to binaries the user can run with sudo (creates single `ALLOWED` Cmnd_Alias) -- `run_commands` (optional) - Custom shell commands to execute during image build (before agent setup) - -**claude.json** - Agent-specific configuration: -```json -{ - "packages": [], - "run_commands": [ - "curl -fsSL --proto-redir '-all,https' --tlsv1.3 https://claude.ai/install.sh | bash", - "mkdir /home/claude/.config" - ], - "terminal_command": ["claude"] -} -``` - -Fields: -- `packages` (optional) - Additional packages for the agent (merged with image packages) -- `run_commands` (optional) - Commands to set up the agent (executed after image setup) -- `terminal_command` (required) - Command to launch the agent (must have at least one element) - -**Using the Config Interface:** - -```go -import "github.com/kortex-hub/kortex-cli/pkg/runtime/podman/config" - -// Create config manager (in Initialize method) -configDir := filepath.Join(storageDir, "config") -cfg, err := config.NewConfig(configDir) -if err != nil { - return fmt.Errorf("failed to create config: %w", err) -} - -// Generate default configs if they don't exist -if err := cfg.GenerateDefaults(); err != nil { - return fmt.Errorf("failed to generate defaults: %w", err) -} - -// Load configurations (in Create method) -imageConfig, err := cfg.LoadImage() -if err != nil { - return fmt.Errorf("failed to load image config: %w", err) -} - -agentConfig, err := cfg.LoadAgent("claude") -if err != nil { - return fmt.Errorf("failed to load agent config: %w", err) -} -``` - -**Validation:** - -The config system validates: -- Image version cannot be empty -- Sudo binaries must be absolute paths -- Terminal command must have at least one element -- All fields are optional except `version` (ImageConfig) and `terminal_command` (AgentConfig) - -**Default Generation:** - -- Default configs are auto-generated on first runtime initialization -- Existing config files are never overwritten - customizations are preserved -- Default image config includes common development tools and packages -- Default Claude config installs Claude Code from the official install script - -**Containerfile Generation:** - -The config system is used to generate Containerfiles dynamically: - -```go -import "github.com/kortex-hub/kortex-cli/pkg/runtime/podman" - -// Generate Containerfile content from configs -containerfileContent := generateContainerfile(imageConfig, agentConfig) - -// Generate sudoers file content from sudo binaries -sudoersContent := generateSudoers(imageConfig.Sudo) -``` - -The `generateContainerfile` function creates a Containerfile with: -- Base image: `registry.fedoraproject.org/fedora:` -- Merged packages from image and agent configs -- User/group setup (hardcoded as `claude:claude`) -- Sudoers configuration with single `ALLOWED` Cmnd_Alias -- Custom RUN commands from both configs (image commands first, then agent commands) - -**Hardcoded Values:** - -These values are not configurable: -- Base image registry: `registry.fedoraproject.org/fedora` (only version tag is configurable) -- Container user: `claude` -- Container group: `claude` -- User UID/GID: Matched to host user's UID/GID at build time - -**Design Principles:** -- Follows interface-based design pattern with unexported implementation -- Uses nested JSON structure for clarity -- Validates all configurations on load to catch errors early -- Separate concerns: base image vs agent-specific settings -- Extensible: easy to add new agent configurations (e.g., `goose.json`, `cursor.json`) - -### Config System - -The config system manages workspace configuration stored in the `.kortex` directory. It provides an interface for reading and validating workspace settings including environment variables and mount points. - -**Key Components:** -- **Config Interface** (`pkg/config/config.go`): Interface for managing configuration directories -- **WorkspaceConfiguration Model**: Imported from `github.com/kortex-hub/kortex-cli-api/workspace-configuration/go` -- **Configuration File**: `workspace.json` within the `.kortex` directory - -**Configuration Structure:** - -The `workspace.json` file follows the nested JSON structure pattern: - -```json -{ - "environment": [ - { - "name": "DEBUG", - "value": "true" - }, - { - "name": "API_KEY", - "secret": "github-token" - } - ], - "mounts": { - "dependencies": ["../main"], - "configs": [".ssh", ".gitconfig"] - } -} -``` - -**Model Fields:** -- `environment` - Environment variables with either hardcoded `value` or `secret` reference (optional) - - `name` - Variable name (must be valid Unix environment variable name) - - `value` - Hardcoded value (mutually exclusive with `secret`, empty strings allowed) - - `secret` - Secret reference (mutually exclusive with `value`, cannot be empty) -- `mounts.dependencies` - Additional source directories to mount (optional) - - Paths must be relative (not absolute) - - Paths cannot be empty - - Relative to workspace sources directory -- `mounts.configs` - Configuration directories to mount (optional) - - Paths must be relative (not absolute) - - Paths cannot be empty - - Relative to `$HOME` - -**Using the Config Interface:** - -```go -import ( - "github.com/kortex-hub/kortex-cli/pkg/config" - workspace "github.com/kortex-hub/kortex-cli-api/workspace-configuration/go" -) - -// Create a config manager for a workspace -cfg, err := config.NewConfig("/path/to/workspace/.kortex") -if err != nil { - return err -} - -// Load and validate the workspace configuration -workspaceCfg, err := cfg.Load() -if err != nil { - if errors.Is(err, config.ErrConfigNotFound) { - // workspace.json doesn't exist, use defaults - } else if errors.Is(err, config.ErrInvalidConfig) { - // Configuration validation failed - } else { - return err - } -} - -// Access configuration values (note: fields are pointers) -if workspaceCfg.Environment != nil { - for _, env := range *workspaceCfg.Environment { - // Use env.Name, env.Value, env.Secret - } -} - -if workspaceCfg.Mounts != nil { - if workspaceCfg.Mounts.Dependencies != nil { - // Use dependency paths - } - if workspaceCfg.Mounts.Configs != nil { - // Use config paths - } -} -``` - -**Configuration Validation:** - -The `Load()` method automatically validates the configuration and returns `ErrInvalidConfig` if any of these rules are violated: - -**Environment Variables:** -- Name cannot be empty -- Name must be a valid Unix environment variable name (starts with letter or underscore, followed by letters, digits, or underscores) -- Exactly one of `value` or `secret` must be defined -- Secret references cannot be empty strings -- Empty values are allowed (valid use case: set env var to empty string) - -**Mount Paths:** -- Dependency paths cannot be empty -- Dependency paths must be relative (not absolute) -- Config paths cannot be empty -- Config paths must be relative (not absolute) - -**Error Handling:** -- `config.ErrInvalidPath` - Configuration path is empty or invalid -- `config.ErrConfigNotFound` - The `workspace.json` file is not found -- `config.ErrInvalidConfig` - Configuration validation failed (includes detailed error message) - -**Design Principles:** -- Configuration directory is NOT automatically created -- Missing configuration directory is treated as empty/default configuration -- All configurations are validated on load to catch errors early -- Follows the module design pattern with interface-based API -- Uses nested JSON structure for clarity and extensibility -- Model types are imported from external API package for consistency - -### Multi-Level Configuration System - -The multi-level configuration system allows users to customize workspace settings at different levels: workspace, project, and agent. Configurations are merged with proper precedence, enabling: -- **Workspace-level config** (`.kortex/workspace.json`) - Shared project configuration committed to repository -- **Project-specific config** (`~/.kortex-cli/config/projects.json`) - User's custom config for specific projects -- **Global config** (empty string `""` key in `projects.json`) - Settings applied to all projects (e.g., `.gitconfig`, SSH keys) -- **Agent-specific config** (`~/.kortex-cli/config/agents.json`) - Per-agent overrides (e.g., Claude, Goose) - -**Key Components:** -- **ConfigMerger** (`pkg/config/merger.go`): Merges multiple `WorkspaceConfiguration` objects -- **AgentConfigLoader** (`pkg/config/agents.go`): Loads agent-specific configuration -- **ProjectConfigLoader** (`pkg/config/projects.go`): Loads project and global configuration -- **Manager Integration** (`pkg/instances/manager.go`): Handles config loading and merging during instance creation - -**Configuration Precedence** (highest to lowest): -1. Agent-specific configuration (from `agents.json`) -2. Project-specific configuration (from `projects.json` using project ID) -3. Global project configuration (from `projects.json` using empty string `""` key) -4. Workspace-level configuration (from `.kortex/workspace.json`) - -**File Locations:** - -All user-specific configuration files are stored under the storage directory (default: `~/.kortex-cli`, configurable via `--storage` flag or `KORTEX_CLI_STORAGE` environment variable): - -- Agent configs: `/config/agents.json` -- Project configs: `/config/projects.json` -- Workspace configs: `.kortex/workspace.json` (in workspace directory) - -**Agent Configuration Structure:** - -`/config/agents.json`: -```json -{ - "claude": { - "environment": [ - { - "name": "DEBUG", - "value": "true" - } - ], - "mounts": { - "configs": [".claude-config"] - } - }, - "goose": { - "environment": [ - { - "name": "GOOSE_MODE", - "value": "verbose" - } - ] - } -} -``` - -**Project Configuration Structure:** - -`/config/projects.json`: -```json -{ - "": { - "mounts": { - "configs": [".gitconfig", ".ssh"] - } - }, - "github.com/kortex-hub/kortex-cli": { - "environment": [ - { - "name": "PROJECT_VAR", - "value": "project-value" - } - ], - "mounts": { - "dependencies": ["../kortex-common"] - } - }, - "/home/user/my/project": { - "environment": [ - { - "name": "LOCAL_DEV", - "value": "true" - } - ] - } -} -``` - -**Special Keys:** -- Empty string `""` in `projects.json` represents global/default configuration applied to all projects -- Useful for common settings like GitHub credentials, SSH keys, or global environment variables -- Project-specific configs override global config - -**Using the Multi-Level Config System:** - -The Manager handles all configuration loading and merging automatically: - -```go -// In command code (e.g., init command) -addedInstance, err := manager.Add(ctx, instances.AddOptions{ - Instance: instance, - RuntimeType: "fake", - WorkspaceConfig: workspaceConfig, // From .kortex/workspace.json - Project: "custom-project", // Optional override - Agent: "claude", // Optional agent name -}) -``` - -The Manager's `Add()` method: -1. Detects project ID (or uses custom override) -2. Loads project config (global `""` + project-specific merged) -3. Loads agent config (if agent name provided) -4. Merges configs: workspace → global → project → agent -5. Passes merged config to runtime - -**Merging Behavior:** - -- **Environment variables**: Later configs override earlier ones by name - - If the same variable appears in multiple configs, the one from the higher-precedence config wins -- **Mount dependencies**: Deduplicated list (preserves order, removes duplicates) -- **Mount configs**: Deduplicated list (preserves order, removes duplicates) - -**Example Merge Flow:** - -Given: -- Workspace config: `DEBUG=workspace`, `WORKSPACE_VAR=value1` -- Global config: `GLOBAL_VAR=global` -- Project config: `DEBUG=project`, `PROJECT_VAR=value2` -- Agent config: `DEBUG=agent`, `AGENT_VAR=value3` - -Result: `DEBUG=agent`, `WORKSPACE_VAR=value1`, `GLOBAL_VAR=global`, `PROJECT_VAR=value2`, `AGENT_VAR=value3` - -**Loading Configuration Programmatically:** - -```go -import "github.com/kortex-hub/kortex-cli/pkg/config" - -// Load project config (includes global + project-specific merged) -projectLoader, err := config.NewProjectConfigLoader(storageDir) -projectConfig, err := projectLoader.Load("github.com/user/repo") - -// Load agent config -agentLoader, err := config.NewAgentConfigLoader(storageDir) -agentConfig, err := agentLoader.Load("claude") - -// Merge configurations -merger := config.NewMerger() -merged := merger.Merge(workspaceConfig, projectConfig) -merged = merger.Merge(merged, agentConfig) -``` - -**Testing Multi-Level Configs:** - -```go -// Create test config files -configDir := filepath.Join(storageDir, "config") -os.MkdirAll(configDir, 0755) - -agentsJSON := `{"claude": {"environment": [{"name": "VAR", "value": "val"}]}}` -os.WriteFile(filepath.Join(configDir, "agents.json"), []byte(agentsJSON), 0644) - -// Run init with agent -rootCmd.SetArgs([]string{"init", sourcesDir, "--runtime", "fake", "--agent", "claude"}) -rootCmd.Execute() -``` - -**Design Principles:** -- Configuration merging is handled by Manager, not commands -- Missing config files return empty configs (not errors) -- Invalid JSON or validation errors are reported -- All loaders follow the module design pattern -- Cross-platform compatible (uses `filepath.Join()`, `t.TempDir()`) -- Storage directory is configurable via `--storage` flag or `KORTEX_CLI_STORAGE` env var - -**Error Handling:** -- `config.ErrInvalidAgentConfig` - Agent configuration is invalid -- `config.ErrInvalidProjectConfig` - Project configuration is invalid -- Both loaders validate configurations using the same validation logic as workspace config +**For Podman runtime configuration details, use:** `/working-with-podman-runtime-config` ### Skills System Skills are reusable capabilities that can be discovered and executed by AI agents: @@ -916,781 +219,77 @@ Skills are reusable capabilities that can be discovered and executed by AI agent 3. Symlink in `.claude/skills/`: `ln -s ../../skills/ .claude/skills/` ### Adding a New Command -1. Create `pkg/cmd/.go` with a `NewCmd()` function that returns `*cobra.Command` -2. In the `NewCmd()` function: - - Create and configure the `cobra.Command` - - **IMPORTANT**: Always define the `Args` field to specify argument validation - - **IMPORTANT**: Always add an `Example` field with usage examples - - Set up any flags or subcommands - - Return the configured command -3. Register the command in `pkg/cmd/root.go` by adding `rootCmd.AddCommand(NewCmd())` in the `NewRootCmd()` function -4. Create corresponding test file `pkg/cmd/_test.go` -5. In tests, create command instances using `NewRootCmd()` or `NewCmd()` as needed -6. **IMPORTANT**: Add a `TestCmd_Examples` function to validate the examples - -**Command Argument Validation:** - -All commands **MUST** declare the `Args` field to specify argument validation behavior. Common options: -- `cobra.NoArgs` - Command accepts no arguments (most common for parent commands and no-arg commands) -- `cobra.ExactArgs(n)` - Command requires exactly n arguments -- `cobra.MinimumNArgs(n)` - Command requires at least n arguments -- `cobra.MaximumNArgs(n)` - Command accepts up to n arguments -- `cobra.RangeArgs(min, max)` - Command accepts between min and max arguments -Example: -```go -// pkg/cmd/example.go -func NewExampleCmd() *cobra.Command { - return &cobra.Command{ - Use: "example", - Short: "An example command", - Example: `# Run the example command -kortex-cli example`, - Args: cobra.NoArgs, // Always declare Args field - Run: func(cmd *cobra.Command, args []string) { - // Command logic here - }, - } -} - -// In pkg/cmd/root.go, add to NewRootCmd(): -rootCmd.AddCommand(NewExampleCmd()) -``` - -**Command Examples:** - -All commands **MUST** include an `Example` field with usage examples. Examples improve help documentation and are automatically validated to ensure they stay accurate as the code evolves. - -**Example Format:** -```go -func NewExampleCmd() *cobra.Command { - return &cobra.Command{ - Use: "example [arg]", - Short: "An example command", - Example: `# Basic usage with comment -kortex-cli example - -# With argument -kortex-cli example value - -# With flag -kortex-cli example --flag value`, - Args: cobra.MaximumNArgs(1), - Run: func(cmd *cobra.Command, args []string) { - // Command logic here - }, - } -} -``` - -**Example Guidelines:** -- Use comments (lines starting with `#`) to describe what each example does -- Show the most common use cases (typically 3-5 examples) -- Include examples for all important flags -- Examples must use the actual binary name (`kortex-cli`) -- All commands and flags in examples must exist -- Keep examples concise and realistic -- Use `--` separator when passing flags to nested commands: `kortex-cli terminal ID -- bash -c 'echo hello'` - - The `--` tells the example validator to stop parsing flags and treat everything after as arguments - - This matches Cobra's behavior for passing arguments to nested commands - -**Validating Examples:** - -Every command with an `Example` field **MUST** have a corresponding validation test: - -```go -func TestCmd_Examples(t *testing.T) { - t.Parallel() - - // Get the command - cmd := NewCmd() - - // Verify Example field is not empty - if cmd.Example == "" { - t.Fatal("Example field should not be empty") - } - - // Parse the examples - commands, err := testutil.ParseExampleCommands(cmd.Example) - if err != nil { - t.Fatalf("Failed to parse examples: %v", err) - } - - // Verify we have the expected number of examples - expectedCount := 3 // Adjust based on your examples - if len(commands) != expectedCount { - t.Errorf("Expected %d example commands, got %d", expectedCount, len(commands)) - } - - // Validate all examples against the root command - rootCmd := NewRootCmd() - err = testutil.ValidateCommandExamples(rootCmd, cmd.Example) - if err != nil { - t.Errorf("Example validation failed: %v", err) - } -} -``` - -**What the validator checks:** -- Binary name is `kortex-cli` -- All commands exist in the command tree -- All flags (both long and short) are valid for the command -- No invalid subcommands are used - -**Reference:** See `pkg/cmd/init.go` and `pkg/cmd/init_test.go` for complete examples. - -**Alias Commands:** - -Alias commands are shortcuts that delegate to existing commands (e.g., `list` as an alias for `workspace list`). For alias commands: - -1. **Inherit the Example field** from the original command -2. **Adapt the examples** to show the alias syntax instead of the full command -3. **Do NOT create validation tests** for aliases (they use the same validation as the original command) - -Use the `AdaptExampleForAlias()` helper function (from `pkg/cmd/helpers.go`) to automatically replace the command name in examples while preserving comments: - -```go -func NewListCmd() *cobra.Command { - // Create the workspace list command - workspaceListCmd := NewWorkspaceListCmd() - - // Create an alias command that delegates to workspace list - cmd := &cobra.Command{ - Use: "list", - Short: workspaceListCmd.Short, - Long: workspaceListCmd.Long, - Example: AdaptExampleForAlias(workspaceListCmd.Example, "workspace list", "list"), - Args: workspaceListCmd.Args, - PreRunE: workspaceListCmd.PreRunE, - RunE: workspaceListCmd.RunE, - } - - // Copy flags from workspace list command - cmd.Flags().AddFlagSet(workspaceListCmd.Flags()) - - return cmd -} -``` - -The `AdaptExampleForAlias()` function: -- Replaces the original command with the alias **only in command lines** (lines starting with `kortex-cli`) -- **Preserves comments unchanged** (lines starting with `#`) -- Maintains formatting and indentation - -**Example transformation:** -```bash -# Original (from workspace list): -# List all workspaces -kortex-cli workspace list - -# List in JSON format -kortex-cli workspace list --output json - -# After AdaptExampleForAlias(..., "workspace list", "list"): -# List all workspaces -kortex-cli list - -# List in JSON format -kortex-cli list --output json -``` - -**Reference:** See `pkg/cmd/list.go` and `pkg/cmd/remove.go` for complete alias examples. - -### Command Implementation Pattern - -Commands should follow a consistent structure for maintainability and testability: - -1. **Command Struct** - Contains all command state: - - Input values from flags/args - - Computed/validated values - - Dependencies (e.g., manager instances) - -2. **preRun Method** - Validates parameters and prepares: - - Parse and validate arguments/flags - - Access global flags (e.g., `--storage`) - - Create dependencies (managers, etc.) - - Convert paths to absolute using `filepath.Abs()` - - Store validated values in struct fields - -3. **run Method** - Executes the command logic: - - Use validated values from struct fields - - Perform the actual operation - - Output results to user +**Available Skills:** +- `/add-command-simple` - For commands without JSON output support +- `/add-command-with-json` - For commands with JSON output support +- `/add-alias-command` - For alias commands that delegate to existing commands +- `/add-parent-command` - For parent commands with subcommands -**Reference:** See `pkg/cmd/init.go` for a complete implementation of this pattern. +**All commands MUST:** +- Define the `Args` field for argument validation (`cobra.NoArgs`, `cobra.ExactArgs(n)`, etc.) +- Include an `Example` field with usage examples +- Have a corresponding `TestCmd_Examples` test function to validate examples -### Flag Binding Best Practices +**For advanced command patterns, use:** `/implementing-command-patterns` -**IMPORTANT**: Always bind command flags directly to struct fields using the `*Var` variants (e.g., `StringVarP`, `BoolVarP`, `IntVarP`) instead of using the non-binding variants and then calling `GetString()`, `GetBool()`, etc. in `preRun`. - -**Benefits:** -- **Cleaner code**: No need to call `cmd.Flags().GetString()` and handle errors -- **Simpler testing**: Tests can set struct fields directly instead of creating and setting flags -- **Earlier binding**: Values are available immediately when preRun is called -- **Less error-prone**: No risk of typos in flag names when retrieving values - -**Pattern:** - -```go -// Command struct with fields for all flags -type myCmd struct { - verbose bool - output string - count int - manager instances.Manager -} - -// Bind flags to struct fields in the command factory -func NewMyCmd() *cobra.Command { - c := &myCmd{} - - cmd := &cobra.Command{ - Use: "my-command", - Short: "My command description", - Args: cobra.NoArgs, - PreRunE: c.preRun, - RunE: c.run, - } - - // GOOD: Bind flags directly to struct fields - cmd.Flags().BoolVarP(&c.verbose, "verbose", "v", false, "Show detailed output") - cmd.Flags().StringVarP(&c.output, "output", "o", "", "Output format (supported: json)") - cmd.Flags().IntVarP(&c.count, "count", "c", 10, "Number of items to process") - - return cmd -} - -// Use the bound values directly in preRun -func (m *myCmd) preRun(cmd *cobra.Command, args []string) error { - // Values are already available from struct fields - if m.output != "" && m.output != "json" { - return fmt.Errorf("unsupported output format: %s", m.output) - } - - // No need to call cmd.Flags().GetString("output") - return nil -} -``` - -**Avoid:** - -```go -// BAD: Don't define flags without binding -cmd.Flags().StringP("output", "o", "", "Output format") - -// BAD: Don't retrieve flag values in preRun -func (m *myCmd) preRun(cmd *cobra.Command, args []string) error { - output, err := cmd.Flags().GetString("output") // Avoid this pattern - if err != nil { - return err - } - m.output = output - // ... -} -``` - -**Testing with Bound Flags:** - -```go -func TestMyCmd_PreRun(t *testing.T) { - t.Run("validates output format", func(t *testing.T) { - // Set struct fields directly - no need to set up flags - c := &myCmd{ - output: "xml", // Invalid format - } - cmd := &cobra.Command{} - - err := c.preRun(cmd, []string{}) - if err == nil { - t.Fatal("Expected error for invalid output format") - } - }) -} -``` - -**Reference:** See `pkg/cmd/init.go`, `pkg/cmd/workspace_remove.go`, and `pkg/cmd/workspace_list.go` for examples of proper flag binding. - -### JSON Output Support Pattern - -When adding JSON output support to commands, follow this pattern to ensure consistent error handling and output formatting: - -**Rules:** - -1. **Check output flag FIRST in preRun** - Validate the output format before any other validation -2. **Set SilenceErrors and SilenceUsage early** - Prevent Cobra's default error output when in JSON mode -3. **Use outputErrorIfJSON for ALL errors** - In preRun, run, and any helper methods (like outputJSON) - -**Pattern:** - -```go -type myCmd struct { - output string // Bound to --output flag - manager instances.Manager -} - -func (m *myCmd) preRun(cmd *cobra.Command, args []string) error { - // 1. FIRST: Validate output format - if m.output != "" && m.output != "json" { - return fmt.Errorf("unsupported output format: %s (supported: json)", m.output) - } - - // 2. EARLY: Silence Cobra's error output in JSON mode - if m.output == "json" { - cmd.SilenceErrors = true - cmd.SilenceUsage = true - } - - // 3. ALL subsequent errors use outputErrorIfJSON - storageDir, err := cmd.Flags().GetString("storage") - if err != nil { - return outputErrorIfJSON(cmd, m.output, fmt.Errorf("failed to read --storage flag: %w", err)) - } - - manager, err := instances.NewManager(storageDir) - if err != nil { - return outputErrorIfJSON(cmd, m.output, fmt.Errorf("failed to create manager: %w", err)) - } - m.manager = manager - - return nil -} - -func (m *myCmd) run(cmd *cobra.Command, args []string) error { - // ALL errors in run use outputErrorIfJSON - data, err := m.manager.GetData() - if err != nil { - return outputErrorIfJSON(cmd, m.output, fmt.Errorf("failed to get data: %w", err)) - } - - if m.output == "json" { - return m.outputJSON(cmd, data) - } - - // Text output - cmd.Println(data) - return nil -} - -func (m *myCmd) outputJSON(cmd *cobra.Command, data interface{}) error { - jsonData, err := json.MarshalIndent(data, "", " ") - if err != nil { - // Even unlikely errors in helper methods use outputErrorIfJSON - return outputErrorIfJSON(cmd, m.output, fmt.Errorf("failed to marshal to JSON: %w", err)) - } - - fmt.Fprintln(cmd.OutOrStdout(), string(jsonData)) - return nil -} -``` - -**Why this pattern:** -- **Consistent error format**: All errors are JSON when `--output=json` is set -- **No Cobra pollution**: SilenceErrors prevents "Error: ..." prefix in JSON output -- **Early detection**: Output flag is validated before expensive operations -- **Helper methods work**: outputErrorIfJSON works in any method that has access to cmd and output flag - -**Helper function:** - -The `outputErrorIfJSON` helper in `pkg/cmd/conversion.go` handles the formatting: - -```go -func outputErrorIfJSON(cmd interface{ OutOrStdout() io.Writer }, output string, err error) error { - if output == "json" { - jsonErr, _ := formatErrorJSON(err) - fmt.Fprintln(cmd.OutOrStdout(), jsonErr) // Errors go to stdout in JSON mode - } - return err // Still return the error for proper exit codes -} -``` - -**Reference:** See `pkg/cmd/init.go`, `pkg/cmd/workspace_remove.go`, and `pkg/cmd/workspace_list.go` for complete implementations. - -### Interactive Commands (No JSON Output) - -Some commands are inherently interactive and do not support JSON output. These commands connect stdin/stdout/stderr directly to a user's terminal. - -**Example: Terminal Command** - -The `terminal` command provides an interactive session with a running workspace instance: - -```go -type workspaceTerminalCmd struct { - manager instances.Manager - id string - command []string -} - -func (w *workspaceTerminalCmd) preRun(cmd *cobra.Command, args []string) error { - w.id = args[0] - - // Extract command from args[1:] if provided - if len(args) > 1 { - w.command = args[1:] - } else { - // Default command (configurable from runtime) - w.command = []string{"claude"} - } - - // Standard setup: storage flag, manager, runtime registration - storageDir, _ := cmd.Flags().GetString("storage") - absStorageDir, _ := filepath.Abs(storageDir) - - manager, err := instances.NewManager(absStorageDir) - if err != nil { - return fmt.Errorf("failed to create manager: %w", err) - } - - if err := runtimesetup.RegisterAll(manager); err != nil { - return fmt.Errorf("failed to register runtimes: %w", err) - } - - w.manager = manager - return nil -} - -func (w *workspaceTerminalCmd) run(cmd *cobra.Command, args []string) error { - // Connect to terminal - this is a blocking interactive call - err := w.manager.Terminal(cmd.Context(), w.id, w.command) - if err != nil { - if errors.Is(err, instances.ErrInstanceNotFound) { - return fmt.Errorf("workspace not found: %s\nUse 'workspace list' to see available workspaces", w.id) - } - return err - } - return nil -} -``` - -**Key differences from JSON-supporting commands:** -- **No `--output` flag** - Interactive commands don't need this -- **No JSON output helpers** - All output goes directly to terminal -- **Simpler error handling** - Just return errors normally (no `outputErrorIfJSON`) -- **Blocking execution** - The command runs until the user exits the interactive session -- **Command arguments** - Accept commands to run inside the instance: `terminal ID bash` or `terminal ID -- bash -c 'echo hello'` - -**Example command registration:** - -```go -func NewWorkspaceTerminalCmd() *cobra.Command { - c := &workspaceTerminalCmd{} - - cmd := &cobra.Command{ - Use: "terminal ID [COMMAND...]", - Short: "Connect to a running workspace with an interactive terminal", - Args: cobra.MinimumNArgs(1), - ValidArgsFunction: completeRunningWorkspaceID, // Only show running workspaces - PreRunE: c.preRun, - RunE: c.run, - } - - // No flags needed - just uses global --storage flag - return cmd -} -``` - -**Reference:** See `pkg/cmd/workspace_terminal.go` for the complete implementation. - -### Testing Pattern for Commands - -Commands should have two types of tests following the pattern in `pkg/cmd/init_test.go`: - -1. **Unit Tests** - Test the `preRun` method directly by calling it on the command struct: - - **IMPORTANT**: Create an instance of the command struct (e.g., `c := &initCmd{}`) - - **IMPORTANT**: Create a mock `*cobra.Command` and set up required flags - - **IMPORTANT**: Call `c.preRun(cmd, args)` directly - DO NOT call `rootCmd.Execute()` - - Use `t.Run()` for subtests within a parent test function - - Test with different argument/flag combinations - - Verify struct fields are set correctly after `preRun()` executes - - Use `t.TempDir()` for temporary directories (automatic cleanup) - - **Example:** - ```go - func TestMyCmd_PreRun(t *testing.T) { - t.Run("sets fields correctly", func(t *testing.T) { - t.Parallel() - - storageDir := t.TempDir() - - c := &myCmd{} // Create command struct instance - cmd := &cobra.Command{} // Create mock cobra command - cmd.Flags().String("storage", storageDir, "test storage flag") - - args := []string{"arg1"} - - err := c.preRun(cmd, args) // Call preRun directly - if err != nil { - t.Fatalf("preRun() failed: %v", err) - } - - // Assert on struct fields - if c.manager == nil { - t.Error("Expected manager to be created") - } - }) - } - ``` - -2. **E2E Tests** - Test the full command execution including Cobra wiring: - - Execute via `rootCmd.Execute()` to test the complete flow - - Use real temp directories with `t.TempDir()` - - Verify output messages - - Verify persistence (check storage/database) - - Verify all field values from `manager.List()` or similar - - Test multiple scenarios (default args, custom args, edge cases) - - Test Cobra's argument validation (e.g., required args, arg counts) - - **Example:** - ```go - func TestMyCmd_E2E(t *testing.T) { - t.Run("executes successfully", func(t *testing.T) { - t.Parallel() - - storageDir := t.TempDir() - - rootCmd := NewRootCmd() // Use full command construction - rootCmd.SetArgs([]string{"mycommand", "arg1", "--storage", storageDir}) - - err := rootCmd.Execute() // Execute the full command - if err != nil { - t.Fatalf("Execute() failed: %v", err) - } - - // Verify results in storage - manager, _ := instances.NewManager(storageDir) - instancesList, _ := manager.List() - // ... assert on results - }) - } - ``` - -**Reference:** See `pkg/cmd/init_test.go`, `pkg/cmd/workspace_list_test.go`, and `pkg/cmd/workspace_remove_test.go` for complete examples of both `preRun` unit tests (in `Test*Cmd_PreRun`) and E2E tests (in `Test*Cmd_E2E`). +**For testing commands, use:** `/testing-commands` ### Working with the Instances Manager -When commands need to interact with workspaces: +The instances manager provides the API for managing workspace instances: ```go -// In preRun - create manager from storage flag -storageDir, _ := cmd.Flags().GetString("storage") +// Create manager manager, err := instances.NewManager(storageDir) -if err != nil { - return fmt.Errorf("failed to create manager: %w", err) -} - -// In run - use manager to add instances -instance, err := instances.NewInstance(sourceDir, configDir) -if err != nil { - return fmt.Errorf("failed to create instance: %w", err) -} - -addedInstance, err := manager.Add(instance) -if err != nil { - return fmt.Errorf("failed to add instance: %w", err) -} - -// List instances -instancesList, err := manager.List() -if err != nil { - return fmt.Errorf("failed to list instances: %w", err) -} - -// Get specific instance -instance, err := manager.Get(id) -if err != nil { - return fmt.Errorf("instance not found: %w", err) -} - -// Delete instance -err := manager.Delete(id) -if err != nil { - return fmt.Errorf("failed to delete instance: %w", err) -} - -// Connect to a running instance with an interactive terminal -// Note: Instance must be running and runtime must implement Terminal interface -err := manager.Terminal(ctx, id, []string{"bash"}) -if err != nil { - return fmt.Errorf("failed to connect to terminal: %w", err) -} -``` - -**Terminal Method:** - -The `Terminal()` method enables interactive terminal sessions with running workspace instances: - -```go -func (m *manager) Terminal(ctx context.Context, id string, command []string) error -``` - -**Behavior:** -- Verifies the instance exists and is in a running state -- Checks if the runtime implements the `runtime.Terminal` interface -- Delegates to the runtime's Terminal implementation -- Returns an error if the instance is not running or runtime doesn't support terminals - -**Example usage in a command:** - -```go -func (w *workspaceTerminalCmd) run(cmd *cobra.Command, args []string) error { - // Start terminal session with the command extracted in preRun - err := w.manager.Terminal(cmd.Context(), w.id, w.command) - if err != nil { - if errors.Is(err, instances.ErrInstanceNotFound) { - return fmt.Errorf("workspace not found: %s\nUse 'workspace list' to see available workspaces", w.id) - } - return err - } - return nil -} -``` - -**Key points:** -- Uses a read lock (doesn't modify instance state) -- Command is a slice of strings: `[]string{"bash"}` or `[]string{"claude-code", "--debug"}` -- Returns `ErrInstanceNotFound` if instance doesn't exist -- Returns an error if instance state is not "running" -- Returns an error if the runtime doesn't implement `runtime.Terminal` interface -### Project Detection and Grouping +// Register runtimes +runtimesetup.RegisterAll(manager) -Each workspace has a `project` field that enables grouping workspaces belonging to the same project across branches, forks, or subdirectories. +// Add instance +manager.Add(ctx, instances.AddOptions{...}) -**Project Identifier Detection:** +// List, Get, Delete instances +manager.List() +manager.Get(id) +manager.Delete(id) -The manager automatically detects the project identifier when adding instances: +// Start, Stop instances +manager.Start(ctx, id) +manager.Stop(ctx, id) -1. **Git repository with remote**: Uses repository remote URL (without `.git`) plus relative path - - Checks `upstream` remote first (useful for forks) - - Falls back to `origin` remote if `upstream` doesn't exist - - Example: `https://github.com/kortex-hub/kortex-cli/` (at root) or `https://github.com/kortex-hub/kortex-cli/pkg/git` (in subdirectory) - -2. **Git repository without remote**: Uses repository root directory plus relative path - - Example: `/home/user/local-repo` (at root) or `/home/user/local-repo/pkg/utils` (in subdirectory) - -3. **Non-git directory**: Uses the source directory path - - Example: `/tmp/workspace` - -**Custom Project Override:** - -Users can override auto-detection with the `--project` flag: - -```go -// Add instance with custom project -addedInstance, err := manager.Add(ctx, instances.AddOptions{ - Instance: instance, - RuntimeType: "fake", - WorkspaceConfig: workspaceConfig, - Project: "custom-project-id", // Optional: overrides auto-detection -}) -``` - -**Implementation Details:** - -- **Package**: `pkg/git` provides git repository detection with testable abstractions -- **Detector Interface**: `git.Detector` with `DetectRepository(ctx, dir)` method -- **Executor Pattern**: `git.Executor` abstracts git command execution for testing -- **Manager Integration**: `manager.detectProject()` is called during `Add()` if no custom project is provided - -**Testing:** - -```go -// Use fake git detector in tests -gitDetector := newFakeGitDetectorWithRepo( - "/repo/root", - "https://github.com/user/repo", - "pkg/subdir", // relative path -) - -manager, _ := newManagerWithFactory( - storageDir, - fakeInstanceFactory, - newFakeGenerator(), - newTestRegistry(tmpDir), - gitDetector, -) +// Interactive terminal +manager.Terminal(ctx, id, []string{"bash"}) ``` -See `pkg/instances/manager_project_test.go` for comprehensive test examples. +**For detailed manager API and project detection, use:** `/working-with-instances-manager` ### Cross-Platform Path Handling ⚠️ **CRITICAL**: All path operations and tests MUST be cross-platform compatible (Linux, macOS, Windows). -**Rules:** -- Always use `filepath.Join()` for path construction (never hardcode "/" or "\\") +**Core Rules:** +- **Host paths** (files on disk): Use `filepath.Join()` - works on Windows, macOS, Linux +- **Container paths** (inside Podman): Use `path.Join()` - containers are always Unix/Linux - Convert relative paths to absolute with `filepath.Abs()` - Never hardcode paths with `~` - use `os.UserHomeDir()` instead -- In tests, use `filepath.Join()` for all path assertions -- **Use `t.TempDir()` for ALL temporary directories in tests - never hardcode paths** - -#### Common Test Failures on Windows - -Tests often fail on Windows due to hardcoded Unix-style paths. These paths get normalized differently by `filepath.Abs()` on Windows vs Unix systems. - -**❌ NEVER do this in tests:** -```go -// BAD - Will fail on Windows because filepath.Abs() normalizes differently -instance, err := instances.NewInstance(instances.NewInstanceParams{ - SourceDir: "/path/to/source", // ❌ Hardcoded Unix path - ConfigDir: "/path/to/config", // ❌ Hardcoded Unix path -}) - -// BAD - Will fail on Windows -invalidPath := "/this/path/does/not/exist" // ❌ Unix-style absolute path - -// BAD - Platform-specific separator -path := dir + "/subdir" // ❌ Hardcoded forward slash -``` - -**✅ ALWAYS do this in tests:** -```go -// GOOD - Cross-platform, works everywhere -sourceDir := t.TempDir() -configDir := t.TempDir() -instance, err := instances.NewInstance(instances.NewInstanceParams{ - SourceDir: sourceDir, // ✅ Real temp directory - ConfigDir: configDir, // ✅ Real temp directory -}) - -// GOOD - Create invalid path cross-platform way -tempDir := t.TempDir() -notADir := filepath.Join(tempDir, "file") -os.WriteFile(notADir, []byte("test"), 0644) -invalidPath := filepath.Join(notADir, "subdir") // ✅ Will fail MkdirAll on all platforms - -// GOOD - Use filepath.Join() -path := filepath.Join(dir, "subdir") // ✅ Cross-platform -``` - -#### Examples for Production Code +- **Use `t.TempDir()` for ALL temporary directories in tests** +**Example:** ```go -// GOOD: Cross-platform path construction -configDir := filepath.Join(sourceDir, ".kortex") -absPath, err := filepath.Abs(relativePath) - -// BAD: Hardcoded separator -configDir := sourceDir + "/.kortex" // Don't do this! - -// GOOD: User home directory -homeDir, err := os.UserHomeDir() -defaultPath := filepath.Join(homeDir, ".kortex-cli") - -// BAD: Hardcoded tilde -defaultPath := "~/.kortex-cli" // Don't do this! +import ( + "path" // For container paths + "path/filepath" // For host paths +) -// GOOD: Test assertions -expectedPath := filepath.Join(".", "relative", "path") -if result != expectedPath { - t.Errorf("Expected %s, got %s", expectedPath, result) -} +// Host path (cross-platform) +configDir := filepath.Join(storageDir, ".kortex") -// GOOD: Temporary directories in tests -tempDir := t.TempDir() // Automatic cleanup -sourcesDir := t.TempDir() +// Container path (always Unix) +workspacePath := path.Join("/workspace", "sources") ``` -**Why this matters:** Tests that pass on Linux/macOS may fail on Windows CI if they use hardcoded Unix paths. Always use `t.TempDir()` and `filepath.Join()` to ensure tests work on all platforms. +**For detailed cross-platform patterns, use:** `/cross-platform-development` ## Documentation Standards @@ -1704,25 +303,6 @@ All markdown files (*.md) in this repository must follow these standards: - For output examples or plain text content, use `text` as the language tag - This ensures markdown linters (markdownlint MD040) pass and improves syntax highlighting -**Examples:** - -````markdown - -```bash -make build -``` - -```text -Error: workspace not found: invalid-id -Use 'workspace list' to see available workspaces -``` - - -``` -make build -``` -```` - **Common Language Tags:** - `bash` - Shell commands and scripts - `go` - Go source code @@ -1748,59 +328,13 @@ Tests follow Go conventions with `*_test.go` files alongside source files. Tests **All tests MUST call `t.Parallel()` as the first line of the test function.** -This ensures faster test execution and better resource utilization. Every test function should start with: +**Exception:** Tests using `t.Setenv()` cannot use `t.Parallel()` on the parent test function. -```go -func TestExample(t *testing.T) { - t.Parallel() +**For general testing best practices, use:** `/testing-best-practices` - // Test code here... -} -``` - -**Exception: Tests using `t.Setenv()`** - -Tests that use `t.Setenv()` to set environment variables **cannot use `t.Parallel()`** on the parent test function. The Go testing framework enforces this restriction because environment variable changes affect the entire process. - -```go -// CORRECT: No t.Parallel() when using t.Setenv() -func TestWithEnvVariable(t *testing.T) { - t.Run("subtest with env var", func(t *testing.T) { - t.Setenv("MY_VAR", "value") - // Test code here... - }) -} - -// INCORRECT: Will panic at runtime -func TestWithEnvVariable(t *testing.T) { - t.Parallel() // ❌ WRONG - cannot use with t.Setenv() - - t.Run("subtest with env var", func(t *testing.T) { - t.Setenv("MY_VAR", "value") - // Test code here... - }) -} -``` - -**Reference:** See `pkg/cmd/root_test.go:TestRootCmd_StorageEnvVariable()` for an example of testing with environment variables. - -### Testing with Fake Objects - -When testing code that uses interfaces (following the Module Design Pattern), **use fake implementations instead of real implementations or mocks**. - -**Pattern:** -1. Create unexported fake structs that implement the interface -2. Use factory injection to provide fakes to the code under test -3. Control fake behavior through constructor parameters or fields - -**Benefits:** -- **No external dependencies** - Fakes are simple structs with no framework requirements -- **Full control** - Control exact behavior through fields/parameters -- **Type-safe** - Compile-time verification that fakes implement interfaces -- **Easy to understand** - Fakes are just plain Go code -- **Flexible** - Can create different factories for different test scenarios +**For command testing patterns, use:** `/testing-commands` -**Reference:** See `pkg/instances/manager_test.go` for a complete implementation of this pattern with factory injection. +**For cross-platform testing, use:** `/cross-platform-development` ## GitHub Actions diff --git a/skills/README.md b/skills/README.md index eb15271..e2460e6 100644 --- a/skills/README.md +++ b/skills/README.md @@ -12,9 +12,34 @@ Each skill is contained in its own subdirectory with a `SKILL.md` file that defi ## Available Skills -- **commit**: Generate conventional commit messages based on staged changes +### Command Development +- **add-command-simple**: Add a simple CLI command without JSON output support +- **add-command-with-json**: Add a new CLI command with JSON output support +- **add-alias-command**: Add an alias command that delegates to an existing command +- **add-parent-command**: Add a parent/root command that has subcommands +- **implementing-command-patterns**: Advanced patterns for implementing commands including flag binding, JSON output, and interactive sessions + +### Runtime Development +- **add-runtime**: Add a new runtime implementation to the kortex-cli runtime system +- **working-with-runtime-system**: Guide to understanding and working with the kortex-cli runtime system architecture + +### Configuration & Integration +- **working-with-config-system**: Guide to workspace configuration for environment variables and mount points at multiple levels +- **working-with-podman-runtime-config**: Guide to configuring the Podman runtime including image setup, agent configuration, and containerfile generation +- **working-with-steplogger**: Complete guide to integrating StepLogger for user progress feedback in commands and runtimes +- **working-with-instances-manager**: Guide to using the instances manager API for workspace management and project detection + +### Testing +- **testing-commands**: Comprehensive guide to testing CLI commands with unit tests, E2E tests, and best practices +- **testing-best-practices**: Testing best practices including parallel execution, fake objects, and factory injection patterns + +### Development Standards +- **cross-platform-development**: Essential patterns for cross-platform compatibility including path handling and testing practices - **copyright-headers**: Add or update Apache License 2.0 copyright headers to source files +### Tools +- **commit**: Generate conventional commit messages based on staged changes + ## Usage Agents can discover skills by scanning this directory for `SKILL.md` files. Each skill's metadata and instructions are contained in its respective file. diff --git a/skills/cross-platform-development/SKILL.md b/skills/cross-platform-development/SKILL.md new file mode 100644 index 0000000..0b21cdb --- /dev/null +++ b/skills/cross-platform-development/SKILL.md @@ -0,0 +1,432 @@ +--- +name: cross-platform-development +description: Essential patterns for cross-platform compatibility including path handling and testing practices +argument-hint: "" +--- + +# Cross-Platform Development + +⚠️ **CRITICAL**: All path operations and tests MUST be cross-platform compatible (Linux, macOS, Windows). + +## Overview + +This skill covers essential patterns for writing code that works correctly across all supported platforms. Tests that pass on Linux/macOS may fail on Windows CI if they don't follow these patterns. + +## Core Rules + +- **Host paths**: Always use `filepath.Join()` for path construction (never hardcode "/" or "\\") +- **Container paths**: Always use `path.Join()` for paths inside containers (containers are always Unix/Linux) +- Convert relative paths to absolute with `filepath.Abs()` +- Never hardcode paths with `~` - use `os.UserHomeDir()` instead +- In tests, use `filepath.Join()` for all path assertions +- **Use `t.TempDir()` for ALL temporary directories in tests - never hardcode paths** + +## Host Paths vs Container Paths + +### Host Paths (Use `filepath.Join`) + +Host paths run on the actual operating system (Windows, macOS, or Linux) and must use OS-specific separators: + +```go +import "path/filepath" + +// GOOD: Host paths use filepath.Join +configDir := filepath.Join(storageDir, ".kortex") +runtimeDir := filepath.Join(storageDir, "runtimes", "podman") +``` + +### Container Paths (Use `path.Join`) + +Container paths are **always Unix/Linux** regardless of the host OS. Podman containers run Linux, so paths inside containers must use forward slashes: + +```go +import "path" // Note: NOT path/filepath + +// GOOD: Container paths use path.Join (always Unix) +containerPath := path.Join("/home", "agent", ".config") +mountPath := path.Join("/workspace", "sources", "pkg") + +// BAD: Don't use filepath.Join for container paths +containerPath := filepath.Join("/home", "agent", ".config") // Wrong on Windows host! +``` + +**Example from Podman runtime:** + +```go +import ( + "path" // For container paths + "path/filepath" // For host paths +) + +// Host path (can be Windows, macOS, Linux) +hostConfigDir := filepath.Join(storageDir, "runtimes", "podman", "config") + +// Container path (always Linux) +containerWorkspace := path.Join("/workspace", "sources") +containerHome := path.Join("/home", "agent") +``` + +## Common Test Failures on Windows + +Tests often fail on Windows due to hardcoded Unix-style paths. These paths get normalized differently by `filepath.Abs()` on Windows vs Unix systems. + +### ❌ NEVER Do This in Tests + +```go +// BAD - Will fail on Windows because filepath.Abs() normalizes differently +instance, err := instances.NewInstance(instances.NewInstanceParams{ + SourceDir: "/path/to/source", // ❌ Hardcoded Unix path + ConfigDir: "/path/to/config", // ❌ Hardcoded Unix path +}) + +// BAD - Will fail on Windows +invalidPath := "/this/path/does/not/exist" // ❌ Unix-style absolute path + +// BAD - Platform-specific separator +path := dir + "/subdir" // ❌ Hardcoded forward slash +``` + +### ✅ ALWAYS Do This in Tests + +```go +// GOOD - Cross-platform, works everywhere +sourceDir := t.TempDir() +configDir := t.TempDir() +instance, err := instances.NewInstance(instances.NewInstanceParams{ + SourceDir: sourceDir, // ✅ Real temp directory + ConfigDir: configDir, // ✅ Real temp directory +}) + +// GOOD - Create invalid path cross-platform way +tempDir := t.TempDir() +notADir := filepath.Join(tempDir, "file") +os.WriteFile(notADir, []byte("test"), 0644) +invalidPath := filepath.Join(notADir, "subdir") // ✅ Will fail MkdirAll on all platforms + +// GOOD - Use filepath.Join() for host paths +path := filepath.Join(dir, "subdir") // ✅ Cross-platform +``` + +## Production Code Patterns + +### Host Path Construction + +```go +import "path/filepath" + +// GOOD: Cross-platform host path construction +configDir := filepath.Join(sourceDir, ".kortex") +absPath, err := filepath.Abs(relativePath) + +// BAD: Hardcoded separator +configDir := sourceDir + "/.kortex" // Don't do this! +``` + +### Container Path Construction + +```go +import "path" + +// GOOD: Container path construction (always Unix) +workspacePath := path.Join("/workspace", "sources") +agentHome := path.Join("/home", "agent", ".config") + +// BAD: Using filepath.Join for container paths +workspacePath := filepath.Join("/workspace", "sources") // Wrong on Windows! +``` + +### User Home Directory + +```go +import "path/filepath" + +// GOOD: User home directory (host path) +homeDir, err := os.UserHomeDir() +defaultPath := filepath.Join(homeDir, ".kortex-cli") + +// BAD: Hardcoded tilde +defaultPath := "~/.kortex-cli" // Don't do this! +``` + +### Test Assertions + +```go +import "path/filepath" + +// GOOD: Test assertions for host paths +expectedPath := filepath.Join(".", "relative", "path") +if result != expectedPath { + t.Errorf("Expected %s, got %s", expectedPath, result) +} + +// BAD: Hardcoded path +expectedPath := "./relative/path" // Don't do this! +``` + +### Temporary Directories in Tests + +```go +// GOOD: Temporary directories in tests +tempDir := t.TempDir() // Automatic cleanup +sourcesDir := t.TempDir() + +// BAD: Hardcoded temp paths +tempDir := "/tmp/test" // Don't do this! +``` + +## Path Handling Best Practices + +### Converting Relative to Absolute + +Always convert relative paths to absolute in command `preRun`: + +```go +import "path/filepath" + +func (c *myCmd) preRun(cmd *cobra.Command, args []string) error { + relativePath := args[0] + + // Convert to absolute path (host path) + absPath, err := filepath.Abs(relativePath) + if err != nil { + return fmt.Errorf("failed to resolve path: %w", err) + } + + c.path = absPath + return nil +} +``` + +### Building Nested Paths + +#### Host Paths + +```go +import "path/filepath" + +// GOOD: Multiple joins for host paths +baseDir := filepath.Join(storageDir, "runtimes") +runtimeDir := filepath.Join(baseDir, "podman") +configDir := filepath.Join(runtimeDir, "config") + +// GOOD: Nested arguments for host paths +configPath := filepath.Join(storageDir, "runtimes", "podman", "config", "image.json") + +// BAD: String concatenation +configPath := storageDir + "/runtimes/podman/config/image.json" // Don't do this! +``` + +#### Container Paths + +```go +import "path" + +// GOOD: Container paths (always Unix) +workspaceSources := path.Join("/workspace", "sources", "pkg", "cmd") +agentConfig := path.Join("/home", "agent", ".config", "claude") + +// BAD: Using filepath for container paths +workspaceSources := filepath.Join("/workspace", "sources", "pkg", "cmd") // Wrong! +``` + +### Checking Path Existence + +```go +import "path/filepath" + +// GOOD: Cross-platform path checking (host paths) +configPath := filepath.Join(configDir, "workspace.json") +if _, err := os.Stat(configPath); err != nil { + if os.IsNotExist(err) { + // File doesn't exist + } +} +``` + +## Testing Patterns + +### Creating Test Directories + +```go +import "path/filepath" + +func TestMyFunction(t *testing.T) { + t.Parallel() + + // Create temp directories - automatically cleaned up + storageDir := t.TempDir() + sourcesDir := t.TempDir() + configDir := t.TempDir() + + // Use in tests (these are host paths) + result, err := MyFunction(sourcesDir, configDir) + // ... +} +``` + +### Testing with Invalid Paths + +```go +import "path/filepath" + +func TestMyFunction_InvalidPath(t *testing.T) { + t.Parallel() + + // Create a file (not a directory) + tempDir := t.TempDir() + notADir := filepath.Join(tempDir, "file") + os.WriteFile(notADir, []byte("test"), 0644) + + // Try to use file as a directory + invalidPath := filepath.Join(notADir, "subdir") + + _, err := MyFunction(invalidPath) + if err == nil { + t.Fatal("Expected error for invalid path") + } +} +``` + +### Path Assertions in Tests + +```go +import "path/filepath" + +func TestMyFunction_ReturnsPath(t *testing.T) { + t.Parallel() + + sourceDir := t.TempDir() + + result, err := MyFunction(sourceDir) + if err != nil { + t.Fatalf("MyFunction() failed: %v", err) + } + + // Build expected path using filepath.Join (host path) + expectedPath := filepath.Join(sourceDir, ".kortex") + if result != expectedPath { + t.Errorf("Expected %s, got %s", expectedPath, result) + } +} +``` + +## Common Pitfalls + +### Hardcoded Separators + +```go +// BAD: Platform-specific +path := basedir + "/config/file.json" +path := basedir + "\\config\\file.json" + +// GOOD: Cross-platform (host paths) +path := filepath.Join(basedir, "config", "file.json") + +// GOOD: Container paths (always Unix) +containerPath := path.Join("/home", "agent", "config", "file.json") +``` + +### Home Directory Expansion + +```go +import "path/filepath" + +// BAD: Tilde doesn't work cross-platform +defaultPath := "~/.kortex-cli" + +// GOOD: Use os.UserHomeDir() +homeDir, err := os.UserHomeDir() +if err != nil { + return "", err +} +defaultPath := filepath.Join(homeDir, ".kortex-cli") +``` + +### Absolute Path Detection + +```go +import "path/filepath" + +// GOOD: Use filepath.IsAbs() for host paths +if filepath.IsAbs(path) { + // Path is absolute +} + +// BAD: String prefix checking +if strings.HasPrefix(path, "/") { // Only works on Unix + // Path is absolute +} +``` + +### Path Cleaning + +```go +import "path/filepath" + +// GOOD: Use filepath.Clean() to normalize host paths +cleanPath := filepath.Clean(userPath) + +// Removes redundant separators and . elements +// Resolves .. elements +// Converts separators to OS-specific +``` + +## Summary: When to Use Which Package + +| Context | Package | Example | +|---------|---------|---------| +| Host paths (files on disk) | `path/filepath` | `filepath.Join(storageDir, "config")` | +| Container paths (inside Podman) | `path` | `path.Join("/workspace", "sources")` | +| URL paths | `path` | `path.Join("/api", "v1", "users")` | + +## Environment Variables + +Handle environment variables in a cross-platform way: + +```go +// GOOD: Use os.Getenv() +homeDir := os.Getenv("HOME") // Unix +if homeDir == "" { + homeDir = os.Getenv("USERPROFILE") // Windows +} + +// BETTER: Use os.UserHomeDir() which handles this +homeDir, err := os.UserHomeDir() +``` + +## File Permissions + +Be aware of cross-platform permission differences: + +```go +// Create directories with appropriate permissions +// 0755 works on Unix, Windows ignores the permission bits +err := os.MkdirAll(dirPath, 0755) + +// Create files +// 0644 works on Unix, Windows ignores the permission bits +err := os.WriteFile(filePath, data, 0644) +``` + +## Why This Matters + +**Tests that pass on Linux/macOS may fail on Windows CI if they use hardcoded Unix paths.** + +Always use: +- `t.TempDir()` for temporary directories in tests +- `filepath.Join()` for host paths (files on disk) +- `path.Join()` for container paths (inside Podman containers, which are always Linux) + +## Related Skills + +- `/testing-commands` - Command testing patterns +- `/testing-best-practices` - General testing best practices +- `/working-with-podman-runtime-config` - Podman runtime configuration + +## References + +- **Go filepath package**: Standard library documentation for host paths +- **Go path package**: Standard library documentation for Unix paths (containers, URLs) +- **Cross-platform tests**: All `*_test.go` files should follow these patterns +- **Example**: `pkg/instances/manager_test.go`, `pkg/cmd/init_test.go` +- **Podman runtime**: `pkg/runtime/podman/` for container path examples diff --git a/skills/implementing-command-patterns/SKILL.md b/skills/implementing-command-patterns/SKILL.md new file mode 100644 index 0000000..b20d943 --- /dev/null +++ b/skills/implementing-command-patterns/SKILL.md @@ -0,0 +1,334 @@ +--- +name: implementing-command-patterns +description: Advanced patterns for implementing commands including flag binding, JSON output, and interactive sessions +argument-hint: "" +--- + +# Implementing Command Patterns + +This skill covers advanced patterns for implementing CLI commands beyond the basics. For creating simple commands, see `/add-command-simple` and `/add-command-with-json` skills. + +## Command Implementation Pattern + +Commands should follow a consistent structure for maintainability and testability: + +### 1. Command Struct + +Contains all command state: +- Input values from flags/args +- Computed/validated values +- Dependencies (e.g., manager instances) + +### 2. preRun Method + +Validates parameters and prepares: +- Parse and validate arguments/flags +- Access global flags (e.g., `--storage`) +- Create dependencies (managers, etc.) +- Convert paths to absolute using `filepath.Abs()` +- Store validated values in struct fields + +### 3. run Method + +Executes the command logic: +- Use validated values from struct fields +- Perform the actual operation +- Output results to user + +**Reference:** See `pkg/cmd/init.go` for a complete implementation of this pattern. + +## Flag Binding Best Practices + +**IMPORTANT**: Always bind command flags directly to struct fields using the `*Var` variants (e.g., `StringVarP`, `BoolVarP`, `IntVarP`) instead of using the non-binding variants and then calling `GetString()`, `GetBool()`, etc. in `preRun`. + +### Benefits + +- **Cleaner code**: No need to call `cmd.Flags().GetString()` and handle errors +- **Simpler testing**: Tests can set struct fields directly instead of creating and setting flags +- **Earlier binding**: Values are available immediately when preRun is called +- **Less error-prone**: No risk of typos in flag names when retrieving values + +### Pattern + +```go +// Command struct with fields for all flags +type myCmd struct { + verbose bool + output string + count int + manager instances.Manager +} + +// Bind flags to struct fields in the command factory +func NewMyCmd() *cobra.Command { + c := &myCmd{} + + cmd := &cobra.Command{ + Use: "my-command", + Short: "My command description", + Args: cobra.NoArgs, + PreRunE: c.preRun, + RunE: c.run, + } + + // GOOD: Bind flags directly to struct fields + cmd.Flags().BoolVarP(&c.verbose, "verbose", "v", false, "Show detailed output") + cmd.Flags().StringVarP(&c.output, "output", "o", "", "Output format (supported: json)") + cmd.Flags().IntVarP(&c.count, "count", "c", 10, "Number of items to process") + + return cmd +} + +// Use the bound values directly in preRun +func (m *myCmd) preRun(cmd *cobra.Command, args []string) error { + // Values are already available from struct fields + if m.output != "" && m.output != "json" { + return fmt.Errorf("unsupported output format: %s", m.output) + } + + // No need to call cmd.Flags().GetString("output") + return nil +} +``` + +### Avoid + +```go +// BAD: Don't define flags without binding +cmd.Flags().StringP("output", "o", "", "Output format") + +// BAD: Don't retrieve flag values in preRun +func (m *myCmd) preRun(cmd *cobra.Command, args []string) error { + output, err := cmd.Flags().GetString("output") // Avoid this pattern + if err != nil { + return err + } + m.output = output + // ... +} +``` + +### Testing with Bound Flags + +```go +func TestMyCmd_PreRun(t *testing.T) { + t.Run("validates output format", func(t *testing.T) { + // Set struct fields directly - no need to set up flags + c := &myCmd{ + output: "xml", // Invalid format + } + cmd := &cobra.Command{} + + err := c.preRun(cmd, []string{}) + if err == nil { + t.Fatal("Expected error for invalid output format") + } + }) +} +``` + +**Reference:** See `pkg/cmd/init.go`, `pkg/cmd/workspace_remove.go`, and `pkg/cmd/workspace_list.go` for examples of proper flag binding. + +## JSON Output Support Pattern + +When adding JSON output support to commands, follow this pattern to ensure consistent error handling and output formatting. + +### Rules + +1. **Check output flag FIRST in preRun** - Validate the output format before any other validation +2. **Set SilenceErrors and SilenceUsage early** - Prevent Cobra's default error output when in JSON mode +3. **Use outputErrorIfJSON for ALL errors** - In preRun, run, and any helper methods (like outputJSON) + +### Pattern + +```go +type myCmd struct { + output string // Bound to --output flag + manager instances.Manager +} + +func (m *myCmd) preRun(cmd *cobra.Command, args []string) error { + // 1. FIRST: Validate output format + if m.output != "" && m.output != "json" { + return fmt.Errorf("unsupported output format: %s (supported: json)", m.output) + } + + // 2. EARLY: Silence Cobra's error output in JSON mode + if m.output == "json" { + cmd.SilenceErrors = true + cmd.SilenceUsage = true + } + + // 3. ALL subsequent errors use outputErrorIfJSON + storageDir, err := cmd.Flags().GetString("storage") + if err != nil { + return outputErrorIfJSON(cmd, m.output, fmt.Errorf("failed to read --storage flag: %w", err)) + } + + manager, err := instances.NewManager(storageDir) + if err != nil { + return outputErrorIfJSON(cmd, m.output, fmt.Errorf("failed to create manager: %w", err)) + } + m.manager = manager + + return nil +} + +func (m *myCmd) run(cmd *cobra.Command, args []string) error { + // ALL errors in run use outputErrorIfJSON + data, err := m.manager.GetData() + if err != nil { + return outputErrorIfJSON(cmd, m.output, fmt.Errorf("failed to get data: %w", err)) + } + + if m.output == "json" { + return m.outputJSON(cmd, data) + } + + // Text output + cmd.Println(data) + return nil +} + +func (m *myCmd) outputJSON(cmd *cobra.Command, data interface{}) error { + jsonData, err := json.MarshalIndent(data, "", " ") + if err != nil { + // Even unlikely errors in helper methods use outputErrorIfJSON + return outputErrorIfJSON(cmd, m.output, fmt.Errorf("failed to marshal to JSON: %w", err)) + } + + fmt.Fprintln(cmd.OutOrStdout(), string(jsonData)) + return nil +} +``` + +### Why This Pattern + +- **Consistent error format**: All errors are JSON when `--output=json` is set +- **No Cobra pollution**: SilenceErrors prevents "Error: ..." prefix in JSON output +- **Early detection**: Output flag is validated before expensive operations +- **Helper methods work**: outputErrorIfJSON works in any method that has access to cmd and output flag + +### Helper Function + +The `outputErrorIfJSON` helper in `pkg/cmd/conversion.go` handles the formatting: + +```go +func outputErrorIfJSON(cmd interface{ OutOrStdout() io.Writer }, output string, err error) error { + if output == "json" { + jsonErr, _ := formatErrorJSON(err) + fmt.Fprintln(cmd.OutOrStdout(), jsonErr) // Errors go to stdout in JSON mode + } + return err // Still return the error for proper exit codes +} +``` + +**Reference:** See `pkg/cmd/init.go`, `pkg/cmd/workspace_remove.go`, and `pkg/cmd/workspace_list.go` for complete implementations. + +## Interactive Commands (No JSON Output) + +Some commands are inherently interactive and do not support JSON output. These commands connect stdin/stdout/stderr directly to a user's terminal. + +### Example: Terminal Command + +The `terminal` command provides an interactive session with a running workspace instance: + +```go +type workspaceTerminalCmd struct { + manager instances.Manager + id string + command []string +} + +func (w *workspaceTerminalCmd) preRun(cmd *cobra.Command, args []string) error { + w.id = args[0] + + // Extract command from args[1:] if provided + if len(args) > 1 { + w.command = args[1:] + } else { + // Default command (configurable from runtime) + w.command = []string{"claude"} + } + + // Standard setup: storage flag, manager, runtime registration + storageDir, err := cmd.Flags().GetString("storage") + if err != nil { + return fmt.Errorf("failed to read --storage flag: %w", err) + } + + absStorageDir, err := filepath.Abs(storageDir) + if err != nil { + return fmt.Errorf("failed to resolve storage path: %w", err) + } + + manager, err := instances.NewManager(absStorageDir) + if err != nil { + return fmt.Errorf("failed to create manager: %w", err) + } + + if err := runtimesetup.RegisterAll(manager); err != nil { + return fmt.Errorf("failed to register runtimes: %w", err) + } + + w.manager = manager + return nil +} + +func (w *workspaceTerminalCmd) run(cmd *cobra.Command, args []string) error { + // Connect to terminal - this is a blocking interactive call + err := w.manager.Terminal(cmd.Context(), w.id, w.command) + if err != nil { + if errors.Is(err, instances.ErrInstanceNotFound) { + return fmt.Errorf("workspace not found: %s\nUse 'workspace list' to see available workspaces", w.id) + } + return err + } + return nil +} +``` + +### Key Differences from JSON-Supporting Commands + +- **No `--output` flag** - Interactive commands don't need this +- **No JSON output helpers** - All output goes directly to terminal +- **Simpler error handling** - Just return errors normally (no `outputErrorIfJSON`) +- **Blocking execution** - The command runs until the user exits the interactive session +- **Command arguments** - Accept commands to run inside the instance: `terminal ID bash` or `terminal ID -- bash -c 'echo hello'` + +### Example Command Registration + +```go +func NewWorkspaceTerminalCmd() *cobra.Command { + c := &workspaceTerminalCmd{} + + cmd := &cobra.Command{ + Use: "terminal ID [COMMAND...]", + Short: "Connect to a running workspace with an interactive terminal", + Args: cobra.MinimumNArgs(1), + ValidArgsFunction: completeRunningWorkspaceID, // Only show running workspaces + PreRunE: c.preRun, + RunE: c.run, + } + + // No flags needed - just uses global --storage flag + return cmd +} +``` + +**Reference:** See `pkg/cmd/workspace_terminal.go` for the complete implementation. + +## Related Skills + +- `/add-command-simple` - Creating simple commands +- `/add-command-with-json` - Creating commands with JSON output +- `/testing-commands` - Testing command implementations +- `/working-with-steplogger` - Adding progress feedback to commands + +## References + +- **Command Pattern Example**: `pkg/cmd/init.go` +- **Flag Binding Examples**: `pkg/cmd/init.go`, `pkg/cmd/workspace_remove.go`, `pkg/cmd/workspace_list.go` +- **JSON Output Examples**: `pkg/cmd/init.go`, `pkg/cmd/workspace_remove.go`, `pkg/cmd/workspace_list.go` +- **Interactive Command Example**: `pkg/cmd/workspace_terminal.go` +- **Helper Functions**: `pkg/cmd/conversion.go` diff --git a/skills/testing-best-practices/SKILL.md b/skills/testing-best-practices/SKILL.md new file mode 100644 index 0000000..3089f64 --- /dev/null +++ b/skills/testing-best-practices/SKILL.md @@ -0,0 +1,489 @@ +--- +name: testing-best-practices +description: Testing best practices including parallel execution, fake objects, and factory injection patterns +argument-hint: "" +--- + +# Testing Best Practices + +This skill covers general testing best practices for the kortex-cli project, including parallel test execution and using fake objects for testability. + +## Parallel Test Execution + +**All tests MUST call `t.Parallel()` as the first line of the test function.** + +This ensures faster test execution and better resource utilization. Every test function should start with: + +```go +func TestExample(t *testing.T) { + t.Parallel() + + // Test code here... +} +``` + +### Benefits of Parallel Tests + +- **Faster CI/CD**: Tests run concurrently, reducing total execution time +- **Better resource utilization**: Makes use of multi-core systems +- **Identifies race conditions**: Helps find concurrency issues early + +### Exception: Tests Using `t.Setenv()` + +Tests that use `t.Setenv()` to set environment variables **cannot use `t.Parallel()`** on the parent test function. The Go testing framework enforces this restriction because environment variable changes affect the entire process. + +```go +// CORRECT: No t.Parallel() when using t.Setenv() +func TestWithEnvVariable(t *testing.T) { + t.Run("subtest with env var", func(t *testing.T) { + t.Setenv("MY_VAR", "value") + // Test code here... + }) +} + +// INCORRECT: Will panic at runtime +func TestWithEnvVariable(t *testing.T) { + t.Parallel() // ❌ WRONG - cannot use with t.Setenv() + + t.Run("subtest with env var", func(t *testing.T) { + t.Setenv("MY_VAR", "value") + // Test code here... + }) +} +``` + +**Reference:** See `pkg/cmd/root_test.go:TestRootCmd_StorageEnvVariable()` for an example of testing with environment variables. + +## Testing with Fake Objects + +When testing code that uses interfaces (following the Module Design Pattern), **use fake implementations instead of real implementations or mocks**. + +### Why Fakes Over Mocks + +- **No external dependencies** - Fakes are simple structs with no framework requirements +- **Full control** - Control exact behavior through fields/parameters +- **Type-safe** - Compile-time verification that fakes implement interfaces +- **Easy to understand** - Fakes are just plain Go code +- **Flexible** - Can create different factories for different test scenarios + +### Pattern + +1. Create unexported fake structs that implement the interface +2. Use factory injection to provide fakes to the code under test +3. Control fake behavior through constructor parameters or fields + +### Example: Fake Instance Factory + +```go +// Fake instance factory for testing +type fakeInstanceFactory struct { + instances map[string]instances.Instance + err error +} + +func newFakeInstanceFactory() *fakeInstanceFactory { + return &fakeInstanceFactory{ + instances: make(map[string]instances.Instance), + } +} + +func (f *fakeInstanceFactory) NewInstance(params instances.NewInstanceParams) (instances.Instance, error) { + if f.err != nil { + return nil, f.err + } + + instance := &fakeInstance{ + id: generateID(params.SourceDir), + sourceDir: params.SourceDir, + configDir: params.ConfigDir, + } + + f.instances[instance.id] = instance + return instance, nil +} + +// Use in tests +func TestManager_Add(t *testing.T) { + t.Parallel() + + storageDir := t.TempDir() + fakeFactory := newFakeInstanceFactory() + + manager, err := newManagerWithFactory(storageDir, fakeFactory) + if err != nil { + t.Fatalf("Failed to create manager: %v", err) + } + + // Test using the fake factory + instance, err := manager.Add(context.Background(), instances.AddOptions{ + // ... + }) +} +``` + +### Example: Fake with Controlled Errors + +```go +// Fake that can be configured to return errors +type fakeGenerator struct { + nextID string + err error +} + +func newFakeGenerator() *fakeGenerator { + return &fakeGenerator{ + nextID: "test-id", + } +} + +func newFakeGeneratorWithError(err error) *fakeGenerator { + return &fakeGenerator{ + err: err, + } +} + +func (f *fakeGenerator) Generate(input string) (string, error) { + if f.err != nil { + return "", f.err + } + return f.nextID, nil +} + +// Use in tests +func TestManager_Add_GeneratorError(t *testing.T) { + t.Parallel() + + storageDir := t.TempDir() + fakeGen := newFakeGeneratorWithError(errors.New("generator error")) + + manager, err := newManagerWithFactory(storageDir, fakeInstanceFactory, fakeGen) + if err != nil { + t.Fatalf("Failed to create manager: %v", err) + } + + _, err = manager.Add(context.Background(), instances.AddOptions{ + // ... + }) + + if err == nil { + t.Fatal("Expected error from generator") + } +} +``` + +### Example: Fake with Behavior Tracking + +```go +// Fake that tracks method calls +type fakeRuntime struct { + createCalls []runtime.CreateParams + startCalls []string + stopCalls []string +} + +func newFakeRuntime() *fakeRuntime { + return &fakeRuntime{ + createCalls: []runtime.CreateParams{}, + startCalls: []string{}, + stopCalls: []string{}, + } +} + +func (f *fakeRuntime) Create(ctx context.Context, params runtime.CreateParams) (runtime.RuntimeInfo, error) { + f.createCalls = append(f.createCalls, params) + return runtime.RuntimeInfo{ + ID: "test-id", + State: "created", + }, nil +} + +func (f *fakeRuntime) Start(ctx context.Context, id string) (runtime.RuntimeInfo, error) { + f.startCalls = append(f.startCalls, id) + return runtime.RuntimeInfo{ + ID: id, + State: "running", + }, nil +} + +// Use in tests +func TestManager_Lifecycle(t *testing.T) { + t.Parallel() + + fakeRT := newFakeRuntime() + + // ... test code that uses the runtime ... + + // Verify behavior + if len(fakeRT.createCalls) != 1 { + t.Errorf("Expected 1 Create() call, got %d", len(fakeRT.createCalls)) + } + + if len(fakeRT.startCalls) != 1 { + t.Errorf("Expected 1 Start() call, got %d", len(fakeRT.startCalls)) + } +} +``` + +### Factory Injection Pattern + +Use factory injection to provide fakes to the code under test: + +```go +// Production constructor +func NewManager(storageDir string) (Manager, error) { + return newManagerWithFactory( + storageDir, + instances.NewInstance, // Real factory + generator.New(), // Real generator + registry.New(), // Real registry + git.NewDetector(), // Real detector + ) +} + +// Test-friendly constructor with factory injection +func newManagerWithFactory( + storageDir string, + instanceFactory instanceFactory, + generator idGenerator, + registry runtimeRegistry, + detector git.Detector, +) (Manager, error) { + // Implementation uses injected dependencies +} + +// In tests +func TestManager(t *testing.T) { + t.Parallel() + + fakeFactory := newFakeInstanceFactory() + fakeGen := newFakeGenerator() + fakeRegistry := newTestRegistry(t.TempDir()) + fakeDetector := newFakeGitDetector() + + manager, err := newManagerWithFactory( + t.TempDir(), + fakeFactory, + fakeGen, + fakeRegistry, + fakeDetector, + ) + // ... test code ... +} +``` + +## Test Organization + +### Naming Conventions + +```go +// Unit tests for specific methods +func TestManager_Add(t *testing.T) { ... } +func TestManager_Delete(t *testing.T) { ... } + +// E2E tests +func TestInitCmd_E2E(t *testing.T) { ... } + +// PreRun validation tests +func TestInitCmd_PreRun(t *testing.T) { ... } + +// Example validation tests +func TestInitCmd_Examples(t *testing.T) { ... } +``` + +### Test Structure + +```go +func TestManager_Add(t *testing.T) { + t.Parallel() + + t.Run("adds instance successfully", func(t *testing.T) { + t.Parallel() + + // Arrange + storageDir := t.TempDir() + manager, _ := NewManager(storageDir) + + // Act + instance, err := manager.Add(context.Background(), options) + + // Assert + if err != nil { + t.Fatalf("Add() failed: %v", err) + } + if instance.ID == "" { + t.Error("Expected non-empty ID") + } + }) + + t.Run("returns error for invalid input", func(t *testing.T) { + t.Parallel() + + // Arrange + storageDir := t.TempDir() + manager, _ := NewManager(storageDir) + + // Act + _, err := manager.Add(context.Background(), invalidOptions) + + // Assert + if err == nil { + t.Fatal("Expected error for invalid input") + } + }) +} +``` + +## Table-Driven Tests + +Use table-driven tests for testing multiple scenarios: + +```go +func TestValidate(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + input string + wantErr bool + errMsg string + }{ + { + name: "valid input", + input: "valid", + wantErr: false, + }, + { + name: "empty input", + input: "", + wantErr: true, + errMsg: "cannot be empty", + }, + { + name: "invalid characters", + input: "invalid@#$", + wantErr: true, + errMsg: "invalid characters", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + + err := Validate(tt.input) + + if (err != nil) != tt.wantErr { + t.Fatalf("Validate() error = %v, wantErr %v", err, tt.wantErr) + } + + if tt.wantErr && !strings.Contains(err.Error(), tt.errMsg) { + t.Errorf("Expected error to contain '%s', got: %s", tt.errMsg, err.Error()) + } + }) + } +} +``` + +## Test Helpers + +Create test helpers to reduce boilerplate: + +```go +// Test helper for creating a test manager +func newTestManager(t *testing.T) instances.Manager { + t.Helper() + + storageDir := t.TempDir() + manager, err := instances.NewManager(storageDir) + if err != nil { + t.Fatalf("Failed to create manager: %v", err) + } + + return manager +} + +// Test helper for adding a test instance +func addTestInstance(t *testing.T, manager instances.Manager) instances.Instance { + t.Helper() + + sourceDir := t.TempDir() + configDir := t.TempDir() + + instance, err := instances.NewInstance(instances.NewInstanceParams{ + SourceDir: sourceDir, + ConfigDir: configDir, + }) + if err != nil { + t.Fatalf("Failed to create instance: %v", err) + } + + added, err := manager.Add(context.Background(), instances.AddOptions{ + Instance: instance, + RuntimeType: "fake", + }) + if err != nil { + t.Fatalf("Failed to add instance: %v", err) + } + + return added +} + +// Use in tests +func TestManager_Delete(t *testing.T) { + t.Parallel() + + manager := newTestManager(t) + instance := addTestInstance(t, manager) + + err := manager.Delete(instance.ID) + if err != nil { + t.Fatalf("Delete() failed: %v", err) + } +} +``` + +## Cleanup and Resource Management + +Use `t.TempDir()` for automatic cleanup: + +```go +func TestWithFiles(t *testing.T) { + t.Parallel() + + // Automatically cleaned up after test + tempDir := t.TempDir() + + // Create test files + testFile := filepath.Join(tempDir, "test.txt") + os.WriteFile(testFile, []byte("test"), 0644) + + // No need to manually clean up - t.TempDir() handles it +} +``` + +Use `t.Cleanup()` for custom cleanup: + +```go +func TestWithCleanup(t *testing.T) { + t.Parallel() + + // Custom cleanup + t.Cleanup(func() { + // Clean up resources + }) + + // Test code... +} +``` + +## Related Skills + +- `/testing-commands` - Command-specific testing patterns +- `/cross-platform-development` - Cross-platform testing practices + +## References + +- **Parallel Tests**: All `*_test.go` files should use `t.Parallel()` +- **Fake Objects**: `pkg/instances/manager_test.go` for complete examples +- **Factory Injection**: `pkg/instances/manager.go` and `pkg/instances/manager_test.go` +- **Go Testing Package**: Standard library documentation diff --git a/skills/testing-commands/SKILL.md b/skills/testing-commands/SKILL.md new file mode 100644 index 0000000..d12f24a --- /dev/null +++ b/skills/testing-commands/SKILL.md @@ -0,0 +1,377 @@ +--- +name: testing-commands +description: Comprehensive guide to testing CLI commands with unit tests, E2E tests, and best practices +argument-hint: "" +--- + +# Testing Commands + +This skill covers comprehensive testing patterns for CLI commands, including unit tests for `preRun` validation and E2E tests for full command execution. + +## Overview + +Commands should have two types of tests: +1. **Unit Tests** - Test the `preRun` method directly to verify validation logic +2. **E2E Tests** - Test the full command execution including Cobra wiring + +## Unit Tests - Testing preRun Directly + +Unit tests focus on validating the `preRun` method logic without executing the full command flow. + +### Pattern + +- **IMPORTANT**: Create an instance of the command struct (e.g., `c := &initCmd{}`) +- **IMPORTANT**: Create a mock `*cobra.Command` and set up required flags +- **IMPORTANT**: Call `c.preRun(cmd, args)` directly - DO NOT call `rootCmd.Execute()` +- Use `t.Run()` for subtests within a parent test function +- Test with different argument/flag combinations +- Verify struct fields are set correctly after `preRun()` executes +- Use `t.TempDir()` for temporary directories (automatic cleanup) + +### Example + +```go +func TestMyCmd_PreRun(t *testing.T) { + t.Run("sets fields correctly", func(t *testing.T) { + t.Parallel() + + storageDir := t.TempDir() + + c := &myCmd{} // Create command struct instance + cmd := &cobra.Command{} // Create mock cobra command + cmd.Flags().String("storage", storageDir, "test storage flag") + + args := []string{"arg1"} + + err := c.preRun(cmd, args) // Call preRun directly + if err != nil { + t.Fatalf("preRun() failed: %v", err) + } + + // Assert on struct fields + if c.manager == nil { + t.Error("Expected manager to be created") + } + }) +} +``` + +## E2E Tests - Testing Full Command Execution + +E2E tests verify the complete command flow including Cobra argument parsing, flag handling, and persistence. + +### Pattern + +- Execute via `rootCmd.Execute()` to test the complete flow +- Use real temp directories with `t.TempDir()` +- Verify output messages +- Verify persistence (check storage/database) +- Verify all field values from `manager.List()` or similar +- Test multiple scenarios (default args, custom args, edge cases) +- Test Cobra's argument validation (e.g., required args, arg counts) + +### Example + +```go +func TestMyCmd_E2E(t *testing.T) { + t.Run("executes successfully", func(t *testing.T) { + t.Parallel() + + storageDir := t.TempDir() + + rootCmd := NewRootCmd() // Use full command construction + rootCmd.SetArgs([]string{"mycommand", "arg1", "--storage", storageDir}) + + err := rootCmd.Execute() // Execute the full command + if err != nil { + t.Fatalf("Execute() failed: %v", err) + } + + // Verify results in storage + manager, _ := instances.NewManager(storageDir) + instancesList, _ := manager.List() + // ... assert on results + }) +} +``` + +## Testing with Captured Output + +Capture command output for verification: + +```go +func TestMyCmd_E2E_Output(t *testing.T) { + t.Run("displays correct output", func(t *testing.T) { + t.Parallel() + + storageDir := t.TempDir() + + rootCmd := NewRootCmd() + rootCmd.SetArgs([]string{"mycommand", "arg1", "--storage", storageDir}) + + // Capture output + var buf bytes.Buffer + rootCmd.SetOut(&buf) + + err := rootCmd.Execute() + if err != nil { + t.Fatalf("Execute() failed: %v", err) + } + + output := buf.String() + if !strings.Contains(output, "expected message") { + t.Errorf("Expected output to contain 'expected message', got: %s", output) + } + }) +} +``` + +## Testing JSON Output + +Test both text and JSON output modes: + +```go +func TestMyCmd_E2E_JSON(t *testing.T) { + t.Run("outputs valid JSON", func(t *testing.T) { + t.Parallel() + + storageDir := t.TempDir() + + rootCmd := NewRootCmd() + rootCmd.SetArgs([]string{"mycommand", "arg1", "--storage", storageDir, "--output", "json"}) + + var buf bytes.Buffer + rootCmd.SetOut(&buf) + + err := rootCmd.Execute() + if err != nil { + t.Fatalf("Execute() failed: %v", err) + } + + // Verify JSON is valid + var result MyResultType + if err := json.Unmarshal(buf.Bytes(), &result); err != nil { + t.Fatalf("Failed to parse JSON output: %v", err) + } + + // Verify JSON fields + if result.Field != "expected value" { + t.Errorf("Expected field to be 'expected value', got: %s", result.Field) + } + }) +} +``` + +## Testing Error Cases + +Test that errors are handled correctly: + +```go +func TestMyCmd_E2E_Errors(t *testing.T) { + t.Run("returns error for invalid argument", func(t *testing.T) { + t.Parallel() + + storageDir := t.TempDir() + + rootCmd := NewRootCmd() + rootCmd.SetArgs([]string{"mycommand", "invalid-arg", "--storage", storageDir}) + + err := rootCmd.Execute() + if err == nil { + t.Fatal("Expected error for invalid argument") + } + + expectedMsg := "expected error message" + if !strings.Contains(err.Error(), expectedMsg) { + t.Errorf("Expected error to contain '%s', got: %s", expectedMsg, err.Error()) + } + }) +} +``` + +## Testing Cobra Argument Validation + +Test that Cobra's `Args` validation works: + +```go +func TestMyCmd_E2E_ArgValidation(t *testing.T) { + t.Run("requires argument", func(t *testing.T) { + t.Parallel() + + storageDir := t.TempDir() + + rootCmd := NewRootCmd() + rootCmd.SetArgs([]string{"mycommand", "--storage", storageDir}) // Missing required arg + + err := rootCmd.Execute() + if err == nil { + t.Fatal("Expected error for missing required argument") + } + }) + + t.Run("rejects too many arguments", func(t *testing.T) { + t.Parallel() + + storageDir := t.TempDir() + + rootCmd := NewRootCmd() + rootCmd.SetArgs([]string{"mycommand", "arg1", "arg2", "--storage", storageDir}) // Too many args + + err := rootCmd.Execute() + if err == nil { + t.Fatal("Expected error for too many arguments") + } + }) +} +``` + +## Testing with Temp Directories + +Always use `t.TempDir()` for temporary directories: + +```go +func TestMyCmd_E2E_Persistence(t *testing.T) { + t.Run("persists data correctly", func(t *testing.T) { + t.Parallel() + + // Create temp directories - automatically cleaned up + storageDir := t.TempDir() + sourcesDir := t.TempDir() + + rootCmd := NewRootCmd() + rootCmd.SetArgs([]string{"init", sourcesDir, "--storage", storageDir}) + + err := rootCmd.Execute() + if err != nil { + t.Fatalf("Execute() failed: %v", err) + } + + // Verify persistence + manager, err := instances.NewManager(storageDir) + if err != nil { + t.Fatalf("Failed to create manager: %v", err) + } + + instances, err := manager.List() + if err != nil { + t.Fatalf("Failed to list instances: %v", err) + } + + if len(instances) != 1 { + t.Errorf("Expected 1 instance, got %d", len(instances)) + } + }) +} +``` + +## Test Organization + +Organize tests into logical groups: + +```go +// TestMyCmd_PreRun tests the preRun validation logic +func TestMyCmd_PreRun(t *testing.T) { + // Unit tests for preRun method +} + +// TestMyCmd_E2E tests the full command execution +func TestMyCmd_E2E(t *testing.T) { + // E2E tests for complete flow +} + +// TestMyCmd_Examples validates example commands +func TestMyCmd_Examples(t *testing.T) { + // Example validation (see CLAUDE.md - Adding a New Command) +} +``` + +## Common Patterns + +### Setting Up Test Data + +```go +func TestMyCmd_E2E(t *testing.T) { + t.Run("works with existing data", func(t *testing.T) { + t.Parallel() + + storageDir := t.TempDir() + + // Set up test data + manager, _ := instances.NewManager(storageDir) + instance, _ := instances.NewInstance(instances.NewInstanceParams{ + SourceDir: t.TempDir(), + ConfigDir: t.TempDir(), + }) + manager.Add(context.Background(), instances.AddOptions{ + Instance: instance, + RuntimeType: "fake", + }) + + // Run command + rootCmd := NewRootCmd() + rootCmd.SetArgs([]string{"workspace", "list", "--storage", storageDir}) + + err := rootCmd.Execute() + if err != nil { + t.Fatalf("Execute() failed: %v", err) + } + }) +} +``` + +### Testing Multiple Scenarios + +```go +func TestMyCmd_E2E(t *testing.T) { + tests := []struct { + name string + args []string + wantErr bool + errMsg string + }{ + { + name: "valid input", + args: []string{"mycommand", "valid-arg"}, + wantErr: false, + }, + { + name: "invalid input", + args: []string{"mycommand", "invalid-arg"}, + wantErr: true, + errMsg: "expected error", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + + storageDir := t.TempDir() + + rootCmd := NewRootCmd() + rootCmd.SetArgs(append(tt.args, "--storage", storageDir)) + + err := rootCmd.Execute() + if (err != nil) != tt.wantErr { + t.Fatalf("Execute() error = %v, wantErr %v", err, tt.wantErr) + } + + if tt.wantErr && !strings.Contains(err.Error(), tt.errMsg) { + t.Errorf("Expected error to contain '%s', got: %s", tt.errMsg, err.Error()) + } + }) + } +} +``` + +## Related Skills + +- `/implementing-command-patterns` - Command implementation patterns +- `/testing-best-practices` - General testing best practices +- `/cross-platform-development` - Cross-platform testing patterns + +## References + +- **Example Tests**: `pkg/cmd/init_test.go`, `pkg/cmd/workspace_list_test.go`, `pkg/cmd/workspace_remove_test.go` +- **Command Pattern**: `pkg/cmd/init.go` +- **Test Utilities**: `pkg/cmd/testutil/` diff --git a/skills/working-with-config-system/SKILL.md b/skills/working-with-config-system/SKILL.md new file mode 100644 index 0000000..b393759 --- /dev/null +++ b/skills/working-with-config-system/SKILL.md @@ -0,0 +1,342 @@ +--- +name: working-with-config-system +description: Guide to workspace configuration for environment variables and mount points at multiple levels +argument-hint: "" +--- + +# Working with the Config System + +The config system manages **workspace configuration** for injecting environment variables and mounting directories into workspaces. This is different from runtime-specific configuration (e.g., Podman image settings). + +**What this config system controls:** +- Environment variables to inject into workspace containers/VMs +- Additional source directories to mount (dependencies) +- Configuration directories to mount from `$HOME` (e.g., `.ssh`, `.gitconfig`) + +**What this does NOT control:** +- Runtime-specific settings (e.g., Podman container image, packages to install) +- See `/working-with-podman-runtime-config` for runtime-specific configuration + +## Overview + +The multi-level configuration system allows users to customize workspace settings at different levels: +- **Workspace-level config** (`.kortex/workspace.json`) - Shared project configuration committed to repository + - Can be configured using the `--workspace-configuration` flag of the `init` command (path to directory containing `workspace.json`) +- **Project-specific config** (`~/.kortex-cli/config/projects.json`) - User's custom config for specific projects +- **Global config** (empty string `""` key in `projects.json`) - Settings applied to all projects +- **Agent-specific config** (`~/.kortex-cli/config/agents.json`) - Per-agent overrides (e.g., Claude, Goose) + +These configurations control what gets injected **into** workspaces (environment variables, mounts), not how the workspace runtime is built or configured. + +## Key Components + +- **Config Interface** (`pkg/config/config.go`): Interface for managing configuration directories +- **ConfigMerger** (`pkg/config/merger.go`): Merges multiple `WorkspaceConfiguration` objects +- **AgentConfigLoader** (`pkg/config/agents.go`): Loads agent-specific configuration +- **ProjectConfigLoader** (`pkg/config/projects.go`): Loads project and global configuration +- **Manager Integration** (`pkg/instances/manager.go`): Handles config loading and merging during instance creation +- **WorkspaceConfiguration Model**: Imported from `github.com/kortex-hub/kortex-cli-api/workspace-configuration/go` + +## Configuration File Locations + +All user-specific configuration files are stored under the storage directory (default: `~/.kortex-cli`, configurable via `--storage` flag or `KORTEX_CLI_STORAGE` environment variable): + +- **Agent configs**: `/config/agents.json` +- **Project configs**: `/config/projects.json` +- **Workspace configs**: `.kortex/workspace.json` (in workspace directory) + - Created/configured via `kortex-cli init --workspace-configuration ` + +## Configuration Precedence + +Configurations are merged from lowest to highest priority (highest wins): +1. **Agent-specific configuration** (from `agents.json`) - HIGHEST PRIORITY +2. **Project-specific configuration** (from `projects.json` using project ID) +3. **Global project configuration** (from `projects.json` using empty string `""` key) +4. **Workspace-level configuration** (from `.kortex/workspace.json`) - LOWEST PRIORITY + +## Configuration Structure + +### Workspace Configuration (`workspace.json`) + +The `workspace.json` file controls what gets injected into the workspace: + +```json +{ + "environment": [ + { + "name": "DEBUG", + "value": "true" + }, + { + "name": "API_KEY", + "secret": "github-token" + } + ], + "mounts": { + "dependencies": ["../main"], + "configs": [".ssh", ".gitconfig"] + } +} +``` + +**Creating workspace configuration:** + +Use the `--workspace-configuration` flag with the `init` command to specify a directory containing `workspace.json`: + +```bash +# Create workspace with custom configuration directory +kortex-cli init /path/to/workspace --workspace-configuration /path/to/config-dir +# This will look for /path/to/config-dir/workspace.json +``` + +**Fields:** +- `environment` - Environment variables to set in the workspace (optional) + - `name` - Variable name (must be valid Unix environment variable name) + - `value` - Hardcoded value (mutually exclusive with `secret`, empty strings allowed) + - `secret` - Secret reference (mutually exclusive with `value`, cannot be empty) +- `mounts.dependencies` - Additional source directories to mount into workspace (optional) + - Paths must be relative (not absolute) + - Paths cannot be empty + - Relative to workspace sources directory +- `mounts.configs` - Configuration directories from `$HOME` to mount into workspace (optional) + - Paths must be relative (not absolute) + - Paths cannot be empty + - Relative to `$HOME` + +### Agent Configuration (`agents.json`) + +Agent-specific overrides for environment variables and mounts: + +```json +{ + "claude": { + "environment": [ + { + "name": "DEBUG", + "value": "true" + } + ], + "mounts": { + "configs": [".claude-config"] + } + }, + "goose": { + "environment": [ + { + "name": "GOOSE_MODE", + "value": "verbose" + } + ] + } +} +``` + +### Project Configuration (`projects.json`) + +Project-specific and global settings for environment variables and mounts: + +```json +{ + "": { + "mounts": { + "configs": [".gitconfig", ".ssh"] + } + }, + "github.com/kortex-hub/kortex-cli": { + "environment": [ + { + "name": "PROJECT_VAR", + "value": "project-value" + } + ], + "mounts": { + "dependencies": ["../kortex-common"] + } + }, + "/home/user/my/project": { + "environment": [ + { + "name": "LOCAL_DEV", + "value": "true" + } + ] + } +} +``` + +**Special Keys:** +- Empty string `""` represents global/default configuration applied to all projects +- Useful for common settings like SSH keys, Git config that should be mounted in all workspaces +- Project-specific configs override global config + +## Using the Config Interface + +### Loading Workspace Configuration + +```go +import ( + "github.com/kortex-hub/kortex-cli/pkg/config" + workspace "github.com/kortex-hub/kortex-cli-api/workspace-configuration/go" +) + +// Create a config manager for a workspace +cfg, err := config.NewConfig("/path/to/workspace/.kortex") +if err != nil { + return err +} + +// Load and validate the workspace configuration +workspaceCfg, err := cfg.Load() +if err != nil { + if errors.Is(err, config.ErrConfigNotFound) { + // workspace.json doesn't exist, use defaults + } else if errors.Is(err, config.ErrInvalidConfig) { + // Configuration validation failed + } else { + return err + } +} + +// Access configuration values (note: fields are pointers) +if workspaceCfg.Environment != nil { + for _, env := range *workspaceCfg.Environment { + // Use env.Name, env.Value, env.Secret + } +} + +if workspaceCfg.Mounts != nil { + if workspaceCfg.Mounts.Dependencies != nil { + // Use dependency paths + } + if workspaceCfg.Mounts.Configs != nil { + // Use config paths + } +} +``` + +## Using the Multi-Level Config System + +The Manager handles all configuration loading and merging automatically: + +```go +// In command code (e.g., init command) +addedInstance, err := manager.Add(ctx, instances.AddOptions{ + Instance: instance, + RuntimeType: "fake", + WorkspaceConfig: workspaceConfig, // From .kortex/workspace.json or --workspace-configuration directory + Project: "custom-project", // Optional override + Agent: "claude", // Optional agent name +}) +``` + +The Manager's `Add()` method: +1. Detects project ID (or uses custom override) +2. Loads project config (global `""` + project-specific merged) +3. Loads agent config (if agent name provided) +4. Merges configs: workspace → global → project → agent +5. Passes merged config to runtime for injection into workspace + +## Merging Behavior + +- **Environment variables**: Later configs override earlier ones by name + - If the same variable appears in multiple configs, the one from the higher-precedence config wins +- **Mount dependencies**: Deduplicated list (preserves order, removes duplicates) +- **Mount configs**: Deduplicated list (preserves order, removes duplicates) + +**Example Merge Flow:** + +Given: +- Workspace config: `DEBUG=workspace`, `WORKSPACE_VAR=value1` +- Global config: `GLOBAL_VAR=global` +- Project config: `DEBUG=project`, `PROJECT_VAR=value2` +- Agent config: `DEBUG=agent`, `AGENT_VAR=value3` + +Result: `DEBUG=agent`, `WORKSPACE_VAR=value1`, `GLOBAL_VAR=global`, `PROJECT_VAR=value2`, `AGENT_VAR=value3` + +## Loading Configuration Programmatically + +```go +import "github.com/kortex-hub/kortex-cli/pkg/config" + +// Load project config (includes global + project-specific merged) +projectLoader, err := config.NewProjectConfigLoader(storageDir) +projectConfig, err := projectLoader.Load("github.com/user/repo") + +// Load agent config +agentLoader, err := config.NewAgentConfigLoader(storageDir) +agentConfig, err := agentLoader.Load("claude") + +// Merge configurations +merger := config.NewMerger() +merged := merger.Merge(workspaceConfig, projectConfig) +merged = merger.Merge(merged, agentConfig) +``` + +## Configuration Validation + +The `Load()` method automatically validates the configuration and returns `ErrInvalidConfig` if any of these rules are violated: + +### Environment Variables + +- Name cannot be empty +- Name must be a valid Unix environment variable name (starts with letter or underscore, followed by letters, digits, or underscores) +- Exactly one of `value` or `secret` must be defined +- Secret references cannot be empty strings +- Empty values are allowed (valid use case: set env var to empty string) + +### Mount Paths + +- Dependency paths cannot be empty +- Dependency paths must be relative (not absolute) +- Config paths cannot be empty +- Config paths must be relative (not absolute) + +## Error Handling + +- `config.ErrInvalidPath` - Configuration path is empty or invalid +- `config.ErrConfigNotFound` - The `workspace.json` file is not found +- `config.ErrInvalidConfig` - Configuration validation failed (includes detailed error message) +- `config.ErrInvalidAgentConfig` - Agent configuration is invalid +- `config.ErrInvalidProjectConfig` - Project configuration is invalid + +## Testing Multi-Level Configs + +```go +// Create test config files +configDir := filepath.Join(storageDir, "config") +os.MkdirAll(configDir, 0755) + +agentsJSON := `{"claude": {"environment": [{"name": "VAR", "value": "val"}]}}` +os.WriteFile(filepath.Join(configDir, "agents.json"), []byte(agentsJSON), 0644) + +// Run init with agent +rootCmd.SetArgs([]string{"init", sourcesDir, "--runtime", "fake", "--agent", "claude"}) +rootCmd.Execute() +``` + +## Design Principles + +- Configuration directory is NOT automatically created +- Missing configuration directory is treated as empty/default configuration +- All configurations are validated on load to catch errors early +- Configuration merging is handled by Manager, not commands +- Missing config files return empty configs (not errors) +- Invalid JSON or validation errors are reported +- All loaders follow the module design pattern +- Cross-platform compatible (uses `filepath.Join()`, `t.TempDir()`) +- Storage directory is configurable via `--storage` flag or `KORTEX_CLI_STORAGE` env var +- Uses nested JSON structure for clarity and extensibility +- Model types are imported from external API package for consistency + +## Related Skills + +- `/working-with-podman-runtime-config` - Configure runtime-specific settings (Podman image, packages, etc.) +- `/working-with-instances-manager` - Using the instances manager API + +## References + +- **Config Interface**: `pkg/config/config.go` +- **ConfigMerger**: `pkg/config/merger.go` +- **AgentConfigLoader**: `pkg/config/agents.go` +- **ProjectConfigLoader**: `pkg/config/projects.go` +- **Manager Integration**: `pkg/instances/manager.go` diff --git a/skills/working-with-instances-manager/SKILL.md b/skills/working-with-instances-manager/SKILL.md new file mode 100644 index 0000000..072b540 --- /dev/null +++ b/skills/working-with-instances-manager/SKILL.md @@ -0,0 +1,319 @@ +--- +name: working-with-instances-manager +description: Guide to using the instances manager API for workspace management and project detection +argument-hint: "" +--- + +# Working with the Instances Manager + +The instances manager provides the API for managing workspace instances throughout their lifecycle. This skill covers the manager API and project detection functionality. + +## Overview + +The instances manager handles: +- Adding and removing workspace instances +- Listing and retrieving instance information +- Starting and stopping instances via runtimes +- Project detection and grouping +- Configuration merging (workspace, project, agent configs) +- Interactive terminal sessions with running instances + +## Creating the Manager + +In command `preRun`, create the manager from the storage flag: + +```go +storageDir, _ := cmd.Flags().GetString("storage") +manager, err := instances.NewManager(storageDir) +if err != nil { + return fmt.Errorf("failed to create manager: %w", err) +} +``` + +## Manager API + +### Add - Create New Instance + +Add a new workspace instance to the manager: + +```go +instance, err := instances.NewInstance(instances.NewInstanceParams{ + SourceDir: sourceDir, + ConfigDir: configDir, +}) +if err != nil { + return fmt.Errorf("failed to create instance: %w", err) +} + +addedInstance, err := manager.Add(ctx, instances.AddOptions{ + Instance: instance, + RuntimeType: "fake", + WorkspaceConfig: workspaceConfig, // From .kortex/workspace.json + Project: "custom-project", // Optional: overrides auto-detection + Agent: "claude", // Optional: agent name for agent-specific config +}) +if err != nil { + return fmt.Errorf("failed to add instance: %w", err) +} +``` + +The `Add()` method: +1. Detects project ID (or uses custom override) +2. Loads project config (global `""` + project-specific merged) +3. Loads agent config (if agent name provided) +4. Merges configs: workspace → global → project → agent +5. Passes merged config to runtime for injection into workspace + +### List - Get All Instances + +List all registered workspace instances: + +```go +instancesList, err := manager.List() +if err != nil { + return fmt.Errorf("failed to list instances: %w", err) +} + +for _, instance := range instancesList { + fmt.Printf("ID: %s, State: %s, Project: %s\n", + instance.ID, instance.State, instance.Project) +} +``` + +### Get - Retrieve Specific Instance + +Get a specific instance by ID: + +```go +instance, err := manager.Get(id) +if err != nil { + if errors.Is(err, instances.ErrInstanceNotFound) { + return fmt.Errorf("workspace not found: %s", id) + } + return fmt.Errorf("instance not found: %w", err) +} + +fmt.Printf("Found instance: %s (State: %s)\n", instance.ID, instance.State) +``` + +### Delete - Remove Instance + +Delete an instance from the manager: + +```go +err := manager.Delete(id) +if err != nil { + if errors.Is(err, instances.ErrInstanceNotFound) { + return fmt.Errorf("workspace not found: %s", id) + } + return fmt.Errorf("failed to delete instance: %w", err) +} +``` + +### Start - Start Instance Runtime + +Start a stopped instance: + +```go +info, err := manager.Start(ctx, id) +if err != nil { + if errors.Is(err, instances.ErrInstanceNotFound) { + return fmt.Errorf("workspace not found: %s", id) + } + return fmt.Errorf("failed to start instance: %w", err) +} + +fmt.Printf("Started instance: %s (State: %s)\n", info.ID, info.State) +``` + +### Stop - Stop Instance Runtime + +Stop a running instance: + +```go +err := manager.Stop(ctx, id) +if err != nil { + if errors.Is(err, instances.ErrInstanceNotFound) { + return fmt.Errorf("workspace not found: %s", id) + } + return fmt.Errorf("failed to stop instance: %w", err) +} +``` + +### Terminal - Interactive Terminal Session + +Connect to a running instance with an interactive terminal: + +```go +err := manager.Terminal(cmd.Context(), id, []string{"bash"}) +if err != nil { + if errors.Is(err, instances.ErrInstanceNotFound) { + return fmt.Errorf("workspace not found: %s\nUse 'workspace list' to see available workspaces", id) + } + return err +} +``` + +**Terminal Method Behavior:** +- Verifies the instance exists and is in a running state +- Checks if the runtime implements the `runtime.Terminal` interface +- Delegates to the runtime's Terminal implementation +- Returns an error if the instance is not running or runtime doesn't support terminals + +**Key Points:** +- Uses a read lock (doesn't modify instance state) +- Command is a slice of strings: `[]string{"bash"}` or `[]string{"claude-code", "--debug"}` +- Returns `ErrInstanceNotFound` if instance doesn't exist +- Returns an error if instance state is not "running" +- Returns an error if the runtime doesn't implement `runtime.Terminal` interface + +**Example usage in a command:** + +```go +func (w *workspaceTerminalCmd) run(cmd *cobra.Command, args []string) error { + // Start terminal session with the command extracted in preRun + err := w.manager.Terminal(cmd.Context(), w.id, w.command) + if err != nil { + if errors.Is(err, instances.ErrInstanceNotFound) { + return fmt.Errorf("workspace not found: %s\nUse 'workspace list' to see available workspaces", w.id) + } + return err + } + return nil +} +``` + +## Project Detection and Grouping + +Each workspace has a `project` field that enables grouping workspaces belonging to the same project across branches, forks, or subdirectories. + +### Project Identifier Detection + +The manager automatically detects the project identifier when adding instances: + +1. **Git repository with remote**: Uses repository remote URL (without `.git`) plus relative path + - Checks `upstream` remote first (useful for forks) + - Falls back to `origin` remote if `upstream` doesn't exist + - Example: `https://github.com/kortex-hub/kortex-cli/` (at root) or `https://github.com/kortex-hub/kortex-cli/pkg/git` (in subdirectory) + +2. **Git repository without remote**: Uses repository root directory plus relative path + - Example: `/home/user/local-repo` (at root) or `/home/user/local-repo/pkg/utils` (in subdirectory) + +3. **Non-git directory**: Uses the source directory path + - Example: `/tmp/workspace` + +### Custom Project Override + +Users can override auto-detection with the `--project` flag: + +```go +// Add instance with custom project +addedInstance, err := manager.Add(ctx, instances.AddOptions{ + Instance: instance, + RuntimeType: "fake", + WorkspaceConfig: workspaceConfig, + Project: "custom-project-id", // Optional: overrides auto-detection +}) +``` + +### Implementation Details + +- **Package**: `pkg/git` provides git repository detection with testable abstractions +- **Detector Interface**: `git.Detector` with `DetectRepository(ctx, dir)` method +- **Executor Pattern**: `git.Executor` abstracts git command execution for testing +- **Manager Integration**: `manager.detectProject()` is called during `Add()` if no custom project is provided + +### Testing with Fake Git Detector + +```go +// Use fake git detector in tests +gitDetector := newFakeGitDetectorWithRepo( + "/repo/root", + "https://github.com/user/repo", + "pkg/subdir", // relative path +) + +manager, _ := newManagerWithFactory( + storageDir, + fakeInstanceFactory, + newFakeGenerator(), + newTestRegistry(tmpDir), + gitDetector, +) +``` + +See `pkg/instances/manager_project_test.go` for comprehensive test examples. + +## Error Handling + +Common errors from the manager: + +```go +// Instance not found +if errors.Is(err, instances.ErrInstanceNotFound) { + return fmt.Errorf("workspace not found: %s", id) +} + +// Runtime not found +if errors.Is(err, runtime.ErrRuntimeNotFound) { + return fmt.Errorf("runtime not found: %s", runtimeType) +} + +// Instance already exists +if errors.Is(err, instances.ErrInstanceExists) { + return fmt.Errorf("workspace already exists: %s", id) +} +``` + +## Example: Complete Command Implementation + +```go +type myCmd struct { + manager instances.Manager +} + +func (c *myCmd) preRun(cmd *cobra.Command, args []string) error { + storageDir, _ := cmd.Flags().GetString("storage") + + manager, err := instances.NewManager(storageDir) + if err != nil { + return fmt.Errorf("failed to create manager: %w", err) + } + + // Register runtimes + if err := runtimesetup.RegisterAll(manager); err != nil { + return fmt.Errorf("failed to register runtimes: %w", err) + } + + c.manager = manager + return nil +} + +func (c *myCmd) run(cmd *cobra.Command, args []string) error { + // Use manager to list instances + instances, err := c.manager.List() + if err != nil { + return fmt.Errorf("failed to list instances: %w", err) + } + + for _, instance := range instances { + cmd.Printf("ID: %s, State: %s, Project: %s\n", + instance.ID, instance.State, instance.Project) + } + + return nil +} +``` + +## Related Skills + +- `/working-with-config-system` - Configuration merging and multi-level configs +- `/working-with-runtime-system` - Runtime system architecture +- `/implementing-command-patterns` - Command implementation patterns + +## References + +- **Manager Interface**: `pkg/instances/manager.go` +- **Git Detection**: `pkg/git/` +- **Project Tests**: `pkg/instances/manager_project_test.go` +- **Example Commands**: `pkg/cmd/init.go`, `pkg/cmd/workspace_list.go`, `pkg/cmd/workspace_terminal.go` diff --git a/skills/working-with-podman-runtime-config/SKILL.md b/skills/working-with-podman-runtime-config/SKILL.md new file mode 100644 index 0000000..e7a7c43 --- /dev/null +++ b/skills/working-with-podman-runtime-config/SKILL.md @@ -0,0 +1,173 @@ +--- +name: working-with-podman-runtime-config +description: Guide to configuring the Podman runtime including image setup, agent configuration, and containerfile generation +argument-hint: "" +--- + +# Working with Podman Runtime Configuration + +The Podman runtime supports configurable image and agent settings through JSON files. This is **runtime-specific configuration** that controls how the Podman container is built and configured. + +**What this config system controls:** +- Base container image (Fedora version) +- Packages to install in the container +- Sudo permissions for binaries +- Custom RUN commands during image build +- Agent-specific setup commands +- Terminal command for the agent + +**What this does NOT control:** +- Environment variables and mounts injected into workspaces +- See `/working-with-config-system` for workspace configuration (env vars, mounts) + +## Overview + +The Podman runtime configuration allows customization of the base image, installed packages, sudo permissions, and agent setup through JSON files stored in the runtime's storage directory. + +## Key Components + +- **Config Interface** (`pkg/runtime/podman/config/config.go`): Interface for managing Podman runtime configuration +- **ImageConfig** (`pkg/runtime/podman/config/types.go`): Base image configuration (Fedora version, packages, sudo binaries, custom RUN commands) +- **AgentConfig** (`pkg/runtime/podman/config/types.go`): Agent-specific configuration (packages, RUN commands, terminal command) +- **Defaults** (`pkg/runtime/podman/config/defaults.go`): Default configurations for image and Claude agent + +## Configuration Storage + +Configuration files are stored in the runtime's storage directory: + +```text +/runtimes/podman/config/ +├── image.json # Base image configuration +└── claude.json # Agent-specific configuration (e.g., for Claude Code) +``` + +## Configuration Files + +### image.json - Base Image Configuration + +```json +{ + "version": "latest", + "packages": ["which", "procps-ng", "wget2", "@development-tools", "jq", "gh", "golang", "golangci-lint", "python3", "python3-pip"], + "sudo": ["/usr/bin/dnf", "/bin/nice", "/bin/kill", "/usr/bin/kill", "/usr/bin/killall"], + "run_commands": [] +} +``` + +**Fields:** +- `version` (required) - Fedora version tag (e.g., "latest", "40", "41") +- `packages` (optional) - DNF packages to install +- `sudo` (optional) - Absolute paths to binaries the user can run with sudo (creates single `ALLOWED` Cmnd_Alias) +- `run_commands` (optional) - Custom shell commands to execute during image build (before agent setup) + +### claude.json - Agent-Specific Configuration + +```json +{ + "packages": [], + "run_commands": [ + "curl -fsSL --proto-redir '-all,https' --tlsv1.3 https://claude.ai/install.sh | bash", + "mkdir /home/agent/.config" + ], + "terminal_command": ["claude"] +} +``` + +**Fields:** +- `packages` (optional) - Additional packages for the agent (merged with image packages) +- `run_commands` (optional) - Commands to set up the agent (executed after image setup) +- `terminal_command` (required) - Command to launch the agent (must have at least one element) + +## Using the Config Interface + +```go +import "github.com/kortex-hub/kortex-cli/pkg/runtime/podman/config" + +// Create config manager (in Initialize method) +configDir := filepath.Join(storageDir, "config") +cfg, err := config.NewConfig(configDir) +if err != nil { + return fmt.Errorf("failed to create config: %w", err) +} + +// Generate default configs if they don't exist +if err := cfg.GenerateDefaults(); err != nil { + return fmt.Errorf("failed to generate defaults: %w", err) +} + +// Load configurations (in Create method) +imageConfig, err := cfg.LoadImage() +if err != nil { + return fmt.Errorf("failed to load image config: %w", err) +} + +agentConfig, err := cfg.LoadAgent("claude") +if err != nil { + return fmt.Errorf("failed to load agent config: %w", err) +} +``` + +## Validation + +The config system validates: +- Image version cannot be empty +- Sudo binaries must be absolute paths +- Terminal command must have at least one element +- All fields are optional except `version` (ImageConfig) and `terminal_command` (AgentConfig) + +## Default Generation + +- Default configs are auto-generated on first runtime initialization +- Existing config files are never overwritten - customizations are preserved +- Default image config includes common development tools and packages +- Default Claude config installs Claude Code from the official install script + +## Containerfile Generation + +The config system is used to generate Containerfiles dynamically: + +```go +import "github.com/kortex-hub/kortex-cli/pkg/runtime/podman" + +// Generate Containerfile content from configs +containerfileContent := generateContainerfile(imageConfig, agentConfig) + +// Generate sudoers file content from sudo binaries +sudoersContent := generateSudoers(imageConfig.Sudo) +``` + +The `generateContainerfile` function creates a Containerfile with: +- Base image: `registry.fedoraproject.org/fedora:` +- Merged packages from image and agent configs +- User/group setup (hardcoded as `agent:agent`) +- Sudoers configuration with single `ALLOWED` Cmnd_Alias +- Custom RUN commands from both configs (image commands first, then agent commands) + +## Hardcoded Values + +These values are not configurable: +- Base image registry: `registry.fedoraproject.org/fedora` (only version tag is configurable) +- Container user: `agent` +- Container group: `agent` +- User UID/GID: Matched to host user's UID/GID at build time + +## Design Principles + +- Follows interface-based design pattern with unexported implementation +- Uses nested JSON structure for clarity +- Validates all configurations on load to catch errors early +- Separate concerns: base image vs agent-specific settings +- Extensible: easy to add new agent configurations (e.g., `goose.json`, `cursor.json`) + +## Related Skills + +- `/working-with-config-system` - Workspace configuration (env vars, mounts) +- `/working-with-runtime-system` - Runtime system architecture +- `/add-runtime` - Creating new runtimes + +## References + +- **Config Interface**: `pkg/runtime/podman/config/config.go` +- **ImageConfig & AgentConfig**: `pkg/runtime/podman/config/types.go` +- **Defaults**: `pkg/runtime/podman/config/defaults.go` +- **Podman Runtime**: `pkg/runtime/podman/` diff --git a/skills/working-with-runtime-system/SKILL.md b/skills/working-with-runtime-system/SKILL.md new file mode 100644 index 0000000..2bb4b47 --- /dev/null +++ b/skills/working-with-runtime-system/SKILL.md @@ -0,0 +1,158 @@ +--- +name: working-with-runtime-system +description: Guide to understanding and working with the kortex-cli runtime system architecture +argument-hint: "" +--- + +# Working with the Runtime System + +The runtime system provides a pluggable architecture for managing workspaces on different container/VM platforms (Podman, MicroVM, Kubernetes, etc.). This skill provides detailed guidance on understanding and working with the runtime system. + +## Overview + +The runtime system enables kortex-cli to support multiple backend platforms through a common interface. Each runtime implementation handles the platform-specific details of creating, starting, stopping, and managing workspace instances. + +## Key Components + +- **Runtime Interface** (`pkg/runtime/runtime.go`): Contract all runtimes must implement +- **Registry** (`pkg/runtime/registry.go`): Manages runtime registration and discovery +- **Runtime Implementations** (`pkg/runtime//`): Platform-specific packages (e.g., `fake`) +- **Centralized Registration** (`pkg/runtimesetup/register.go`): Automatically registers all available runtimes + +## Runtime Registration in Commands + +Commands use `runtimesetup.RegisterAll()` to automatically register all available runtimes: + +```go +import "github.com/kortex-hub/kortex-cli/pkg/runtimesetup" + +// In command preRun +manager, err := instances.NewManager(storageDir) +if err != nil { + return err +} + +// Register all available runtimes +if err := runtimesetup.RegisterAll(manager); err != nil { + return err +} +``` + +This automatically registers all runtimes from `pkg/runtimesetup/register.go` that report as available (e.g., only registers Podman if `podman` CLI is installed). + +## Optional Runtime Interfaces + +Some runtimes may implement additional optional interfaces to provide extended functionality beyond the base Runtime interface. These are checked at runtime using type assertions, allowing runtimes to opt-in to features they support. + +### StorageAware Interface + +The StorageAware interface enables runtimes to persist data in a dedicated storage directory managed by the registry. + +```go +type StorageAware interface { + Initialize(storageDir string) error +} +``` + +**How it works:** + +When a runtime implements StorageAware, the registry will: +1. Create a directory at `//` +2. Call `Initialize(storageDir)` with the path +3. The runtime can use this directory to persist instance data, configuration, or other state + +**Example implementation:** + +```go +type myRuntime struct { + storageDir string + storageFile string + instances map[string]Instance +} + +// Implement StorageAware +func (r *myRuntime) Initialize(storageDir string) error { + r.storageDir = storageDir + r.storageFile = filepath.Join(storageDir, "instances.json") + + // Load existing state from disk + return r.loadFromDisk() +} + +func (r *myRuntime) Create(ctx context.Context, params runtime.CreateParams) (runtime.RuntimeInfo, error) { + // ... create instance logic ... + + // Persist to storage directory + if err := r.saveToDisk(); err != nil { + return runtime.RuntimeInfo{}, fmt.Errorf("failed to persist instance: %w", err) + } + + return info, nil +} +``` + +### Terminal Interface + +The Terminal interface enables interactive terminal sessions for connecting to running instances. This is used by the `terminal` command. + +```go +type Terminal interface { + // Terminal starts an interactive terminal session inside a running instance. + // The agent parameter is used to load agent-specific configuration for the terminal session. + // The command is executed with stdin/stdout/stderr connected directly to the user's terminal. + Terminal(ctx context.Context, agent string, instanceID string, command []string) error +} +``` + +**Example implementation (Podman runtime):** + +```go +func (p *podmanRuntime) Terminal(ctx context.Context, agent string, instanceID string, command []string) error { + if agent == "" { + return fmt.Errorf("%w: agent is required", runtime.ErrInvalidParams) + } + if instanceID == "" { + return fmt.Errorf("%w: instance ID is required", runtime.ErrInvalidParams) + } + if len(command) == 0 { + return fmt.Errorf("%w: command is required", runtime.ErrInvalidParams) + } + + // Build podman exec -it + args := []string{"exec", "-it", instanceID} + args = append(args, command...) + + return p.executor.RunInteractive(ctx, args...) +} +``` + +**How optional interfaces work:** + +The Terminal interface follows the same pattern as `StorageAware` - it's optional, and runtimes that don't support interactive sessions simply don't implement it. The instances manager checks for Terminal support at runtime using type assertion: + +```go +if terminalRuntime, ok := runtime.(Terminal); ok { + return terminalRuntime.Terminal(ctx, agent, instanceID, command) +} +return errors.New("runtime does not support terminal sessions") +``` + +This pattern allows runtimes to provide additional capabilities without requiring all runtimes to implement every possible feature. + +## Adding a New Runtime + +Use the `/add-runtime` skill which provides step-by-step instructions for creating a new runtime implementation. The `fake` runtime in `pkg/runtime/fake/` serves as a reference implementation. + +## Related Skills + +- `/add-runtime` - Step-by-step guide to create a new runtime implementation +- `/working-with-steplogger` - Add progress feedback to runtime operations +- `/working-with-podman-runtime-config` - Configure the Podman runtime + +## References + +- **Runtime Interface**: `pkg/runtime/runtime.go` +- **Registry**: `pkg/runtime/registry.go` +- **Registration**: `pkg/runtimesetup/register.go` +- **Reference Implementation**: `pkg/runtime/fake/` +- **Podman Implementation**: `pkg/runtime/podman/` diff --git a/skills/working-with-steplogger/SKILL.md b/skills/working-with-steplogger/SKILL.md new file mode 100644 index 0000000..9b01455 --- /dev/null +++ b/skills/working-with-steplogger/SKILL.md @@ -0,0 +1,246 @@ +--- +name: working-with-steplogger +description: Complete guide to integrating StepLogger for user progress feedback in commands and runtimes +argument-hint: "" +--- + +# Working with StepLogger + +The StepLogger system provides user-facing progress feedback during runtime operations. It displays operational steps with spinners and completion messages in text mode, improving the user experience for long-running operations. + +## Overview + +StepLogger enables commands and runtimes to show users what's happening during multi-step operations like creating containers, building images, or starting instances. It automatically handles different output modes (text with spinners vs JSON with silence). + +## Key Components + +- **StepLogger Interface** (`pkg/steplogger/steplogger.go`): Contract for logging operational steps +- **TextLogger** (`pkg/steplogger/text.go`): Implementation with spinner animations for text output +- **NoOpLogger** (`pkg/steplogger/noop.go`): Silent implementation for JSON mode and tests +- **Context Integration** (`pkg/steplogger/context.go`): Attach/retrieve loggers from context + +## Injecting StepLogger into Context + +Commands are responsible for creating and injecting the appropriate StepLogger into the context before calling runtime methods: + +```go +func (c *myCmd) run(cmd *cobra.Command, args []string) error { + // Create appropriate logger based on output mode + var logger steplogger.StepLogger + if c.output == "json" { + // No step logging in JSON mode + logger = steplogger.NewNoOpLogger() + } else { + // Use text logger with spinners for text output + logger = steplogger.NewTextLogger(cmd.ErrOrStderr()) + } + defer logger.Complete() + + // Attach logger to context + ctx := steplogger.WithLogger(cmd.Context(), logger) + + // Pass context to runtime methods + info, err := runtime.Create(ctx, params) + if err != nil { + return err + } + + return nil +} +``` + +## Logger Selection Rules + +- **JSON mode** (`--output json`): Use `steplogger.NewNoOpLogger()` - completely silent, no output +- **Text mode** (default): Use `steplogger.NewTextLogger(cmd.ErrOrStderr())` - displays spinners and messages to stderr + +## Important Notes + +- Always call `defer logger.Complete()` immediately after creating the logger +- Attach the logger to context using `steplogger.WithLogger(cmd.Context(), logger)` +- Pass the context (not `cmd.Context()`) to all runtime methods +- Output to stderr (`cmd.ErrOrStderr()`) so it doesn't interfere with stdout JSON output + +## Using StepLogger in Runtime Methods + +All runtime methods that accept a `context.Context` should use the StepLogger for user feedback. + +**Note for Runtime Implementers:** You don't need to create or inject the StepLogger - it's already in the context. Simply retrieve it using `steplogger.FromContext(ctx)` and use it as shown below: + +```go +func (r *myRuntime) Create(ctx context.Context, params runtime.CreateParams) (runtime.RuntimeInfo, error) { + logger := steplogger.FromContext(ctx) + defer logger.Complete() + + // Step 1: Create resources + logger.Start("Creating workspace directory", "Workspace directory created") + if err := r.createDirectory(params.Name); err != nil { + logger.Fail(err) + return runtime.RuntimeInfo{}, err + } + + // Step 2: Build image + logger.Start("Building container image", "Container image built") + if err := r.buildImage(ctx, params.Name); err != nil { + logger.Fail(err) + return runtime.RuntimeInfo{}, err + } + + // Step 3: Create instance + logger.Start("Creating instance", "Instance created") + info, err := r.createInstance(ctx, params) + if err != nil { + logger.Fail(err) + return runtime.RuntimeInfo{}, err + } + + return info, nil +} +``` + +## StepLogger Methods + +### Start(inProgress, completed string) + +Begin a new step with progress and completion messages. + +- Automatically completes the previous step if one exists +- `inProgress`: Message shown while the step is running (e.g., "Building container image") +- `completed`: Message shown when the step completes (e.g., "Container image built") + +### Complete() + +Mark the current step as successfully completed. + +- Typically called with `defer` at the start of the method to complete the last step + +### Fail(err error) + +Mark the current step as failed. + +- Displays the error message to the user +- Should be called before returning the error + +## Best Practices + +1. **Always call `Complete()` with defer** at the start of the method +2. **Use descriptive messages** that inform users about what's happening +3. **Call `Fail()` before returning errors** to show which step failed +4. **Retrieve logger from context** using `steplogger.FromContext(ctx)` +5. **Don't worry about JSON mode** - the NoOpLogger is automatically used when `--output json` is set + +## Example Pattern for All Runtime Methods + +```go +// Create +func (r *myRuntime) Create(ctx context.Context, params runtime.CreateParams) (runtime.RuntimeInfo, error) { + logger := steplogger.FromContext(ctx) + defer logger.Complete() + + logger.Start("Creating resource", "Resource created") + // ... implementation ... +} + +// Start +func (r *myRuntime) Start(ctx context.Context, id string) (runtime.RuntimeInfo, error) { + logger := steplogger.FromContext(ctx) + defer logger.Complete() + + logger.Start("Starting instance", "Instance started") + // ... implementation ... +} + +// Stop +func (r *myRuntime) Stop(ctx context.Context, id string) error { + logger := steplogger.FromContext(ctx) + defer logger.Complete() + + logger.Start("Stopping instance", "Instance stopped") + // ... implementation ... +} + +// Remove +func (r *myRuntime) Remove(ctx context.Context, id string) error { + logger := steplogger.FromContext(ctx) + defer logger.Complete() + + logger.Start("Removing instance", "Instance removed") + // ... implementation ... +} +``` + +## Automatic Behavior + +- **Text Mode**: Displays animated spinners during operations and completion checkmarks +- **JSON Mode**: Silent - no output to avoid polluting JSON responses +- **Testing**: Use `steplogger.NewNoOpLogger()` or don't attach a logger (defaults to NoOp) + +## Testing StepLogger + +Create a fake step logger to verify step behavior in tests: + +```go +// Create fake logger +type fakeStepLogger struct { + startCalls []stepCall + failCalls []error + completeCalls int +} + +type stepCall struct { + inProgress string + completed string +} + +func (f *fakeStepLogger) Start(inProgress, completed string) { + f.startCalls = append(f.startCalls, stepCall{inProgress, completed}) +} + +func (f *fakeStepLogger) Fail(err error) { + f.failCalls = append(f.failCalls, err) +} + +func (f *fakeStepLogger) Complete() { + f.completeCalls++ +} + +// Use in tests +func TestCreate_StepLogger(t *testing.T) { + fakeLogger := &fakeStepLogger{} + ctx := steplogger.WithLogger(context.Background(), fakeLogger) + + _, err := runtime.Create(ctx, params) + + // Verify step calls + if len(fakeLogger.startCalls) != 3 { + t.Errorf("Expected 3 Start() calls, got %d", len(fakeLogger.startCalls)) + } + if fakeLogger.completeCalls != 1 { + t.Errorf("Expected 1 Complete() call, got %d", fakeLogger.completeCalls) + } +} +``` + +## Reference Implementation + +See `pkg/runtime/podman/` for complete examples: +- `create.go` - Multi-step Create operation with 4 steps +- `start.go` - Start operation with verification step +- `stop.go` - Simple single-step Stop operation +- `remove.go` - Remove operation with state checking + +See `pkg/runtime/podman/steplogger_test.go` and step logger tests in `create_test.go`, `start_test.go`, `stop_test.go`, `remove_test.go`. + +## Related Skills + +- `/working-with-runtime-system` - Runtime architecture overview +- `/add-runtime` - Creating new runtimes with StepLogger +- `/implementing-command-patterns` - Command patterns including StepLogger integration + +## References + +- **StepLogger Interface**: `pkg/steplogger/steplogger.go` +- **TextLogger**: `pkg/steplogger/text.go` +- **NoOpLogger**: `pkg/steplogger/noop.go` +- **Context Integration**: `pkg/steplogger/context.go` +- **Example Commands**: `pkg/cmd/init.go`, `pkg/cmd/workspace_start.go`, `pkg/cmd/workspace_stop.go`, `pkg/cmd/workspace_remove.go`