From 9063f3c0547434dd199b3f4a9b5073eebd16f7f3 Mon Sep 17 00:00:00 2001 From: Joshua Gilman Date: Sun, 7 Jun 2026 22:50:57 -0700 Subject: [PATCH] feat(cli): move version to a subcommand and make -v verbosity P1 foundation (PR-1.2) of the CLI UX overhaul. Drop cobra's Version: field and the --version flag so -v is free, and add a `yacd version` subcommand that prints the same build-metadata line to stdout. Add the persistent UX flags --verbose/-v (count), --quiet/-q, --non-interactive, --color, and --no-color; this PR consumes only -v and -q (the rest are registered for the foundation and wired to ui.IO in a later phase). Verbose logging now runs on slog: PersistentPreRunE reads -v/-q directly off cmd.Flags() (flag-only, env-immune) without changing loadRuntimeConfig's signature, raises the resolved --log-level by the verbose count via raise() (additive along errorinfo->debug" stepping and gives a warn+-v=>debug example). Additive-by-one is the only model that delivers goal 5's graduated -vvv control and matches the stepping description, so raise("warn",1)=="info". BREAKING (pre-1.0, intentional): `yacd --version` is removed; use `yacd version`. -v/--verbose is always verbosity. Co-Authored-By: Claude Opus 4.8 (1M context) --- cli/internal/cli/config.go | 37 ++++++++++++++++++-- cli/internal/cli/config_test.go | 61 +++++++++++++++++++++++++++++++++ cli/internal/cli/options.go | 1 + cli/internal/cli/root.go | 30 +++++++++++++--- cli/internal/cli/root_test.go | 55 +++++++++++++++++++++++------ cli/internal/cli/version.go | 24 +++++++++++++ 6 files changed, 190 insertions(+), 18 deletions(-) create mode 100644 cli/internal/cli/config_test.go create mode 100644 cli/internal/cli/version.go diff --git a/cli/internal/cli/config.go b/cli/internal/cli/config.go index 58c9ab0..a97129c 100644 --- a/cli/internal/cli/config.go +++ b/cli/internal/cli/config.go @@ -116,10 +116,41 @@ func loadRuntimeConfig(vp *viper.Viper) (RuntimeConfig, error) { return config, nil } +// raise lifts the base log level by verbose steps along the ordering +// error < warn < info < debug. It never lowers the level and caps at debug; a +// base outside the known set (already rejected by loadRuntimeConfig) is +// returned unchanged. This is the one two-key composition: -v is additive over +// the resolved --log-level. +func raise(base string, verbose int) string { + if verbose <= 0 { + return base + } + order := []string{"error", "warn", "info", "debug"} + index := -1 + for i, level := range order { + if level == base { + index = i + break + } + } + if index < 0 { + return base + } + if raised := index + verbose; raised < len(order) { + return order[raised] + } + return order[len(order)-1] +} + // newLogger constructs the slog logger for the active command from the -// resolved RuntimeConfig. JSON format is selected explicitly; everything -// else falls back to text. -func newLogger(config RuntimeConfig, out io.Writer) *slog.Logger { +// resolved RuntimeConfig. When quiet is set the logger is forced off (the +// global mute overrides -v/--log-level); the final returned error still prints +// through the exit handler, which is not the logger. JSON format is selected +// explicitly; everything else falls back to text. +func newLogger(config RuntimeConfig, out io.Writer, quiet bool) *slog.Logger { + if quiet { + return slog.New(slog.DiscardHandler) + } var level slog.Level switch config.LogLevel { case "debug": diff --git a/cli/internal/cli/config_test.go b/cli/internal/cli/config_test.go new file mode 100644 index 0000000..e8e3789 --- /dev/null +++ b/cli/internal/cli/config_test.go @@ -0,0 +1,61 @@ +package cli + +import ( + "bytes" + "context" + "log/slog" + "testing" + + "github.com/stretchr/testify/assert" +) + +func TestRaise(t *testing.T) { + t.Parallel() + + cases := []struct { + name string + base string + verbose int + want string + }{ + {"no verbose keeps base", "info", 0, "info"}, + {"info plus one is debug", "info", 1, "debug"}, + {"info caps at debug", "info", 5, "debug"}, + {"warn steps to info", "warn", 1, "info"}, + {"warn two steps to debug", "warn", 2, "debug"}, + {"error three steps to debug", "error", 3, "debug"}, + {"debug never lowers", "debug", 2, "debug"}, + {"warn unchanged without verbose", "warn", 0, "warn"}, + {"unknown base returned unchanged", "trace", 2, "trace"}, + {"negative verbose returned unchanged", "info", -1, "info"}, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + assert.Equal(t, tc.want, raise(tc.base, tc.verbose)) + }) + } +} + +func TestNewLoggerQuietForcesOff(t *testing.T) { + t.Parallel() + + var buf bytes.Buffer + logger := newLogger(RuntimeConfig{LogLevel: "debug"}, &buf, true) + logger.Error("boom") + + assert.False(t, logger.Enabled(context.Background(), slog.LevelError), + "quiet forces the logger off, overriding the level") + assert.Empty(t, buf.String(), "a quiet logger writes nothing") +} + +func TestNewLoggerRespectsLevel(t *testing.T) { + t.Parallel() + + var buf bytes.Buffer + logger := newLogger(RuntimeConfig{LogLevel: "info"}, &buf, false) + + assert.True(t, logger.Enabled(context.Background(), slog.LevelInfo)) + assert.False(t, logger.Enabled(context.Background(), slog.LevelDebug), + "the info level suppresses debug records") +} diff --git a/cli/internal/cli/options.go b/cli/internal/cli/options.go index 3b4f0ec..10c0d41 100644 --- a/cli/internal/cli/options.go +++ b/cli/internal/cli/options.go @@ -163,6 +163,7 @@ type commandContext struct { operatorInstaller OperatorInstallerFactory clusterState ClusterStateFactory k3dVersion string + build BuildInfo // managedEngaged records, for the duration of one command, whether the // shared target resolver fell through to the tracked managed devnet // cluster. The managed-reconcile wrapper reads it to decide whether a diff --git a/cli/internal/cli/root.go b/cli/internal/cli/root.go index 28f18e1..ac99b93 100644 --- a/cli/internal/cli/root.go +++ b/cli/internal/cli/root.go @@ -1,7 +1,6 @@ package cli import ( - "fmt" "io" "log/slog" "strings" @@ -95,13 +94,13 @@ func NewRootCommand(options Options) *cobra.Command { operatorInstaller: options.OperatorInstallerFactory, clusterState: options.ClusterStateFactory, k3dVersion: ghrelease.DefaultK3dPin.Version, + build: options.Build, logger: slog.New(slog.NewTextHandler(options.Err, &slog.HandlerOptions{Level: slog.LevelInfo})), } root := &cobra.Command{ Use: "yacd", Short: "YACD developer CLI", - Version: options.Build.Version, SilenceUsage: true, SilenceErrors: true, PersistentPreRunE: func(cmd *cobra.Command, _ []string) error { @@ -112,11 +111,23 @@ func NewRootCommand(options Options) *cobra.Command { if err != nil { return err } - ctx.logger = newLogger(runtimeConfig, ctx.err) + // -v/-q are flag-only knobs read straight off cmd.Flags() so an + // ambient YACD_VERBOSE/YACD_QUIET cannot raise verbosity or mute a + // scripted run. -v is additive over the resolved --log-level; -q + // forces the logger off entirely. + verbosity, err := cmd.Flags().GetCount("verbose") + if err != nil { + return err + } + quiet, err := cmd.Flags().GetBool("quiet") + if err != nil { + return err + } + runtimeConfig.LogLevel = raise(runtimeConfig.LogLevel, verbosity) + ctx.logger = newLogger(runtimeConfig, ctx.err, quiet) return nil }, } - root.SetVersionTemplate(fmt.Sprintf("yacd %s (%s) built %s\n", options.Build.Version, options.Build.Commit, options.Build.Date)) root.SetIn(options.In) root.SetOut(options.Out) root.SetErr(options.Err) @@ -126,6 +137,14 @@ func NewRootCommand(options Options) *cobra.Command { root.PersistentFlags().StringP("namespace", "n", "", "Kubernetes namespace") root.PersistentFlags().String("log-level", "info", "Log level: debug, info, warn, error") root.PersistentFlags().String("log-format", "text", "Log format: text, json") + // Session/TTY decisions. These are flag-only (no env tier by design) and are + // read directly off cmd.Flags() wherever they are consumed, so no ambient + // YACD_* can drive them or leak into a child environment. + root.PersistentFlags().CountP("verbose", "v", "Increase log verbosity (-v, -vv, -vvv)") + root.PersistentFlags().BoolP("quiet", "q", false, "Suppress progress, logs, and warnings; print only data and errors") + root.PersistentFlags().Bool("non-interactive", false, "Never prompt; fail instead of waiting for input") + root.PersistentFlags().String("color", "auto", "Color output: auto, always, never") + root.PersistentFlags().Bool("no-color", false, "Disable color output") // The network verbs resolve their target through the shared resolver, so // each is wrapped to clear a stale managed-cluster record when it targeted @@ -142,6 +161,9 @@ func NewRootCommand(options Options) *cobra.Command { root.AddCommand(newDevnetCommand(ctx)) // init only prints an embedded template — no cluster contact, so no reconcile. root.AddCommand(newInitCommand(ctx)) + // version replaces cobra's removed --version flag (dropped so -v is free for + // verbosity); it prints build metadata to stdout and contacts nothing. + root.AddCommand(newVersionCommand(ctx)) // install targets an arbitrary cluster by explicit-or-ambient kubeconfig; it // deliberately accepts --kubeconfig/--context and never consults the managed // devnet record, so it is neither wrapped in withManagedReconcile nor gated diff --git a/cli/internal/cli/root_test.go b/cli/internal/cli/root_test.go index 16943ad..7ee13fe 100644 --- a/cli/internal/cli/root_test.go +++ b/cli/internal/cli/root_test.go @@ -9,22 +9,55 @@ import ( "github.com/stretchr/testify/require" ) -func TestVersionFlagPrintsBuildMetadata(t *testing.T) { +func testBuildInfo() BuildInfo { + return BuildInfo{Version: "0.1.0", Commit: "abc1234", Date: "2026-05-22T10:00:00Z"} +} + +func TestVersionSubcommandPrintsBuildMetadata(t *testing.T) { t.Parallel() var stdout, stderr bytes.Buffer - root := NewRootCommand(Options{ - Out: &stdout, - Err: &stderr, - Build: BuildInfo{ - Version: "0.1.0", - Commit: "abc1234", - Date: "2026-05-22T10:00:00Z", - }, - }) - root.SetArgs([]string{"--version"}) + root := NewRootCommand(Options{Out: &stdout, Err: &stderr, Build: testBuildInfo()}) + root.SetArgs([]string{"version"}) require.NoError(t, root.ExecuteContext(context.Background())) assert.Equal(t, "yacd 0.1.0 (abc1234) built 2026-05-22T10:00:00Z\n", stdout.String()) assert.Empty(t, stderr.String()) } + +func TestVersionFlagIsRemoved(t *testing.T) { + t.Parallel() + + var stdout, stderr bytes.Buffer + root := NewRootCommand(Options{Out: &stdout, Err: &stderr, Build: testBuildInfo()}) + root.SetArgs([]string{"--version"}) + + err := root.ExecuteContext(context.Background()) + require.Error(t, err, "--version must no longer be a registered flag") + assert.Contains(t, err.Error(), "unknown flag") + assert.Empty(t, stdout.String(), "no version string prints for the removed flag") +} + +func TestVersionSubcommandPrintsDataUnderQuiet(t *testing.T) { + t.Parallel() + + var stdout, stderr bytes.Buffer + root := NewRootCommand(Options{Out: &stdout, Err: &stderr, Build: testBuildInfo()}) + root.SetArgs([]string{"-q", "version"}) + + require.NoError(t, root.ExecuteContext(context.Background())) + assert.Equal(t, "yacd 0.1.0 (abc1234) built 2026-05-22T10:00:00Z\n", stdout.String(), + "data still prints to stdout under --quiet") +} + +func TestVerboseShorthandIsVerbosityNotVersion(t *testing.T) { + t.Parallel() + + var stdout, stderr bytes.Buffer + root := NewRootCommand(Options{Out: &stdout, Err: &stderr, Build: testBuildInfo()}) + root.SetArgs([]string{"-vvv", "version"}) + + require.NoError(t, root.ExecuteContext(context.Background()), + "-v is the verbosity count, not --version") + assert.Contains(t, stdout.String(), "yacd 0.1.0") +} diff --git a/cli/internal/cli/version.go b/cli/internal/cli/version.go new file mode 100644 index 0000000..96545ab --- /dev/null +++ b/cli/internal/cli/version.go @@ -0,0 +1,24 @@ +package cli + +import ( + "fmt" + + "github.com/spf13/cobra" +) + +// newVersionCommand prints the build metadata to stdout. It replaces cobra's +// removed --version flag, so -v stays free for verbosity, and is the only +// surface for the version string. The output matches the previous --version +// template byte-for-byte. +func newVersionCommand(commandContext *commandContext) *cobra.Command { + return &cobra.Command{ + Use: "version", + Short: "Print the yacd version", + Args: cobra.NoArgs, + RunE: func(_ *cobra.Command, _ []string) error { + _, err := fmt.Fprintf(commandContext.out, "yacd %s (%s) built %s\n", + commandContext.build.Version, commandContext.build.Commit, commandContext.build.Date) + return err + }, + } +}