diff --git a/README.md b/README.md index e95aed4..8d00c5f 100644 --- a/README.md +++ b/README.md @@ -30,3 +30,92 @@ A standardized protocol for connecting AI agents to external data sources and to ### Skills Pre-configured capabilities or specialized functions that can be enabled for an agent. Skills extend what an agent can do, such as code review, testing, or specific domain knowledge. + +### Workspace +A registered directory containing your project source code and its configuration. Each workspace is tracked by kortex-cli with a unique ID and name for easy management. + +## Commands + +### `init` - Register a New Workspace + +Registers a new workspace with kortex-cli, making it available for agent launch and configuration. + +#### Usage + +```bash +kortex-cli init [sources-directory] [flags] +``` + +#### Arguments + +- `sources-directory` - Path to the directory containing your project source files (optional, defaults to current directory `.`) + +#### Flags + +- `--workspace-configuration ` - Directory for workspace configuration files (default: `/.kortex`) +- `--name, -n ` - Human-readable name for the workspace (default: generated from sources directory) +- `--verbose, -v` - Show detailed output including all workspace information +- `--storage ` - Storage directory for kortex-cli data (default: `$HOME/.kortex-cli`) + +#### Examples + +**Register the current directory:** +```bash +kortex-cli init +``` +Output: `a1b2c3d4e5f6...` (workspace ID) + +**Register a specific directory:** +```bash +kortex-cli init /path/to/myproject +``` + +**Register with a custom name:** +```bash +kortex-cli init /path/to/myproject --name "my-awesome-project" +``` + +**Register with custom configuration location:** +```bash +kortex-cli init /path/to/myproject --workspace-configuration /path/to/config +``` + +**View detailed output:** +```bash +kortex-cli init --verbose +``` +Output: +``` +Registered workspace: + ID: a1b2c3d4e5f6... + Name: myproject + Sources directory: /absolute/path/to/myproject + Configuration directory: /absolute/path/to/myproject/.kortex +``` + +#### Workspace Naming + +- If `--name` is not provided, the name is automatically generated from the last component of the sources directory path +- If a workspace with the same name already exists, kortex-cli automatically appends an increment (`-2`, `-3`, etc.) to ensure uniqueness + +**Examples:** +```bash +# First workspace in /home/user/project +kortex-cli init /home/user/project +# Name: "project" + +# Second workspace with the same directory name +kortex-cli init /home/user/another-location/project --name "project" +# Name: "project-2" + +# Third workspace with the same name +kortex-cli init /tmp/project --name "project" +# Name: "project-3" +``` + +#### Notes + +- All directory paths are converted to absolute paths for consistency +- The workspace ID is a unique identifier generated automatically +- Workspaces can be listed using the `workspace list` command +- The default configuration directory (`.kortex`) is created inside the sources directory unless specified otherwise diff --git a/pkg/cmd/init.go b/pkg/cmd/init.go index 0e70a9a..a920413 100644 --- a/pkg/cmd/init.go +++ b/pkg/cmd/init.go @@ -30,6 +30,7 @@ import ( type initCmd struct { sourcesDir string workspaceConfigDir string + name string absSourcesDir string absConfigDir string manager instances.Manager @@ -82,7 +83,11 @@ func (i *initCmd) preRun(cmd *cobra.Command, args []string) error { // 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) + instance, err := instances.NewInstance(instances.NewInstanceParams{ + SourceDir: i.absSourcesDir, + ConfigDir: i.absConfigDir, + Name: i.name, + }) if err != nil { return fmt.Errorf("failed to create instance: %w", err) } @@ -96,6 +101,7 @@ func (i *initCmd) run(cmd *cobra.Command, args []string) error { if i.verbose { cmd.Printf("Registered workspace:\n") cmd.Printf(" ID: %s\n", addedInstance.GetID()) + cmd.Printf(" Name: %s\n", addedInstance.GetName()) cmd.Printf(" Sources directory: %s\n", addedInstance.GetSourceDir()) cmd.Printf(" Configuration directory: %s\n", addedInstance.GetConfigDir()) } else { @@ -123,6 +129,9 @@ The workspace configuration directory defaults to .kortex/ inside the sources di // Add workspace-configuration flag cmd.Flags().StringVar(&c.workspaceConfigDir, "workspace-configuration", "", "Directory for workspace configuration (default: /.kortex)") + // Add name flag + cmd.Flags().StringVarP(&c.name, "name", "n", "", "Name for the workspace (default: generated from sources directory)") + // Add verbose flag cmd.Flags().BoolVarP(&c.verbose, "verbose", "v", false, "Show detailed output") diff --git a/pkg/cmd/init_test.go b/pkg/cmd/init_test.go index ef2b645..d11eb7e 100644 --- a/pkg/cmd/init_test.go +++ b/pkg/cmd/init_test.go @@ -276,6 +276,11 @@ func TestInitCmd_E2E(t *testing.T) { t.Error("Expected instance to have a non-empty ID") } + // Verify instance has a non-empty Name + if inst.GetName() == "" { + t.Error("Expected instance to have a non-empty Name") + } + // Verify output contains only the ID (default non-verbose output) output := strings.TrimSpace(buf.String()) if output != inst.GetID() { @@ -651,4 +656,186 @@ func TestInitCmd_E2E(t *testing.T) { t.Errorf("Expected verbose output to contain instance ID %s, got: %s", inst.GetID(), output) } }) + + t.Run("generates default name from source 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 name is generated from source directory + 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] + expectedName := filepath.Base(sourcesDir) + + if inst.GetName() != expectedName { + t.Errorf("Expected name %s, got %s", expectedName, inst.GetName()) + } + }) + + t.Run("uses custom name from flag", func(t *testing.T) { + t.Parallel() + + storageDir := t.TempDir() + sourcesDir := t.TempDir() + customName := "my-workspace" + + rootCmd := NewRootCmd() + buf := new(bytes.Buffer) + rootCmd.SetOut(buf) + rootCmd.SetArgs([]string{"--storage", storageDir, "init", sourcesDir, "--name", customName}) + + err := rootCmd.Execute() + if err != nil { + t.Fatalf("Execute() failed: %v", err) + } + + // Verify instance name is the custom name + 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] + + if inst.GetName() != customName { + t.Errorf("Expected name %s, got %s", customName, inst.GetName()) + } + }) + + t.Run("generates unique names with increments", func(t *testing.T) { + t.Parallel() + + storageDir := t.TempDir() + // Create three temp directories with the same base name pattern + parentDir := t.TempDir() + sourcesDir1 := filepath.Join(parentDir, "project") + sourcesDir2 := filepath.Join(parentDir, "project-other") + sourcesDir3 := filepath.Join(parentDir, "project-another") + + // Register first workspace with name "project" + 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 with the same name "project" (should become "project-2") + rootCmd2 := NewRootCmd() + buf2 := new(bytes.Buffer) + rootCmd2.SetOut(buf2) + rootCmd2.SetArgs([]string{"--storage", storageDir, "init", sourcesDir2, "--name", "project"}) + + err = rootCmd2.Execute() + if err != nil { + t.Fatalf("Execute() failed for second workspace: %v", err) + } + + // Register third workspace with the same name "project" (should become "project-3") + rootCmd3 := NewRootCmd() + buf3 := new(bytes.Buffer) + rootCmd3.SetOut(buf3) + rootCmd3.SetArgs([]string{"--storage", storageDir, "init", sourcesDir3, "--name", "project"}) + + err = rootCmd3.Execute() + if err != nil { + t.Fatalf("Execute() failed for third workspace: %v", err) + } + + // Verify all three instances have unique names + 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) != 3 { + t.Fatalf("Expected 3 instances, got %d", len(instancesList)) + } + + // Verify names are unique + names := make(map[string]bool) + for _, inst := range instancesList { + if names[inst.GetName()] { + t.Errorf("Duplicate name found: %s", inst.GetName()) + } + names[inst.GetName()] = true + } + + // Verify expected names are present + expectedNames := []string{"project", "project-2", "project-3"} + for _, expectedName := range expectedNames { + if !names[expectedName] { + t.Errorf("Expected name %s not found in instances", expectedName) + } + } + }) + + t.Run("verbose output includes name", func(t *testing.T) { + t.Parallel() + + storageDir := t.TempDir() + sourcesDir := t.TempDir() + customName := "my-workspace" + + rootCmd := NewRootCmd() + buf := new(bytes.Buffer) + rootCmd.SetOut(buf) + rootCmd.SetArgs([]string{"--storage", storageDir, "init", sourcesDir, "--name", customName, "--verbose"}) + + err := rootCmd.Execute() + if err != nil { + t.Fatalf("Execute() failed: %v", err) + } + + output := buf.String() + + // Verify verbose output contains the name + if !strings.Contains(output, "Name:") { + t.Errorf("Expected verbose output to contain 'Name:', got: %s", output) + } + if !strings.Contains(output, customName) { + t.Errorf("Expected verbose output to contain name %s, got: %s", customName, output) + } + }) } diff --git a/pkg/cmd/workspace_list_test.go b/pkg/cmd/workspace_list_test.go index c6cb11c..5f52e2d 100644 --- a/pkg/cmd/workspace_list_test.go +++ b/pkg/cmd/workspace_list_test.go @@ -94,7 +94,10 @@ func TestWorkspaceListCmd_E2E(t *testing.T) { t.Fatalf("Failed to create manager: %v", err) } - instance, err := instances.NewInstance(sourcesDir, filepath.Join(sourcesDir, ".kortex")) + instance, err := instances.NewInstance(instances.NewInstanceParams{ + SourceDir: sourcesDir, + ConfigDir: filepath.Join(sourcesDir, ".kortex"), + }) if err != nil { t.Fatalf("Failed to create instance: %v", err) } @@ -138,12 +141,18 @@ func TestWorkspaceListCmd_E2E(t *testing.T) { t.Fatalf("Failed to create manager: %v", err) } - instance1, err := instances.NewInstance(sourcesDir1, filepath.Join(sourcesDir1, ".kortex")) + instance1, err := instances.NewInstance(instances.NewInstanceParams{ + SourceDir: sourcesDir1, + ConfigDir: filepath.Join(sourcesDir1, ".kortex"), + }) if err != nil { t.Fatalf("Failed to create instance 1: %v", err) } - instance2, err := instances.NewInstance(sourcesDir2, filepath.Join(sourcesDir2, ".kortex")) + instance2, err := instances.NewInstance(instances.NewInstanceParams{ + SourceDir: sourcesDir2, + ConfigDir: filepath.Join(sourcesDir2, ".kortex"), + }) if err != nil { t.Fatalf("Failed to create instance 2: %v", err) } @@ -197,7 +206,10 @@ func TestWorkspaceListCmd_E2E(t *testing.T) { t.Fatalf("Failed to create manager: %v", err) } - instance, err := instances.NewInstance(sourcesDir, filepath.Join(sourcesDir, ".kortex")) + instance, err := instances.NewInstance(instances.NewInstanceParams{ + SourceDir: sourcesDir, + ConfigDir: filepath.Join(sourcesDir, ".kortex"), + }) if err != nil { t.Fatalf("Failed to create instance: %v", err) } diff --git a/pkg/instances/instance.go b/pkg/instances/instance.go index f8639b8..8d30670 100644 --- a/pkg/instances/instance.go +++ b/pkg/instances/instance.go @@ -41,16 +41,20 @@ type InstancePaths struct { type InstanceData struct { // ID is the unique identifier for the instance ID string `json:"id"` + // Name is the human-readable name for the instance + Name string `json:"name"` // Paths contains the source and configuration directories Paths InstancePaths `json:"paths"` } // Instance represents a workspace instance with source and configuration directories. -// Each instance is uniquely identified by its ID. +// Each instance is uniquely identified by its ID and has a human-readable name. // Instances must be created using NewInstance to ensure proper validation and path normalization. type Instance interface { // GetID returns the unique identifier for the instance. GetID() string + // GetName returns the human-readable name for the instance. + GetName() string // GetSourceDir returns the source directory. // The path is always absolute. GetSourceDir() string @@ -67,6 +71,8 @@ type Instance interface { type instance struct { // ID is the unique identifier for the instance ID string + // Name is the human-readable name for the instance + Name string // SourceDir is the directory containing source files. // This is always stored as an absolute path. SourceDir string @@ -83,6 +89,11 @@ func (i *instance) GetID() string { return i.ID } +// GetName returns the human-readable name for the instance +func (i *instance) GetName() string { + return i.Name +} + // GetSourceDir returns the source directory func (i *instance) GetSourceDir() string { return i.SourceDir @@ -107,7 +118,8 @@ func (i *instance) IsAccessible() bool { // Dump returns the serializable data of the instance func (i *instance) Dump() InstanceData { return InstanceData{ - ID: i.ID, + ID: i.ID, + Name: i.Name, Paths: InstancePaths{ Source: i.SourceDir, Configuration: i.ConfigDir, @@ -115,30 +127,41 @@ func (i *instance) Dump() InstanceData { } } +// NewInstanceParams contains the parameters for creating a new Instance +type NewInstanceParams struct { + // SourceDir is the directory containing source files + SourceDir string + // ConfigDir is the directory containing workspace configuration + ConfigDir string + // Name is the human-readable name for the instance (optional, will be generated if empty) + Name string +} + // NewInstance creates a new Instance with validated and normalized paths. // Both sourceDir and configDir are converted to absolute paths. -// The ID is empty and will be generated by the Manager when the instance is added to storage. -func NewInstance(sourceDir, configDir string) (Instance, error) { - if sourceDir == "" { +// The ID and Name (if empty) will be generated by the Manager when the instance is added to storage. +func NewInstance(params NewInstanceParams) (Instance, error) { + if params.SourceDir == "" { return nil, ErrInvalidPath } - if configDir == "" { + if params.ConfigDir == "" { return nil, ErrInvalidPath } // Convert to absolute paths - absSourceDir, err := filepath.Abs(sourceDir) + absSourceDir, err := filepath.Abs(params.SourceDir) if err != nil { return nil, err } - absConfigDir, err := filepath.Abs(configDir) + absConfigDir, err := filepath.Abs(params.ConfigDir) if err != nil { return nil, err } return &instance{ ID: "", // ID will be set by Manager when added + Name: params.Name, SourceDir: absSourceDir, ConfigDir: absConfigDir, }, nil @@ -150,6 +173,9 @@ func NewInstanceFromData(data InstanceData) (Instance, error) { if data.ID == "" { return nil, errors.New("instance ID cannot be empty") } + if data.Name == "" { + return nil, errors.New("instance name cannot be empty") + } if data.Paths.Source == "" { return nil, ErrInvalidPath } @@ -159,6 +185,7 @@ func NewInstanceFromData(data InstanceData) (Instance, error) { return &instance{ ID: data.ID, + Name: data.Name, SourceDir: data.Paths.Source, ConfigDir: data.Paths.Configuration, }, nil diff --git a/pkg/instances/instance_test.go b/pkg/instances/instance_test.go index d0729df..b32a713 100644 --- a/pkg/instances/instance_test.go +++ b/pkg/instances/instance_test.go @@ -30,7 +30,10 @@ func TestNewInstance(t *testing.T) { sourceDir := filepath.Join(tmpDir, "test-source") configDir := filepath.Join(tmpDir, "test-config") - inst, err := NewInstance(sourceDir, configDir) + inst, err := NewInstance(NewInstanceParams{ + SourceDir: sourceDir, + ConfigDir: configDir, + }) if err != nil { t.Fatalf("NewInstance() unexpected error = %v", err) } @@ -41,6 +44,9 @@ func TestNewInstance(t *testing.T) { if inst.GetID() != "" { t.Errorf("GetID() = %v, want empty string (ID should be set by Manager)", inst.GetID()) } + if inst.GetName() != "" { + t.Errorf("GetName() = %v, want empty string (Name should be set by Manager if not provided)", inst.GetName()) + } if inst.GetSourceDir() != sourceDir { t.Errorf("GetSourceDir() = %v, want %v", inst.GetSourceDir(), sourceDir) } @@ -52,7 +58,10 @@ func TestNewInstance(t *testing.T) { t.Run("converts relative paths to absolute", func(t *testing.T) { t.Parallel() - inst, err := NewInstance("source", "config") + inst, err := NewInstance(NewInstanceParams{ + SourceDir: "source", + ConfigDir: "config", + }) if err != nil { t.Fatalf("NewInstance() unexpected error = %v", err) } @@ -81,11 +90,35 @@ func TestNewInstance(t *testing.T) { } }) + t.Run("creates instance with custom name", func(t *testing.T) { + t.Parallel() + + tmpDir := t.TempDir() + sourceDir := filepath.Join(tmpDir, "test-source") + configDir := filepath.Join(tmpDir, "test-config") + + inst, err := NewInstance(NewInstanceParams{ + SourceDir: sourceDir, + ConfigDir: configDir, + Name: "my-workspace", + }) + if err != nil { + t.Fatalf("NewInstance() unexpected error = %v", err) + } + + if inst.GetName() != "my-workspace" { + t.Errorf("GetName() = %v, want 'my-workspace'", inst.GetName()) + } + }) + t.Run("returns error for empty source dir", func(t *testing.T) { t.Parallel() tmpDir := t.TempDir() - _, err := NewInstance("", filepath.Join(tmpDir, "config")) + _, err := NewInstance(NewInstanceParams{ + SourceDir: "", + ConfigDir: filepath.Join(tmpDir, "config"), + }) if err != ErrInvalidPath { t.Errorf("NewInstance() error = %v, want %v", err, ErrInvalidPath) } @@ -95,7 +128,10 @@ func TestNewInstance(t *testing.T) { t.Parallel() tmpDir := t.TempDir() - _, err := NewInstance(filepath.Join(tmpDir, "source"), "") + _, err := NewInstance(NewInstanceParams{ + SourceDir: filepath.Join(tmpDir, "source"), + ConfigDir: "", + }) if err != ErrInvalidPath { t.Errorf("NewInstance() error = %v, want %v", err, ErrInvalidPath) } @@ -104,7 +140,10 @@ func TestNewInstance(t *testing.T) { t.Run("returns error for both empty", func(t *testing.T) { t.Parallel() - _, err := NewInstance("", "") + _, err := NewInstance(NewInstanceParams{ + SourceDir: "", + ConfigDir: "", + }) if err != ErrInvalidPath { t.Errorf("NewInstance() error = %v, want %v", err, ErrInvalidPath) } @@ -128,7 +167,10 @@ func TestInstance_IsAccessible(t *testing.T) { t.Fatalf("Failed to create config dir: %v", err) } - inst, err := NewInstance(sourceDir, configDir) + inst, err := NewInstance(NewInstanceParams{ + SourceDir: sourceDir, + ConfigDir: configDir, + }) if err != nil { t.Fatalf("NewInstance() unexpected error = %v", err) } @@ -148,7 +190,10 @@ func TestInstance_IsAccessible(t *testing.T) { t.Fatalf("Failed to create config dir: %v", err) } - inst, err := NewInstance(filepath.Join(tmpDir, "nonexistent"), configDir) + inst, err := NewInstance(NewInstanceParams{ + SourceDir: filepath.Join(tmpDir, "nonexistent"), + ConfigDir: configDir, + }) if err != nil { t.Fatalf("NewInstance() unexpected error = %v", err) } @@ -168,7 +213,10 @@ func TestInstance_IsAccessible(t *testing.T) { t.Fatalf("Failed to create source dir: %v", err) } - inst, err := NewInstance(sourceDir, filepath.Join(tmpDir, "nonexistent")) + inst, err := NewInstance(NewInstanceParams{ + SourceDir: sourceDir, + ConfigDir: filepath.Join(tmpDir, "nonexistent"), + }) if err != nil { t.Fatalf("NewInstance() unexpected error = %v", err) } @@ -183,10 +231,10 @@ func TestInstance_IsAccessible(t *testing.T) { tmpDir := t.TempDir() - inst, err := NewInstance( - filepath.Join(tmpDir, "nonexistent1"), - filepath.Join(tmpDir, "nonexistent2"), - ) + inst, err := NewInstance(NewInstanceParams{ + SourceDir: filepath.Join(tmpDir, "nonexistent1"), + ConfigDir: filepath.Join(tmpDir, "nonexistent2"), + }) if err != nil { t.Fatalf("NewInstance() unexpected error = %v", err) } diff --git a/pkg/instances/manager.go b/pkg/instances/manager.go index 994e5f9..0ac3fbf 100644 --- a/pkg/instances/manager.go +++ b/pkg/instances/manager.go @@ -17,6 +17,7 @@ package instances import ( "encoding/json" "errors" + "fmt" "os" "path/filepath" "sync" @@ -92,7 +93,8 @@ func newManagerWithFactory(storageDir string, factory InstanceFactory, gen gener // Add registers a new instance. // The instance must be created using NewInstance to ensure proper validation. // A unique ID is generated for the instance when it's added to storage. -// Returns the instance with its generated ID. +// If the instance name is empty, a unique name is generated from the source directory. +// Returns the instance with its generated ID and name. func (m *manager) Add(inst Instance) (Instance, error) { if inst == nil { return nil, errors.New("instance cannot be nil") @@ -123,9 +125,19 @@ func (m *manager) Add(inst Instance) (Instance, error) { } } - // Create a new instance with the unique ID + // Generate a unique name if not provided + name := inst.GetName() + if name == "" { + name = m.generateUniqueName(inst.GetSourceDir(), instances) + } else { + // Ensure the provided name is unique + name = m.ensureUniqueName(name, instances) + } + + // Create a new instance with the unique ID and name instanceWithID := &instance{ ID: uniqueID, + Name: name, SourceDir: inst.GetSourceDir(), ConfigDir: inst.GetConfigDir(), } @@ -225,6 +237,42 @@ func (m *manager) Reconcile() ([]string, error) { return removed, nil } +// generateUniqueName generates a unique name from the source directory +// by extracting the last component of the path and adding an increment if needed +func (m *manager) generateUniqueName(sourceDir string, instances []Instance) string { + // Extract the last component of the source directory + baseName := filepath.Base(sourceDir) + return m.ensureUniqueName(baseName, instances) +} + +// ensureUniqueName ensures the name is unique by adding an increment if needed +func (m *manager) ensureUniqueName(name string, instances []Instance) string { + // Check if the name is already in use + nameExists := func(checkName string) bool { + for _, inst := range instances { + if inst.GetName() == checkName { + return true + } + } + return false + } + + // If the name is not in use, return it + if !nameExists(name) { + return name + } + + // Find a unique name by adding an increment + counter := 2 + for { + uniqueName := fmt.Sprintf("%s-%d", name, counter) + if !nameExists(uniqueName) { + return uniqueName + } + counter++ + } +} + // loadInstances reads instances from the storage file func (m *manager) loadInstances() ([]Instance, error) { // If file doesn't exist, return empty list diff --git a/pkg/instances/manager_test.go b/pkg/instances/manager_test.go index e755321..8bdb6e0 100644 --- a/pkg/instances/manager_test.go +++ b/pkg/instances/manager_test.go @@ -27,6 +27,7 @@ import ( // fakeInstance is a test double for the Instance interface type fakeInstance struct { id string + name string sourceDir string configDir string accessible bool @@ -39,6 +40,10 @@ func (f *fakeInstance) GetID() string { return f.id } +func (f *fakeInstance) GetName() string { + return f.name +} + func (f *fakeInstance) GetSourceDir() string { return f.sourceDir } @@ -53,7 +58,8 @@ func (f *fakeInstance) IsAccessible() bool { func (f *fakeInstance) Dump() InstanceData { return InstanceData{ - ID: f.id, + ID: f.id, + Name: f.name, Paths: InstancePaths{ Source: f.sourceDir, Configuration: f.configDir, @@ -61,13 +67,23 @@ func (f *fakeInstance) Dump() InstanceData { } } +// newFakeInstanceParams contains the parameters for creating a fake instance +type newFakeInstanceParams struct { + ID string + Name string + SourceDir string + ConfigDir string + Accessible bool +} + // newFakeInstance creates a new fake instance for testing -func newFakeInstance(id, sourceDir, configDir string, accessible bool) Instance { +func newFakeInstance(params newFakeInstanceParams) Instance { return &fakeInstance{ - id: id, - sourceDir: sourceDir, - configDir: configDir, - accessible: accessible, + id: params.ID, + name: params.Name, + sourceDir: params.SourceDir, + configDir: params.ConfigDir, + accessible: params.Accessible, } } @@ -76,6 +92,9 @@ func fakeInstanceFactory(data InstanceData) (Instance, error) { if data.ID == "" { return nil, errors.New("instance ID cannot be empty") } + if data.Name == "" { + return nil, errors.New("instance name cannot be empty") + } if data.Paths.Source == "" { return nil, ErrInvalidPath } @@ -86,6 +105,7 @@ func fakeInstanceFactory(data InstanceData) (Instance, error) { // Tests can verify accessibility behavior separately return &fakeInstance{ id: data.ID, + name: data.Name, sourceDir: data.Paths.Source, configDir: data.Paths.Configuration, accessible: true, @@ -199,7 +219,11 @@ func TestNewManager(t *testing.T) { // We can't directly access storageFile since it's on the unexported struct, // but we can verify behavior by adding an instance and checking file creation instanceTmpDir := t.TempDir() - inst := newFakeInstance("", filepath.Join(instanceTmpDir, "source"), filepath.Join(instanceTmpDir, "config"), true) + inst := newFakeInstance(newFakeInstanceParams{ + SourceDir: filepath.Join(instanceTmpDir, "source"), + ConfigDir: filepath.Join(instanceTmpDir, "config"), + Accessible: true, + }) _, _ = manager.Add(inst) expectedFile := filepath.Join(tmpDir, DefaultStorageFileName) @@ -219,7 +243,11 @@ func TestManager_Add(t *testing.T) { manager, _ := newManagerWithFactory(tmpDir, fakeInstanceFactory, newFakeGenerator()) instanceTmpDir := t.TempDir() - inst := newFakeInstance("", filepath.Join(instanceTmpDir, "source"), filepath.Join(instanceTmpDir, "config"), true) + inst := newFakeInstance(newFakeInstanceParams{ + SourceDir: filepath.Join(instanceTmpDir, "source"), + ConfigDir: filepath.Join(instanceTmpDir, "config"), + Accessible: true, + }) added, err := manager.Add(inst) if err != nil { t.Fatalf("Add() unexpected error = %v", err) @@ -267,8 +295,16 @@ func TestManager_Add(t *testing.T) { instanceTmpDir := t.TempDir() // Create instances without IDs (empty ID) - inst1 := newFakeInstance("", filepath.Join(instanceTmpDir, "source1"), filepath.Join(instanceTmpDir, "config1"), true) - inst2 := newFakeInstance("", filepath.Join(instanceTmpDir, "source2"), filepath.Join(instanceTmpDir, "config2"), true) + inst1 := newFakeInstance(newFakeInstanceParams{ + SourceDir: filepath.Join(instanceTmpDir, "source1"), + ConfigDir: filepath.Join(instanceTmpDir, "config1"), + Accessible: true, + }) + inst2 := newFakeInstance(newFakeInstanceParams{ + SourceDir: filepath.Join(instanceTmpDir, "source2"), + ConfigDir: filepath.Join(instanceTmpDir, "config2"), + Accessible: true, + }) added1, _ := manager.Add(inst1) added2, _ := manager.Add(inst2) @@ -309,7 +345,11 @@ func TestManager_Add(t *testing.T) { manager, _ := newManagerWithFactory(tmpDir, fakeInstanceFactory, newFakeGenerator()) instanceTmpDir := t.TempDir() - inst := newFakeInstance("", filepath.Join(instanceTmpDir, "source"), filepath.Join(instanceTmpDir, "config"), true) + inst := newFakeInstance(newFakeInstanceParams{ + SourceDir: filepath.Join(instanceTmpDir, "source"), + ConfigDir: filepath.Join(instanceTmpDir, "config"), + Accessible: true, + }) _, _ = manager.Add(inst) // Check that JSON file exists and is readable @@ -330,9 +370,21 @@ func TestManager_Add(t *testing.T) { manager, _ := newManagerWithFactory(tmpDir, fakeInstanceFactory, newFakeGenerator()) instanceTmpDir := t.TempDir() - inst1 := newFakeInstance("", filepath.Join(instanceTmpDir, "source1"), filepath.Join(instanceTmpDir, "config1"), true) - inst2 := newFakeInstance("", filepath.Join(instanceTmpDir, "source2"), filepath.Join(instanceTmpDir, "config2"), true) - inst3 := newFakeInstance("", filepath.Join(instanceTmpDir, "source3"), filepath.Join(instanceTmpDir, "config3"), true) + inst1 := newFakeInstance(newFakeInstanceParams{ + SourceDir: filepath.Join(instanceTmpDir, "source1"), + ConfigDir: filepath.Join(instanceTmpDir, "config1"), + Accessible: true, + }) + inst2 := newFakeInstance(newFakeInstanceParams{ + SourceDir: filepath.Join(instanceTmpDir, "source2"), + ConfigDir: filepath.Join(instanceTmpDir, "config2"), + Accessible: true, + }) + inst3 := newFakeInstance(newFakeInstanceParams{ + SourceDir: filepath.Join(instanceTmpDir, "source3"), + ConfigDir: filepath.Join(instanceTmpDir, "config3"), + Accessible: true, + }) _, _ = manager.Add(inst1) _, _ = manager.Add(inst2) @@ -370,8 +422,16 @@ func TestManager_List(t *testing.T) { manager, _ := newManagerWithFactory(tmpDir, fakeInstanceFactory, newFakeGenerator()) instanceTmpDir := t.TempDir() - inst1 := newFakeInstance("", filepath.Join(instanceTmpDir, "source1"), filepath.Join(instanceTmpDir, "config1"), true) - inst2 := newFakeInstance("", filepath.Join(instanceTmpDir, "source2"), filepath.Join(instanceTmpDir, "config2"), true) + inst1 := newFakeInstance(newFakeInstanceParams{ + SourceDir: filepath.Join(instanceTmpDir, "source1"), + ConfigDir: filepath.Join(instanceTmpDir, "config1"), + Accessible: true, + }) + inst2 := newFakeInstance(newFakeInstanceParams{ + SourceDir: filepath.Join(instanceTmpDir, "source2"), + ConfigDir: filepath.Join(instanceTmpDir, "config2"), + Accessible: true, + }) _, _ = manager.Add(inst1) _, _ = manager.Add(inst2) @@ -417,7 +477,11 @@ func TestManager_Get(t *testing.T) { instanceTmpDir := t.TempDir() expectedSource := filepath.Join(instanceTmpDir, "source") expectedConfig := filepath.Join(instanceTmpDir, "config") - inst := newFakeInstance("", expectedSource, expectedConfig, true) + inst := newFakeInstance(newFakeInstanceParams{ + SourceDir: expectedSource, + ConfigDir: expectedConfig, + Accessible: true, + }) added, _ := manager.Add(inst) generatedID := added.GetID() @@ -462,7 +526,11 @@ func TestManager_Delete(t *testing.T) { instanceTmpDir := t.TempDir() sourceDir := filepath.Join(instanceTmpDir, "source") configDir := filepath.Join(instanceTmpDir, "config") - inst := newFakeInstance("", sourceDir, configDir, true) + inst := newFakeInstance(newFakeInstanceParams{ + SourceDir: sourceDir, + ConfigDir: configDir, + Accessible: true, + }) added, _ := manager.Add(inst) generatedID := added.GetID() @@ -502,8 +570,16 @@ func TestManager_Delete(t *testing.T) { config1 := filepath.Join(instanceTmpDir, "config1") source2 := filepath.Join(instanceTmpDir, "source2") config2 := filepath.Join(instanceTmpDir, "config2") - inst1 := newFakeInstance("", source1, config1, true) - inst2 := newFakeInstance("", source2, config2, true) + inst1 := newFakeInstance(newFakeInstanceParams{ + SourceDir: source1, + ConfigDir: config1, + Accessible: true, + }) + inst2 := newFakeInstance(newFakeInstanceParams{ + SourceDir: source2, + ConfigDir: config2, + Accessible: true, + }) added1, _ := manager.Add(inst1) added2, _ := manager.Add(inst2) @@ -531,7 +607,11 @@ func TestManager_Delete(t *testing.T) { tmpDir := t.TempDir() manager1, _ := newManagerWithFactory(tmpDir, fakeInstanceFactory, newFakeGenerator()) - inst := newFakeInstance("", filepath.Join(string(filepath.Separator), "tmp", "source"), filepath.Join(string(filepath.Separator), "tmp", "config"), true) + inst := newFakeInstance(newFakeInstanceParams{ + SourceDir: filepath.Join(string(filepath.Separator), "tmp", "source"), + ConfigDir: filepath.Join(string(filepath.Separator), "tmp", "config"), + Accessible: true, + }) added, _ := manager1.Add(inst) generatedID := added.GetID() @@ -559,11 +639,15 @@ func TestManager_Reconcile(t *testing.T) { if data.ID == "" { return nil, errors.New("instance ID cannot be empty") } + if data.Name == "" { + return nil, errors.New("instance name cannot be empty") + } if data.Paths.Source == "" || data.Paths.Configuration == "" { return nil, ErrInvalidPath } return &fakeInstance{ id: data.ID, + name: data.Name, sourceDir: data.Paths.Source, configDir: data.Paths.Configuration, accessible: false, // Always inaccessible for this test @@ -573,7 +657,11 @@ func TestManager_Reconcile(t *testing.T) { // Add instance that is inaccessible instanceTmpDir := t.TempDir() - inst := newFakeInstance("", filepath.Join(instanceTmpDir, "nonexistent-source"), filepath.Join(instanceTmpDir, "config"), false) + inst := newFakeInstance(newFakeInstanceParams{ + SourceDir: filepath.Join(instanceTmpDir, "nonexistent-source"), + ConfigDir: filepath.Join(instanceTmpDir, "config"), + Accessible: false, + }) _, _ = manager.Add(inst) removed, err := manager.Reconcile() @@ -595,11 +683,15 @@ func TestManager_Reconcile(t *testing.T) { if data.ID == "" { return nil, errors.New("instance ID cannot be empty") } + if data.Name == "" { + return nil, errors.New("instance name cannot be empty") + } if data.Paths.Source == "" || data.Paths.Configuration == "" { return nil, ErrInvalidPath } return &fakeInstance{ id: data.ID, + name: data.Name, sourceDir: data.Paths.Source, configDir: data.Paths.Configuration, accessible: false, // Always inaccessible for this test @@ -609,7 +701,11 @@ func TestManager_Reconcile(t *testing.T) { // Add instance that is inaccessible instanceTmpDir := t.TempDir() - inst := newFakeInstance("", filepath.Join(instanceTmpDir, "source"), filepath.Join(instanceTmpDir, "nonexistent-config"), false) + inst := newFakeInstance(newFakeInstanceParams{ + SourceDir: filepath.Join(instanceTmpDir, "source"), + ConfigDir: filepath.Join(instanceTmpDir, "nonexistent-config"), + Accessible: false, + }) _, _ = manager.Add(inst) removed, err := manager.Reconcile() @@ -631,11 +727,15 @@ func TestManager_Reconcile(t *testing.T) { if data.ID == "" { return nil, errors.New("instance ID cannot be empty") } + if data.Name == "" { + return nil, errors.New("instance name cannot be empty") + } if data.Paths.Source == "" || data.Paths.Configuration == "" { return nil, ErrInvalidPath } return &fakeInstance{ id: data.ID, + name: data.Name, sourceDir: data.Paths.Source, configDir: data.Paths.Configuration, accessible: false, // Always inaccessible for this test @@ -645,7 +745,11 @@ func TestManager_Reconcile(t *testing.T) { instanceTmpDir := t.TempDir() inaccessibleSource := filepath.Join(instanceTmpDir, "nonexistent-source") - inst := newFakeInstance("", inaccessibleSource, filepath.Join(instanceTmpDir, "config"), false) + inst := newFakeInstance(newFakeInstanceParams{ + SourceDir: inaccessibleSource, + ConfigDir: filepath.Join(instanceTmpDir, "config"), + Accessible: false, + }) added, _ := manager.Add(inst) generatedID := added.GetID() @@ -690,11 +794,19 @@ func TestManager_Reconcile(t *testing.T) { accessibleConfig := filepath.Join(instanceTmpDir, "accessible-config") // Add accessible instance - accessible := newFakeInstance("", accessibleSource, accessibleConfig, true) + accessible := newFakeInstance(newFakeInstanceParams{ + SourceDir: accessibleSource, + ConfigDir: accessibleConfig, + Accessible: true, + }) _, _ = manager.Add(accessible) // Add inaccessible instance - inaccessible := newFakeInstance("", inaccessibleSource, filepath.Join(instanceTmpDir, "nonexistent-config"), false) + inaccessible := newFakeInstance(newFakeInstanceParams{ + SourceDir: inaccessibleSource, + ConfigDir: filepath.Join(instanceTmpDir, "nonexistent-config"), + Accessible: false, + }) _, _ = manager.Add(inaccessible) removed, err := manager.Reconcile() @@ -723,7 +835,11 @@ func TestManager_Reconcile(t *testing.T) { manager, _ := newManagerWithFactory(tmpDir, fakeInstanceFactory, newFakeGenerator()) instanceTmpDir := t.TempDir() - inst := newFakeInstance("", filepath.Join(instanceTmpDir, "source"), filepath.Join(instanceTmpDir, "config"), true) + inst := newFakeInstance(newFakeInstanceParams{ + SourceDir: filepath.Join(instanceTmpDir, "source"), + ConfigDir: filepath.Join(instanceTmpDir, "config"), + Accessible: true, + }) _, _ = manager.Add(inst) removed, err := manager.Reconcile() @@ -766,7 +882,11 @@ func TestManager_Persistence(t *testing.T) { manager1, _ := newManagerWithFactory(tmpDir, fakeInstanceFactory, newFakeGenerator()) expectedSource := filepath.Join(instanceTmpDir, "source") expectedConfig := filepath.Join(instanceTmpDir, "config") - inst := newFakeInstance("", expectedSource, expectedConfig, true) + inst := newFakeInstance(newFakeInstanceParams{ + SourceDir: expectedSource, + ConfigDir: expectedConfig, + Accessible: true, + }) added, _ := manager1.Add(inst) generatedID := added.GetID() @@ -798,7 +918,11 @@ func TestManager_Persistence(t *testing.T) { instanceTmpDir := t.TempDir() expectedSource := filepath.Join(instanceTmpDir, "source") expectedConfig := filepath.Join(instanceTmpDir, "config") - inst := newFakeInstance("", expectedSource, expectedConfig, true) + inst := newFakeInstance(newFakeInstanceParams{ + SourceDir: expectedSource, + ConfigDir: expectedConfig, + Accessible: true, + }) added, _ := manager.Add(inst) generatedID := added.GetID() @@ -853,7 +977,11 @@ func TestManager_ConcurrentAccess(t *testing.T) { defer wg.Done() sourceDir := filepath.Join(instanceTmpDir, "source", string(rune('a'+id))) configDir := filepath.Join(instanceTmpDir, "config", string(rune('a'+id))) - inst := newFakeInstance("", sourceDir, configDir, true) + inst := newFakeInstance(newFakeInstanceParams{ + SourceDir: sourceDir, + ConfigDir: configDir, + Accessible: true, + }) _, _ = manager.Add(inst) }(i) } @@ -877,7 +1005,11 @@ func TestManager_ConcurrentAccess(t *testing.T) { for i := 0; i < 5; i++ { sourceDir := filepath.Join(instanceTmpDir, "source", string(rune('a'+i))) configDir := filepath.Join(instanceTmpDir, "config", string(rune('a'+i))) - inst := newFakeInstance("", sourceDir, configDir, true) + inst := newFakeInstance(newFakeInstanceParams{ + SourceDir: sourceDir, + ConfigDir: configDir, + Accessible: true, + }) _, _ = manager.Add(inst) } @@ -902,3 +1034,242 @@ func TestManager_ConcurrentAccess(t *testing.T) { } }) } + +func TestManager_ensureUniqueName(t *testing.T) { + t.Parallel() + + t.Run("returns original name when no conflict", func(t *testing.T) { + t.Parallel() + + tmpDir := t.TempDir() + m, _ := newManagerWithFactory(tmpDir, fakeInstanceFactory, newFakeGenerator()) + + // Cast to concrete type to access unexported methods + mgr := m.(*manager) + + instances := []Instance{ + newFakeInstance(newFakeInstanceParams{ + ID: "id1", + Name: "workspace1", + SourceDir: "/path/source1", + ConfigDir: "/path/config1", + Accessible: true, + }), + newFakeInstance(newFakeInstanceParams{ + ID: "id2", + Name: "workspace2", + SourceDir: "/path/source2", + ConfigDir: "/path/config2", + Accessible: true, + }), + } + + result := mgr.ensureUniqueName("myworkspace", instances) + + if result != "myworkspace" { + t.Errorf("ensureUniqueName() = %v, want myworkspace", result) + } + }) + + t.Run("adds increment when name conflicts", func(t *testing.T) { + t.Parallel() + + tmpDir := t.TempDir() + m, _ := newManagerWithFactory(tmpDir, fakeInstanceFactory, newFakeGenerator()) + + mgr := m.(*manager) + + instances := []Instance{ + newFakeInstance(newFakeInstanceParams{ + ID: "id1", + Name: "myworkspace", + SourceDir: "/path/source1", + ConfigDir: "/path/config1", + Accessible: true, + }), + } + + result := mgr.ensureUniqueName("myworkspace", instances) + + if result != "myworkspace-2" { + t.Errorf("ensureUniqueName() = %v, want myworkspace-2", result) + } + }) + + t.Run("increments until unique name is found", func(t *testing.T) { + t.Parallel() + + tmpDir := t.TempDir() + m, _ := newManagerWithFactory(tmpDir, fakeInstanceFactory, newFakeGenerator()) + + mgr := m.(*manager) + + instances := []Instance{ + newFakeInstance(newFakeInstanceParams{ + ID: "id1", + Name: "myworkspace", + SourceDir: "/path/source1", + ConfigDir: "/path/config1", + Accessible: true, + }), + newFakeInstance(newFakeInstanceParams{ + ID: "id2", + Name: "myworkspace-2", + SourceDir: "/path/source2", + ConfigDir: "/path/config2", + Accessible: true, + }), + newFakeInstance(newFakeInstanceParams{ + ID: "id3", + Name: "myworkspace-3", + SourceDir: "/path/source3", + ConfigDir: "/path/config3", + Accessible: true, + }), + } + + result := mgr.ensureUniqueName("myworkspace", instances) + + if result != "myworkspace-4" { + t.Errorf("ensureUniqueName() = %v, want myworkspace-4", result) + } + }) + + t.Run("handles double digit increments", func(t *testing.T) { + t.Parallel() + + tmpDir := t.TempDir() + m, _ := newManagerWithFactory(tmpDir, fakeInstanceFactory, newFakeGenerator()) + + mgr := m.(*manager) + + // Create instances with names up to myworkspace-10 + instances := []Instance{} + instances = append(instances, newFakeInstance(newFakeInstanceParams{ + ID: "id0", + Name: "myworkspace", + SourceDir: "/path/source0", + ConfigDir: "/path/config0", + Accessible: true, + })) + for i := 2; i <= 10; i++ { + name := fmt.Sprintf("myworkspace-%d", i) + id := fmt.Sprintf("id%d", i) + instances = append(instances, newFakeInstance(newFakeInstanceParams{ + ID: id, + Name: name, + SourceDir: fmt.Sprintf("/path/source%d", i), + ConfigDir: fmt.Sprintf("/path/config%d", i), + Accessible: true, + })) + } + + result := mgr.ensureUniqueName("myworkspace", instances) + + if result != "myworkspace-11" { + t.Errorf("ensureUniqueName() = %v, want myworkspace-11", result) + } + }) + + t.Run("works with empty instance list", func(t *testing.T) { + t.Parallel() + + tmpDir := t.TempDir() + m, _ := newManagerWithFactory(tmpDir, fakeInstanceFactory, newFakeGenerator()) + + mgr := m.(*manager) + + instances := []Instance{} + + result := mgr.ensureUniqueName("myworkspace", instances) + + if result != "myworkspace" { + t.Errorf("ensureUniqueName() = %v, want myworkspace", result) + } + }) +} + +func TestManager_generateUniqueName(t *testing.T) { + t.Parallel() + + t.Run("generates name from source directory basename", func(t *testing.T) { + t.Parallel() + + tmpDir := t.TempDir() + m, _ := newManagerWithFactory(tmpDir, fakeInstanceFactory, newFakeGenerator()) + + mgr := m.(*manager) + + instances := []Instance{} + + result := mgr.generateUniqueName("/home/user/myproject", instances) + + if result != "myproject" { + t.Errorf("generateUniqueName() = %v, want myproject", result) + } + }) + + t.Run("handles conflicting names from different paths", func(t *testing.T) { + t.Parallel() + + tmpDir := t.TempDir() + m, _ := newManagerWithFactory(tmpDir, fakeInstanceFactory, newFakeGenerator()) + + mgr := m.(*manager) + + instances := []Instance{ + newFakeInstance(newFakeInstanceParams{ + ID: "id1", + Name: "myproject", + SourceDir: "/home/user/myproject", + ConfigDir: "/home/user/myproject/.kortex", + Accessible: true, + }), + } + + result := mgr.generateUniqueName("/home/otheruser/myproject", instances) + + if result != "myproject-2" { + t.Errorf("generateUniqueName() = %v, want myproject-2", result) + } + }) + + t.Run("handles Windows-style paths", func(t *testing.T) { + t.Parallel() + + tmpDir := t.TempDir() + m, _ := newManagerWithFactory(tmpDir, fakeInstanceFactory, newFakeGenerator()) + + mgr := m.(*manager) + + instances := []Instance{} + + // filepath.Base works cross-platform + result := mgr.generateUniqueName(filepath.Join("C:", "Users", "user", "myproject"), instances) + + if result != "myproject" { + t.Errorf("generateUniqueName() = %v, want myproject", result) + } + }) + + t.Run("handles current directory", func(t *testing.T) { + t.Parallel() + + tmpDir := t.TempDir() + m, _ := newManagerWithFactory(tmpDir, fakeInstanceFactory, newFakeGenerator()) + + mgr := m.(*manager) + + instances := []Instance{} + + // Get the current working directory + wd, _ := os.Getwd() + expectedName := filepath.Base(wd) + + result := mgr.generateUniqueName(wd, instances) + + if result != expectedName { + t.Errorf("generateUniqueName() = %v, want %v", result, expectedName) + } + }) +}