From 9d87457c7949cec6315407aca2ce37c244f3739f Mon Sep 17 00:00:00 2001 From: Joshua Gilman Date: Mon, 8 Jun 2026 07:23:32 -0700 Subject: [PATCH] feat(cli): replace per-command --json with a global --output/-o MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit P1 foundation (PR-1.3) of the CLI UX overhaul. Add a persistent --output/-o (text|json, env YACD_OUTPUT) as the single machine-output toggle, validated once in PersistentPreRunE, and remove the per-command --json flags and all YACD_JSON handling from list/info/wallet outright (pre-1.0 clean cut, no alias). The JSON branches now gate on commandContext.outputJSON() (a viper-backed read of the validated --output value); the frozen JSON byte shapes are unchanged. PR-1.4 swaps these reads to cc.io.JSON() once the resolved ui.IO is threaded — routing through cc.io now is impossible because cc.io does not exist until then. Also introduce formatText/formatJSON constants to keep the new enum handling under the goconst threshold. Tests/Chainsaw: the --json invocations move to -o json (byte-identical output); new coverage for --json being unknown, YACD_OUTPUT validation/precedence, the output default, and the outputJSON helper. BREAKING (pre-1.0, intentional): --json and YACD_JSON are removed; use --output json / -o json (YACD_OUTPUT=json). Co-Authored-By: Claude Opus 4.8 (1M context) --- cli/internal/cli/config.go | 48 +++++++++--- cli/internal/cli/info.go | 3 +- cli/internal/cli/info_test.go | 6 +- cli/internal/cli/list.go | 4 +- cli/internal/cli/list_test.go | 2 +- cli/internal/cli/output_test.go | 75 +++++++++++++++++++ cli/internal/cli/root.go | 3 +- cli/internal/cli/wallet.go | 9 +-- .../chainsaw/manager-smoke/chainsaw-test.yaml | 2 +- 9 files changed, 126 insertions(+), 26 deletions(-) create mode 100644 cli/internal/cli/output_test.go diff --git a/cli/internal/cli/config.go b/cli/internal/cli/config.go index a97129c..6af9297 100644 --- a/cli/internal/cli/config.go +++ b/cli/internal/cli/config.go @@ -11,6 +11,13 @@ import ( "github.com/spf13/viper" ) +// formatText and formatJSON are the shared values for the --log-format and +// --output enums. +const ( + formatText = "text" + 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. @@ -32,6 +39,10 @@ type RuntimeConfig struct { // LogFormat is text or json. LogFormat string + + // 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 } // initializeConfig wires viper to the root persistent flags. It is called @@ -39,7 +50,7 @@ type RuntimeConfig struct { // once per execution before any RunE. func initializeConfig(cmd *cobra.Command, vp *viper.Viper) error { vp.SetDefault("log-level", "info") - vp.SetDefault("log-format", "text") + vp.SetDefault("log-format", formatText) vp.SetEnvPrefix("YACD") // Environment variable names cannot contain "-" or ".", so the replacer // canonicalises both to "_". Flag and viper-key names already use "-". @@ -62,6 +73,9 @@ func initializeConfig(cmd *cobra.Command, vp *viper.Viper) error { if err := bindFlag(vp, "log-format", rootFlags.Lookup("log-format")); err != nil { return err } + if err := bindFlag(vp, "output", rootFlags.Lookup("output")); err != nil { + return err + } if err := vp.BindPFlags(cmd.Flags()); err != nil { return fmt.Errorf("bind command flags: %w", err) } @@ -89,17 +103,21 @@ func bindFlag(vp *viper.Viper, key string, flag *pflag.Flag) error { // directly. func loadRuntimeConfig(vp *viper.Viper) (RuntimeConfig, error) { config := RuntimeConfig{ - Kubeconfig: strings.TrimSpace(vp.GetString("kubeconfig")), - KubeContext: strings.TrimSpace(vp.GetString("kube-context")), - Namespace: strings.TrimSpace(vp.GetString("namespace")), - LogLevel: strings.TrimSpace(vp.GetString("log-level")), - LogFormat: strings.TrimSpace(vp.GetString("log-format")), + Kubeconfig: strings.TrimSpace(vp.GetString("kubeconfig")), + KubeContext: strings.TrimSpace(vp.GetString("kube-context")), + Namespace: strings.TrimSpace(vp.GetString("namespace")), + LogLevel: strings.TrimSpace(vp.GetString("log-level")), + LogFormat: strings.TrimSpace(vp.GetString("log-format")), + OutputFormat: strings.TrimSpace(vp.GetString("output")), } if config.LogLevel == "" { config.LogLevel = "info" } if config.LogFormat == "" { - config.LogFormat = "text" + config.LogFormat = formatText + } + if config.OutputFormat == "" { + config.OutputFormat = formatText } switch config.LogLevel { @@ -108,14 +126,26 @@ func loadRuntimeConfig(vp *viper.Viper) (RuntimeConfig, error) { return RuntimeConfig{}, fmt.Errorf("unsupported log level %q", config.LogLevel) } switch config.LogFormat { - case "text", "json": + case formatText, formatJSON: default: return RuntimeConfig{}, fmt.Errorf("unsupported log format %q", config.LogFormat) } + switch config.OutputFormat { + case formatText, formatJSON: + default: + return RuntimeConfig{}, fmt.Errorf("unsupported output format %q", config.OutputFormat) + } 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) +} + // 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 @@ -164,7 +194,7 @@ func newLogger(config RuntimeConfig, out io.Writer, quiet bool) *slog.Logger { } handlerOptions := &slog.HandlerOptions{Level: level} - if config.LogFormat == "json" { + if config.LogFormat == formatJSON { return slog.New(slog.NewJSONHandler(out, handlerOptions)) } diff --git a/cli/internal/cli/info.go b/cli/internal/cli/info.go index c41ab16..96d8877 100644 --- a/cli/internal/cli/info.go +++ b/cli/internal/cli/info.go @@ -45,7 +45,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.viper.GetBool("json") { + if commandContext.outputJSON() { encoded, err := json.MarshalIndent(info, "", " ") if err != nil { return fmt.Errorf("marshal info JSON: %w", err) @@ -59,7 +59,6 @@ func newInfoCommand(commandContext *commandContext) *cobra.Command { return printInfo(commandContext.out, info) }, } - cmd.Flags().Bool("json", false, "Print machine-readable JSON") return cmd } diff --git a/cli/internal/cli/info_test.go b/cli/internal/cli/info_test.go index 8ddac39..09ddec2 100644 --- a/cli/internal/cli/info_test.go +++ b/cli/internal/cli/info_test.go @@ -39,7 +39,7 @@ func TestInfoReadsGlobalKubeEnvironment(t *testing.T) { Viper: viper.New(), KubeClientFactory: factory, }) - root.SetArgs([]string{"info", "devnet", "--json"}) + root.SetArgs([]string{"info", "devnet", "-o", "json"}) require.NoError(t, root.ExecuteContext(context.Background())) assert.Equal(t, "dev-context", capturedConfig.Context) @@ -75,7 +75,7 @@ func TestInfoDefaultsNamespaceToName(t *testing.T) { Viper: viper.New(), KubeClientFactory: kubeClientFactory(client), }) - root.SetArgs([]string{"info", "devnet", "--json"}) + root.SetArgs([]string{"info", "devnet", "-o", "json"}) require.NoError(t, root.ExecuteContext(context.Background())) assert.Contains(t, stdout.String(), `"namespace": "devnet"`) @@ -101,7 +101,7 @@ func TestInfoOmitsWalletWhenFaucetWalletAbsent(t *testing.T) { Viper: viper.New(), KubeClientFactory: kubeClientFactory(client), }) - root.SetArgs([]string{"info", "devnet", "--json"}) + root.SetArgs([]string{"info", "devnet", "-o", "json"}) require.NoError(t, root.ExecuteContext(context.Background())) assert.NotContains(t, stdout.String(), `"wallet"`) diff --git a/cli/internal/cli/list.go b/cli/internal/cli/list.go index a5dbb9e..9f5429e 100644 --- a/cli/internal/cli/list.go +++ b/cli/internal/cli/list.go @@ -27,7 +27,7 @@ func newListCommand(commandContext *commandContext) *cobra.Command { if err != nil { return err } - jsonOutput := commandContext.viper.GetBool("json") + jsonOutput := commandContext.outputJSON() kubeClient, _, err := commandContext.resolveKubeClient(runtimeConfig) if err != nil { @@ -62,8 +62,6 @@ func newListCommand(commandContext *commandContext) *cobra.Command { }, } - cmd.Flags().Bool("json", false, "Print machine-readable JSON") - return cmd } diff --git a/cli/internal/cli/list_test.go b/cli/internal/cli/list_test.go index 0aea802..c5f39b7 100644 --- a/cli/internal/cli/list_test.go +++ b/cli/internal/cli/list_test.go @@ -169,7 +169,7 @@ func TestListJSONOutputShape(t *testing.T) { Viper: viper.New(), KubeClientFactory: kubeClientFactory(client), }) - root.SetArgs([]string{"list", "-n", "team-a", "--json"}) + root.SetArgs([]string{"list", "-n", "team-a", "-o", "json"}) require.NoError(t, root.ExecuteContext(context.Background())) diff --git a/cli/internal/cli/output_test.go b/cli/internal/cli/output_test.go new file mode 100644 index 0000000..3bca1e2 --- /dev/null +++ b/cli/internal/cli/output_test.go @@ -0,0 +1,75 @@ +package cli + +import ( + "bytes" + "context" + "testing" + + "github.com/spf13/viper" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestJSONFlagIsRemoved(t *testing.T) { + t.Parallel() + + var stdout, stderr bytes.Buffer + root := NewRootCommand(Options{Out: &stdout, Err: &stderr, Build: testBuildInfo()}) + root.SetArgs([]string{"list", "--json"}) + + err := root.ExecuteContext(context.Background()) + require.Error(t, err, "--json must no longer be a registered flag") + assert.Contains(t, err.Error(), "unknown flag") +} + +func TestLoadRuntimeConfigRejectsUnknownOutput(t *testing.T) { + t.Parallel() + + vp := viper.New() + vp.Set("output", "xml") + + _, err := loadRuntimeConfig(vp) + require.Error(t, err) + assert.Contains(t, err.Error(), "unsupported output format") +} + +func TestLoadRuntimeConfigOutputDefaultsToText(t *testing.T) { + t.Parallel() + + config, err := loadRuntimeConfig(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()) + }) + } +} + +// TestOutputEnvIsReadAndValidated proves YACD_OUTPUT flows through the env +// binding: a bogus value is rejected in PersistentPreRunE before the command +// runs, which can only happen if the env var is read for the output format. +func TestOutputEnvIsReadAndValidated(t *testing.T) { + t.Setenv("YACD_OUTPUT", "bogus") + + 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) + assert.Contains(t, err.Error(), "unsupported output format") + assert.Empty(t, stdout.String(), "the command must not run when output is invalid") +} diff --git a/cli/internal/cli/root.go b/cli/internal/cli/root.go index ac99b93..87d2665 100644 --- a/cli/internal/cli/root.go +++ b/cli/internal/cli/root.go @@ -136,7 +136,7 @@ func NewRootCommand(options Options) *cobra.Command { 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", "text", "Log format: text, json") + 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. @@ -145,6 +145,7 @@ func NewRootCommand(options Options) *cobra.Command { 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") // The network verbs resolve their target through the shared resolver, so // each is wrapped to clear a stale managed-cluster record when it targeted diff --git a/cli/internal/cli/wallet.go b/cli/internal/cli/wallet.go index 345592b..051af78 100644 --- a/cli/internal/cli/wallet.go +++ b/cli/internal/cli/wallet.go @@ -126,14 +126,13 @@ func newWalletListCommand(commandContext *commandContext) *cobra.Command { return err } - if commandContext.viper.GetBool("json") { + if commandContext.outputJSON() { return writeJSON(commandContext.out, newWalletListItems(wallets)) } return printWalletList(commandContext.out, wallets) }, } - cmd.Flags().Bool("json", false, "Print machine-readable JSON") return cmd } @@ -152,7 +151,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.viper.GetBool("json") + jsonOutput := commandContext.outputJSON() lovelace, topupRequested, err := parseOptionalLovelace(topup) if err != nil { @@ -215,7 +214,6 @@ func newWalletAddCommand(commandContext *commandContext) *cobra.Command { cmd.Flags().String("topup", "", "Fund the new wallet with this many lovelace from the faucet") cmd.Flags().Bool("await", false, "Wait for the funding transaction to confirm on-chain (requires --topup)") cmd.Flags().Duration("await-timeout", 2*time.Minute, "Maximum time to wait for --await confirmation") - cmd.Flags().Bool("json", false, "Print machine-readable JSON") addChainOverrideFlags(cmd) return cmd @@ -238,7 +236,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.viper.GetBool("json") + jsonOutput := commandContext.outputJSON() if err := requireAwaitTimeout(await, awaitTimeout); err != nil { return err } @@ -277,7 +275,6 @@ func newWalletTopUpCommand(commandContext *commandContext) *cobra.Command { cmd.Flags().String("from", "", "Source wallet name to fund from (default: the faucet wallet)") cmd.Flags().Bool("await", false, "Wait for the funding transaction to confirm on-chain") cmd.Flags().Duration("await-timeout", 2*time.Minute, "Maximum time to wait for --await confirmation") - cmd.Flags().Bool("json", false, "Print machine-readable JSON") addChainOverrideFlags(cmd) return cmd diff --git a/test/chainsaw/manager-smoke/chainsaw-test.yaml b/test/chainsaw/manager-smoke/chainsaw-test.yaml index 1b95f60..c6f0ec5 100644 --- a/test/chainsaw/manager-smoke/chainsaw-test.yaml +++ b/test/chainsaw/manager-smoke/chainsaw-test.yaml @@ -542,7 +542,7 @@ spec: content: | set -eu export KUBECTL_KUBERC="${KUBECTL_KUBERC:-false}" - info_json="$(go run ./cli/cmd/yacd --namespace yacd-smoke info phase4-smoke --json)" + info_json="$(go run ./cli/cmd/yacd --namespace yacd-smoke info phase4-smoke -o json)" printf '%s\n' "$info_json" INFO_JSON="$info_json" python3 - <<'PY' import json