diff --git a/AGENTS.md b/AGENTS.md index 6cd147b..83f90fe 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -129,6 +129,63 @@ All modules (packages outside of `cmd/`) MUST follow the interface-based design **This pattern is MANDATORY for all new modules in `pkg/`.** +### JSON Storage Structure + +When designing JSON storage structures for persistent data, use **nested objects with subfields** instead of flat structures with naming conventions. + +**Preferred Pattern (nested structure):** +```json +{ + "id": "dc610bffa75f21b5b043f98aff12b157fb16fae6c0ac3139c28f85d6defbe017", + "paths": { + "source": "/Users/user/project", + "configuration": "/Users/user/project/.kortex" + } +} +``` + +**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 +- **Extensibility** - Easy to add new subfields without polluting the top level +- **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"` +} +``` + ### Skills System Skills are reusable capabilities that can be discovered and executed by AI agents: - **Location**: `skills//SKILL.md` @@ -170,6 +227,130 @@ func NewExampleCmd() *cobra.Command { rootCmd.AddCommand(NewExampleCmd()) ``` +### 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. + +### 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: + - Use `t.Run()` for subtests within a parent test function + - Test with different argument/flag combinations + - Verify struct fields are set correctly + - Use `t.TempDir()` for temporary directories (automatic cleanup) + +2. **E2E Tests** - Test the full command execution: + - Execute via `rootCmd.Execute()` + - 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) + +**Reference:** See `pkg/cmd/init_test.go` for complete examples of both `preRun` unit tests (in `TestInitCmd_PreRun`) and E2E tests (in `TestInitCmd_E2E`). + +### Working with the Instances Manager + +When commands need to interact with workspaces: + +```go +// In preRun - create manager from storage flag +storageDir, _ := cmd.Flags().GetString("storage") +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) +} +``` + +### Cross-Platform Path Handling + +**IMPORTANT**: All path operations must be cross-platform compatible (Linux, macOS, Windows). + +**Rules:** +- Always use `filepath.Join()` for path construction (never hardcode "/" or "\\") +- 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 temporary directories in tests + +**Examples:** + +```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! + +// GOOD: Test assertions +expectedPath := filepath.Join(".", "relative", "path") +if result != expectedPath { + t.Errorf("Expected %s, got %s", expectedPath, result) +} + +// GOOD: Temporary directories in tests +tempDir := t.TempDir() // Automatic cleanup +sourcesDir := t.TempDir() +``` + ## Copyright Headers All source files must include Apache License 2.0 copyright headers with Red Hat copyright. Use the `/copyright-headers` skill to add or update headers automatically. The current year is 2026. diff --git a/pkg/cmd/init.go b/pkg/cmd/init.go new file mode 100644 index 0000000..0e70a9a --- /dev/null +++ b/pkg/cmd/init.go @@ -0,0 +1,130 @@ +/********************************************************************** + * Copyright (C) 2026 Red Hat, Inc. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + * + * SPDX-License-Identifier: Apache-2.0 + **********************************************************************/ + +package cmd + +import ( + "fmt" + "path/filepath" + + "github.com/kortex-hub/kortex-cli/pkg/instances" + "github.com/spf13/cobra" +) + +// initCmd contains the configuration for the init command +type initCmd struct { + sourcesDir string + workspaceConfigDir string + absSourcesDir string + absConfigDir string + manager instances.Manager + verbose bool +} + +// preRun validates the parameters and flags +func (i *initCmd) preRun(cmd *cobra.Command, args []string) error { + // Get storage directory from global flag + storageDir, err := cmd.Flags().GetString("storage") + if err != nil { + return fmt.Errorf("failed to read --storage flag: %w", err) + } + + // Create manager + manager, err := instances.NewManager(storageDir) + if err != nil { + return fmt.Errorf("failed to create manager: %w", err) + } + i.manager = manager + + // Get sources directory (default to current directory) + i.sourcesDir = "." + if len(args) > 0 { + i.sourcesDir = args[0] + } + + // Convert to absolute path for clarity + absSourcesDir, err := filepath.Abs(i.sourcesDir) + if err != nil { + return fmt.Errorf("failed to resolve sources directory path: %w", err) + } + i.absSourcesDir = absSourcesDir + + // If workspace-configuration flag was not explicitly set, default to .kortex/ inside sources directory + if !cmd.Flags().Changed("workspace-configuration") { + i.workspaceConfigDir = filepath.Join(i.sourcesDir, ".kortex") + } + + // Convert workspace config to absolute path + absConfigDir, err := filepath.Abs(i.workspaceConfigDir) + if err != nil { + return fmt.Errorf("failed to resolve workspace configuration directory path: %w", err) + } + i.absConfigDir = absConfigDir + + return nil +} + +// run executes the init command logic +func (i *initCmd) run(cmd *cobra.Command, args []string) error { + // Create a new instance + instance, err := instances.NewInstance(i.absSourcesDir, i.absConfigDir) + if err != nil { + return fmt.Errorf("failed to create instance: %w", err) + } + + // Add the instance to the manager + addedInstance, err := i.manager.Add(instance) + if err != nil { + return fmt.Errorf("failed to add instance: %w", err) + } + + if i.verbose { + cmd.Printf("Registered workspace:\n") + cmd.Printf(" ID: %s\n", addedInstance.GetID()) + cmd.Printf(" Sources directory: %s\n", addedInstance.GetSourceDir()) + cmd.Printf(" Configuration directory: %s\n", addedInstance.GetConfigDir()) + } else { + cmd.Println(addedInstance.GetID()) + } + + return nil +} + +func NewInitCmd() *cobra.Command { + c := &initCmd{} + + cmd := &cobra.Command{ + Use: "init [sources-directory]", + Short: "Register a new workspace", + Long: `Register a new workspace with the specified sources directory and configuration location. + +The sources directory defaults to the current directory (.) if not specified. +The workspace configuration directory defaults to .kortex/ inside the sources directory if not specified.`, + Args: cobra.MaximumNArgs(1), + PreRunE: c.preRun, + RunE: c.run, + } + + // Add workspace-configuration flag + cmd.Flags().StringVar(&c.workspaceConfigDir, "workspace-configuration", "", "Directory for workspace configuration (default: /.kortex)") + + // Add verbose flag + cmd.Flags().BoolVarP(&c.verbose, "verbose", "v", false, "Show detailed output") + + return cmd +} diff --git a/pkg/cmd/init_test.go b/pkg/cmd/init_test.go new file mode 100644 index 0000000..ef2b645 --- /dev/null +++ b/pkg/cmd/init_test.go @@ -0,0 +1,654 @@ +/********************************************************************** + * Copyright (C) 2026 Red Hat, Inc. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + * + * SPDX-License-Identifier: Apache-2.0 + **********************************************************************/ + +package cmd + +import ( + "bytes" + "path/filepath" + "strings" + "testing" + + "github.com/kortex-hub/kortex-cli/pkg/instances" + "github.com/spf13/cobra" +) + +func TestInitCmd_PreRun(t *testing.T) { + t.Parallel() + + t.Run("default arguments", func(t *testing.T) { + t.Parallel() + + tempDir := t.TempDir() + + c := &initCmd{} + cmd := &cobra.Command{} + cmd.Flags().String("workspace-configuration", "", "test flag") + cmd.Flags().String("storage", tempDir, "test storage flag") + + args := []string{} + + err := c.preRun(cmd, args) + if err != nil { + t.Fatalf("preRun() failed: %v", err) + } + + if c.manager == nil { + t.Error("Expected manager to be created") + } + + if c.sourcesDir != "." { + t.Errorf("Expected sourcesDir to be '.', got %s", c.sourcesDir) + } + + expectedAbsSourcesDir, _ := filepath.Abs(".") + if c.absSourcesDir != expectedAbsSourcesDir { + t.Errorf("Expected absSourcesDir to be %s, got %s", expectedAbsSourcesDir, c.absSourcesDir) + } + + expectedConfigDir := filepath.Join(".", ".kortex") + if c.workspaceConfigDir != expectedConfigDir { + t.Errorf("Expected workspaceConfigDir to be %s, got %s", expectedConfigDir, c.workspaceConfigDir) + } + + expectedAbsConfigDir, _ := filepath.Abs(expectedConfigDir) + if c.absConfigDir != expectedAbsConfigDir { + t.Errorf("Expected absConfigDir to be %s, got %s", expectedAbsConfigDir, c.absConfigDir) + } + }) + + t.Run("with sources directory", func(t *testing.T) { + t.Parallel() + + tempDir := t.TempDir() + sourcesDir := t.TempDir() + + c := &initCmd{} + cmd := &cobra.Command{} + cmd.Flags().String("workspace-configuration", "", "test flag") + cmd.Flags().String("storage", tempDir, "test storage flag") + + args := []string{sourcesDir} + + err := c.preRun(cmd, args) + if err != nil { + t.Fatalf("preRun() failed: %v", err) + } + + if c.manager == nil { + t.Error("Expected manager to be created") + } + + if c.sourcesDir != sourcesDir { + t.Errorf("Expected sourcesDir to be %s, got %s", sourcesDir, c.sourcesDir) + } + + expectedAbsSourcesDir, _ := filepath.Abs(sourcesDir) + if c.absSourcesDir != expectedAbsSourcesDir { + t.Errorf("Expected absSourcesDir to be %s, got %s", expectedAbsSourcesDir, c.absSourcesDir) + } + + expectedConfigDir := filepath.Join(sourcesDir, ".kortex") + if c.workspaceConfigDir != expectedConfigDir { + t.Errorf("Expected workspaceConfigDir to be %s, got %s", expectedConfigDir, c.workspaceConfigDir) + } + + expectedAbsConfigDir, _ := filepath.Abs(expectedConfigDir) + if c.absConfigDir != expectedAbsConfigDir { + t.Errorf("Expected absConfigDir to be %s, got %s", expectedAbsConfigDir, c.absConfigDir) + } + }) + + t.Run("with workspace configuration flag", func(t *testing.T) { + t.Parallel() + + tempDir := t.TempDir() + configDir := t.TempDir() + + c := &initCmd{ + workspaceConfigDir: configDir, + } + cmd := &cobra.Command{} + cmd.Flags().String("workspace-configuration", "", "test flag") + cmd.Flags().Set("workspace-configuration", configDir) + cmd.Flags().String("storage", tempDir, "test storage flag") + + args := []string{} + + err := c.preRun(cmd, args) + if err != nil { + t.Fatalf("preRun() failed: %v", err) + } + + if c.manager == nil { + t.Error("Expected manager to be created") + } + + if c.sourcesDir != "." { + t.Errorf("Expected sourcesDir to be '.', got %s", c.sourcesDir) + } + + if c.workspaceConfigDir != configDir { + t.Errorf("Expected workspaceConfigDir to be %s, got %s", configDir, c.workspaceConfigDir) + } + + expectedAbsConfigDir, _ := filepath.Abs(configDir) + if c.absConfigDir != expectedAbsConfigDir { + t.Errorf("Expected absConfigDir to be %s, got %s", expectedAbsConfigDir, c.absConfigDir) + } + }) + + t.Run("with both arguments", func(t *testing.T) { + t.Parallel() + + tempDir := t.TempDir() + sourcesDir := t.TempDir() + configDir := t.TempDir() + + c := &initCmd{ + workspaceConfigDir: configDir, + } + cmd := &cobra.Command{} + cmd.Flags().String("workspace-configuration", "", "test flag") + cmd.Flags().Set("workspace-configuration", configDir) + cmd.Flags().String("storage", tempDir, "test storage flag") + + args := []string{sourcesDir} + + err := c.preRun(cmd, args) + if err != nil { + t.Fatalf("preRun() failed: %v", err) + } + + if c.manager == nil { + t.Error("Expected manager to be created") + } + + if c.sourcesDir != sourcesDir { + t.Errorf("Expected sourcesDir to be %s, got %s", sourcesDir, c.sourcesDir) + } + + expectedAbsSourcesDir, _ := filepath.Abs(sourcesDir) + if c.absSourcesDir != expectedAbsSourcesDir { + t.Errorf("Expected absSourcesDir to be %s, got %s", expectedAbsSourcesDir, c.absSourcesDir) + } + + if c.workspaceConfigDir != configDir { + t.Errorf("Expected workspaceConfigDir to be %s, got %s", configDir, c.workspaceConfigDir) + } + + expectedAbsConfigDir, _ := filepath.Abs(configDir) + if c.absConfigDir != expectedAbsConfigDir { + t.Errorf("Expected absConfigDir to be %s, got %s", expectedAbsConfigDir, c.absConfigDir) + } + }) + + t.Run("relative sources directory", func(t *testing.T) { + t.Parallel() + + tempDir := t.TempDir() + relativePath := filepath.Join(".", "relative", "path") + + c := &initCmd{} + cmd := &cobra.Command{} + cmd.Flags().String("workspace-configuration", "", "test flag") + cmd.Flags().String("storage", tempDir, "test storage flag") + + args := []string{relativePath} + + err := c.preRun(cmd, args) + if err != nil { + t.Fatalf("preRun() failed: %v", err) + } + + if c.manager == nil { + t.Error("Expected manager to be created") + } + + if c.sourcesDir != relativePath { + t.Errorf("Expected sourcesDir to be %s, got %s", relativePath, c.sourcesDir) + } + + expectedAbsSourcesDir, _ := filepath.Abs(relativePath) + if c.absSourcesDir != expectedAbsSourcesDir { + t.Errorf("Expected absSourcesDir to be %s, got %s", expectedAbsSourcesDir, c.absSourcesDir) + } + + expectedConfigDir := filepath.Join(relativePath, ".kortex") + if c.workspaceConfigDir != expectedConfigDir { + t.Errorf("Expected workspaceConfigDir to be %s, got %s", expectedConfigDir, c.workspaceConfigDir) + } + }) +} + +func TestInitCmd_E2E(t *testing.T) { + t.Parallel() + + t.Run("registers workspace with default arguments", func(t *testing.T) { + t.Parallel() + + storageDir := t.TempDir() + + rootCmd := NewRootCmd() + buf := new(bytes.Buffer) + rootCmd.SetOut(buf) + rootCmd.SetArgs([]string{"--storage", storageDir, "init"}) + + err := rootCmd.Execute() + if err != nil { + t.Fatalf("Execute() failed: %v", err) + } + + // Verify instance was created + manager, err := instances.NewManager(storageDir) + if err != nil { + t.Fatalf("Failed to create manager: %v", err) + } + + instancesList, err := manager.List() + if err != nil { + t.Fatalf("Failed to list instances: %v", err) + } + + if len(instancesList) != 1 { + t.Fatalf("Expected 1 instance, got %d", len(instancesList)) + } + + inst := instancesList[0] + + // Verify instance has a non-empty ID + if inst.GetID() == "" { + t.Error("Expected instance to have a non-empty ID") + } + + // Verify output contains only the ID (default non-verbose output) + output := strings.TrimSpace(buf.String()) + if output != inst.GetID() { + t.Errorf("Expected output to be just the ID %s, got: %s", inst.GetID(), output) + } + + // Verify sources directory is current directory (absolute) + expectedAbsSourcesDir, _ := filepath.Abs(".") + if inst.GetSourceDir() != expectedAbsSourcesDir { + t.Errorf("Expected source dir %s, got %s", expectedAbsSourcesDir, inst.GetSourceDir()) + } + + // Verify config directory defaults to .kortex in current directory + expectedConfigDir := filepath.Join(expectedAbsSourcesDir, ".kortex") + if inst.GetConfigDir() != expectedConfigDir { + t.Errorf("Expected config dir %s, got %s", expectedConfigDir, inst.GetConfigDir()) + } + + // Verify paths are absolute + if !filepath.IsAbs(inst.GetSourceDir()) { + t.Errorf("Expected source dir to be absolute, got %s", inst.GetSourceDir()) + } + if !filepath.IsAbs(inst.GetConfigDir()) { + t.Errorf("Expected config dir to be absolute, got %s", inst.GetConfigDir()) + } + }) + + t.Run("registers workspace with custom sources directory", func(t *testing.T) { + t.Parallel() + + storageDir := t.TempDir() + sourcesDir := t.TempDir() + + rootCmd := NewRootCmd() + buf := new(bytes.Buffer) + rootCmd.SetOut(buf) + rootCmd.SetArgs([]string{"--storage", storageDir, "init", sourcesDir}) + + err := rootCmd.Execute() + if err != nil { + t.Fatalf("Execute() failed: %v", err) + } + + // Verify instance was created with correct paths + manager, err := instances.NewManager(storageDir) + if err != nil { + t.Fatalf("Failed to create manager: %v", err) + } + + instancesList, err := manager.List() + if err != nil { + t.Fatalf("Failed to list instances: %v", err) + } + + if len(instancesList) != 1 { + t.Fatalf("Expected 1 instance, got %d", len(instancesList)) + } + + inst := instancesList[0] + + // Verify instance has a non-empty ID + if inst.GetID() == "" { + t.Error("Expected instance to have a non-empty ID") + } + + // Verify output contains only the ID (default non-verbose output) + output := strings.TrimSpace(buf.String()) + if output != inst.GetID() { + t.Errorf("Expected output to be just the ID %s, got: %s", inst.GetID(), output) + } + + expectedAbsSourcesDir, _ := filepath.Abs(sourcesDir) + if inst.GetSourceDir() != expectedAbsSourcesDir { + t.Errorf("Expected source dir %s, got %s", expectedAbsSourcesDir, inst.GetSourceDir()) + } + + expectedConfigDir := filepath.Join(expectedAbsSourcesDir, ".kortex") + if inst.GetConfigDir() != expectedConfigDir { + t.Errorf("Expected config dir %s, got %s", expectedConfigDir, inst.GetConfigDir()) + } + + // Verify paths are absolute + if !filepath.IsAbs(inst.GetSourceDir()) { + t.Errorf("Expected source dir to be absolute, got %s", inst.GetSourceDir()) + } + if !filepath.IsAbs(inst.GetConfigDir()) { + t.Errorf("Expected config dir to be absolute, got %s", inst.GetConfigDir()) + } + }) + + t.Run("registers workspace with custom configuration directory", func(t *testing.T) { + t.Parallel() + + storageDir := t.TempDir() + configDir := t.TempDir() + + rootCmd := NewRootCmd() + buf := new(bytes.Buffer) + rootCmd.SetOut(buf) + rootCmd.SetArgs([]string{"--storage", storageDir, "init", "--workspace-configuration", configDir}) + + err := rootCmd.Execute() + if err != nil { + t.Fatalf("Execute() failed: %v", err) + } + + // Verify instance was created with correct paths + manager, err := instances.NewManager(storageDir) + if err != nil { + t.Fatalf("Failed to create manager: %v", err) + } + + instancesList, err := manager.List() + if err != nil { + t.Fatalf("Failed to list instances: %v", err) + } + + if len(instancesList) != 1 { + t.Fatalf("Expected 1 instance, got %d", len(instancesList)) + } + + inst := instancesList[0] + + // Verify instance has a non-empty ID + if inst.GetID() == "" { + t.Error("Expected instance to have a non-empty ID") + } + + // Verify output contains only the ID (default non-verbose output) + output := strings.TrimSpace(buf.String()) + if output != inst.GetID() { + t.Errorf("Expected output to be just the ID %s, got: %s", inst.GetID(), output) + } + + // Verify sources directory defaults to current directory + expectedAbsSourcesDir, _ := filepath.Abs(".") + if inst.GetSourceDir() != expectedAbsSourcesDir { + t.Errorf("Expected source dir %s, got %s", expectedAbsSourcesDir, inst.GetSourceDir()) + } + + expectedAbsConfigDir, _ := filepath.Abs(configDir) + if inst.GetConfigDir() != expectedAbsConfigDir { + t.Errorf("Expected config dir %s, got %s", expectedAbsConfigDir, inst.GetConfigDir()) + } + + // Verify paths are absolute + if !filepath.IsAbs(inst.GetSourceDir()) { + t.Errorf("Expected source dir to be absolute, got %s", inst.GetSourceDir()) + } + if !filepath.IsAbs(inst.GetConfigDir()) { + t.Errorf("Expected config dir to be absolute, got %s", inst.GetConfigDir()) + } + }) + + t.Run("registers workspace with both custom directories", func(t *testing.T) { + t.Parallel() + + storageDir := t.TempDir() + sourcesDir := t.TempDir() + configDir := t.TempDir() + + rootCmd := NewRootCmd() + buf := new(bytes.Buffer) + rootCmd.SetOut(buf) + rootCmd.SetArgs([]string{"--storage", storageDir, "init", sourcesDir, "--workspace-configuration", configDir}) + + err := rootCmd.Execute() + if err != nil { + t.Fatalf("Execute() failed: %v", err) + } + + // Verify instance was created with correct paths + manager, err := instances.NewManager(storageDir) + if err != nil { + t.Fatalf("Failed to create manager: %v", err) + } + + instancesList, err := manager.List() + if err != nil { + t.Fatalf("Failed to list instances: %v", err) + } + + if len(instancesList) != 1 { + t.Fatalf("Expected 1 instance, got %d", len(instancesList)) + } + + inst := instancesList[0] + + // Verify instance has a non-empty ID + if inst.GetID() == "" { + t.Error("Expected instance to have a non-empty ID") + } + + // Verify output contains only the ID (default non-verbose output) + output := strings.TrimSpace(buf.String()) + if output != inst.GetID() { + t.Errorf("Expected output to be just the ID %s, got: %s", inst.GetID(), output) + } + + expectedAbsSourcesDir, _ := filepath.Abs(sourcesDir) + if inst.GetSourceDir() != expectedAbsSourcesDir { + t.Errorf("Expected source dir %s, got %s", expectedAbsSourcesDir, inst.GetSourceDir()) + } + + expectedAbsConfigDir, _ := filepath.Abs(configDir) + if inst.GetConfigDir() != expectedAbsConfigDir { + t.Errorf("Expected config dir %s, got %s", expectedAbsConfigDir, inst.GetConfigDir()) + } + + // Verify paths are absolute + if !filepath.IsAbs(inst.GetSourceDir()) { + t.Errorf("Expected source dir to be absolute, got %s", inst.GetSourceDir()) + } + if !filepath.IsAbs(inst.GetConfigDir()) { + t.Errorf("Expected config dir to be absolute, got %s", inst.GetConfigDir()) + } + }) + + t.Run("registers multiple workspaces", func(t *testing.T) { + t.Parallel() + + storageDir := t.TempDir() + sourcesDir1 := t.TempDir() + sourcesDir2 := t.TempDir() + + // Register first workspace + rootCmd1 := NewRootCmd() + buf1 := new(bytes.Buffer) + rootCmd1.SetOut(buf1) + rootCmd1.SetArgs([]string{"--storage", storageDir, "init", sourcesDir1}) + + err := rootCmd1.Execute() + if err != nil { + t.Fatalf("Execute() failed for first workspace: %v", err) + } + + // Register second workspace + rootCmd2 := NewRootCmd() + buf2 := new(bytes.Buffer) + rootCmd2.SetOut(buf2) + rootCmd2.SetArgs([]string{"--storage", storageDir, "init", sourcesDir2}) + + err = rootCmd2.Execute() + if err != nil { + t.Fatalf("Execute() failed for second workspace: %v", err) + } + + // Verify both instances exist + manager, err := instances.NewManager(storageDir) + if err != nil { + t.Fatalf("Failed to create manager: %v", err) + } + + instancesList, err := manager.List() + if err != nil { + t.Fatalf("Failed to list instances: %v", err) + } + + if len(instancesList) != 2 { + t.Fatalf("Expected 2 instances, got %d", len(instancesList)) + } + + // Verify both instances have unique IDs + if instancesList[0].GetID() == "" || instancesList[1].GetID() == "" { + t.Error("Expected both instances to have non-empty IDs") + } + if instancesList[0].GetID() == instancesList[1].GetID() { + t.Error("Expected instances to have unique IDs") + } + + // Verify both instances have correct source directories + expectedAbsSourcesDir1, _ := filepath.Abs(sourcesDir1) + expectedAbsSourcesDir2, _ := filepath.Abs(sourcesDir2) + + foundDir1 := false + foundDir2 := false + for _, inst := range instancesList { + if inst.GetSourceDir() == expectedAbsSourcesDir1 { + foundDir1 = true + // Verify config dir for first workspace + expectedConfigDir1 := filepath.Join(expectedAbsSourcesDir1, ".kortex") + if inst.GetConfigDir() != expectedConfigDir1 { + t.Errorf("Expected config dir %s for first workspace, got %s", expectedConfigDir1, inst.GetConfigDir()) + } + } + if inst.GetSourceDir() == expectedAbsSourcesDir2 { + foundDir2 = true + // Verify config dir for second workspace + expectedConfigDir2 := filepath.Join(expectedAbsSourcesDir2, ".kortex") + if inst.GetConfigDir() != expectedConfigDir2 { + t.Errorf("Expected config dir %s for second workspace, got %s", expectedConfigDir2, inst.GetConfigDir()) + } + } + + // Verify paths are absolute + if !filepath.IsAbs(inst.GetSourceDir()) { + t.Errorf("Expected source dir to be absolute, got %s", inst.GetSourceDir()) + } + if !filepath.IsAbs(inst.GetConfigDir()) { + t.Errorf("Expected config dir to be absolute, got %s", inst.GetConfigDir()) + } + } + + if !foundDir1 { + t.Errorf("Expected to find instance with source dir %s", expectedAbsSourcesDir1) + } + if !foundDir2 { + t.Errorf("Expected to find instance with source dir %s", expectedAbsSourcesDir2) + } + }) + + t.Run("registers workspace with verbose flag", func(t *testing.T) { + t.Parallel() + + storageDir := t.TempDir() + sourcesDir := t.TempDir() + + rootCmd := NewRootCmd() + buf := new(bytes.Buffer) + rootCmd.SetOut(buf) + rootCmd.SetArgs([]string{"--storage", storageDir, "init", sourcesDir, "--verbose"}) + + err := rootCmd.Execute() + if err != nil { + t.Fatalf("Execute() failed: %v", err) + } + + output := buf.String() + + // Verify verbose output contains expected strings + if !strings.Contains(output, "Registered workspace:") { + t.Errorf("Expected verbose output to contain 'Registered workspace:', got: %s", output) + } + if !strings.Contains(output, "ID:") { + t.Errorf("Expected verbose output to contain 'ID:', got: %s", output) + } + if !strings.Contains(output, "Sources directory:") { + t.Errorf("Expected verbose output to contain 'Sources directory:', got: %s", output) + } + if !strings.Contains(output, "Configuration directory:") { + t.Errorf("Expected verbose output to contain 'Configuration directory:', got: %s", output) + } + + // Verify instance was created with correct paths + manager, err := instances.NewManager(storageDir) + if err != nil { + t.Fatalf("Failed to create manager: %v", err) + } + + instancesList, err := manager.List() + if err != nil { + t.Fatalf("Failed to list instances: %v", err) + } + + if len(instancesList) != 1 { + t.Fatalf("Expected 1 instance, got %d", len(instancesList)) + } + + inst := instancesList[0] + + // Verify verbose output contains the actual values + expectedAbsSourcesDir, _ := filepath.Abs(sourcesDir) + if !strings.Contains(output, expectedAbsSourcesDir) { + t.Errorf("Expected verbose output to contain sources directory %s, got: %s", expectedAbsSourcesDir, output) + } + + expectedConfigDir := filepath.Join(expectedAbsSourcesDir, ".kortex") + if !strings.Contains(output, expectedConfigDir) { + t.Errorf("Expected verbose output to contain config directory %s, got: %s", expectedConfigDir, output) + } + + if !strings.Contains(output, inst.GetID()) { + t.Errorf("Expected verbose output to contain instance ID %s, got: %s", inst.GetID(), output) + } + }) +} diff --git a/pkg/cmd/root.go b/pkg/cmd/root.go index 9017a9e..8a5b8d1 100644 --- a/pkg/cmd/root.go +++ b/pkg/cmd/root.go @@ -45,6 +45,7 @@ func NewRootCmd() *cobra.Command { // Add subcommands rootCmd.AddCommand(NewVersionCmd()) + rootCmd.AddCommand(NewInitCmd()) // Global flags rootCmd.PersistentFlags().String("storage", defaultStoragePath, "Directory where kortex-cli will store all its files") diff --git a/pkg/instances/instance.go b/pkg/instances/instance.go index 5e635a1..f8639b8 100644 --- a/pkg/instances/instance.go +++ b/pkg/instances/instance.go @@ -29,14 +29,20 @@ var ( ErrInvalidPath = errors.New("invalid path") ) +// InstancePaths represents the paths in an instance +type InstancePaths struct { + // Source is the directory containing source files (absolute path) + Source string `json:"source"` + // Configuration is the directory containing workspace configuration (absolute path) + Configuration string `json:"configuration"` +} + // InstanceData represents the serializable data of an instance type InstanceData struct { // ID is the unique identifier for the instance ID string `json:"id"` - // SourceDir is the directory containing source files (absolute path) - SourceDir string `json:"source_dir"` - // ConfigDir is the directory containing workspace configuration (absolute path) - ConfigDir string `json:"config_dir"` + // Paths contains the source and configuration directories + Paths InstancePaths `json:"paths"` } // Instance represents a workspace instance with source and configuration directories. @@ -101,9 +107,11 @@ func (i *instance) IsAccessible() bool { // Dump returns the serializable data of the instance func (i *instance) Dump() InstanceData { return InstanceData{ - ID: i.ID, - SourceDir: i.SourceDir, - ConfigDir: i.ConfigDir, + ID: i.ID, + Paths: InstancePaths{ + Source: i.SourceDir, + Configuration: i.ConfigDir, + }, } } @@ -142,17 +150,17 @@ func NewInstanceFromData(data InstanceData) (Instance, error) { if data.ID == "" { return nil, errors.New("instance ID cannot be empty") } - if data.SourceDir == "" { + if data.Paths.Source == "" { return nil, ErrInvalidPath } - if data.ConfigDir == "" { + if data.Paths.Configuration == "" { return nil, ErrInvalidPath } return &instance{ ID: data.ID, - SourceDir: data.SourceDir, - ConfigDir: data.ConfigDir, + SourceDir: data.Paths.Source, + ConfigDir: data.Paths.Configuration, }, nil } diff --git a/pkg/instances/manager_test.go b/pkg/instances/manager_test.go index 9a6d9cd..e755321 100644 --- a/pkg/instances/manager_test.go +++ b/pkg/instances/manager_test.go @@ -53,9 +53,11 @@ func (f *fakeInstance) IsAccessible() bool { func (f *fakeInstance) Dump() InstanceData { return InstanceData{ - ID: f.id, - SourceDir: f.sourceDir, - ConfigDir: f.configDir, + ID: f.id, + Paths: InstancePaths{ + Source: f.sourceDir, + Configuration: f.configDir, + }, } } @@ -74,18 +76,18 @@ func fakeInstanceFactory(data InstanceData) (Instance, error) { if data.ID == "" { return nil, errors.New("instance ID cannot be empty") } - if data.SourceDir == "" { + if data.Paths.Source == "" { return nil, ErrInvalidPath } - if data.ConfigDir == "" { + if data.Paths.Configuration == "" { return nil, ErrInvalidPath } // For testing, we assume instances are accessible by default // Tests can verify accessibility behavior separately return &fakeInstance{ id: data.ID, - sourceDir: data.SourceDir, - configDir: data.ConfigDir, + sourceDir: data.Paths.Source, + configDir: data.Paths.Configuration, accessible: true, }, nil } @@ -557,13 +559,13 @@ func TestManager_Reconcile(t *testing.T) { if data.ID == "" { return nil, errors.New("instance ID cannot be empty") } - if data.SourceDir == "" || data.ConfigDir == "" { + if data.Paths.Source == "" || data.Paths.Configuration == "" { return nil, ErrInvalidPath } return &fakeInstance{ id: data.ID, - sourceDir: data.SourceDir, - configDir: data.ConfigDir, + sourceDir: data.Paths.Source, + configDir: data.Paths.Configuration, accessible: false, // Always inaccessible for this test }, nil } @@ -593,13 +595,13 @@ func TestManager_Reconcile(t *testing.T) { if data.ID == "" { return nil, errors.New("instance ID cannot be empty") } - if data.SourceDir == "" || data.ConfigDir == "" { + if data.Paths.Source == "" || data.Paths.Configuration == "" { return nil, ErrInvalidPath } return &fakeInstance{ id: data.ID, - sourceDir: data.SourceDir, - configDir: data.ConfigDir, + sourceDir: data.Paths.Source, + configDir: data.Paths.Configuration, accessible: false, // Always inaccessible for this test }, nil } @@ -629,13 +631,13 @@ func TestManager_Reconcile(t *testing.T) { if data.ID == "" { return nil, errors.New("instance ID cannot be empty") } - if data.SourceDir == "" || data.ConfigDir == "" { + if data.Paths.Source == "" || data.Paths.Configuration == "" { return nil, ErrInvalidPath } return &fakeInstance{ id: data.ID, - sourceDir: data.SourceDir, - configDir: data.ConfigDir, + sourceDir: data.Paths.Source, + configDir: data.Paths.Configuration, accessible: false, // Always inaccessible for this test }, nil } @@ -672,14 +674,14 @@ func TestManager_Reconcile(t *testing.T) { if data.ID == "" { return nil, errors.New("instance ID cannot be empty") } - if data.SourceDir == "" || data.ConfigDir == "" { + if data.Paths.Source == "" || data.Paths.Configuration == "" { return nil, ErrInvalidPath } - accessible := data.SourceDir == accessibleSource + accessible := data.Paths.Source == accessibleSource return &fakeInstance{ id: data.ID, - sourceDir: data.SourceDir, - configDir: data.ConfigDir, + sourceDir: data.Paths.Source, + configDir: data.Paths.Configuration, accessible: accessible, }, nil } @@ -823,11 +825,11 @@ func TestManager_Persistence(t *testing.T) { if instances[0].ID != generatedID { t.Errorf("JSON ID = %v, want %v", instances[0].ID, generatedID) } - if instances[0].SourceDir != expectedSource { - t.Errorf("JSON SourceDir = %v, want %v", instances[0].SourceDir, expectedSource) + if instances[0].Paths.Source != expectedSource { + t.Errorf("JSON Paths.Source = %v, want %v", instances[0].Paths.Source, expectedSource) } - if instances[0].ConfigDir != expectedConfig { - t.Errorf("JSON ConfigDir = %v, want %v", instances[0].ConfigDir, expectedConfig) + if instances[0].Paths.Configuration != expectedConfig { + t.Errorf("JSON Paths.Configuration = %v, want %v", instances[0].Paths.Configuration, expectedConfig) } }) }