From dc6eb19c702f9ec487a54e223609a6f1a2fad6c1 Mon Sep 17 00:00:00 2001 From: Philippe Martin Date: Thu, 12 Mar 2026 10:15:43 +0100 Subject: [PATCH 1/6] chore(deps): update kortex-cli-api to 8f627ff7db68 Updates the kortex-cli-api dependency to the latest version. Co-Authored-By: Claude Code (Claude Sonnet 4.5) Signed-off-by: Philippe Martin --- go.mod | 2 +- go.sum | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/go.mod b/go.mod index 11f8206..a3adc3a 100644 --- a/go.mod +++ b/go.mod @@ -3,7 +3,7 @@ module github.com/kortex-hub/kortex-cli go 1.25.7 require ( - github.com/kortex-hub/kortex-cli-api/cli/go v0.0.0-20260309100818-e0ea254e6f13 + github.com/kortex-hub/kortex-cli-api/cli/go v0.0.0-20260312091024-8f627ff7db68 github.com/spf13/cobra v1.10.2 ) diff --git a/go.sum b/go.sum index 5ed660e..92e54b0 100644 --- a/go.sum +++ b/go.sum @@ -1,8 +1,8 @@ github.com/cpuguy83/go-md2man/v2 v2.0.6/go.mod h1:oOW0eioCTA6cOiMLiUPZOpcVxMig6NIQQ7OS05n1F4g= github.com/inconshreveable/mousetrap v1.1.0 h1:wN+x4NVGpMsO7ErUn/mUI3vEoE6Jt13X2s0bqwp9tc8= github.com/inconshreveable/mousetrap v1.1.0/go.mod h1:vpF70FUmC8bwa3OWnCshd2FqLfsEA9PFc4w1p2J65bw= -github.com/kortex-hub/kortex-cli-api/cli/go v0.0.0-20260309100818-e0ea254e6f13 h1:JkzrfheDTegKzYzjOfo6K5oEgMmT1+1+oqxTfRghJCw= -github.com/kortex-hub/kortex-cli-api/cli/go v0.0.0-20260309100818-e0ea254e6f13/go.mod h1:jWKudiw26CIjuOsROQ80FOuuk2m2/5BVkuATpmD5xQQ= +github.com/kortex-hub/kortex-cli-api/cli/go v0.0.0-20260312091024-8f627ff7db68 h1:+rG9db97UcYp3RvWwyd3yQQDghsJgEPp2bFZcqz8qfc= +github.com/kortex-hub/kortex-cli-api/cli/go v0.0.0-20260312091024-8f627ff7db68/go.mod h1:jWKudiw26CIjuOsROQ80FOuuk2m2/5BVkuATpmD5xQQ= github.com/russross/blackfriday/v2 v2.1.0/go.mod h1:+Rmxgy9KzJVeS9/2gXHxylqXiyQDYRxCVz55jmeOWTM= github.com/spf13/cobra v1.10.2 h1:DMTTonx5m65Ic0GOoRY2c16WCbHxOOw6xxezuLaBpcU= github.com/spf13/cobra v1.10.2/go.mod h1:7C1pvHqHw5A4vrJfjNwvOdzYu0Gml16OCs2GRiTUUS4= From 8748386ae91772a9227ed645cc77e58a45727cfa Mon Sep 17 00:00:00 2001 From: Philippe Martin Date: Thu, 12 Mar 2026 15:33:43 +0100 Subject: [PATCH 2/6] feat(cmd): add JSON output support to init, remove, and list commands Adds --output flag to init, remove, and list commands enabling JSON output for programmatic consumption. Creates shared conversion helpers to ensure consistent JSON formatting across all commands following the OpenAPI schema from kortex-cli-api package. Key changes: - New conversion helpers in pkg/cmd/conversion.go with outputErrorIfJSON - Init command returns WorkspaceId by default, full Workspace with --verbose - Remove and list commands return appropriate JSON structures - All errors formatted as JSON when --output=json is set - Comprehensive tests and AGENTS.md documentation added Co-Authored-By: Claude Code (Claude Sonnet 4.5) Signed-off-by: Philippe Martin --- AGENTS.md | 186 ++++++++++++++++ pkg/cmd/conversion.go | 74 +++++++ pkg/cmd/conversion_test.go | 369 +++++++++++++++++++++++++++++++ pkg/cmd/init.go | 66 +++++- pkg/cmd/init_test.go | 302 +++++++++++++++++++++++++ pkg/cmd/workspace_list.go | 45 ++-- pkg/cmd/workspace_list_test.go | 56 ++++- pkg/cmd/workspace_remove.go | 51 ++++- pkg/cmd/workspace_remove_test.go | 301 +++++++++++++++++++++++++ 9 files changed, 1397 insertions(+), 53 deletions(-) create mode 100644 pkg/cmd/conversion.go create mode 100644 pkg/cmd/conversion_test.go diff --git a/AGENTS.md b/AGENTS.md index cc32420..269440b 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -401,6 +401,192 @@ Commands should follow a consistent structure for maintainability and testabilit **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. + ### Testing Pattern for Commands Commands should have two types of tests following the pattern in `pkg/cmd/init_test.go`: diff --git a/pkg/cmd/conversion.go b/pkg/cmd/conversion.go new file mode 100644 index 0000000..f4a6080 --- /dev/null +++ b/pkg/cmd/conversion.go @@ -0,0 +1,74 @@ +/********************************************************************** + * 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 ( + "encoding/json" + "fmt" + "io" + + api "github.com/kortex-hub/kortex-cli-api/cli/go" + "github.com/kortex-hub/kortex-cli/pkg/instances" +) + +// instanceToWorkspaceId converts an Instance to an api.WorkspaceId +func instanceToWorkspaceId(instance instances.Instance) api.WorkspaceId { + return api.WorkspaceId{ + Id: instance.GetID(), + } +} + +// instanceToWorkspace converts an Instance to an api.Workspace +func instanceToWorkspace(instance instances.Instance) api.Workspace { + return api.Workspace{ + Id: instance.GetID(), + Name: instance.GetName(), + Paths: api.WorkspacePaths{ + Configuration: instance.GetConfigDir(), + Source: instance.GetSourceDir(), + }, + } +} + +// formatErrorJSON formats an error as JSON using api.Error schema +func formatErrorJSON(err error) (string, error) { + if err == nil { + return "", nil + } + errorResponse := api.Error{ + Error: err.Error(), + } + + jsonData, jsonErr := json.MarshalIndent(errorResponse, "", " ") + if jsonErr != nil { + return "", fmt.Errorf("failed to marshal error to JSON: %w", jsonErr) + } + + return string(jsonData), nil +} + +// outputErrorIfJSON outputs the error as JSON if output mode is "json", then returns the error. +// This helper reduces code duplication for error handling in commands. +func outputErrorIfJSON(cmd interface{ OutOrStdout() io.Writer }, output string, err error) error { + if output == "json" { + jsonErr, _ := formatErrorJSON(err) + fmt.Fprintln(cmd.OutOrStdout(), jsonErr) + } + return err +} diff --git a/pkg/cmd/conversion_test.go b/pkg/cmd/conversion_test.go new file mode 100644 index 0000000..5f62300 --- /dev/null +++ b/pkg/cmd/conversion_test.go @@ -0,0 +1,369 @@ +/********************************************************************** + * 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" + "encoding/json" + "errors" + "fmt" + "testing" + + api "github.com/kortex-hub/kortex-cli-api/cli/go" + "github.com/kortex-hub/kortex-cli/pkg/instances" + "github.com/spf13/cobra" +) + +func TestInstanceToWorkspaceId(t *testing.T) { + t.Parallel() + + t.Run("converts instance to workspace ID", func(t *testing.T) { + t.Parallel() + + instance, err := instances.NewInstance(instances.NewInstanceParams{ + SourceDir: "/path/to/source", + ConfigDir: "/path/to/config", + Name: "test-workspace", + }) + if err != nil { + t.Fatalf("Failed to create instance: %v", err) + } + + // Manually set ID (in real usage, Manager sets this) + instanceData := instance.Dump() + instanceData.ID = "test-id-123" + instance, _ = instances.NewInstanceFromData(instanceData) + + result := instanceToWorkspaceId(instance) + + if result.Id != "test-id-123" { + t.Errorf("Expected ID 'test-id-123', got '%s'", result.Id) + } + + // Verify it only contains ID field by marshaling and checking JSON structure + jsonData, err := json.Marshal(result) + if err != nil { + t.Fatalf("Failed to marshal result: %v", err) + } + + var parsed map[string]interface{} + if err := json.Unmarshal(jsonData, &parsed); err != nil { + t.Fatalf("Failed to unmarshal JSON: %v", err) + } + + if len(parsed) != 1 { + t.Errorf("Expected only 1 field, got %d: %v", len(parsed), parsed) + } + + if _, exists := parsed["id"]; !exists { + t.Error("Expected 'id' field in JSON") + } + }) +} + +func TestInstanceToWorkspace(t *testing.T) { + t.Parallel() + + t.Run("converts instance to full workspace", func(t *testing.T) { + t.Parallel() + + instance, err := instances.NewInstance(instances.NewInstanceParams{ + SourceDir: "/path/to/source", + ConfigDir: "/path/to/config", + Name: "test-workspace", + }) + if err != nil { + t.Fatalf("Failed to create instance: %v", err) + } + + // Manually set ID (in real usage, Manager sets this) + instanceData := instance.Dump() + instanceData.ID = "test-id-456" + instance, _ = instances.NewInstanceFromData(instanceData) + + result := instanceToWorkspace(instance) + + if result.Id != "test-id-456" { + t.Errorf("Expected ID 'test-id-456', got '%s'", result.Id) + } + + if result.Name != "test-workspace" { + t.Errorf("Expected name 'test-workspace', got '%s'", result.Name) + } + + if result.Paths.Source != "/path/to/source" { + t.Errorf("Expected source '/path/to/source', got '%s'", result.Paths.Source) + } + + if result.Paths.Configuration != "/path/to/config" { + t.Errorf("Expected config '/path/to/config', got '%s'", result.Paths.Configuration) + } + }) + + t.Run("includes all required fields", func(t *testing.T) { + t.Parallel() + + instance, err := instances.NewInstance(instances.NewInstanceParams{ + SourceDir: "/some/path", + ConfigDir: "/config/path", + Name: "my-workspace", + }) + if err != nil { + t.Fatalf("Failed to create instance: %v", err) + } + + // Set ID + instanceData := instance.Dump() + instanceData.ID = "full-test-id" + instance, _ = instances.NewInstanceFromData(instanceData) + + result := instanceToWorkspace(instance) + + // Marshal to JSON to verify structure + jsonData, err := json.Marshal(result) + if err != nil { + t.Fatalf("Failed to marshal result: %v", err) + } + + var parsed map[string]interface{} + if err := json.Unmarshal(jsonData, &parsed); err != nil { + t.Fatalf("Failed to unmarshal JSON: %v", err) + } + + // Verify all expected fields exist + if _, exists := parsed["id"]; !exists { + t.Error("Expected 'id' field in JSON") + } + if _, exists := parsed["name"]; !exists { + t.Error("Expected 'name' field in JSON") + } + if _, exists := parsed["paths"]; !exists { + t.Error("Expected 'paths' field in JSON") + } + + // Verify paths structure + paths, ok := parsed["paths"].(map[string]interface{}) + if !ok { + t.Fatal("Expected 'paths' to be an object") + } + + if _, exists := paths["source"]; !exists { + t.Error("Expected 'paths.source' field in JSON") + } + if _, exists := paths["configuration"]; !exists { + t.Error("Expected 'paths.configuration' field in JSON") + } + }) +} + +func TestFormatErrorJSON(t *testing.T) { + t.Parallel() + + t.Run("formats error as JSON", func(t *testing.T) { + t.Parallel() + + err := errors.New("something went wrong") + result, jsonErr := formatErrorJSON(err) + + if jsonErr != nil { + t.Fatalf("formatErrorJSON failed: %v", jsonErr) + } + + // Parse the JSON to verify structure + var errorResponse api.Error + if err := json.Unmarshal([]byte(result), &errorResponse); err != nil { + t.Fatalf("Failed to unmarshal error JSON: %v", err) + } + + if errorResponse.Error != "something went wrong" { + t.Errorf("Expected error message 'something went wrong', got '%s'", errorResponse.Error) + } + }) + + t.Run("includes only error field", func(t *testing.T) { + t.Parallel() + + err := errors.New("test error") + result, jsonErr := formatErrorJSON(err) + + if jsonErr != nil { + t.Fatalf("formatErrorJSON failed: %v", jsonErr) + } + + var parsed map[string]interface{} + if err := json.Unmarshal([]byte(result), &parsed); err != nil { + t.Fatalf("Failed to unmarshal JSON: %v", err) + } + + if len(parsed) != 1 { + t.Errorf("Expected only 1 field, got %d: %v", len(parsed), parsed) + } + + if _, exists := parsed["error"]; !exists { + t.Error("Expected 'error' field in JSON") + } + }) + + t.Run("handles complex error messages", func(t *testing.T) { + t.Parallel() + + complexErr := errors.New("failed to process: invalid input \"test\"\nUse --help for more information") + result, jsonErr := formatErrorJSON(complexErr) + + if jsonErr != nil { + t.Fatalf("formatErrorJSON failed: %v", jsonErr) + } + + var errorResponse api.Error + if err := json.Unmarshal([]byte(result), &errorResponse); err != nil { + t.Fatalf("Failed to unmarshal error JSON: %v", err) + } + + expectedMsg := "failed to process: invalid input \"test\"\nUse --help for more information" + if errorResponse.Error != expectedMsg { + t.Errorf("Expected error message '%s', got '%s'", expectedMsg, errorResponse.Error) + } + }) +} + +func TestOutputErrorIfJSON(t *testing.T) { + t.Parallel() + + t.Run("outputs JSON when output is json", func(t *testing.T) { + t.Parallel() + + cmd := &cobra.Command{} + buf := new(bytes.Buffer) + cmd.SetOut(buf) + + testErr := errors.New("test error message") + returnedErr := outputErrorIfJSON(cmd, "json", testErr) + + // Verify the error is returned unchanged + if returnedErr != testErr { + t.Errorf("Expected returned error to be the same as input error") + } + + // Verify JSON was written to output + var errorResponse api.Error + if err := json.Unmarshal(buf.Bytes(), &errorResponse); err != nil { + t.Fatalf("Failed to unmarshal JSON output: %v\nOutput was: %s", err, buf.String()) + } + + if errorResponse.Error != "test error message" { + t.Errorf("Expected error message 'test error message', got '%s'", errorResponse.Error) + } + }) + + t.Run("does not output JSON when output is empty", func(t *testing.T) { + t.Parallel() + + cmd := &cobra.Command{} + buf := new(bytes.Buffer) + cmd.SetOut(buf) + + testErr := errors.New("test error message") + returnedErr := outputErrorIfJSON(cmd, "", testErr) + + // Verify the error is returned unchanged + if returnedErr != testErr { + t.Errorf("Expected returned error to be the same as input error") + } + + // Verify nothing was written to output + if buf.Len() != 0 { + t.Errorf("Expected no output, got: %s", buf.String()) + } + }) + + t.Run("does not output JSON when output is text", func(t *testing.T) { + t.Parallel() + + cmd := &cobra.Command{} + buf := new(bytes.Buffer) + cmd.SetOut(buf) + + testErr := errors.New("test error message") + returnedErr := outputErrorIfJSON(cmd, "text", testErr) + + // Verify the error is returned unchanged + if returnedErr != testErr { + t.Errorf("Expected returned error to be the same as input error") + } + + // Verify nothing was written to output + if buf.Len() != 0 { + t.Errorf("Expected no output, got: %s", buf.String()) + } + }) + + t.Run("formats complex error messages correctly", func(t *testing.T) { + t.Parallel() + + cmd := &cobra.Command{} + buf := new(bytes.Buffer) + cmd.SetOut(buf) + + testErr := errors.New("workspace not found: abc123\nUse 'workspace list' to see available workspaces") + returnedErr := outputErrorIfJSON(cmd, "json", testErr) + + // Verify the error is returned unchanged + if returnedErr != testErr { + t.Errorf("Expected returned error to be the same as input error") + } + + // Verify JSON was written with full error message + var errorResponse api.Error + if err := json.Unmarshal(buf.Bytes(), &errorResponse); err != nil { + t.Fatalf("Failed to unmarshal JSON output: %v", err) + } + + expectedMsg := "workspace not found: abc123\nUse 'workspace list' to see available workspaces" + if errorResponse.Error != expectedMsg { + t.Errorf("Expected error message '%s', got '%s'", expectedMsg, errorResponse.Error) + } + }) + + t.Run("handles wrapped errors", func(t *testing.T) { + t.Parallel() + + cmd := &cobra.Command{} + buf := new(bytes.Buffer) + cmd.SetOut(buf) + + baseErr := errors.New("base error") + wrappedErr := fmt.Errorf("wrapped error: %w", baseErr) + returnedErr := outputErrorIfJSON(cmd, "json", wrappedErr) + + // Verify the error is returned unchanged + if returnedErr != wrappedErr { + t.Errorf("Expected returned error to be the same as input error") + } + + // Verify JSON contains the full wrapped error message + var errorResponse api.Error + if err := json.Unmarshal(buf.Bytes(), &errorResponse); err != nil { + t.Fatalf("Failed to unmarshal JSON output: %v", err) + } + + if errorResponse.Error != "wrapped error: base error" { + t.Errorf("Expected error message 'wrapped error: base error', got '%s'", errorResponse.Error) + } + }) +} diff --git a/pkg/cmd/init.go b/pkg/cmd/init.go index b19513d..948d7e8 100644 --- a/pkg/cmd/init.go +++ b/pkg/cmd/init.go @@ -19,6 +19,7 @@ package cmd import ( + "encoding/json" "fmt" "os" "path/filepath" @@ -36,26 +37,39 @@ type initCmd struct { absConfigDir string manager instances.Manager verbose bool + output string } // preRun validates the parameters and flags func (i *initCmd) preRun(cmd *cobra.Command, args []string) error { + // Validate output format if specified + if i.output != "" && i.output != "json" { + return fmt.Errorf("unsupported output format: %s (supported: json)", i.output) + } + + // Silence Cobra's error and usage output when JSON mode is enabled + // This prevents "Error: ..." and usage info from being printed + if i.output == "json" { + cmd.SilenceErrors = true + cmd.SilenceUsage = true + } + // 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) + return outputErrorIfJSON(cmd, i.output, fmt.Errorf("failed to read --storage flag: %w", err)) } // Convert to absolute path absStorageDir, err := filepath.Abs(storageDir) if err != nil { - return fmt.Errorf("failed to resolve storage directory path: %w", err) + return outputErrorIfJSON(cmd, i.output, fmt.Errorf("failed to resolve storage directory path: %w", err)) } // Create manager manager, err := instances.NewManager(absStorageDir) if err != nil { - return fmt.Errorf("failed to create manager: %w", err) + return outputErrorIfJSON(cmd, i.output, fmt.Errorf("failed to create manager: %w", err)) } i.manager = manager @@ -68,19 +82,19 @@ func (i *initCmd) preRun(cmd *cobra.Command, args []string) error { // 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) + return outputErrorIfJSON(cmd, i.output, fmt.Errorf("failed to resolve sources directory path: %w", err)) } i.absSourcesDir = absSourcesDir // Verify that the sources directory exists and is a directory fileInfo, err := os.Stat(i.absSourcesDir) if os.IsNotExist(err) { - return fmt.Errorf("sources directory does not exist: %s", i.absSourcesDir) + return outputErrorIfJSON(cmd, i.output, fmt.Errorf("sources directory does not exist: %s", i.absSourcesDir)) } else if err != nil { - return fmt.Errorf("failed to check sources directory: %w", err) + return outputErrorIfJSON(cmd, i.output, fmt.Errorf("failed to check sources directory: %w", err)) } if !fileInfo.IsDir() { - return fmt.Errorf("sources path is not a directory: %s", i.absSourcesDir) + return outputErrorIfJSON(cmd, i.output, fmt.Errorf("sources path is not a directory: %s", i.absSourcesDir)) } // If workspace-configuration flag was not explicitly set, default to .kortex/ inside sources directory @@ -91,7 +105,7 @@ func (i *initCmd) preRun(cmd *cobra.Command, args []string) error { // 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) + return outputErrorIfJSON(cmd, i.output, fmt.Errorf("failed to resolve workspace configuration directory path: %w", err)) } i.absConfigDir = absConfigDir @@ -107,15 +121,21 @@ func (i *initCmd) run(cmd *cobra.Command, args []string) error { Name: i.name, }) if err != nil { - return fmt.Errorf("failed to create instance: %w", err) + return outputErrorIfJSON(cmd, i.output, 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) + return outputErrorIfJSON(cmd, i.output, err) } + // Handle JSON output + if i.output == "json" { + return i.outputJSON(cmd, addedInstance) + } + + // Handle text output if i.verbose { cmd.Printf("Registered workspace:\n") cmd.Printf(" ID: %s\n", addedInstance.GetID()) @@ -129,6 +149,29 @@ func (i *initCmd) run(cmd *cobra.Command, args []string) error { return nil } +// outputJSON outputs the instance as JSON based on verbose flag +func (i *initCmd) outputJSON(cmd *cobra.Command, instance instances.Instance) error { + var jsonData []byte + var err error + + if i.verbose { + // Verbose mode: return full Workspace + workspace := instanceToWorkspace(instance) + jsonData, err = json.MarshalIndent(workspace, "", " ") + } else { + // Default mode: return only WorkspaceId + workspaceId := instanceToWorkspaceId(instance) + jsonData, err = json.MarshalIndent(workspaceId, "", " ") + } + + if err != nil { + return outputErrorIfJSON(cmd, i.output, fmt.Errorf("failed to marshal to JSON: %w", err)) + } + + fmt.Fprintln(cmd.OutOrStdout(), string(jsonData)) + return nil +} + func NewInitCmd() *cobra.Command { c := &initCmd{} @@ -164,5 +207,8 @@ kortex-cli init --verbose`, // Add verbose flag cmd.Flags().BoolVarP(&c.verbose, "verbose", "v", false, "Show detailed output") + // Add output flag + cmd.Flags().StringVarP(&c.output, "output", "o", "", "Output format (supported: json)") + return cmd } diff --git a/pkg/cmd/init_test.go b/pkg/cmd/init_test.go index 8632e6d..931c0df 100644 --- a/pkg/cmd/init_test.go +++ b/pkg/cmd/init_test.go @@ -20,11 +20,13 @@ package cmd import ( "bytes" + "encoding/json" "os" "path/filepath" "strings" "testing" + api "github.com/kortex-hub/kortex-cli-api/cli/go" "github.com/kortex-hub/kortex-cli/pkg/cmd/testutil" "github.com/kortex-hub/kortex-cli/pkg/instances" "github.com/spf13/cobra" @@ -309,6 +311,114 @@ func TestInitCmd_PreRun(t *testing.T) { t.Errorf("Expected error to contain 'sources path is not a directory', got: %v", err) } }) + + t.Run("accepts empty output flag", func(t *testing.T) { + t.Parallel() + + tempDir := t.TempDir() + + c := &initCmd{ + output: "", // Default empty output + } + 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.output != "" { + t.Errorf("Expected output to be empty, got %s", c.output) + } + }) + + t.Run("accepts json output format", func(t *testing.T) { + t.Parallel() + + tempDir := t.TempDir() + + c := &initCmd{ + output: "json", + } + 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.output != "json" { + t.Errorf("Expected output to be 'json', got %s", c.output) + } + }) + + t.Run("rejects invalid output format", func(t *testing.T) { + t.Parallel() + + tempDir := t.TempDir() + + c := &initCmd{ + output: "xml", + } + 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.Fatal("Expected preRun() to fail with invalid output format") + } + + if !strings.Contains(err.Error(), "unsupported output format") { + t.Errorf("Expected error to contain 'unsupported output format', got: %v", err) + } + if !strings.Contains(err.Error(), "xml") { + t.Errorf("Expected error to mention 'xml', got: %v", err) + } + }) + + t.Run("outputs JSON error when manager creation fails with json output", func(t *testing.T) { + t.Parallel() + + // Use an invalid storage path to cause manager creation to fail + invalidStorage := "/this/path/does/not/exist/and/should/fail" + + c := &initCmd{ + output: "json", + } + cmd := &cobra.Command{} + buf := new(bytes.Buffer) + cmd.SetOut(buf) + cmd.Flags().String("workspace-configuration", "", "test flag") + cmd.Flags().String("storage", invalidStorage, "test storage flag") + + args := []string{} + + err := c.preRun(cmd, args) + if err == nil { + t.Fatal("Expected preRun() to fail with invalid storage path") + } + + // Verify JSON error was output + var errorResponse api.Error + if jsonErr := json.Unmarshal(buf.Bytes(), &errorResponse); jsonErr != nil { + t.Fatalf("Failed to unmarshal error JSON: %v\nOutput was: %s", jsonErr, buf.String()) + } + + if !strings.Contains(errorResponse.Error, "failed to create manager") { + t.Errorf("Expected error to contain 'failed to create manager', got: %s", errorResponse.Error) + } + }) } func TestInitCmd_E2E(t *testing.T) { @@ -1003,6 +1113,198 @@ func TestInitCmd_E2E(t *testing.T) { t.Errorf("Expected 0 instances, got %d", len(instancesList)) } }) + + t.Run("json output returns workspace ID by default", 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, "--output", "json"}) + + err := rootCmd.Execute() + if err != nil { + t.Fatalf("Execute() failed: %v", err) + } + + // Parse JSON output + var workspaceId api.WorkspaceId + if err := json.Unmarshal(buf.Bytes(), &workspaceId); err != nil { + t.Fatalf("Failed to unmarshal JSON: %v", err) + } + + // Verify ID is not empty + if workspaceId.Id == "" { + t.Error("Expected non-empty ID in JSON output") + } + + // Verify only ID field exists + var parsed map[string]interface{} + if err := json.Unmarshal(buf.Bytes(), &parsed); err != nil { + t.Fatalf("Failed to unmarshal to map: %v", err) + } + + if len(parsed) != 1 { + t.Errorf("Expected only 1 field in JSON, got %d: %v", len(parsed), parsed) + } + + if _, exists := parsed["id"]; !exists { + t.Error("Expected 'id' field in JSON") + } + + // Verify instance was actually created + manager, err := instances.NewManager(storageDir) + if err != nil { + t.Fatalf("Failed to create manager: %v", err) + } + + instance, err := manager.Get(workspaceId.Id) + if err != nil { + t.Fatalf("Failed to get instance: %v", err) + } + + if instance.GetID() != workspaceId.Id { + t.Errorf("Expected instance ID %s, got %s", workspaceId.Id, instance.GetID()) + } + }) + + t.Run("json output with verbose returns full workspace", 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, "--output", "json", "--verbose"}) + + err := rootCmd.Execute() + if err != nil { + t.Fatalf("Execute() failed: %v", err) + } + + // Parse JSON output + var workspace api.Workspace + if err := json.Unmarshal(buf.Bytes(), &workspace); err != nil { + t.Fatalf("Failed to unmarshal JSON: %v", err) + } + + // Verify all fields are populated + if workspace.Id == "" { + t.Error("Expected non-empty ID in JSON output") + } + + if workspace.Name == "" { + t.Error("Expected non-empty Name in JSON output") + } + + if workspace.Paths.Source == "" { + t.Error("Expected non-empty Source path in JSON output") + } + + if workspace.Paths.Configuration == "" { + t.Error("Expected non-empty Configuration path in JSON output") + } + + // Verify paths are absolute + if !filepath.IsAbs(workspace.Paths.Source) { + t.Errorf("Expected absolute source path, got %s", workspace.Paths.Source) + } + + if !filepath.IsAbs(workspace.Paths.Configuration) { + t.Errorf("Expected absolute configuration path, got %s", workspace.Paths.Configuration) + } + + // Verify all expected fields exist + var parsed map[string]interface{} + if err := json.Unmarshal(buf.Bytes(), &parsed); err != nil { + t.Fatalf("Failed to unmarshal to map: %v", err) + } + + if _, exists := parsed["id"]; !exists { + t.Error("Expected 'id' field in JSON") + } + if _, exists := parsed["name"]; !exists { + t.Error("Expected 'name' field in JSON") + } + if _, exists := parsed["paths"]; !exists { + t.Error("Expected 'paths' field in JSON") + } + }) + + t.Run("json output error for non-existent directory", func(t *testing.T) { + t.Parallel() + + storageDir := t.TempDir() + nonExistentDir := filepath.Join(storageDir, "does-not-exist") + + rootCmd := NewRootCmd() + buf := new(bytes.Buffer) + rootCmd.SetOut(buf) + rootCmd.SetArgs([]string{"--storage", storageDir, "init", nonExistentDir, "--output", "json"}) + + err := rootCmd.Execute() + if err == nil { + t.Fatal("Expected Execute() to fail with non-existent directory") + } + + // Parse JSON error output + var errorResponse api.Error + if err := json.Unmarshal(buf.Bytes(), &errorResponse); err != nil { + t.Fatalf("Failed to unmarshal error JSON: %v", err) + } + + // Verify error message + if !strings.Contains(errorResponse.Error, "sources directory does not exist") { + t.Errorf("Expected error to contain 'sources directory does not exist', got: %s", errorResponse.Error) + } + + // Verify only error field exists + var parsed map[string]interface{} + if err := json.Unmarshal(buf.Bytes(), &parsed); err != nil { + t.Fatalf("Failed to unmarshal to map: %v", err) + } + + if len(parsed) != 1 { + t.Errorf("Expected only 1 field in error JSON, got %d: %v", len(parsed), parsed) + } + + if _, exists := parsed["error"]; !exists { + t.Error("Expected 'error' field in JSON") + } + }) + + t.Run("json output uses custom name", func(t *testing.T) { + t.Parallel() + + storageDir := t.TempDir() + sourcesDir := t.TempDir() + customName := "my-custom-workspace" + + rootCmd := NewRootCmd() + buf := new(bytes.Buffer) + rootCmd.SetOut(buf) + rootCmd.SetArgs([]string{"--storage", storageDir, "init", sourcesDir, "--name", customName, "--output", "json", "--verbose"}) + + err := rootCmd.Execute() + if err != nil { + t.Fatalf("Execute() failed: %v", err) + } + + // Parse JSON output + var workspace api.Workspace + if err := json.Unmarshal(buf.Bytes(), &workspace); err != nil { + t.Fatalf("Failed to unmarshal JSON: %v", err) + } + + if workspace.Name != customName { + t.Errorf("Expected name %s in JSON output, got %s", customName, workspace.Name) + } + }) } func TestInitCmd_Examples(t *testing.T) { diff --git a/pkg/cmd/workspace_list.go b/pkg/cmd/workspace_list.go index d01867b..921a7f6 100644 --- a/pkg/cmd/workspace_list.go +++ b/pkg/cmd/workspace_list.go @@ -36,34 +36,34 @@ type workspaceListCmd struct { // preRun validates the parameters and flags func (w *workspaceListCmd) preRun(cmd *cobra.Command, args []string) error { + // Validate output format if specified + if w.output != "" && w.output != "json" { + return fmt.Errorf("unsupported output format: %s (supported: json)", w.output) + } + + // Silence Cobra's error and usage output when JSON mode is enabled + // This prevents "Error: ..." and usage info from being printed + if w.output == "json" { + cmd.SilenceErrors = true + cmd.SilenceUsage = true + } + // 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) + return outputErrorIfJSON(cmd, w.output, fmt.Errorf("failed to read --storage flag: %w", err)) } // Convert to absolute path absStorageDir, err := filepath.Abs(storageDir) if err != nil { - return fmt.Errorf("failed to resolve storage directory path: %w", err) - } - - // Get output format flag - output, err := cmd.Flags().GetString("output") - if err != nil { - return fmt.Errorf("failed to read --output flag: %w", err) - } - - // Validate output format if specified - if output != "" && output != "json" { - return fmt.Errorf("unsupported output format: %s (supported: json)", output) + return outputErrorIfJSON(cmd, w.output, fmt.Errorf("failed to resolve storage directory path: %w", err)) } - w.output = output // Create manager manager, err := instances.NewManager(absStorageDir) if err != nil { - return fmt.Errorf("failed to create manager: %w", err) + return outputErrorIfJSON(cmd, w.output, fmt.Errorf("failed to create manager: %w", err)) } w.manager = manager @@ -75,7 +75,7 @@ func (w *workspaceListCmd) run(cmd *cobra.Command, args []string) error { // Get all instances instancesList, err := w.manager.List() if err != nil { - return fmt.Errorf("failed to list instances: %w", err) + return outputErrorIfJSON(cmd, w.output, fmt.Errorf("failed to list instances: %w", err)) } // Handle JSON output format @@ -105,14 +105,7 @@ func (w *workspaceListCmd) outputJSON(cmd *cobra.Command, instancesList []instan // Convert instances to API Workspace format workspaces := make([]api.Workspace, 0, len(instancesList)) for _, instance := range instancesList { - workspace := api.Workspace{ - Id: instance.GetID(), - Name: instance.GetName(), - Paths: api.WorkspacePaths{ - Configuration: instance.GetConfigDir(), - Source: instance.GetSourceDir(), - }, - } + workspace := instanceToWorkspace(instance) workspaces = append(workspaces, workspace) } @@ -124,7 +117,7 @@ func (w *workspaceListCmd) outputJSON(cmd *cobra.Command, instancesList []instan // Marshal to JSON with indentation jsonData, err := json.MarshalIndent(workspacesList, "", " ") if err != nil { - return fmt.Errorf("failed to marshal workspaces to JSON: %w", err) + return outputErrorIfJSON(cmd, w.output, fmt.Errorf("failed to marshal workspaces to JSON: %w", err)) } // Output the JSON to stdout @@ -152,7 +145,7 @@ kortex-cli workspace list -o json`, RunE: c.run, } - cmd.Flags().StringP("output", "o", "", "Output format (supported: json)") + cmd.Flags().StringVarP(&c.output, "output", "o", "", "Output format (supported: json)") return cmd } diff --git a/pkg/cmd/workspace_list_test.go b/pkg/cmd/workspace_list_test.go index 2f8dbb2..d968ebb 100644 --- a/pkg/cmd/workspace_list_test.go +++ b/pkg/cmd/workspace_list_test.go @@ -55,7 +55,6 @@ func TestWorkspaceListCmd_PreRun(t *testing.T) { c := &workspaceListCmd{} cmd := &cobra.Command{} cmd.Flags().String("storage", storageDir, "test storage flag") - cmd.Flags().String("output", "", "test output flag") args := []string{} @@ -74,10 +73,11 @@ func TestWorkspaceListCmd_PreRun(t *testing.T) { storageDir := t.TempDir() - c := &workspaceListCmd{} + c := &workspaceListCmd{ + output: "", + } cmd := &cobra.Command{} cmd.Flags().String("storage", storageDir, "test storage flag") - cmd.Flags().String("output", "", "test output flag") args := []string{} @@ -96,11 +96,11 @@ func TestWorkspaceListCmd_PreRun(t *testing.T) { storageDir := t.TempDir() - c := &workspaceListCmd{} + c := &workspaceListCmd{ + output: "json", + } cmd := &cobra.Command{} cmd.Flags().String("storage", storageDir, "test storage flag") - cmd.Flags().String("output", "", "test output flag") - cmd.Flags().Set("output", "json") args := []string{} @@ -119,11 +119,11 @@ func TestWorkspaceListCmd_PreRun(t *testing.T) { storageDir := t.TempDir() - c := &workspaceListCmd{} + c := &workspaceListCmd{ + output: "xml", + } cmd := &cobra.Command{} cmd.Flags().String("storage", storageDir, "test storage flag") - cmd.Flags().String("output", "", "test output flag") - cmd.Flags().Set("output", "xml") args := []string{} @@ -142,11 +142,11 @@ func TestWorkspaceListCmd_PreRun(t *testing.T) { storageDir := t.TempDir() - c := &workspaceListCmd{} + c := &workspaceListCmd{ + output: "yaml", + } cmd := &cobra.Command{} cmd.Flags().String("storage", storageDir, "test storage flag") - cmd.Flags().String("output", "", "test output flag") - cmd.Flags().Set("output", "yaml") args := []string{} @@ -159,6 +159,38 @@ func TestWorkspaceListCmd_PreRun(t *testing.T) { t.Errorf("Expected error to contain 'unsupported output format', got: %v", err) } }) + + t.Run("outputs JSON error when manager creation fails with json output", func(t *testing.T) { + t.Parallel() + + // Use an invalid storage path to cause manager creation to fail + invalidStorage := "/this/path/does/not/exist/and/should/fail" + + c := &workspaceListCmd{ + output: "json", + } + cmd := &cobra.Command{} + buf := new(bytes.Buffer) + cmd.SetOut(buf) + cmd.Flags().String("storage", invalidStorage, "test storage flag") + + args := []string{} + + err := c.preRun(cmd, args) + if err == nil { + t.Fatal("Expected preRun() to fail with invalid storage path") + } + + // Verify JSON error was output + var errorResponse api.Error + if jsonErr := json.Unmarshal(buf.Bytes(), &errorResponse); jsonErr != nil { + t.Fatalf("Failed to unmarshal error JSON: %v\nOutput was: %s", jsonErr, buf.String()) + } + + if !strings.Contains(errorResponse.Error, "failed to create manager") { + t.Errorf("Expected error to contain 'failed to create manager', got: %s", errorResponse.Error) + } + }) } func TestWorkspaceListCmd_E2E(t *testing.T) { diff --git a/pkg/cmd/workspace_remove.go b/pkg/cmd/workspace_remove.go index 75cb38e..4ef8158 100644 --- a/pkg/cmd/workspace_remove.go +++ b/pkg/cmd/workspace_remove.go @@ -19,10 +19,12 @@ package cmd import ( + "encoding/json" "errors" "fmt" "path/filepath" + api "github.com/kortex-hub/kortex-cli-api/cli/go" "github.com/kortex-hub/kortex-cli/pkg/instances" "github.com/spf13/cobra" ) @@ -31,28 +33,41 @@ import ( type workspaceRemoveCmd struct { manager instances.Manager id string + output string } // preRun validates the parameters and flags func (w *workspaceRemoveCmd) preRun(cmd *cobra.Command, args []string) error { + // Validate output format if specified + if w.output != "" && w.output != "json" { + return fmt.Errorf("unsupported output format: %s (supported: json)", w.output) + } + + // Silence Cobra's error and usage output when JSON mode is enabled + // This prevents "Error: ..." and usage info from being printed + if w.output == "json" { + cmd.SilenceErrors = true + cmd.SilenceUsage = true + } + w.id = args[0] // 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) + return outputErrorIfJSON(cmd, w.output, fmt.Errorf("failed to read --storage flag: %w", err)) } // Normalize storage path to absolute path absStorageDir, err := filepath.Abs(storageDir) if err != nil { - return fmt.Errorf("failed to resolve absolute path for storage directory: %w", err) + return outputErrorIfJSON(cmd, w.output, fmt.Errorf("failed to resolve absolute path for storage directory: %w", err)) } // Create manager manager, err := instances.NewManager(absStorageDir) if err != nil { - return fmt.Errorf("failed to create manager: %w", err) + return outputErrorIfJSON(cmd, w.output, fmt.Errorf("failed to create manager: %w", err)) } w.manager = manager @@ -65,16 +80,40 @@ func (w *workspaceRemoveCmd) run(cmd *cobra.Command, args []string) error { err := w.manager.Delete(w.id) if err != nil { if errors.Is(err, instances.ErrInstanceNotFound) { + if w.output == "json" { + return outputErrorIfJSON(cmd, w.output, fmt.Errorf("workspace not found: %s", w.id)) + } return fmt.Errorf("workspace not found: %s\nUse 'workspace list' to see available workspaces", w.id) } - return fmt.Errorf("failed to remove workspace: %w", err) + return outputErrorIfJSON(cmd, w.output, err) } - // Output only the ID + // Handle JSON output + if w.output == "json" { + return w.outputJSON(cmd) + } + + // Output only the ID (text mode) cmd.Println(w.id) return nil } +// outputJSON outputs the workspace ID as JSON +func (w *workspaceRemoveCmd) outputJSON(cmd *cobra.Command) error { + // Return only the ID (per OpenAPI spec) + workspaceId := api.WorkspaceId{ + Id: w.id, + } + + jsonData, err := json.MarshalIndent(workspaceId, "", " ") + if err != nil { + return outputErrorIfJSON(cmd, w.output, fmt.Errorf("failed to marshal to JSON: %w", err)) + } + + fmt.Fprintln(cmd.OutOrStdout(), string(jsonData)) + return nil +} + func NewWorkspaceRemoveCmd() *cobra.Command { c := &workspaceRemoveCmd{} @@ -89,5 +128,7 @@ kortex-cli workspace remove abc123`, RunE: c.run, } + cmd.Flags().StringVarP(&c.output, "output", "o", "", "Output format (supported: json)") + return cmd } diff --git a/pkg/cmd/workspace_remove_test.go b/pkg/cmd/workspace_remove_test.go index 9a8224b..ff84e50 100644 --- a/pkg/cmd/workspace_remove_test.go +++ b/pkg/cmd/workspace_remove_test.go @@ -20,10 +20,12 @@ package cmd import ( "bytes" + "encoding/json" "path/filepath" "strings" "testing" + api "github.com/kortex-hub/kortex-cli-api/cli/go" "github.com/kortex-hub/kortex-cli/pkg/cmd/testutil" "github.com/kortex-hub/kortex-cli/pkg/instances" "github.com/spf13/cobra" @@ -69,6 +71,110 @@ func TestWorkspaceRemoveCmd_PreRun(t *testing.T) { t.Errorf("Expected id to be 'test-workspace-id', got %s", c.id) } }) + + t.Run("accepts empty output flag", func(t *testing.T) { + t.Parallel() + + storageDir := t.TempDir() + + c := &workspaceRemoveCmd{ + output: "", + } + cmd := &cobra.Command{} + cmd.Flags().String("storage", storageDir, "test storage flag") + + args := []string{"test-id"} + + err := c.preRun(cmd, args) + if err != nil { + t.Fatalf("preRun() failed: %v", err) + } + + if c.output != "" { + t.Errorf("Expected output to be empty, got %s", c.output) + } + }) + + t.Run("accepts json output format", func(t *testing.T) { + t.Parallel() + + storageDir := t.TempDir() + + c := &workspaceRemoveCmd{ + output: "json", + } + cmd := &cobra.Command{} + cmd.Flags().String("storage", storageDir, "test storage flag") + + args := []string{"test-id"} + + err := c.preRun(cmd, args) + if err != nil { + t.Fatalf("preRun() failed: %v", err) + } + + if c.output != "json" { + t.Errorf("Expected output to be 'json', got %s", c.output) + } + }) + + t.Run("rejects invalid output format", func(t *testing.T) { + t.Parallel() + + storageDir := t.TempDir() + + c := &workspaceRemoveCmd{ + output: "yaml", + } + cmd := &cobra.Command{} + cmd.Flags().String("storage", storageDir, "test storage flag") + + args := []string{"test-id"} + + err := c.preRun(cmd, args) + if err == nil { + t.Fatal("Expected preRun() to fail with invalid output format") + } + + if !strings.Contains(err.Error(), "unsupported output format") { + t.Errorf("Expected error to contain 'unsupported output format', got: %v", err) + } + if !strings.Contains(err.Error(), "yaml") { + t.Errorf("Expected error to mention 'yaml', got: %v", err) + } + }) + + t.Run("outputs JSON error when manager creation fails with json output", func(t *testing.T) { + t.Parallel() + + // Use an invalid storage path to cause manager creation to fail + invalidStorage := "/this/path/does/not/exist/and/should/fail" + + c := &workspaceRemoveCmd{ + output: "json", + } + cmd := &cobra.Command{} + buf := new(bytes.Buffer) + cmd.SetOut(buf) + cmd.Flags().String("storage", invalidStorage, "test storage flag") + + args := []string{"test-id"} + + err := c.preRun(cmd, args) + if err == nil { + t.Fatal("Expected preRun() to fail with invalid storage path") + } + + // Verify JSON error was output + var errorResponse api.Error + if jsonErr := json.Unmarshal(buf.Bytes(), &errorResponse); jsonErr != nil { + t.Fatalf("Failed to unmarshal error JSON: %v\nOutput was: %s", jsonErr, buf.String()) + } + + if !strings.Contains(errorResponse.Error, "failed to create manager") { + t.Errorf("Expected error to contain 'failed to create manager', got: %s", errorResponse.Error) + } + }) } func TestWorkspaceRemoveCmd_E2E(t *testing.T) { @@ -327,6 +433,201 @@ func TestWorkspaceRemoveCmd_E2E(t *testing.T) { t.Errorf("Expected 0 instances after removal, got %d", len(instancesList)) } }) + + t.Run("json output returns workspace ID", func(t *testing.T) { + t.Parallel() + + storageDir := t.TempDir() + sourcesDir := t.TempDir() + + // Create a workspace + manager, err := instances.NewManager(storageDir) + if err != nil { + t.Fatalf("Failed to create manager: %v", err) + } + + instance, err := instances.NewInstance(instances.NewInstanceParams{ + SourceDir: sourcesDir, + ConfigDir: filepath.Join(sourcesDir, ".kortex"), + }) + if err != nil { + t.Fatalf("Failed to create instance: %v", err) + } + + addedInstance, err := manager.Add(instance) + if err != nil { + t.Fatalf("Failed to add instance: %v", err) + } + + instanceID := addedInstance.GetID() + + // Remove with JSON output + rootCmd := NewRootCmd() + buf := new(bytes.Buffer) + rootCmd.SetOut(buf) + rootCmd.SetArgs([]string{"workspace", "remove", instanceID, "--storage", storageDir, "--output", "json"}) + + err = rootCmd.Execute() + if err != nil { + t.Fatalf("Execute() failed: %v", err) + } + + // Parse JSON output + var workspaceId api.WorkspaceId + if err := json.Unmarshal(buf.Bytes(), &workspaceId); err != nil { + t.Fatalf("Failed to unmarshal JSON: %v", err) + } + + // Verify ID matches + if workspaceId.Id != instanceID { + t.Errorf("Expected ID %s in JSON output, got %s", instanceID, workspaceId.Id) + } + + // Verify only ID field exists + var parsed map[string]interface{} + if err := json.Unmarshal(buf.Bytes(), &parsed); err != nil { + t.Fatalf("Failed to unmarshal to map: %v", err) + } + + if len(parsed) != 1 { + t.Errorf("Expected only 1 field in JSON, got %d: %v", len(parsed), parsed) + } + + if _, exists := parsed["id"]; !exists { + t.Error("Expected 'id' field in JSON") + } + + // Verify workspace is actually removed + instancesList, err := manager.List() + if err != nil { + t.Fatalf("Failed to list instances: %v", err) + } + + if len(instancesList) != 0 { + t.Errorf("Expected 0 instances after removal, got %d", len(instancesList)) + } + }) + + t.Run("json output error for non-existent workspace", func(t *testing.T) { + t.Parallel() + + storageDir := t.TempDir() + + rootCmd := NewRootCmd() + buf := new(bytes.Buffer) + rootCmd.SetOut(buf) + rootCmd.SetArgs([]string{"workspace", "remove", "invalid-id", "--storage", storageDir, "--output", "json"}) + + err := rootCmd.Execute() + if err == nil { + t.Fatal("Expected Execute() to fail with non-existent workspace") + } + + // Parse JSON error output + var errorResponse api.Error + if err := json.Unmarshal(buf.Bytes(), &errorResponse); err != nil { + t.Fatalf("Failed to unmarshal error JSON: %v", err) + } + + // Verify error message + if !strings.Contains(errorResponse.Error, "workspace not found") { + t.Errorf("Expected error to contain 'workspace not found', got: %s", errorResponse.Error) + } + + if !strings.Contains(errorResponse.Error, "invalid-id") { + t.Errorf("Expected error to contain 'invalid-id', got: %s", errorResponse.Error) + } + + // Verify only error field exists + var parsed map[string]interface{} + if err := json.Unmarshal(buf.Bytes(), &parsed); err != nil { + t.Fatalf("Failed to unmarshal to map: %v", err) + } + + if len(parsed) != 1 { + t.Errorf("Expected only 1 field in error JSON, got %d: %v", len(parsed), parsed) + } + + if _, exists := parsed["error"]; !exists { + t.Error("Expected 'error' field in JSON") + } + }) + + t.Run("json output removes correct workspace when multiple exist", func(t *testing.T) { + t.Parallel() + + storageDir := t.TempDir() + sourcesDir1 := t.TempDir() + sourcesDir2 := t.TempDir() + + // Create two workspaces + manager, err := instances.NewManager(storageDir) + if err != nil { + t.Fatalf("Failed to create manager: %v", err) + } + + 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(instances.NewInstanceParams{ + SourceDir: sourcesDir2, + ConfigDir: filepath.Join(sourcesDir2, ".kortex"), + }) + if err != nil { + t.Fatalf("Failed to create instance 2: %v", err) + } + + addedInstance1, err := manager.Add(instance1) + if err != nil { + t.Fatalf("Failed to add instance 1: %v", err) + } + + addedInstance2, err := manager.Add(instance2) + if err != nil { + t.Fatalf("Failed to add instance 2: %v", err) + } + + // Remove first workspace with JSON output + rootCmd := NewRootCmd() + buf := new(bytes.Buffer) + rootCmd.SetOut(buf) + rootCmd.SetArgs([]string{"workspace", "remove", addedInstance1.GetID(), "--storage", storageDir, "--output", "json"}) + + err = rootCmd.Execute() + if err != nil { + t.Fatalf("Execute() failed: %v", err) + } + + // Parse JSON output + var workspaceId api.WorkspaceId + if err := json.Unmarshal(buf.Bytes(), &workspaceId); err != nil { + t.Fatalf("Failed to unmarshal JSON: %v", err) + } + + // Verify correct ID was removed + if workspaceId.Id != addedInstance1.GetID() { + t.Errorf("Expected ID %s in JSON output, got %s", addedInstance1.GetID(), workspaceId.Id) + } + + // Verify only instance2 remains + instancesList, err := manager.List() + if err != nil { + t.Fatalf("Failed to list instances: %v", err) + } + + if len(instancesList) != 1 { + t.Fatalf("Expected 1 instance after removal, got %d", len(instancesList)) + } + + if instancesList[0].GetID() != addedInstance2.GetID() { + t.Errorf("Expected remaining instance ID %s, got %s", addedInstance2.GetID(), instancesList[0].GetID()) + } + }) } func TestWorkspaceRemoveCmd_Examples(t *testing.T) { From 45953febcf6d4a2495e06cbfdf4cf3fb35c87373 Mon Sep 17 00:00:00 2001 From: Philippe Martin Date: Thu, 12 Mar 2026 11:50:27 +0100 Subject: [PATCH 3/6] docs: add scenario for programmatic workspace management Adds a new "Scenarios" section demonstrating how to manage workspaces from UIs or scripts using JSON output. Includes a complete workflow with 6 steps showing list, init, remove operations, error handling examples, best practices, and a bash script pattern. Closes #54 Co-Authored-By: Claude Code (Claude Sonnet 4.5) Signed-off-by: Philippe Martin --- README.md | 252 +++++++++++++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 251 insertions(+), 1 deletion(-) diff --git a/README.md b/README.md index af7aace..b624832 100644 --- a/README.md +++ b/README.md @@ -36,6 +36,187 @@ Pre-configured capabilities or specialized functions that can be enabled for an ### 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. +## Scenarios + +### Managing Workspaces from a UI or Programmatically + +This scenario demonstrates how to manage workspaces programmatically using JSON output, which is ideal for UIs, scripts, or automation tools. All commands support the `--output json` (or `-o json`) flag for machine-readable output. + +**Step 1: Check existing workspaces** + +```bash +$ kortex-cli workspace list -o json +``` + +```json +{ + "items": [] +} +``` + +Exit code: `0` (success, but no workspaces registered) + +**Step 2: Register a new workspace** + +```bash +$ kortex-cli init /path/to/project -o json +``` + +```json +{ + "id": "2c5f16046476be368fcada501ac6cdc6bbd34ea80eb9ceb635530c0af64681ea" +} +``` + +Exit code: `0` (success) + +**Step 3: Register with verbose output to get full details** + +```bash +$ kortex-cli init /path/to/another-project -o json -v +``` + +```json +{ + "id": "f6e5d4c3b2a1098765432109876543210987654321098765432109876543210a", + "name": "another-project", + "paths": { + "source": "/absolute/path/to/another-project", + "configuration": "/absolute/path/to/another-project/.kortex" + } +} +``` + +Exit code: `0` (success) + +**Step 4: List all workspaces** + +```bash +$ kortex-cli workspace list -o json +``` + +```json +{ + "items": [ + { + "id": "2c5f16046476be368fcada501ac6cdc6bbd34ea80eb9ceb635530c0af64681ea", + "name": "project", + "paths": { + "source": "/absolute/path/to/project", + "configuration": "/absolute/path/to/project/.kortex" + } + }, + { + "id": "f6e5d4c3b2a1098765432109876543210987654321098765432109876543210a", + "name": "another-project", + "paths": { + "source": "/absolute/path/to/another-project", + "configuration": "/absolute/path/to/another-project/.kortex" + } + } + ] +} +``` + +Exit code: `0` (success) + +**Step 5: Remove a workspace** + +```bash +$ kortex-cli workspace remove 2c5f16046476be368fcada501ac6cdc6bbd34ea80eb9ceb635530c0af64681ea -o json +``` + +```json +{ + "id": "2c5f16046476be368fcada501ac6cdc6bbd34ea80eb9ceb635530c0af64681ea" +} +``` + +Exit code: `0` (success) + +**Step 6: Verify removal** + +```bash +$ kortex-cli workspace list -o json +``` + +```json +{ + "items": [ + { + "id": "f6e5d4c3b2a1098765432109876543210987654321098765432109876543210a", + "name": "another-project", + "paths": { + "source": "/absolute/path/to/another-project", + "configuration": "/absolute/path/to/another-project/.kortex" + } + } + ] +} +``` + +Exit code: `0` (success) + +#### Error Handling + +All errors are returned in JSON format when using `--output json`, with the error written to **stdout** (not stderr) and a non-zero exit code. + +**Error: Non-existent directory** + +```bash +$ kortex-cli init /tmp/no-exist -o json +``` + +```json +{ + "error": "sources directory does not exist: /tmp/no-exist" +} +``` + +Exit code: `1` (error) + +**Error: Workspace not found** + +```bash +$ kortex-cli workspace remove unknown-id -o json +``` + +```json +{ + "error": "workspace not found: unknown-id" +} +``` + +Exit code: `1` (error) + +#### Best Practices for Programmatic Usage + +1. **Always check the exit code** to determine success (0) or failure (non-zero) +2. **Parse stdout** for JSON output in both success and error cases +4. **Use verbose mode** with init (`-v`) when you need full workspace details immediately after creation +6. **Handle both success and error JSON structures** in your code: + - Success responses have specific fields (e.g., `id`, `items`, `name`, `paths`) + - Error responses always have an `error` field + +**Example script pattern:** + +```bash +#!/bin/bash + +# Register a workspace +output=$(kortex-cli init /path/to/project -o json) +exit_code=$? + +if [ $exit_code -eq 0 ]; then + workspace_id=$(echo "$output" | jq -r '.id') + echo "Workspace created: $workspace_id" +else + error_msg=$(echo "$output" | jq -r '.error') + echo "Error: $error_msg" + exit 1 +fi +``` + ## Commands ### `init` - Register a New Workspace @@ -57,6 +238,7 @@ kortex-cli init [sources-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 +- `--output, -o ` - Output format (supported: `json`) - `--storage ` - Storage directory for kortex-cli data (default: `$HOME/.kortex-cli`) #### Examples @@ -95,6 +277,38 @@ Registered workspace: Configuration directory: /absolute/path/to/myproject/.kortex ``` +**JSON output (default - ID only):** +```bash +kortex-cli init /path/to/myproject --output json +``` +Output: +```json +{ + "id": "a1b2c3d4e5f6..." +} +``` + +**JSON output with verbose flag (full workspace details):** +```bash +kortex-cli init /path/to/myproject --output json --verbose +``` +Output: +```json +{ + "id": "a1b2c3d4e5f6...", + "name": "myproject", + "paths": { + "source": "/absolute/path/to/myproject", + "configuration": "/absolute/path/to/myproject/.kortex" + } +} +``` + +**JSON output with short flags:** +```bash +kortex-cli init -o json -v +``` + #### Workspace Naming - If `--name` is not provided, the name is automatically generated from the last component of the sources directory path @@ -121,6 +335,10 @@ kortex-cli init /tmp/project --name "project" - 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 +- JSON output format is useful for scripting and automation +- Without `--verbose`, JSON output returns only the workspace ID +- With `--verbose`, JSON output includes full workspace details (ID, name, paths) +- **JSON error handling**: When `--output json` is used, errors are written to stdout (not stderr) in JSON format, and the CLI exits with code 1. Always check the exit code to determine success/failure ### `workspace list` - List All Registered Workspaces @@ -200,6 +418,7 @@ kortex-cli list -o json - When no workspaces are registered, the command displays "No workspaces registered" - The JSON output format is useful for scripting and automation - All paths are displayed as absolute paths for consistency +- **JSON error handling**: When `--output json` is used, errors are written to stdout (not stderr) in JSON format, and the CLI exits with code 1. Always check the exit code to determine success/failure ### `workspace remove` - Remove a Workspace @@ -218,6 +437,7 @@ kortex-cli remove ID [flags] #### Flags +- `--output, -o ` - Output format (supported: `json`) - `--storage ` - Storage directory for kortex-cli data (default: `$HOME/.kortex-cli`) #### Examples @@ -242,9 +462,25 @@ kortex-cli list kortex-cli remove a1b2c3d4e5f6... ``` +**JSON output:** +```bash +kortex-cli workspace remove a1b2c3d4e5f6... --output json +``` +Output: +```json +{ + "id": "a1b2c3d4e5f6..." +} +``` + +**JSON output with short flag:** +```bash +kortex-cli remove a1b2c3d4e5f6... -o json +``` + #### Error Handling -**Workspace not found:** +**Workspace not found (text format):** ```bash kortex-cli remove invalid-id ``` @@ -254,9 +490,23 @@ Error: workspace not found: invalid-id Use 'workspace list' to see available workspaces ``` +**Workspace not found (JSON format):** +```bash +kortex-cli remove invalid-id --output json +``` +Output: +```json +{ + "error": "workspace not found: invalid-id" +} +``` + #### Notes - The workspace ID is required and can be obtained using the `workspace list` or `list` command - Removing a workspace only unregisters it from kortex-cli; it does not delete any files from the sources or configuration directories - If the workspace ID is not found, the command will fail with a helpful error message - Upon successful removal, the command outputs the ID of the removed workspace +- JSON output format is useful for scripting and automation +- When using `--output json`, errors are also returned in JSON format for consistent parsing +- **JSON error handling**: When `--output json` is used, errors are written to stdout (not stderr) in JSON format, and the CLI exits with code 1. Always check the exit code to determine success/failure From cb338d950f393c0736927c9bb87f6cc38ee6dce9 Mon Sep 17 00:00:00 2001 From: Philippe Martin Date: Thu, 12 Mar 2026 12:02:40 +0100 Subject: [PATCH 4/6] fix(test): use cross-platform path for manager creation failure tests Replaces hardcoded Unix path "/this/path/does/not/exist" with a cross-platform approach that creates a file in tempDir and uses it as a parent directory. This reliably fails os.MkdirAll on all platforms (Windows, Linux, macOS). Co-Authored-By: Claude Code (Claude Sonnet 4.5) Signed-off-by: Philippe Martin --- pkg/cmd/init_test.go | 9 +++++++-- pkg/cmd/workspace_list_test.go | 10 ++++++++-- pkg/cmd/workspace_remove_test.go | 10 ++++++++-- 3 files changed, 23 insertions(+), 6 deletions(-) diff --git a/pkg/cmd/init_test.go b/pkg/cmd/init_test.go index 931c0df..2d9e2ec 100644 --- a/pkg/cmd/init_test.go +++ b/pkg/cmd/init_test.go @@ -390,8 +390,13 @@ func TestInitCmd_PreRun(t *testing.T) { t.Run("outputs JSON error when manager creation fails with json output", func(t *testing.T) { t.Parallel() - // Use an invalid storage path to cause manager creation to fail - invalidStorage := "/this/path/does/not/exist/and/should/fail" + tempDir := t.TempDir() + // Create a file and try to use it as a parent directory - will fail cross-platform + notADir := filepath.Join(tempDir, "file") + if err := os.WriteFile(notADir, []byte("test"), 0644); err != nil { + t.Fatalf("Failed to create test file: %v", err) + } + invalidStorage := filepath.Join(notADir, "subdir") c := &initCmd{ output: "json", diff --git a/pkg/cmd/workspace_list_test.go b/pkg/cmd/workspace_list_test.go index d968ebb..cec5d65 100644 --- a/pkg/cmd/workspace_list_test.go +++ b/pkg/cmd/workspace_list_test.go @@ -21,6 +21,7 @@ package cmd import ( "bytes" "encoding/json" + "os" "path/filepath" "strings" "testing" @@ -163,8 +164,13 @@ func TestWorkspaceListCmd_PreRun(t *testing.T) { t.Run("outputs JSON error when manager creation fails with json output", func(t *testing.T) { t.Parallel() - // Use an invalid storage path to cause manager creation to fail - invalidStorage := "/this/path/does/not/exist/and/should/fail" + tempDir := t.TempDir() + // Create a file and try to use it as a parent directory - will fail cross-platform + notADir := filepath.Join(tempDir, "file") + if err := os.WriteFile(notADir, []byte("test"), 0644); err != nil { + t.Fatalf("Failed to create test file: %v", err) + } + invalidStorage := filepath.Join(notADir, "subdir") c := &workspaceListCmd{ output: "json", diff --git a/pkg/cmd/workspace_remove_test.go b/pkg/cmd/workspace_remove_test.go index ff84e50..29704a2 100644 --- a/pkg/cmd/workspace_remove_test.go +++ b/pkg/cmd/workspace_remove_test.go @@ -21,6 +21,7 @@ package cmd import ( "bytes" "encoding/json" + "os" "path/filepath" "strings" "testing" @@ -147,8 +148,13 @@ func TestWorkspaceRemoveCmd_PreRun(t *testing.T) { t.Run("outputs JSON error when manager creation fails with json output", func(t *testing.T) { t.Parallel() - // Use an invalid storage path to cause manager creation to fail - invalidStorage := "/this/path/does/not/exist/and/should/fail" + tempDir := t.TempDir() + // Create a file and try to use it as a parent directory - will fail cross-platform + notADir := filepath.Join(tempDir, "file") + if err := os.WriteFile(notADir, []byte("test"), 0644); err != nil { + t.Fatalf("Failed to create test file: %v", err) + } + invalidStorage := filepath.Join(notADir, "subdir") c := &workspaceRemoveCmd{ output: "json", From 977b762db6d3ce593b2e5b11583076b75c7c4813 Mon Sep 17 00:00:00 2001 From: Philippe Martin Date: Thu, 12 Mar 2026 12:04:33 +0100 Subject: [PATCH 5/6] docs: fix numbered list Signed-off-by: Philippe Martin --- README.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/README.md b/README.md index b624832..604880d 100644 --- a/README.md +++ b/README.md @@ -193,8 +193,8 @@ Exit code: `1` (error) 1. **Always check the exit code** to determine success (0) or failure (non-zero) 2. **Parse stdout** for JSON output in both success and error cases -4. **Use verbose mode** with init (`-v`) when you need full workspace details immediately after creation -6. **Handle both success and error JSON structures** in your code: +3. **Use verbose mode** with init (`-v`) when you need full workspace details immediately after creation +4. **Handle both success and error JSON structures** in your code: - Success responses have specific fields (e.g., `id`, `items`, `name`, `paths`) - Error responses always have an `error` field From cb6bb6ca7c15e1b1ba2968e9973dfaaeee703123 Mon Sep 17 00:00:00 2001 From: Philippe Martin Date: Thu, 12 Mar 2026 13:34:19 +0100 Subject: [PATCH 6/6] fix(test): use t.TempDir() in conversion tests and enhance docs Fixes Windows test failures by replacing hardcoded Unix paths like "/path/to/source" with t.TempDir(). Enhances AGENTS.md with prominent warnings, common mistakes section, and clear examples of what NOT to do in tests to prevent future cross-platform issues. Co-Authored-By: Claude Code (Claude Sonnet 4.5) Signed-off-by: Philippe Martin --- AGENTS.md | 47 +++++++++++++++++++++++++++++++++++--- pkg/cmd/conversion_test.go | 29 +++++++++++++++-------- 2 files changed, 63 insertions(+), 13 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index 269440b..bb90931 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -706,16 +706,55 @@ if err != nil { ### Cross-Platform Path Handling -**IMPORTANT**: All path operations must be cross-platform compatible (Linux, macOS, Windows). +⚠️ **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 "\\") - 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 +- **Use `t.TempDir()` for ALL temporary directories in tests - never hardcode paths** -**Examples:** +#### 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 ```go // GOOD: Cross-platform path construction @@ -743,6 +782,8 @@ tempDir := t.TempDir() // Automatic cleanup sourcesDir := t.TempDir() ``` +**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. + ## Documentation Standards ### Markdown Best Practices diff --git a/pkg/cmd/conversion_test.go b/pkg/cmd/conversion_test.go index 5f62300..260c9b5 100644 --- a/pkg/cmd/conversion_test.go +++ b/pkg/cmd/conversion_test.go @@ -36,9 +36,12 @@ func TestInstanceToWorkspaceId(t *testing.T) { t.Run("converts instance to workspace ID", func(t *testing.T) { t.Parallel() + sourceDir := t.TempDir() + configDir := t.TempDir() + instance, err := instances.NewInstance(instances.NewInstanceParams{ - SourceDir: "/path/to/source", - ConfigDir: "/path/to/config", + SourceDir: sourceDir, + ConfigDir: configDir, Name: "test-workspace", }) if err != nil { @@ -83,9 +86,12 @@ func TestInstanceToWorkspace(t *testing.T) { t.Run("converts instance to full workspace", func(t *testing.T) { t.Parallel() + sourceDir := t.TempDir() + configDir := t.TempDir() + instance, err := instances.NewInstance(instances.NewInstanceParams{ - SourceDir: "/path/to/source", - ConfigDir: "/path/to/config", + SourceDir: sourceDir, + ConfigDir: configDir, Name: "test-workspace", }) if err != nil { @@ -107,21 +113,24 @@ func TestInstanceToWorkspace(t *testing.T) { t.Errorf("Expected name 'test-workspace', got '%s'", result.Name) } - if result.Paths.Source != "/path/to/source" { - t.Errorf("Expected source '/path/to/source', got '%s'", result.Paths.Source) + if result.Paths.Source != sourceDir { + t.Errorf("Expected source '%s', got '%s'", sourceDir, result.Paths.Source) } - if result.Paths.Configuration != "/path/to/config" { - t.Errorf("Expected config '/path/to/config', got '%s'", result.Paths.Configuration) + if result.Paths.Configuration != configDir { + t.Errorf("Expected config '%s', got '%s'", configDir, result.Paths.Configuration) } }) t.Run("includes all required fields", func(t *testing.T) { t.Parallel() + sourceDir := t.TempDir() + configDir := t.TempDir() + instance, err := instances.NewInstance(instances.NewInstanceParams{ - SourceDir: "/some/path", - ConfigDir: "/config/path", + SourceDir: sourceDir, + ConfigDir: configDir, Name: "my-workspace", }) if err != nil {