refactor: replace package-level globals with CLI struct dependency injection (#219)

* refactor: replace package-level globals with CLI struct dependency injection

## What

Introduce a CLI struct that holds logger, writer, build info, and
rootCmd. Convert all command files from init() with package-level vars
to methods on CLI. Delete execute.go and replace with cli.go. Update
main.go to use cmd.NewCLI(...).Execute().

## Why

Package-level mutable globals made tests non-parallelizable and created
implicit coupling between command files. The list command needed a
runtime wrapper hack because writer was nil at init() time. Dependency
injection via a struct makes dependencies explicit and testable.

## Notes

- The exported cmd.Execute() function is replaced by cmd.NewCLI().Execute() — any external callers would break, but main.go is the only caller
- The list.go runtime wrapper indirection is eliminated — c.writer is available when commands run via PersistentPreRun
- discoverPluginNames now accepts a logger interface parameter instead of using the package global, allowing nil in tests
- The exported Run() function is now unexported as c.run() — it was only called internally
- Tests use a newTestCLI helper that constructs isolated CLI instances with a buffer-backed writer, enabling future parallel test execution

Signed-off-by: jmeridth <jmeridth@gmail.com>

* fix: route all version output through c.writer and add NewCLI wiring test

## What

Use c.writer for non-verbose version output instead of fmt.Println to
stdout. Add a test that verifies NewCLI registers all expected
subcommands.

## Why

Non-verbose version output bypassed c.writer, making it uncapturable in
tests and inconsistent with verbose mode and all other commands. The
NewCLI constructor wires all subcommands but had no test to catch
regressions in that wiring.

## Notes

- The non-verbose version test now asserts the version string appears in the buffer and that verbose-only fields (Commit:) are absent, rather than asserting an empty buffer

Signed-off-by: jmeridth <jmeridth@gmail.com>

---------

Signed-off-by: jmeridth <jmeridth@gmail.com>

Author: jmeridth

cmd/cli.go

@@ -0,0 +1,87 @@
+// Package cmd provides the command-line interface for pvtr.
+// It defines the root command and all subcommands, handles configuration,
+// and manages the execution flow of the application.
+package cmd
+
+import (
+	"log"
+	"os"
+	"path/filepath"
+	"text/tabwriter"
+
+	hclog "github.com/hashicorp/go-hclog"
+	"github.com/spf13/cobra"
+	"github.com/spf13/viper"
+
+	"github.com/privateerproj/privateer-sdk/command"
+	"github.com/privateerproj/privateer-sdk/config"
+)
+
+// CLI holds the shared dependencies for all pvtr commands.
+type CLI struct {
+	buildVersion       string
+	buildGitCommitHash string
+	buildTime          string
+
+	logger  hclog.Logger
+	writer  *tabwriter.Writer
+	rootCmd *cobra.Command
+}
+
+// NewCLI creates a CLI instance with the given build metadata
+// and registers all subcommands.
+func NewCLI(version, commitHash, builtAt string) *CLI {
+	c := &CLI{
+		buildVersion:       version,
+		buildGitCommitHash: commitHash,
+		buildTime:          builtAt,
+	}
+
+	c.rootCmd = &cobra.Command{
+		Use:              "pvtr",
+		Short:            "pvtr root command",
+		PersistentPreRun: c.persistentPreRun,
+	}
+
+	command.SetBase(c.rootCmd)
+	c.rootCmd.PersistentFlags().StringP("binaries-path", "b", defaultBinariesPath(), "Path to the directory where plugins are installed")
+	_ = viper.BindPFlag("binaries-path", c.rootCmd.PersistentFlags().Lookup("binaries-path"))
+
+	c.addRunCmd()
+	c.addEnvCmd()
+	c.addVersionCmd()
+	c.addListCmd()
+	c.addGenPluginCmd()
+
+	return c
+}
+
+// Execute runs the root command. This is called by main.main().
+func (c *CLI) Execute() {
+	err := c.rootCmd.Execute()
+	if err != nil {
+		os.Exit(1)
+	}
+}
+
+// persistentPreRun initializes the logger and writer for use by all commands.
+func (c *CLI) persistentPreRun(cmd *cobra.Command, args []string) {
+	cfg := config.NewConfig(nil)
+	c.logger = cfg.Logger
+
+	c.writer = tabwriter.NewWriter(os.Stdout, 1, 1, 1, ' ', 0)
+	command.ReadConfig()
+}
+
+// defaultBinariesPath returns the default path where plugins are installed.
+// It constructs a path in the user's home directory under .privateer/bin.
+// If the home directory cannot be determined, it falls back to a relative
+// path (./.privateer/bin) and logs a warning.
+func defaultBinariesPath() string {
+	home, err := os.UserHomeDir()
+	if err != nil {
+		log.Printf("Warning: could not determine home directory: %v", err)
+		return filepath.Join(".", ".privateer", "bin")
+	}
+	return filepath.Join(home, ".privateer", "bin")
+}

cmd/cli_test.go

@@ -0,0 +1,28 @@
+package cmd
+
+import (
+	"testing"
+)
+
+func TestNewCLI_RegistersAllSubcommands(t *testing.T) {
+	c := NewCLI("1.0.0", "abc123", "2026-01-01T00:00:00Z")
+
+	expectedCommands := []string{
+		"run",
+		"env",
+		"version",
+		"list",
+		"generate-plugin",
+	}
+
+	registered := make(map[string]bool)
+	for _, cmd := range c.rootCmd.Commands() {
+		registered[cmd.Name()] = true
+	}
+
+	for _, name := range expectedCommands {
+		if !registered[name] {
+			t.Errorf("expected subcommand %q to be registered, but it was not", name)
+		}
+	}
+}

cmd/env.go

@@ -13,49 +13,51 @@ import (
 	"github.com/spf13/viper"
 )
 
-// envCmd represents the env command, which displays runtime environment details.
-var envCmd = &cobra.Command{
-	Use:     "env",
-	Aliases: []string{"info"},
-	Short:   "Display runtime environment details.",
-	Long:    `Display the binary path, config location, plugins directory, installed plugins, version, and build information.`,
-	Run: func(cmd *cobra.Command, args []string) {
-		binaryPath, err := os.Executable()
-		if err != nil {
-			binaryPath = "unknown"
-		}
+func (c *CLI) addEnvCmd() {
+	envCmd := &cobra.Command{
+		Use:     "env",
+		Aliases: []string{"info"},
+		Short:   "Display runtime environment details.",
+		Long:    `Display the binary path, config location, plugins directory, installed plugins, version, and build information.`,
+		Run: func(cmd *cobra.Command, args []string) {
+			binaryPath, err := os.Executable()
+			if err != nil {
+				binaryPath = "unknown"
+			}
 
-		configFile := viper.ConfigFileUsed()
-		var configStatus string
-		if configFile == "" {
-			configStatus = "none"
-		} else if _, err := os.Stat(configFile); err == nil {
-			configStatus = fmt.Sprintf("%s (found)", configFile)
-		} else {
-			configStatus = fmt.Sprintf("%s (not found)", configFile)
-		}
+			configFile := viper.ConfigFileUsed()
+			var configStatus string
+			if configFile == "" {
+				configStatus = "none"
+			} else if _, err := os.Stat(configFile); err == nil {
+				configStatus = fmt.Sprintf("%s (found)", configFile)
+			} else {
+				configStatus = fmt.Sprintf("%s (not found)", configFile)
+			}
 
-		pluginsDir := viper.GetString("binaries-path")
-		pluginNames := discoverPluginNames(pluginsDir)
+			pluginsDir := viper.GetString("binaries-path")
+			pluginNames := discoverPluginNames(c.logger, pluginsDir)
 
-		_, _ = fmt.Fprintf(writer, "Binary:\t%s\n", binaryPath)
-		_, _ = fmt.Fprintf(writer, "Config:\t%s\n", configStatus)
-		_, _ = fmt.Fprintf(writer, "Plugins Dir:\t%s\n", pluginsDir)
-		_, _ = fmt.Fprintf(writer, "Plugins:\t%s\n", pluginNames)
-		_, _ = fmt.Fprintf(writer, "Version:\t%s\n", buildVersion)
-		_, _ = fmt.Fprintf(writer, "Commit:\t%s\n", buildGitCommitHash)
-		_, _ = fmt.Fprintf(writer, "Build Time:\t%s\n", buildTime)
-		_, _ = fmt.Fprintf(writer, "Go Version:\t%s\n", runtime.Version())
-		_, _ = fmt.Fprintf(writer, "OS/Arch:\t%s/%s\n", runtime.GOOS, runtime.GOARCH)
-		if err := writer.Flush(); err != nil {
-			log.Printf("Error flushing writer: %v", err)
-		}
-	},
+			_, _ = fmt.Fprintf(c.writer, "Binary:\t%s\n", binaryPath)
+			_, _ = fmt.Fprintf(c.writer, "Config:\t%s\n", configStatus)
+			_, _ = fmt.Fprintf(c.writer, "Plugins Dir:\t%s\n", pluginsDir)
+			_, _ = fmt.Fprintf(c.writer, "Plugins:\t%s\n", pluginNames)
+			_, _ = fmt.Fprintf(c.writer, "Version:\t%s\n", c.buildVersion)
+			_, _ = fmt.Fprintf(c.writer, "Commit:\t%s\n", c.buildGitCommitHash)
+			_, _ = fmt.Fprintf(c.writer, "Build Time:\t%s\n", c.buildTime)
+			_, _ = fmt.Fprintf(c.writer, "Go Version:\t%s\n", runtime.Version())
+			_, _ = fmt.Fprintf(c.writer, "OS/Arch:\t%s/%s\n", runtime.GOOS, runtime.GOARCH)
+			if err := c.writer.Flush(); err != nil {
+				log.Printf("Error flushing writer: %v", err)
+			}
+		},
+	}
+	c.rootCmd.AddCommand(envCmd)
 }
 
-func discoverPluginNames(pluginsDir string) string {
+func discoverPluginNames(logger interface{ Error(string, ...interface{}) }, pluginsDir string) string {
 	pluginPaths, err := hcplugin.Discover("*", pluginsDir)
-	if err != nil {
+	if err != nil && logger != nil {
 		logger.Error("error discovering plugins", "dir", pluginsDir, "error", err)
 	}
 	var names []string
@@ -71,7 +73,3 @@ func discoverPluginNames(pluginsDir string) string {
 	}
 	return strings.Join(names, ", ")
 }
-
-func init() {
-	rootCmd.AddCommand(envCmd)
-}

cmd/env_test.go

@@ -8,16 +8,27 @@ import (
 	"strings"
 	"testing"
 	"text/tabwriter"
+
+	"github.com/spf13/cobra"
 )
 
+func newTestCLI(buf *bytes.Buffer, version, commit, buildTime string) *CLI {
+	c := &CLI{
+		buildVersion:       version,
+		buildGitCommitHash: commit,
+		buildTime:          buildTime,
+		writer:             tabwriter.NewWriter(buf, 1, 1, 1, ' ', 0),
+	}
+	c.rootCmd = &cobra.Command{Use: "pvtr"}
+	return c
+}
+
 func TestEnvCmd_ContainsExpectedFields(t *testing.T) {
 	var buf bytes.Buffer
-	writer = tabwriter.NewWriter(&buf, 1, 1, 1, ' ', 0)
-
-	buildVersion = "1.2.3"
-	buildGitCommitHash = "a1b2c3d"
-	buildTime = "2026-01-01T00:00:00Z"
+	c := newTestCLI(&buf, "1.2.3", "a1b2c3d", "2026-01-01T00:00:00Z")
+	c.addEnvCmd()
 
+	envCmd, _, _ := c.rootCmd.Find([]string{"env"})
 	envCmd.Run(envCmd, []string{})
 
 	output := buf.String()
@@ -42,12 +53,10 @@ func TestEnvCmd_ContainsExpectedFields(t *testing.T) {
 
 func TestEnvCmd_DisplaysBuildInfo(t *testing.T) {
 	var buf bytes.Buffer
-	writer = tabwriter.NewWriter(&buf, 1, 1, 1, ' ', 0)
-
-	buildVersion = "test-version"
-	buildGitCommitHash = "e4f5a6b"
-	buildTime = "2026-02-15T00:00:00Z"
+	c := newTestCLI(&buf, "test-version", "e4f5a6b", "2026-02-15T00:00:00Z")
+	c.addEnvCmd()
 
+	envCmd, _, _ := c.rootCmd.Find([]string{"env"})
 	envCmd.Run(envCmd, []string{})
 
 	output := buf.String()
@@ -72,8 +81,10 @@ func TestEnvCmd_DisplaysBuildInfo(t *testing.T) {
 
 func TestEnvCmd_ShowsBinaryPath(t *testing.T) {
 	var buf bytes.Buffer
-	writer = tabwriter.NewWriter(&buf, 1, 1, 1, ' ', 0)
+	c := newTestCLI(&buf, "", "", "")
+	c.addEnvCmd()
 
+	envCmd, _, _ := c.rootCmd.Find([]string{"env"})
 	envCmd.Run(envCmd, []string{})
 
 	output := buf.String()
@@ -91,14 +102,14 @@ func TestDiscoverPluginNames_EmptyDir(t *testing.T) {
 	}
 	defer func() { _ = os.RemoveAll(tmpDir) }()
 
-	result := discoverPluginNames(tmpDir)
+	result := discoverPluginNames(nil, tmpDir)
 	if result != "none" {
 		t.Errorf("expected 'none' for empty dir, got: %s", result)
 	}
 }
 
 func TestDiscoverPluginNames_NonexistentDir(t *testing.T) {
-	result := discoverPluginNames("/nonexistent/path")
+	result := discoverPluginNames(nil, "/nonexistent/path")
 	if result != "none" {
 		t.Errorf("expected 'none' for nonexistent dir, got: %s", result)
 	}
@@ -118,11 +129,10 @@ func TestDiscoverPluginNames_FiltersPvtrAndPrivateer(t *testing.T) {
 		}
 	}
 
-	result := discoverPluginNames(tmpDir)
+	result := discoverPluginNames(nil, tmpDir)
 
 	// Exact-name binaries should be filtered out
 	for _, filtered := range []string{"pvtr", "privateer"} {
-		// Check the result doesn't contain the exact name as a standalone entry
 		for _, entry := range strings.Split(result, ", ") {
 			if entry == filtered {
 				t.Errorf("expected %q to be filtered out, got: %s", filtered, result)