From cc50a7570dd5a43f3435477e332068cdeee29db7 Mon Sep 17 00:00:00 2001 From: Joshua Gilman Date: Mon, 8 Jun 2026 07:42:43 -0700 Subject: [PATCH] refactor(cli): resolve runtime config once and thread ui.IO MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit P1 foundation (PR-1.4) of the CLI UX overhaul — the merge pass. Resolve the full runtime config exactly once in the root PersistentPreRunE and cache it, the single ui.IO, the logger, and outputExplicit on commandContext. Delete the ten per-command loadRuntimeConfig re-calls (up/down/exec/info/install/list/run/ connect/wallet/target) in favour of cc.runtimeConfig. loadRuntimeConfig now takes (cmd, vp): precedence values come from viper, the flag-only session knobs (-v/-q/--non-interactive/--color/--no-color) come straight off cmd.Flags(), -v raises the resolved --log-level, and --color/ --no-color fold into a ui.ColorMode. resolveUX turns the requested policy into final color/non-interactive verdicts from the raw injected streams (NO_COLOR supreme; the latch one-way). The signature change is atomic with the re-call deletions because resolveWallet and rejectExplicitTarget have no cmd in scope. The ui.IO seam replaces the ad-hoc logger and JSON toggle: ctx.io.NewSlogLogger (DiscardHandler under -q) supersedes newLogger, and list/info/wallet read cc.io.JSON() instead of the viper-backed outputJSON() helper (both removed). The global flags are extracted into addGlobalFlags so tests can drive loadRuntimeConfig directly. ui gains an exported IsTerminalReader for the latch. No user-visible behaviour change: the resolved values and output bytes are identical; resolution simply happens once. Co-Authored-By: Claude Opus 4.8 (1M context) --- cli/internal/cli/config.go | 155 ++++++++++++++++++++++++-------- cli/internal/cli/config_test.go | 128 +++++++++++++++++++++++--- cli/internal/cli/connect.go | 5 +- cli/internal/cli/down.go | 5 +- cli/internal/cli/exec.go | 5 +- cli/internal/cli/info.go | 7 +- cli/internal/cli/install.go | 5 +- cli/internal/cli/list.go | 7 +- cli/internal/cli/options.go | 8 ++ cli/internal/cli/output_test.go | 23 +---- cli/internal/cli/root.go | 62 +++++++------ cli/internal/cli/run.go | 5 +- cli/internal/cli/target.go | 5 +- cli/internal/cli/up.go | 5 +- cli/internal/cli/wallet.go | 11 +-- cli/internal/ui/io.go | 8 +- 16 files changed, 297 insertions(+), 147 deletions(-) diff --git a/cli/internal/cli/config.go b/cli/internal/cli/config.go index 6af9297..2b1c4cd 100644 --- a/cli/internal/cli/config.go +++ b/cli/internal/cli/config.go @@ -4,8 +4,10 @@ import ( "fmt" "io" "log/slog" + "os" "strings" + "github.com/meigma/yacd/cli/internal/ui" "github.com/spf13/cobra" "github.com/spf13/pflag" "github.com/spf13/viper" @@ -18,9 +20,10 @@ const ( formatJSON = "json" ) -// RuntimeConfig is the resolved persistent-flag payload. Each field is the -// trimmed, validated value of the corresponding root flag, with the viper -// precedence (flag > env > default) already applied. +// RuntimeConfig is the resolved persistent-flag payload, computed once in the +// root PersistentPreRunE and cached on commandContext. Precedence-bearing +// fields come from viper (flag > env > default); the session/TTY knobs come +// straight off cmd.Flags() so no ambient YACD_* can drive them. type RuntimeConfig struct { // Kubeconfig is the optional kubeconfig path; empty defers to standard // loading rules. @@ -34,7 +37,7 @@ type RuntimeConfig struct { // defers to environment or kubeconfig defaults. Namespace string - // LogLevel is one of debug, info, warn, error. + // LogLevel is one of debug, info, warn, error, after the -v raise. LogLevel string // LogFormat is text or json. @@ -43,6 +46,18 @@ type RuntimeConfig struct { // OutputFormat is the data-plane format: text or json. It is the sole // machine-output toggle (the per-command --json flags are removed). OutputFormat string + + // Verbosity is the raw -v count. + Verbosity int + + // Quiet is -q: the global mute (human plane, progress, warnings, logger). + Quiet bool + + // NonInteractive disables prompts (the requested half of the latch). + NonInteractive bool + + // Color is the requested color policy before NO_COLOR and TTY folding. + Color ui.ColorMode } // initializeConfig wires viper to the root persistent flags. It is called @@ -97,11 +112,12 @@ func bindFlag(vp *viper.Viper, key string, flag *pflag.Flag) error { return nil } -// loadRuntimeConfig reads the trimmed runtime values from viper and -// validates the log-level/format enums. The empty-string defaults are -// applied here so callers can compare against the package-known set -// directly. -func loadRuntimeConfig(vp *viper.Viper) (RuntimeConfig, error) { +// loadRuntimeConfig resolves the full runtime payload once. The precedence +// values are read through viper; the flag-only session knobs (-v/-q/ +// --non-interactive/--color/--no-color) are read straight off cmd.Flags() so an +// ambient YACD_* cannot drive them or leak into a child environment. -v is +// additive over the resolved --log-level. All enums are validated here. +func loadRuntimeConfig(cmd *cobra.Command, vp *viper.Viper) (RuntimeConfig, error) { config := RuntimeConfig{ Kubeconfig: strings.TrimSpace(vp.GetString("kubeconfig")), KubeContext: strings.TrimSpace(vp.GetString("kube-context")), @@ -120,11 +136,39 @@ func loadRuntimeConfig(vp *viper.Viper) (RuntimeConfig, error) { config.OutputFormat = formatText } + flags := cmd.Flags() + verbosity, err := flags.GetCount("verbose") + if err != nil { + return RuntimeConfig{}, err + } + quiet, err := flags.GetBool("quiet") + if err != nil { + return RuntimeConfig{}, err + } + nonInteractive, err := flags.GetBool("non-interactive") + if err != nil { + return RuntimeConfig{}, err + } + noColor, err := flags.GetBool("no-color") + if err != nil { + return RuntimeConfig{}, err + } + colorValue, err := flags.GetString("color") + if err != nil { + return RuntimeConfig{}, err + } + + config.Verbosity = verbosity + config.Quiet = quiet + config.NonInteractive = nonInteractive + switch config.LogLevel { case "debug", "info", "warn", "error": default: return RuntimeConfig{}, fmt.Errorf("unsupported log level %q", config.LogLevel) } + config.LogLevel = raise(config.LogLevel, verbosity) + switch config.LogFormat { case formatText, formatJSON: default: @@ -136,14 +180,67 @@ func loadRuntimeConfig(vp *viper.Viper) (RuntimeConfig, error) { return RuntimeConfig{}, fmt.Errorf("unsupported output format %q", config.OutputFormat) } + color, err := resolveColorMode(colorValue, noColor, flags.Changed("color")) + if err != nil { + return RuntimeConfig{}, err + } + config.Color = color + return config, nil } -// outputJSON reports whether the resolved --output format is json. The format -// is validated once in PersistentPreRunE, so this read is safe; it is the sole -// machine-output toggle now that the per-command --json flags are gone. -func (commandContext *commandContext) outputJSON() bool { - return strings.EqualFold(strings.TrimSpace(commandContext.viper.GetString("output")), formatJSON) +// resolveColorMode folds --color and --no-color into a single requested policy. +// An explicit --color wins over --no-color; otherwise --no-color forces never. +// NO_COLOR is applied later, in resolveUX, where it is supreme. +func resolveColorMode(value string, noColor, colorChanged bool) (ui.ColorMode, error) { + if noColor && !colorChanged { + return ui.ColorNever, nil + } + switch strings.ToLower(strings.TrimSpace(value)) { + case "", "auto": + return ui.ColorAuto, nil + case "always": + return ui.ColorAlways, nil + case "never": + return ui.ColorNever, nil + default: + return ui.ColorAuto, fmt.Errorf("unsupported color mode %q", value) + } +} + +// resolveUX turns the requested color policy and interactivity into final +// verdicts from the raw injected streams. NO_COLOR is supreme over +// --color=always; the non-interactive latch is one-way (any non-TTY stream or +// the flag disables prompts). +func resolveUX(config RuntimeConfig, errOut io.Writer, in io.Reader) (color, nonInteractive bool) { + errTTY := ui.IsTerminal(errOut) + inTTY := ui.IsTerminalReader(in) + + switch { + case os.Getenv("NO_COLOR") != "": + color = false + case config.Color == ui.ColorAlways: + color = true + case config.Color == ui.ColorNever: + color = false + default: // ColorAuto + color = errTTY + } + + nonInteractive = config.NonInteractive || !errTTY || !inTTY + return color, nonInteractive +} + +// view adapts the command-layer RuntimeConfig into the ui package's input +// shape, keeping ui free of any dependency on this package. +func (config RuntimeConfig) view() ui.RuntimeView { + return ui.RuntimeView{ + OutputJSON: config.OutputFormat == formatJSON, + Quiet: config.Quiet, + Verbosity: config.Verbosity, + LogLevel: config.LogLevel, + LogFormat: config.LogFormat, + } } // raise lifts the base log level by verbose steps along the ordering @@ -172,31 +269,17 @@ func raise(base string, verbose int) string { return order[len(order)-1] } -// newLogger constructs the slog logger for the active command from the -// 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 { +// slogLevel maps the resolved log-level string to a slog.Level. Unknown values +// (already rejected by loadRuntimeConfig) fall back to info. +func slogLevel(level string) slog.Level { + switch level { case "debug": - level = slog.LevelDebug + return slog.LevelDebug case "warn": - level = slog.LevelWarn + return slog.LevelWarn case "error": - level = slog.LevelError + return slog.LevelError default: - level = slog.LevelInfo + return slog.LevelInfo } - - handlerOptions := &slog.HandlerOptions{Level: level} - if config.LogFormat == formatJSON { - return slog.New(slog.NewJSONHandler(out, handlerOptions)) - } - - return slog.New(slog.NewTextHandler(out, handlerOptions)) } diff --git a/cli/internal/cli/config_test.go b/cli/internal/cli/config_test.go index e8e3789..fcfe1df 100644 --- a/cli/internal/cli/config_test.go +++ b/cli/internal/cli/config_test.go @@ -2,13 +2,30 @@ package cli import ( "bytes" - "context" "log/slog" + "strings" "testing" + "github.com/meigma/yacd/cli/internal/ui" + "github.com/spf13/cobra" + "github.com/spf13/viper" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" ) +// globalFlagsCommand returns a command carrying the global persistent flags, +// parsed from args, so loadRuntimeConfig and the resolution helpers can be +// exercised without standing up the whole root command. Precedence-bearing +// values (log-level, output) are driven through viper in those tests; the +// flag-only knobs come from these parsed flags. +func globalFlagsCommand(t *testing.T, args ...string) *cobra.Command { + t.Helper() + cmd := &cobra.Command{RunE: func(*cobra.Command, []string) error { return nil }} + addGlobalFlags(cmd.Flags()) + require.NoError(t, cmd.ParseFlags(args)) + return cmd +} + func TestRaise(t *testing.T) { t.Parallel() @@ -37,25 +54,108 @@ func TestRaise(t *testing.T) { } } -func TestNewLoggerQuietForcesOff(t *testing.T) { +func TestSlogLevel(t *testing.T) { + t.Parallel() + + assert.Equal(t, slog.LevelDebug, slogLevel("debug")) + assert.Equal(t, slog.LevelInfo, slogLevel("info")) + assert.Equal(t, slog.LevelWarn, slogLevel("warn")) + assert.Equal(t, slog.LevelError, slogLevel("error")) + assert.Equal(t, slog.LevelInfo, slogLevel("unknown")) +} + +func TestResolveColorMode(t *testing.T) { + t.Parallel() + + cases := []struct { + name string + value string + noColor bool + colorChanged bool + want ui.ColorMode + wantErr bool + }{ + {"auto default", "auto", false, false, ui.ColorAuto, false}, + {"empty is auto", "", false, false, ui.ColorAuto, false}, + {"always", "always", false, true, ui.ColorAlways, false}, + {"never", "never", false, true, ui.ColorNever, false}, + {"no-color forces never", "auto", true, false, ui.ColorNever, false}, + {"explicit color beats no-color", "always", true, true, ui.ColorAlways, false}, + {"unknown errors", "purple", false, true, ui.ColorAuto, true}, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + got, err := resolveColorMode(tc.value, tc.noColor, tc.colorChanged) + if tc.wantErr { + require.Error(t, err) + return + } + require.NoError(t, err) + assert.Equal(t, tc.want, got) + }) + } +} + +func TestResolveUXNonTTYBuffers(t *testing.T) { t.Parallel() - var buf bytes.Buffer - logger := newLogger(RuntimeConfig{LogLevel: "debug"}, &buf, true) - logger.Error("boom") + // Buffers are non-TTY, so default (auto) color is off and the latch forces + // non-interactive — the determinism backstop tests rely on. + color, nonInteractive := resolveUX(RuntimeConfig{Color: ui.ColorAuto}, &bytes.Buffer{}, strings.NewReader("")) + assert.False(t, color, "auto color is off on a non-TTY stream") + assert.True(t, nonInteractive, "a non-TTY stream is always non-interactive") +} + +func TestResolveUXColorAlwaysForcesColor(t *testing.T) { + // Neutralise any ambient NO_COLOR (CI sets it) so this asserts the + // --color=always override alone. Not parallel: it mutates the environment. + t.Setenv("NO_COLOR", "") + + // --color=always is an explicit override: it forces color even off-TTY + // (design §2.4); only NO_COLOR overrides it. + color, _ := resolveUX(RuntimeConfig{Color: ui.ColorAlways}, &bytes.Buffer{}, strings.NewReader("")) + assert.True(t, color) +} + +func TestResolveUXNoColorEnvIsSupreme(t *testing.T) { + // NO_COLOR beats --color=always. + t.Setenv("NO_COLOR", "1") + + color, _ := resolveUX(RuntimeConfig{Color: ui.ColorAlways}, &bytes.Buffer{}, strings.NewReader("")) + assert.False(t, color, "NO_COLOR is supreme over --color=always") +} + +func TestResolveUXNonInteractiveFlag(t *testing.T) { + t.Parallel() - 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") + _, nonInteractive := resolveUX(RuntimeConfig{NonInteractive: true}, &bytes.Buffer{}, strings.NewReader("")) + assert.True(t, nonInteractive) } -func TestNewLoggerRespectsLevel(t *testing.T) { +func TestLoadRuntimeConfigResolvesFlags(t *testing.T) { t.Parallel() - var buf bytes.Buffer - logger := newLogger(RuntimeConfig{LogLevel: "info"}, &buf, false) + cmd := globalFlagsCommand(t, "-vv", "-q", "--non-interactive") + vp := viper.New() + vp.Set("log-level", "warn") + vp.Set("output", "json") + + config, err := loadRuntimeConfig(cmd, vp) + require.NoError(t, err) + + assert.Equal(t, "debug", config.LogLevel, "warn raised two steps by -vv") + assert.Equal(t, 2, config.Verbosity) + assert.True(t, config.Quiet) + assert.True(t, config.NonInteractive) + assert.Equal(t, formatJSON, config.OutputFormat) +} + +func TestLoadRuntimeConfigRejectsUnknownColor(t *testing.T) { + t.Parallel() - assert.True(t, logger.Enabled(context.Background(), slog.LevelInfo)) - assert.False(t, logger.Enabled(context.Background(), slog.LevelDebug), - "the info level suppresses debug records") + cmd := globalFlagsCommand(t, "--color", "purple") + _, err := loadRuntimeConfig(cmd, viper.New()) + require.Error(t, err) + assert.Contains(t, err.Error(), "unsupported color mode") } diff --git a/cli/internal/cli/connect.go b/cli/internal/cli/connect.go index 3925f2b..5c75071 100644 --- a/cli/internal/cli/connect.go +++ b/cli/internal/cli/connect.go @@ -47,10 +47,7 @@ another. Dropped forwards are re-established automatically. The endpoints file's ports are only live while connect is running.`, Args: cobra.ExactArgs(1), RunE: func(cmd *cobra.Command, args []string) error { - runtimeConfig, err := loadRuntimeConfig(commandContext.viper) - if err != nil { - return err - } + runtimeConfig := commandContext.runtimeConfig name, namespace, err := resolveIdentity(args[0], runtimeConfig) if err != nil { return err diff --git a/cli/internal/cli/down.go b/cli/internal/cli/down.go index c01e86c..c4c9682 100644 --- a/cli/internal/cli/down.go +++ b/cli/internal/cli/down.go @@ -18,10 +18,7 @@ func newDownCommand(commandContext *commandContext) *cobra.Command { Short: "Delete a YACD environment and wait for clean removal", Args: cobra.ExactArgs(1), RunE: func(cmd *cobra.Command, args []string) error { - runtimeConfig, err := loadRuntimeConfig(commandContext.viper) - if err != nil { - return err - } + runtimeConfig := commandContext.runtimeConfig name, namespace, err := resolveIdentity(args[0], runtimeConfig) if err != nil { return err diff --git a/cli/internal/cli/exec.go b/cli/internal/cli/exec.go index 2a3052a..79e21e2 100644 --- a/cli/internal/cli/exec.go +++ b/cli/internal/cli/exec.go @@ -66,10 +66,7 @@ piped or non-terminal invocations (CI) stream without a TTY.`, yacd exec my-net -- sh`, Args: cobra.MinimumNArgs(1), RunE: func(cmd *cobra.Command, args []string) error { - runtimeConfig, err := loadRuntimeConfig(commandContext.viper) - if err != nil { - return err - } + runtimeConfig := commandContext.runtimeConfig name, namespace, err := resolveIdentity(args[0], runtimeConfig) if err != nil { return err diff --git a/cli/internal/cli/info.go b/cli/internal/cli/info.go index 96d8877..fc2120f 100644 --- a/cli/internal/cli/info.go +++ b/cli/internal/cli/info.go @@ -19,10 +19,7 @@ func newInfoCommand(commandContext *commandContext) *cobra.Command { Short: "Print CardanoNetwork status and connection information", Args: cobra.ExactArgs(1), RunE: func(cmd *cobra.Command, args []string) error { - runtimeConfig, err := loadRuntimeConfig(commandContext.viper) - if err != nil { - return err - } + runtimeConfig := commandContext.runtimeConfig name, namespace, err := resolveIdentity(args[0], runtimeConfig) if err != nil { return err @@ -45,7 +42,7 @@ func newInfoCommand(commandContext *commandContext) *cobra.Command { if faucetWallet, walletErr := walletstore.NewStore(kubeClient, namespace, name).Faucet(cmd.Context()); walletErr == nil { info.Wallet = &walletOutput{Address: faucetWallet.Address, KeySecretName: faucetWallet.SecretName} } - if commandContext.outputJSON() { + if commandContext.io.JSON() { encoded, err := json.MarshalIndent(info, "", " ") if err != nil { return fmt.Errorf("marshal info JSON: %w", err) diff --git a/cli/internal/cli/install.go b/cli/internal/cli/install.go index d06da74..ce01691 100644 --- a/cli/internal/cli/install.go +++ b/cli/internal/cli/install.go @@ -50,10 +50,7 @@ func newInstallCommand(commandContext *commandContext) *cobra.Command { "image and is not a supported configuration.", Args: cobra.NoArgs, RunE: func(cmd *cobra.Command, _ []string) error { - runtimeConfig, err := loadRuntimeConfig(commandContext.viper) - if err != nil { - return err - } + runtimeConfig := commandContext.runtimeConfig waitReady := commandContext.viper.GetBool("wait") timeout := commandContext.viper.GetDuration("timeout") diff --git a/cli/internal/cli/list.go b/cli/internal/cli/list.go index 9f5429e..318c5cd 100644 --- a/cli/internal/cli/list.go +++ b/cli/internal/cli/list.go @@ -23,11 +23,8 @@ func newListCommand(commandContext *commandContext) *cobra.Command { Short: "List YACD environments across all namespaces (or one with -n)", Args: cobra.NoArgs, RunE: func(cmd *cobra.Command, _ []string) error { - runtimeConfig, err := loadRuntimeConfig(commandContext.viper) - if err != nil { - return err - } - jsonOutput := commandContext.outputJSON() + runtimeConfig := commandContext.runtimeConfig + jsonOutput := commandContext.io.JSON() kubeClient, _, err := commandContext.resolveKubeClient(runtimeConfig) if err != nil { diff --git a/cli/internal/cli/options.go b/cli/internal/cli/options.go index 10c0d41..f3b7ca9 100644 --- a/cli/internal/cli/options.go +++ b/cli/internal/cli/options.go @@ -10,6 +10,7 @@ import ( "github.com/meigma/yacd/cli/internal/clusterstate" "github.com/meigma/yacd/cli/internal/kube" "github.com/meigma/yacd/cli/internal/operator" + "github.com/meigma/yacd/cli/internal/ui" "github.com/meigma/yacd/internal/cardano/tx" "github.com/spf13/viper" ) @@ -170,4 +171,11 @@ type commandContext struct { // failure warrants an orphaned-state check. managedEngaged bool logger *slog.Logger + + // runtimeConfig, io, and outputExplicit are resolved once in the root + // PersistentPreRunE and read by every command, replacing the former + // per-command loadRuntimeConfig re-calls. + runtimeConfig RuntimeConfig + io ui.IO + outputExplicit bool } diff --git a/cli/internal/cli/output_test.go b/cli/internal/cli/output_test.go index 3bca1e2..521ac8e 100644 --- a/cli/internal/cli/output_test.go +++ b/cli/internal/cli/output_test.go @@ -28,7 +28,7 @@ func TestLoadRuntimeConfigRejectsUnknownOutput(t *testing.T) { vp := viper.New() vp.Set("output", "xml") - _, err := loadRuntimeConfig(vp) + _, err := loadRuntimeConfig(globalFlagsCommand(t), vp) require.Error(t, err) assert.Contains(t, err.Error(), "unsupported output format") } @@ -36,26 +36,9 @@ func TestLoadRuntimeConfigRejectsUnknownOutput(t *testing.T) { func TestLoadRuntimeConfigOutputDefaultsToText(t *testing.T) { t.Parallel() - config, err := loadRuntimeConfig(viper.New()) + config, err := loadRuntimeConfig(globalFlagsCommand(t), viper.New()) require.NoError(t, err) - assert.Equal(t, "text", config.OutputFormat) -} - -func TestOutputJSONHelper(t *testing.T) { - t.Parallel() - - cases := map[string]bool{"json": true, "JSON": true, "text": false, "": false} - for value, want := range cases { - t.Run(value, func(t *testing.T) { - t.Parallel() - vp := viper.New() - if value != "" { - vp.Set("output", value) - } - cc := &commandContext{viper: vp} - assert.Equal(t, want, cc.outputJSON()) - }) - } + assert.Equal(t, formatText, config.OutputFormat) } // TestOutputEnvIsReadAndValidated proves YACD_OUTPUT flows through the env diff --git a/cli/internal/cli/root.go b/cli/internal/cli/root.go index 87d2665..bda45bc 100644 --- a/cli/internal/cli/root.go +++ b/cli/internal/cli/root.go @@ -16,8 +16,10 @@ import ( "github.com/meigma/yacd/cli/internal/operator/ssa" "github.com/meigma/yacd/cli/internal/toolbin" "github.com/meigma/yacd/cli/internal/toolbin/ghrelease" + "github.com/meigma/yacd/cli/internal/ui" "github.com/meigma/yacd/internal/cardano/tx" "github.com/spf13/cobra" + "github.com/spf13/pflag" "github.com/spf13/viper" ) @@ -107,24 +109,19 @@ func NewRootCommand(options Options) *cobra.Command { if err := initializeConfig(cmd, ctx.viper); err != nil { return err } - runtimeConfig, err := loadRuntimeConfig(ctx.viper) + runtimeConfig, err := loadRuntimeConfig(cmd, ctx.viper) if err != nil { return 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) + // Resolve once: cache the config, build the single ui.IO from the + // raw injected writers (before any os.Stdout swap), and derive the + // logger from it. Every command reads ctx.runtimeConfig/ctx.io + // instead of re-resolving. + ctx.runtimeConfig = runtimeConfig + color, nonInteractive := resolveUX(runtimeConfig, ctx.err, ctx.in) + ctx.io = ui.New(ui.ConfigFromRuntime(runtimeConfig.view(), color, nonInteractive), ctx.in, ctx.out, ctx.err) + ctx.logger = ctx.io.NewSlogLogger(slogLevel(runtimeConfig.LogLevel), runtimeConfig.LogFormat == formatJSON) + ctx.outputExplicit = cmd.Flags().Changed("output") return nil }, } @@ -132,20 +129,7 @@ func NewRootCommand(options Options) *cobra.Command { root.SetOut(options.Out) root.SetErr(options.Err) - root.PersistentFlags().String("kubeconfig", "", "Path to the kubeconfig file") - root.PersistentFlags().String("context", "", "Kubeconfig context to use") - root.PersistentFlags().StringP("namespace", "n", "", "Kubernetes namespace") - root.PersistentFlags().String("log-level", "info", "Log level: debug, info, warn, error") - root.PersistentFlags().String("log-format", formatText, "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") - root.PersistentFlags().StringP("output", "o", formatText, "Output format: text, json") + addGlobalFlags(root.PersistentFlags()) // The network verbs resolve their target through the shared resolver, so // each is wrapped to clear a stale managed-cluster record when it targeted @@ -173,3 +157,23 @@ func NewRootCommand(options Options) *cobra.Command { return root } + +// addGlobalFlags registers the persistent flags shared by every command. It is +// called on the root's persistent flag set and reused by tests that exercise +// loadRuntimeConfig directly. The session/TTY knobs (--verbose/--quiet/ +// --non-interactive/--color/--no-color) are flag-only by design: they have no +// env tier and are read straight off cmd.Flags() so no ambient YACD_* can drive +// them or leak into a child environment. +func addGlobalFlags(flags *pflag.FlagSet) { + flags.String("kubeconfig", "", "Path to the kubeconfig file") + flags.String("context", "", "Kubeconfig context to use") + flags.StringP("namespace", "n", "", "Kubernetes namespace") + flags.String("log-level", "info", "Log level: debug, info, warn, error") + flags.String("log-format", formatText, "Log format: text, json") + flags.CountP("verbose", "v", "Increase log verbosity (-v, -vv, -vvv)") + flags.BoolP("quiet", "q", false, "Suppress progress, logs, and warnings; print only data and errors") + flags.Bool("non-interactive", false, "Never prompt; fail instead of waiting for input") + flags.String("color", "auto", "Color output: auto, always, never") + flags.Bool("no-color", false, "Disable color output") + flags.StringP("output", "o", formatText, "Output format: text, json") +} diff --git a/cli/internal/cli/run.go b/cli/internal/cli/run.go index 0e76de7..3dcf4af 100644 --- a/cli/internal/cli/run.go +++ b/cli/internal/cli/run.go @@ -36,10 +36,7 @@ to the command instead of being parsed by yacd.`, yacd run my-net`, Args: cobra.MinimumNArgs(1), RunE: func(cmd *cobra.Command, args []string) error { - runtimeConfig, err := loadRuntimeConfig(commandContext.viper) - if err != nil { - return err - } + runtimeConfig := commandContext.runtimeConfig name, namespace, err := resolveIdentity(args[0], runtimeConfig) if err != nil { return err diff --git a/cli/internal/cli/target.go b/cli/internal/cli/target.go index 7d5f5e2..3807ae7 100644 --- a/cli/internal/cli/target.go +++ b/cli/internal/cli/target.go @@ -67,10 +67,7 @@ func (commandContext *commandContext) resolveKubeClient(cfg RuntimeConfig) (kube // silently ignoring them while mutating the ambient kubeconfig would be // surprising. They remain supported on the network verbs (up, info, ...). func rejectExplicitTarget(commandContext *commandContext) error { - runtimeConfig, err := loadRuntimeConfig(commandContext.viper) - if err != nil { - return err - } + runtimeConfig := commandContext.runtimeConfig if runtimeConfig.Kubeconfig != "" || runtimeConfig.KubeContext != "" { return fmt.Errorf( "devnet manages its own cluster and kubeconfig; --kubeconfig/--context " + diff --git a/cli/internal/cli/up.go b/cli/internal/cli/up.go index ada3626..72633f7 100644 --- a/cli/internal/cli/up.go +++ b/cli/internal/cli/up.go @@ -25,10 +25,7 @@ func newUpCommand(commandContext *commandContext) *cobra.Command { Short: "Create or update a YACD environment and wait for readiness", Args: cobra.ExactArgs(1), RunE: func(cmd *cobra.Command, args []string) error { - runtimeConfig, err := loadRuntimeConfig(commandContext.viper) - if err != nil { - return err - } + runtimeConfig := commandContext.runtimeConfig name, namespace, err := resolveIdentity(args[0], runtimeConfig) if err != nil { return err diff --git a/cli/internal/cli/wallet.go b/cli/internal/cli/wallet.go index 051af78..c93d76c 100644 --- a/cli/internal/cli/wallet.go +++ b/cli/internal/cli/wallet.go @@ -76,10 +76,7 @@ type walletContext struct { // store (no live network) still get a fetched network, which doubles as an // existence check so a typo names a clear not-found rather than an empty list. func (commandContext *commandContext) resolveWallet(ctx context.Context, net string) (*walletContext, error) { - runtimeConfig, err := loadRuntimeConfig(commandContext.viper) - if err != nil { - return nil, err - } + runtimeConfig := commandContext.runtimeConfig name, namespace, err := resolveIdentity(net, runtimeConfig) if err != nil { return nil, err @@ -126,7 +123,7 @@ func newWalletListCommand(commandContext *commandContext) *cobra.Command { return err } - if commandContext.outputJSON() { + if commandContext.io.JSON() { return writeJSON(commandContext.out, newWalletListItems(wallets)) } @@ -151,7 +148,7 @@ func newWalletAddCommand(commandContext *commandContext) *cobra.Command { topup := strings.TrimSpace(commandContext.viper.GetString("topup")) await := commandContext.viper.GetBool("await") awaitTimeout := commandContext.viper.GetDuration("await-timeout") - jsonOutput := commandContext.outputJSON() + jsonOutput := commandContext.io.JSON() lovelace, topupRequested, err := parseOptionalLovelace(topup) if err != nil { @@ -236,7 +233,7 @@ func newWalletTopUpCommand(commandContext *commandContext) *cobra.Command { source := strings.TrimSpace(commandContext.viper.GetString("from")) await := commandContext.viper.GetBool("await") awaitTimeout := commandContext.viper.GetDuration("await-timeout") - jsonOutput := commandContext.outputJSON() + jsonOutput := commandContext.io.JSON() if err := requireAwaitTimeout(await, awaitTimeout); err != nil { return err } diff --git a/cli/internal/ui/io.go b/cli/internal/ui/io.go index 2d8a0f6..0d155e6 100644 --- a/cli/internal/ui/io.go +++ b/cli/internal/ui/io.go @@ -61,11 +61,13 @@ func (i IO) ErrIsTTY() bool { return i.errIsTTY } // NonInteractive and both stdin and the human stream are terminals. It is false // whenever a prompt could hang a script. func (i IO) Interactive() bool { - return !i.cfg.NonInteractive && i.errIsTTY && isTerminalReader(i.in) + return !i.cfg.NonInteractive && i.errIsTTY && IsTerminalReader(i.in) } -// isTerminalReader reports whether r is backed by a terminal file descriptor. -func isTerminalReader(r io.Reader) bool { +// IsTerminalReader reports whether r is backed by a terminal file descriptor. +// It is the input-stream counterpart to IsTerminal; the command layer's +// interactivity latch consults it before an IO is constructed. +func IsTerminalReader(r io.Reader) bool { file, ok := r.(*os.File) return ok && term.IsTerminal(int(file.Fd())) }