refactor: simplify SDK auth, CLI output helpers, and alias CRUD

cli/internal/clientutil/client.go

@@ -0,0 +1,43 @@
+// Package clientutil centralizes construction of the syfthub.Client used by
+// the CLI, so that config-to-option translation lives in exactly one place.
+package clientutil
+
+import (
+	"time"
+
+	"github.com/OpenMined/syfthub/cli/internal/nodeconfig"
+	"github.com/openmined/syfthub/sdk/golang/syfthub"
+)
+
+// NewClient builds a *syfthub.Client from the given NodeConfig.
+//
+// If aggregatorAlias is non-empty, the resolved aggregator URL
+// (cfg.GetAggregatorURL(aggregatorAlias)) is applied when non-empty.
+// If timeoutOverride > 0, it is used in place of cfg.TimeoutDuration().
+// Any extra options are appended last and therefore take precedence over
+// the config-derived ones.
+func NewClient(cfg *nodeconfig.NodeConfig, aggregatorAlias string, timeoutOverride time.Duration, extra ...syfthub.Option) (*syfthub.Client, error) {
+	opts := []syfthub.Option{
+		syfthub.WithBaseURL(cfg.HubURL),
+	}
+
+	timeout := timeoutOverride
+	if timeout <= 0 {
+		timeout = cfg.TimeoutDuration()
+	}
+	opts = append(opts, syfthub.WithTimeout(timeout))
+
+	if cfg.HasAPIToken() {
+		opts = append(opts, syfthub.WithAPIToken(cfg.APIToken))
+	}
+
+	if aggregatorAlias != "" {
+		if url := cfg.GetAggregatorURL(aggregatorAlias); url != "" {
+			opts = append(opts, syfthub.WithAggregatorURL(url))
+		}
+	}
+
+	opts = append(opts, extra...)
+
+	return syfthub.NewClient(opts...)
+}

cli/internal/cmd/add.go

