Improve error handling and logging

This commit enhances error handling and logging across various components, including service initialization, package management, and API responses. It ensures that critical errors are logged with sufficient context and that failures during operations like file operations, service initialization, and transaction commits are handled more gracefully.

Key improvements include:

- More robust error checking and logging for service initialization in `container.go`.
- Improved error handling for file operations in CLI commands (`list.go`, `logs.go`, `package_service.go`, `git.go`, `github.go`, `runner.go`).
- Enhanced transaction management by using a helper function `rollbackTx` to consistently handle rollback scenarios and log potential errors.
- Added `fmt.Errorf` with `%w` for better error wrapping in `core/services` and `packages` directories.
- Refined process management with more explicit error handling during `Stop` and `Wait` operations.
- Improved error reporting for MCP server operations, including graceful shutdown procedures and configuration saving.
- Added specific error handling for SSE write operations in UI handlers to prevent panics and log issues.
- Added a fallback for crypto/rand in `id_generator.go` to ensure ID generation in all environments.
- General cleanup and minor refactors for better code readability and maintainability.

Author: santoshkumarradha

control-plane/internal/application/container.go

@@ -3,14 +3,16 @@ package application
 import (
 	"crypto/sha256"
 	"encoding/hex"
+	"path/filepath"
+
 	"github.com/your-org/agentfield/control-plane/internal/cli/framework"
 	"github.com/your-org/agentfield/control-plane/internal/config"
 	"github.com/your-org/agentfield/control-plane/internal/core/services"
 	"github.com/your-org/agentfield/control-plane/internal/infrastructure/process"
 	"github.com/your-org/agentfield/control-plane/internal/infrastructure/storage"
+	"github.com/your-org/agentfield/control-plane/internal/logger"
 	didServices "github.com/your-org/agentfield/control-plane/internal/services"
 	storageInterface "github.com/your-org/agentfield/control-plane/internal/storage"
-	"path/filepath"
 )
 
 // CreateServiceContainer creates and wires up all services for the CLI commands
@@ -72,15 +74,21 @@ func CreateServiceContainer(cfg *config.Config, agentfieldHome string) *framewor
 			// Generate af server ID based on agentfield home directory
 			// This ensures each agentfield instance has a unique ID while being deterministic
 			agentfieldServerID := generateAgentFieldServerID(agentfieldHome)
-			didService.Initialize(agentfieldServerID)
-
-			// Create VC service with database storage (required)
-			if storageProvider != nil {
-				vcService = didServices.NewVCService(&cfg.Features.DID, didService, storageProvider)
-			}
-
-			if vcService != nil {
-				vcService.Initialize()
+			if err := didService.Initialize(agentfieldServerID); err != nil {
+				logger.Logger.Warn().Err(err).Msg("failed to initialize DID service")
+				didService = nil
+			} else {
+				// Create VC service with database storage (required)
+				if storageProvider != nil {
+					vcService = didServices.NewVCService(&cfg.Features.DID, didService, storageProvider)
+				}
+
+				if vcService != nil {
+					if err := vcService.Initialize(); err != nil {
+						logger.Logger.Warn().Err(err).Msg("failed to initialize VC service")
+						vcService = nil
+					}
+				}
 			}
 		}
 	}

control-plane/internal/cli/add.go

@@ -88,7 +88,7 @@ Template Variables:
 		RunE: func(cmd *cobra.Command, args []string) error {
 			opts.Source = args[0]
 			// If --alias flag is not used, and a second positional arg is present, use it as alias.
-			if cmd.Flags().Changed("alias") == false && len(args) > 1 {
+			if !cmd.Flags().Changed("alias") && len(args) > 1 {
 				opts.Alias = args[1]
 			}
 			// verbose flag is typically a persistent flag from the root command.
@@ -169,9 +169,8 @@ func NewMCPAddCommand(projectDir string, opts *MCPAddOptions, verboseFlag bool)
 	}
 
 	// Determine final alias
-	finalAlias := opts.Alias
-	if finalAlias == "" {
-		finalAlias = deriveAliasLocally(opts.Source) // Using local helper for now
+	if opts.Alias == "" {
+		opts.Alias = deriveAliasLocally(opts.Source) // Using local helper for now
 	}
 
 	// Construct MCPServerConfig (this will be part of the MCPAddCommand or its options)

control-plane/internal/cli/init.go

@@ -39,7 +39,7 @@ var (
 			Foreground(lipgloss.Color("196")).
 			Bold(true)
 
-	successStyle = lipgloss.NewStyle().
+	successStyle = lipgloss.NewStyle(). //nolint:unused // Reserved for future use
 			Foreground(lipgloss.Color("42")).
 			Bold(true)
 )
@@ -56,7 +56,7 @@ type initModel struct {
 	textInput      string
 	err            error
 	done           bool
-	nonInteractive bool
+	nonInteractive bool //nolint:unused // Reserved for future use
 }
 
 func (m initModel) Init() tea.Cmd {
@@ -413,9 +413,15 @@ Example:
 	cmd.Flags().StringVarP(&authorEmail, "email", "e", "", "Author email for the project")
 	cmd.Flags().BoolVar(&nonInteractive, "non-interactive", false, "Run in non-interactive mode (use defaults)")
 
-	viper.BindPFlag("language", cmd.Flags().Lookup("language"))
-	viper.BindPFlag("author.name", cmd.Flags().Lookup("author"))
-	viper.BindPFlag("author.email", cmd.Flags().Lookup("email"))
+	if err := viper.BindPFlag("language", cmd.Flags().Lookup("language")); err != nil {
+		printError("failed to bind language flag: %v", err)
+	}
+	if err := viper.BindPFlag("author.name", cmd.Flags().Lookup("author")); err != nil {
+		printError("failed to bind author flag: %v", err)
+	}
+	if err := viper.BindPFlag("author.email", cmd.Flags().Lookup("email")); err != nil {
+		printError("failed to bind email flag: %v", err)
+	}
 
 	return cmd
 }

control-plane/internal/cli/list.go

@@ -1,6 +1,7 @@
 package cli
 
 import (
+	"errors"
 	"fmt"
 	"os"
 	"path/filepath"
@@ -37,7 +38,13 @@ func runListCommand(cmd *cobra.Command, args []string) {
 	}
 
 	if data, err := os.ReadFile(registryPath); err == nil {
-		yaml.Unmarshal(data, registry)
+		if err := yaml.Unmarshal(data, registry); err != nil {
+			cmd.PrintErrf("failed to parse registry: %v\n", err)
+			return
+		}
+	} else if !errors.Is(err, os.ErrNotExist) {
+		cmd.PrintErrf("failed to read registry: %v\n", err)
+		return
 	}
 
 	if len(registry.Installed) == 0 {

control-plane/internal/cli/logs.go

@@ -1,6 +1,7 @@
 package cli
 
 import (
+	"errors"
 	"fmt"
 	"os"
 	"os/exec" // Added missing import
@@ -70,7 +71,11 @@ func (lv *LogViewer) ViewLogs(agentNodeName string) error {
 	}
 
 	if data, err := os.ReadFile(registryPath); err == nil {
-		yaml.Unmarshal(data, registry)
+		if err := yaml.Unmarshal(data, registry); err != nil {
+			return fmt.Errorf("failed to parse registry: %w", err)
+		}
+	} else if !errors.Is(err, os.ErrNotExist) {
+		return fmt.Errorf("failed to read registry: %w", err)
 	}
 
 	agentNode, exists := registry.Installed[agentNodeName]

control-plane/internal/cli/mcp.go

@@ -538,7 +538,7 @@ func runMCPRemoveCommand(cmd *cobra.Command, args []string, force bool) error {
 
 	if err := manager.Remove(alias); err != nil {
 		if !force && strings.Contains(err.Error(), "is running") {
-			PrintError(fmt.Sprintf("MCP server is running. Stop it first or use --force"))
+			PrintError("MCP server is running. Stop it first or use --force")
 			return err
 		}
 		PrintError(fmt.Sprintf("Failed to remove MCP server: %v", err))

control-plane/internal/cli/vc.go

@@ -5,7 +5,6 @@ import (
 	"encoding/base64"
 	"encoding/json"
 	"fmt"
-	"io/ioutil"
 	"net/http"
 	"os"
 	"strings"
@@ -167,7 +166,7 @@ func verifyVC(vcFilePath string, options VerifyOptions) error {
 
 	// Step 1: Read and parse VC file
 	step1 := VerificationStep{Step: 1, Description: "Reading VC file"}
-	vcData, err := ioutil.ReadFile(vcFilePath)
+	vcData, err := os.ReadFile(vcFilePath)
 	if err != nil {
 		step1.Success = false
 		step1.Error = fmt.Sprintf("Failed to read VC file: %v", err)
@@ -500,6 +499,7 @@ func extractPublicKeyFromDIDDoc(didDoc map[string]interface{}) (map[string]inter
 	return publicKeyJwk, nil
 }
 
+//nolint:unused // Reserved for future signature verification
 func verifyVCSignature(vcDoc types.VCDocument, resolution DIDResolutionInfo) (bool, error) {
 	// Create canonical representation for verification
 	vcCopy := vcDoc
@@ -533,6 +533,7 @@ func verifyVCSignature(vcDoc types.VCDocument, resolution DIDResolutionInfo) (bo
 	return ed25519.Verify(publicKey, canonicalBytes, signatureBytes), nil
 }
 
+//nolint:unused // Reserved for future signature verification
 func verifyWorkflowVCSignature(vcDoc types.WorkflowVCDocument, resolution DIDResolutionInfo) (bool, error) {
 	// Create canonical representation for verification
 	vcCopy := vcDoc

control-plane/internal/core/services/agent_service.go

@@ -4,6 +4,7 @@ package services
 import (
 	"context"
 	"encoding/json"
+	"errors"
 	"fmt"
 	"net/http"
 	"os"
@@ -103,7 +104,9 @@ func (as *DefaultAgentService) RunAgent(name string, options domain.RunOptions)
 	// 5. Wait for agent node to be ready
 	if err := as.waitForAgentNode(port, 10*time.Second); err != nil {
 		// Kill the process if it failed to start properly
-		as.processManager.Stop(pid)
+		if stopErr := as.processManager.Stop(pid); stopErr != nil {
+			return nil, fmt.Errorf("agent node failed to start: %w (additionally failed to stop process: %v)", err, stopErr)
+		}
 		return nil, fmt.Errorf("agent node failed to start: %w", err)
 	}
 
@@ -549,7 +552,11 @@ func (as *DefaultAgentService) updateRuntimeInfo(agentNodeName string, port, pid
 	// Load registry
 	registry := &packages.InstallationRegistry{}
 	if data, err := os.ReadFile(registryPath); err == nil {
-		yaml.Unmarshal(data, registry)
+		if err := yaml.Unmarshal(data, registry); err != nil {
+			return fmt.Errorf("failed to parse registry: %w", err)
+		}
+	} else if !errors.Is(err, os.ErrNotExist) {
+		return fmt.Errorf("failed to read registry: %w", err)
 	}
 
 	// Update runtime info

control-plane/internal/core/services/dev_service.go

@@ -152,7 +152,9 @@ func (ds *DefaultDevService) runDev(packagePath string, options domain.DevOption
 	return nil
 }
 
-// getFreePort finds an available port in the range 8001-8999
+// getFreePort finds an available port in the range 8001-8999.
+//
+//nolint:unused // retained for future dev-service enhancements
 func (ds *DefaultDevService) getFreePort() (int, error) {
 	for port := 8001; port <= 8999; port++ {
 		if ds.isPortAvailable(port) {
@@ -162,7 +164,9 @@ func (ds *DefaultDevService) getFreePort() (int, error) {
 	return 0, fmt.Errorf("no free port available in range 8001-8999")
 }
 
-// isPortAvailable checks if a port is available
+// isPortAvailable checks if a port is available.
+//
+//nolint:unused // retained for future dev-service enhancements
 func (ds *DefaultDevService) isPortAvailable(port int) bool {
 	conn, err := net.Listen("tcp", fmt.Sprintf(":%d", port))
 	if err != nil {
@@ -270,7 +274,9 @@ func (ds *DefaultDevService) discoverAgentPort(timeout time.Duration) (int, erro
 	return 0, fmt.Errorf("could not discover agent port within %v after %d attempts", timeout, checkCount)
 }
 
-// waitForAgent waits for the agent to become ready in dev mode
+// waitForAgent waits for the agent to become ready in dev mode.
+//
+//nolint:unused // retained for future dev-service enhancements
 func (ds *DefaultDevService) waitForAgent(port int, timeout time.Duration) error {
 	client := &http.Client{Timeout: 2 * time.Second}
 	deadline := time.Now().Add(timeout)

control-plane/internal/core/services/package_service.go

@@ -307,15 +307,19 @@ type Spinner struct {
 }
 
 // Color helper methods
-func (ps *DefaultPackageService) green(text string) string  { return green(text) }
+func (ps *DefaultPackageService) green(text string) string { return green(text) }
+
+//nolint:unused // retained for console color helpers
 func (ps *DefaultPackageService) red(text string) string    { return red(text) }
 func (ps *DefaultPackageService) yellow(text string) string { return yellow(text) }
 func (ps *DefaultPackageService) blue(text string) string   { return blue(text) }
 func (ps *DefaultPackageService) cyan(text string) string   { return cyan(text) }
 func (ps *DefaultPackageService) gray(text string) string   { return gray(text) }
 func (ps *DefaultPackageService) bold(text string) string   { return bold(text) }
 func (ps *DefaultPackageService) statusSuccess() string     { return statusSuccess }
-func (ps *DefaultPackageService) statusError() string       { return statusError }
+
+//nolint:unused // retained for console status helpers
+func (ps *DefaultPackageService) statusError() string { return statusError }
 
 // newSpinner creates a new spinner with the given message
 func (ps *DefaultPackageService) newSpinner(message string) *Spinner {
@@ -424,7 +428,9 @@ func (ps *DefaultPackageService) isPackageInstalled(packageName string) bool {
 	}
 
 	if data, err := os.ReadFile(registryPath); err == nil {
-		yaml.Unmarshal(data, registry)
+		if err := yaml.Unmarshal(data, registry); err != nil {
+			return false
+		}
 	}
 
 	_, exists := registry.Installed[packageName]
@@ -508,11 +514,9 @@ func (ps *DefaultPackageService) installDependencies(packagePath string, metadat
 			pipPath = filepath.Join(venvPath, "Scripts", "pip.exe") // Windows
 		}
 
-		// Upgrade pip first
+		// Upgrade pip first (ignore failures)
 		cmd = exec.Command(pipPath, "install", "--upgrade", "pip")
-		if _, err := cmd.CombinedOutput(); err != nil {
-			// Ignore pip upgrade failures
-		}
+		_, _ = cmd.CombinedOutput()
 
 		// Install from requirements.txt if it exists
 		requirementsPath := filepath.Join(packagePath, "requirements.txt")
@@ -561,7 +565,9 @@ func (ps *DefaultPackageService) updateRegistry(metadata *packages.PackageMetada
 	}
 
 	if data, err := os.ReadFile(registryPath); err == nil {
-		yaml.Unmarshal(data, registry)
+		if err := yaml.Unmarshal(data, registry); err != nil {
+			return fmt.Errorf("failed to parse registry: %w", err)
+		}
 	}
 
 	// Add/update package entry