@@ -2,9 +2,6 @@ package cmd
 
 import (
 	"github.com/spf13/cobra"
-
-	"github.com/OpenMined/syfthub/cli/internal/config"
-	"github.com/OpenMined/syfthub/cli/internal/output"
 )
 
 var addCmd = &cobra.Command{
@@ -24,7 +21,9 @@ var addAggregatorCmd = &cobra.Command{
 	Use:   "aggregator <alias> <url>",
 	Short: "Add an aggregator alias",
 	Args:  cobra.ExactArgs(2),
-	RunE:  runAddAggregator,
+	RunE: func(cmd *cobra.Command, args []string) error {
+		return runAddAlias(aggregatorKind, args[0], args[1], addAggregatorDefault, addAggregatorJSONOutput)
+	},
 }
 
 // Add accounting subcommand
@@ -37,7 +36,9 @@ var addAccountingCmd = &cobra.Command{
 	Use:   "accounting <alias> <url>",
 	Short: "Add an accounting service alias",
 	Args:  cobra.ExactArgs(2),
-	RunE:  runAddAccounting,
+	RunE: func(cmd *cobra.Command, args []string) error {
+		return runAddAlias(accountingKind, args[0], args[1], addAccountingDefault, addAccountingJSONOutput)
+	},
 }
 
 func init() {
@@ -53,111 +54,3 @@ func init() {
 	addCmd.AddCommand(addAggregatorCmd)
 	addCmd.AddCommand(addAccountingCmd)
 }
-
-func runAddAggregator(cmd *cobra.Command, args []string) error {
-	alias := args[0]
-	url := args[1]
-
-	cfg := config.Load()
-
-	if _, exists := cfg.Aggregators[alias]; exists {
-		if addAggregatorJSONOutput {
-			output.JSON(map[string]interface{}{
-				"status":  "error",
-				"message": "Aggregator '" + alias + "' already exists",
-			})
-		} else {
-			output.Error("Aggregator '%s' already exists. Use 'syft update aggregator' to modify it.", alias)
-		}
-		return nil
-	}
-
-	cfg.Aggregators[alias] = config.AggregatorConfig{URL: url}
-
-	if addAggregatorDefault {
-		cfg.DefaultAggregator = alias
-	}
-
-	if err := cfg.Save(); err != nil {
-		if addAggregatorJSONOutput {
-			output.JSON(map[string]interface{}{
-				"status":  "error",
-				"message": err.Error(),
-			})
-		} else {
-			output.Error("Failed to save config: %v", err)
-		}
-		return err
-	}
-
-	if addAggregatorJSONOutput {
-		output.JSON(map[string]interface{}{
-			"status":     "success",
-			"alias":      alias,
-			"url":        url,
-			"is_default": addAggregatorDefault,
-		})
-	} else {
-		msg := "Added aggregator '" + alias + "' -> " + url
-		if addAggregatorDefault {
-			msg += " (default)"
-		}
-		output.Success(msg)
-	}
-
-	return nil
-}
-
-func runAddAccounting(cmd *cobra.Command, args []string) error {
-	alias := args[0]
-	url := args[1]
-
-	cfg := config.Load()
-
-	if _, exists := cfg.AccountingServices[alias]; exists {
-		if addAccountingJSONOutput {
-			output.JSON(map[string]interface{}{
-				"status":  "error",
-				"message": "Accounting service '" + alias + "' already exists",
-			})
-		} else {
-			output.Error("Accounting service '%s' already exists. Use 'syft update accounting' to modify it.", alias)
-		}
-		return nil
-	}
-
-	cfg.AccountingServices[alias] = config.AccountingConfig{URL: url}
-
-	if addAccountingDefault {
-		cfg.DefaultAccounting = alias
-	}
-
-	if err := cfg.Save(); err != nil {
-		if addAccountingJSONOutput {
-			output.JSON(map[string]interface{}{
-				"status":  "error",
-				"message": err.Error(),
-			})
-		} else {
-			output.Error("Failed to save config: %v", err)
-		}
-		return err
-	}
-
-	if addAccountingJSONOutput {
-		output.JSON(map[string]interface{}{
-			"status":     "success",
-			"alias":      alias,
-			"url":        url,
-			"is_default": addAccountingDefault,
-		})
-	} else {
-		msg := "Added accounting service '" + alias + "' -> " + url
-		if addAccountingDefault {
-			msg += " (default)"
-		}
-		output.Success(msg)
-	}
-
-	return nil
-}

cli/internal/cmd/agent.go

@@ -8,10 +8,10 @@ import (
 	"os/signal"
 	"strings"
 	"syscall"
-	"time"
 
 	"github.com/spf13/cobra"
 
+	"github.com/OpenMined/syfthub/cli/internal/clientutil"
 	"github.com/OpenMined/syfthub/cli/internal/config"
 	"github.com/OpenMined/syfthub/cli/internal/output"
 	"github.com/openmined/syfthub/sdk/golang/syfthub"
@@ -115,20 +115,7 @@ func runAgent(cmd *cobra.Command, args []string) error {
 
 	cfg := config.Load()
 
-	aggregatorURL := cfg.GetAggregatorURL(agentAggregator)
-
-	opts := []syfthub.Option{
-		syfthub.WithBaseURL(cfg.HubURL),
-		syfthub.WithTimeout(time.Duration(cfg.Timeout) * time.Second),
-	}
-	if aggregatorURL != "" {
-		opts = append(opts, syfthub.WithAggregatorURL(aggregatorURL))
-	}
-	if cfg.HasAPIToken() {
-		opts = append(opts, syfthub.WithAPIToken(cfg.APIToken))
-	}
-
-	client, err := syfthub.NewClient(opts...)
+	client, err := clientutil.NewClient(cfg, agentAggregator, 0)
 	if err != nil {
 		output.Error("Failed to create client: %v", err)
 		return err