From 890004ed575ac234c7566b0e7984c28d75ae8035 Mon Sep 17 00:00:00 2001 From: Justice Perez White Date: Thu, 30 Apr 2026 16:08:12 -0500 Subject: [PATCH 1/9] prompts --- cli-stdout-pollution-fix-prompt-brief.md | 59 +++++++ cli-stdout-pollution-fix-prompt.md | 208 +++++++++++++++++++++++ 2 files changed, 267 insertions(+) create mode 100644 cli-stdout-pollution-fix-prompt-brief.md create mode 100644 cli-stdout-pollution-fix-prompt.md diff --git a/cli-stdout-pollution-fix-prompt-brief.md b/cli-stdout-pollution-fix-prompt-brief.md new file mode 100644 index 000000000..724ddb5eb --- /dev/null +++ b/cli-stdout-pollution-fix-prompt-brief.md @@ -0,0 +1,59 @@ +# Task: fix stdout pollution in `replicated` CLI (Shortcut #136974) + +Repo: `/Users/jpwhite/code/replicated` (Go, cobra). Ship all five fixes in **one PR** on branch `justice/sc-136974/cli-stdout-warnings-corrupt-redirected-output`. Multiple commits on the branch are fine. Findings below are already verified — implement, don't re-research. + +## Bug + +Commands that emit structured output (kubeconfig YAML, `-o json`) write warning/info text to stdout, corrupting redirected output. Cleanest reproduction: `replicated cluster kubeconfig --name X --stdout > kc.yaml` puts a cobra deprecation warning as the first line of the file, which then breaks `kubectl`. Root cause is `cli/cmd/root.go:120` calling `SetOut(stdout)`, plus several direct `fmt.Print*` info/error calls scattered across the CLI. + +## Fixes + +**1. `cli/cmd/root.go:120`** — change `runCmds.rootCmd.SetOut(stdout)` to use `stderr`. Aligns with cobra's default. Behavior change: `--help` output moves to stderr. If you want to keep `--help` on stdout, add a custom `SetHelpFunc` that writes to stdout while leaving usage-on-error on stderr. Either approach is acceptable; document choice in the commit. Safe because nothing in the codebase calls `cmd.Print*` directly and command output flows through `r.w`, not cobra's writer. + +**2. `cli/cmd/cluster_kubeconfig.go`** — three info/error prints leak to stdout. Move them to stderr using `fmt.Fprintf(os.Stderr, ...)` (matches existing convention in `cli/cmd/`): +- Line 126 — "kubeconfig written to %s" message +- Line 183 — "failed to remove backup kubeconfig" defer +- Line 218 — " ✓ Updated kubernetes context" success message + +**Leave line 111 alone** — `fmt.Println(string(kubeconfig))` is the legitimate `--stdout` data path. Bonus: line 111 should be `fmt.Print` (not `Println`) since the kubeconfig already ends in `\n`. + +Do not add a `stderr` field to the `runners` struct. Use `os.Stderr` directly. + +**3. `cli/cmd/lint.go`** — auto-discovery branch (gated at line 149: `if autoDiscoveryMode`) writes status text to `r.w` regardless of output format, breaking `-o json`. Existing tests miss it because their fixtures all have a populated `.replicated` config. Gate every `fmt.Fprintf(r.w, ...)` inside that block on `r.outputFormat == "table"` — specifically lines 150, 193, 194, 195, 196, 197, and 203. The auto-discovery logic itself must still run; only the prints get gated. Match the gating pattern already used elsewhere in the file (e.g., lines 250, 264, 271). + +**4. `pkg/logger/logger.go`** — TTY detection at lines 110, 143, 181, 210, 229 hardcodes `isatty.IsTerminal(os.Stdout.Fd())` regardless of the writer the logger was given. Replace with a helper that inspects `l.w` (type-asserts to `*os.File`; returns false otherwise — `tabwriter.Writer` will correctly be treated as non-TTY). Also change the two `release_download.go` callers passing `os.Stdout` to pass `os.Stderr` (lines 108 and 283). Don't touch `pkg/kots/release/save_test.go:32`. + +**5. Spinner gating in -o json commands** — apply the existing pattern from `cli/cmd/app_hostname_ls.go:80-81` (`showSpinners := outputFormat == "table"`, gate each spinner call). Sites to fix: +- `cli/cmd/app_rm.go:59, 89` — `ActionWithSpinner`, not gated +- `cli/cmd/release_create.go:432, 452` — `ChildActionWithoutSpinner`, not gated. Note the spinners on lines 292/362/387/410/436 are mitigated by `log.Silence()` at line 235, but verify Silence covers `ChildActionWithoutSpinner` — if not, gate those too. + +Do **not** modify `installer_create.go` or `cluster_prepare.go`; they don't support `-o json` today. + +## Guardrails + +- Don't try to fix the broader pattern of ~169 ungated `fmt.Fprintf(r.w, ...)` calls across the CLI. Stay scoped to the five fixes above. +- Don't refactor adjacent code, add comments explaining the fix, or add backwards-compat shims. +- Run `go build ./...`, `go vet ./...`, and `go test ./cli/cmd/... ./pkg/logger/...` before each commit. +- Don't push the branch. Stop when done and report the branch name. + +## Verification + +After implementing, run: + +```bash +go build -o /tmp/replicated ./cli +# Fix #1 + #2: kubeconfig must be clean YAML +/tmp/replicated cluster kubeconfig --name "" --stdout > /tmp/kc.yaml +head -1 /tmp/kc.yaml # must be: apiVersion: v1 + +# Fix #3: lint JSON must parse in an empty directory +mkdir -p /tmp/lint-empty && cd /tmp/lint-empty +/tmp/replicated release lint -o json > out.json 2>err.log +jq -e . out.json # must succeed + +# Fix #5: app rm JSON must parse +/tmp/replicated app rm -o json -f > out.json 2>err.log +jq -e . out.json # must succeed +``` + +A more detailed companion doc with full code snippets and rationale lives at `cli-stdout-pollution-fix-prompt.md` in this repo. This brief is the source of truth for *what* to change; the long doc has *why*. diff --git a/cli-stdout-pollution-fix-prompt.md b/cli-stdout-pollution-fix-prompt.md new file mode 100644 index 000000000..9b365e51d --- /dev/null +++ b/cli-stdout-pollution-fix-prompt.md @@ -0,0 +1,208 @@ +# Task: fix stdout pollution in `replicated` CLI (Shortcut #136974) + +You are working in the `replicated` CLI repo at `/Users/jpwhite/code/replicated` (Go, cobra-based). Implement the five fixes below. Each section names exact file:line references and the change to make. All findings have already been verified by independent research — do not re-verify; just implement. + +## Background + +**Bug**: Commands that emit structured output (kubeconfig YAML, `-o json`) write warning/info text to stdout, corrupting redirected output. Friction-log report: + +> `replicated cluster kubeconfig > kubeconfig.yaml` includes warning lines inline in stdout. The resulting file contains both the kubeconfig YAML and warning text, breaking subsequent `kubectl` calls. + +**Reproduction (cleanest case)**: +```bash +replicated cluster kubeconfig --name "some cluster" --stdout > kubeconfig.yaml +head -1 kubeconfig.yaml +# Output: "Flag --name has been deprecated, use ID_OR_NAME arguments instead" +``` + +The deprecation warning is emitted by cobra to stdout because `cli/cmd/root.go:120` explicitly calls `SetOut(stdout)`. There are also several direct `fmt.Print*` info/error calls in `cluster_kubeconfig.go` and a similar bug in `lint.go` for `-o json` output. + +## PR structure + +All five fixes ship in **one PR** that closes Shortcut #136974. Use a single branch named `justice/sc-136974/cli-stdout-warnings-corrupt-redirected-output`. You may make multiple commits on that branch (one per fix is fine, or group related ones), but the deliverable is one PR. + +--- + +## Fix #1 — Cobra writer should default to stderr + +**File**: `cli/cmd/root.go:120` + +**Current**: +```go +if stdout != nil { + runCmds.rootCmd.SetOut(stdout) +} +``` + +**Change**: Replace `stdout` with `stderr` so cobra's auto-emitted text (deprecation warnings, usage-on-error, any `c.Print*` calls) goes to stderr. This is cobra's default — we are aligning with it. + +```go +if stderr != nil { + runCmds.rootCmd.SetOut(stderr) +} +``` + +**Why this is safe**: +- The codebase has zero direct calls to `cmd.Print`, `cmd.Println`, `cmd.Printf`. Confirmed via grep. +- All command output is written through `r.w` (a tabwriter wrapping stdout) — independent of cobra's writer. +- `runCmds.rootCmd.SetErr(stderr)` is already called on line 117, so error messages are unaffected. +- No tests assert against cobra's writer for usage/help/deprecation text. + +**Behavior change to be aware of**: `replicated --help` will now write help text to stderr instead of stdout. This matches cobra's default. Many users redirect `--help` (e.g. `replicated --help | grep something`) — that will still work because help is shown on the terminal regardless of stream — but a script doing `replicated --help > help.txt` will produce an empty file. If you want to preserve `--help` on stdout while moving usage-on-error to stderr, also add a custom help function: + +```go +runCmds.rootCmd.SetHelpFunc(func(cmd *cobra.Command, args []string) { + cmd.SetOut(stdout) + // call cobra's default help + ... +}) +``` + +Decide which approach you prefer. The minimal one-line change is acceptable; the help-preserving approach is more polite. Document the choice in the commit message. + +## Fix #2 — `cluster_kubeconfig.go` info/error prints + +**File**: `cli/cmd/cluster_kubeconfig.go` + +Three `fmt.Printf`/`fmt.Println` calls leak diagnostic text to stdout. Move them to stderr. + +| Line | Current | Change to | +|------|---------|-----------| +| 126 | `fmt.Printf("kubeconfig written to %s\n", r.args.kubeconfigPath)` | `fmt.Fprintf(os.Stderr, "kubeconfig written to %s\n", r.args.kubeconfigPath)` | +| 183 | `fmt.Printf("failed to remove backup kubeconfig: %s\n", err.Error())` | `fmt.Fprintf(os.Stderr, "failed to remove backup kubeconfig: %s\n", err.Error())` | +| 218 | `fmt.Printf(" ✓ Updated kubernetes context '%s' in '%s'\n", mergedConfig.CurrentContext, kubeconfigPaths[0])` | `fmt.Fprintf(os.Stderr, " ✓ Updated kubernetes context '%s' in '%s'\n", mergedConfig.CurrentContext, kubeconfigPaths[0])` | + +Use `fmt.Fprintf(os.Stderr, ...)` directly — that matches the existing convention in `cli/cmd/` (see `root.go:380, 382-384, 395, 400, 528-530`, `enterprise_portal_preview.go:156, 185, 193, 196, 199, 201`). Do **not** add a stderr field to the `runners` struct. + +**Leave line 111 as-is** — `fmt.Println(string(kubeconfig))` is the `--stdout` data path and correctly belongs on stdout. + +**Bonus fix on line 111**: change `fmt.Println(string(kubeconfig))` to `fmt.Print(string(kubeconfig))` (or `os.Stdout.Write(kubeconfig)`). The kubeconfig payload already ends in `\n`; `Println` adds a second newline. Minor — kubectl tolerates it — but worth fixing while you're in the file. + +**Test impact**: There are no tests for `cluster kubeconfig` in this repo. Verify manually: + +```bash +# After the fix, this file should contain ONLY YAML, no leading deprecation warning: +go build -o /tmp/replicated ./cli +/tmp/replicated cluster kubeconfig --name "" --stdout > /tmp/kc.yaml +head -1 /tmp/kc.yaml # should be: apiVersion: v1 + +# This file should be a valid kubeconfig (currently it contains the success message): +KUBECONFIG=/tmp/test-kc /tmp/replicated cluster kubeconfig 2>/tmp/stderr +cat /tmp/stderr # success message lives here now +``` + +--- + +## Fix #3 — `lint.go` corrupts `-o json` output in auto-discovery mode + +**File**: `cli/cmd/lint.go` + +When `replicated release lint -o json` runs in a directory without a fully-populated `.replicated` config, the auto-discovery branch (gated at line 149: `if autoDiscoveryMode { ... }`) writes informational text to `r.w` (stdout) regardless of output format, breaking JSON parsers. + +**Why existing tests miss it**: `lint_test.go` fixtures all have a populated `.replicated` config, so `autoDiscoveryMode` is false in tests. + +**Offending lines** (all inside the `if autoDiscoveryMode` block at line 149): + +- 150: `fmt.Fprintf(r.w, "No .replicated config found. Auto-discovering lintable resources in current directory...\n\n")` +- 193: `fmt.Fprintf(r.w, "Discovered resources:\n")` +- 194-197: the four `fmt.Fprintf(r.w, " - %d ...")` lines +- 203: `fmt.Fprintf(r.w, "No lintable resources found in current directory.\n")` + +**Fix**: Wrap each of those `fmt.Fprintf(r.w, ...)` calls with `if r.outputFormat == "table" { ... }`. Or, more cleanly, define `showAutoDiscoveryMessages := r.outputFormat == "table"` once at the top of the auto-discovery block and gate each print on it. Other prints in the file (e.g., lines 250, 264, 271, 287, 299, 308, 320, 334, 373, 898) already use the `outputFormat == "table"` gate — match that pattern. + +The auto-discovery **logic itself must still run** — it populates `config` for the rest of the function. Only the prints get gated. + +**Verify after fix**: + +```bash +cd /tmp && mkdir lint-test && cd lint-test +go run /Users/jpwhite/code/replicated/cli release lint -o json > out.json 2>err.log +jq -e . out.json # must succeed (clean JSON) +cat err.log # nothing on stderr is required either, but spurious is fine +``` + +--- + +## Fix #4 — `logger.go` TTY check is hardcoded to `os.Stdout.Fd()` + +**File**: `pkg/logger/logger.go` + +The logger struct holds `w io.Writer`, but its TTY checks at lines **110, 143, 181, 210, 229** all examine `os.Stdout.Fd()` regardless of which writer the caller passed in. Fix the check so it inspects the configured writer. + +**Pattern**: replace each `isatty.IsTerminal(os.Stdout.Fd())` with a helper that checks `l.w`: + +```go +func (l *Logger) isTTY() bool { + if f, ok := l.w.(*os.File); ok { + return isatty.IsTerminal(f.Fd()) + } + return false +} +``` + +Then replace the five call sites. A `tabwriter.Writer` will fail the type assertion and return false (correct — non-TTY behavior). `os.Stdout` and `os.Stderr` will succeed. + +**Also fix the two `release_download.go` callers** that pass `os.Stdout` directly: +- `cli/cmd/release_download.go:108` — change `logger.NewLogger(os.Stdout)` to `logger.NewLogger(os.Stderr)` +- `cli/cmd/release_download.go:283` — same change + +`release download` produces no stdout data (it writes to disk), so progress messages belong on stderr by convention. Don't touch the test file `pkg/kots/release/save_test.go:32`. + +**Test impact**: no existing tests assert on logger output destinations. Verify manually: + +```bash +go run ./cli release download 2>err.log >out.log +# out.log should be empty, err.log contains the action messages +``` + +--- + +## Fix #5 — Audit logger spinner call sites in commands with `-o json` + +**File**: multiple under `cli/cmd/` + +Even with fix #4, the initial `" • "` text from `ActionWithSpinner` and `ActionWithoutSpinner` still prints to whatever writer the logger has. For commands that share their writer with structured output (`r.w` = stdout), this leaks into `-o json` payloads. + +**The architecturally clean fix** is to make the default logger writer in this codebase be stderr. But that's a larger refactor than the ticket calls for. Instead, apply per-call-site gating using the existing pattern at `cli/cmd/app_hostname_ls.go:80-81`: + +```go +showSpinners := outputFormat == "table" +log := logger.NewLogger(r.w) +// ... +if showSpinners { + log.ActionWithSpinner("Fetching app") +} +``` + +**Sites to fix** (commands that support `-o json` and have non-gated logger calls): + +- `cli/cmd/app_rm.go:59` — `ActionWithSpinner` not gated +- `cli/cmd/app_rm.go:89` — `ActionWithSpinner` not gated +- `cli/cmd/release_create.go:432` — `ChildActionWithoutSpinner` not gated (note: spinners on lines 292, 362, 387, 410, 436 are partially mitigated by `log.Silence()` at line 235, but the `ChildActionWithoutSpinner` calls aren't covered by Silence — verify this and gate them) +- `cli/cmd/release_create.go:452` — same as above + +**Do not modify** `installer_create.go` and `cluster_prepare.go` — they don't currently support `-o json`, so leave them alone (this is a "fix only what's needed" instruction; do not add gates for hypothetical future structured-output support). + +**Verify**: +```bash +go run ./cli app rm -o json -f 2>err.log >out.log +jq -e . out.log # must succeed +``` + +--- + +## General implementation guidelines + +- Single PR, single branch (`justice/sc-136974/cli-stdout-warnings-corrupt-redirected-output`). Multiple commits on the branch are fine — group related fixes together. Use Conventional Commits format. +- Don't add comments explaining the fix — the commit message and PR description handle that. +- Don't refactor adjacent code. Don't add backwards-compat shims. Don't add fields to the `runners` struct. +- Run `go build ./...` and `go vet ./...` before each commit. +- Run existing tests in affected packages: `go test ./cli/cmd/... ./pkg/logger/...` +- Don't push the branch unless the user asks. Stop when done and report the branch name. + +## What NOT to do + +- Don't try to "fix everything" — there are ~169 ungated `fmt.Fprintf(r.w, ...)` calls across the CLI for status text. Most are in commands that don't support structured output. Do not try to gate or move them. Stay focused on the five fixes above. +- Don't add a `stderr` field to the `runners` struct. Use `os.Stderr` directly. +- Don't change `cluster_kubeconfig.go:111` to write to stderr — that's the legitimate stdout data path. +- Don't change the version-update banner code (`pkg/version/version.go`) — `PrintIfUpgradeAvailable` already correctly writes to stderr. From f14857a267ffc73be30c0256d8f0d705036d0957 Mon Sep 17 00:00:00 2001 From: Justice Perez White Date: Fri, 1 May 2026 10:33:05 -0500 Subject: [PATCH 2/9] Revert "prompts" This reverts commit 890004ed575ac234c7566b0e7984c28d75ae8035. --- cli-stdout-pollution-fix-prompt-brief.md | 59 ------- cli-stdout-pollution-fix-prompt.md | 208 ----------------------- 2 files changed, 267 deletions(-) delete mode 100644 cli-stdout-pollution-fix-prompt-brief.md delete mode 100644 cli-stdout-pollution-fix-prompt.md diff --git a/cli-stdout-pollution-fix-prompt-brief.md b/cli-stdout-pollution-fix-prompt-brief.md deleted file mode 100644 index 724ddb5eb..000000000 --- a/cli-stdout-pollution-fix-prompt-brief.md +++ /dev/null @@ -1,59 +0,0 @@ -# Task: fix stdout pollution in `replicated` CLI (Shortcut #136974) - -Repo: `/Users/jpwhite/code/replicated` (Go, cobra). Ship all five fixes in **one PR** on branch `justice/sc-136974/cli-stdout-warnings-corrupt-redirected-output`. Multiple commits on the branch are fine. Findings below are already verified — implement, don't re-research. - -## Bug - -Commands that emit structured output (kubeconfig YAML, `-o json`) write warning/info text to stdout, corrupting redirected output. Cleanest reproduction: `replicated cluster kubeconfig --name X --stdout > kc.yaml` puts a cobra deprecation warning as the first line of the file, which then breaks `kubectl`. Root cause is `cli/cmd/root.go:120` calling `SetOut(stdout)`, plus several direct `fmt.Print*` info/error calls scattered across the CLI. - -## Fixes - -**1. `cli/cmd/root.go:120`** — change `runCmds.rootCmd.SetOut(stdout)` to use `stderr`. Aligns with cobra's default. Behavior change: `--help` output moves to stderr. If you want to keep `--help` on stdout, add a custom `SetHelpFunc` that writes to stdout while leaving usage-on-error on stderr. Either approach is acceptable; document choice in the commit. Safe because nothing in the codebase calls `cmd.Print*` directly and command output flows through `r.w`, not cobra's writer. - -**2. `cli/cmd/cluster_kubeconfig.go`** — three info/error prints leak to stdout. Move them to stderr using `fmt.Fprintf(os.Stderr, ...)` (matches existing convention in `cli/cmd/`): -- Line 126 — "kubeconfig written to %s" message -- Line 183 — "failed to remove backup kubeconfig" defer -- Line 218 — " ✓ Updated kubernetes context" success message - -**Leave line 111 alone** — `fmt.Println(string(kubeconfig))` is the legitimate `--stdout` data path. Bonus: line 111 should be `fmt.Print` (not `Println`) since the kubeconfig already ends in `\n`. - -Do not add a `stderr` field to the `runners` struct. Use `os.Stderr` directly. - -**3. `cli/cmd/lint.go`** — auto-discovery branch (gated at line 149: `if autoDiscoveryMode`) writes status text to `r.w` regardless of output format, breaking `-o json`. Existing tests miss it because their fixtures all have a populated `.replicated` config. Gate every `fmt.Fprintf(r.w, ...)` inside that block on `r.outputFormat == "table"` — specifically lines 150, 193, 194, 195, 196, 197, and 203. The auto-discovery logic itself must still run; only the prints get gated. Match the gating pattern already used elsewhere in the file (e.g., lines 250, 264, 271). - -**4. `pkg/logger/logger.go`** — TTY detection at lines 110, 143, 181, 210, 229 hardcodes `isatty.IsTerminal(os.Stdout.Fd())` regardless of the writer the logger was given. Replace with a helper that inspects `l.w` (type-asserts to `*os.File`; returns false otherwise — `tabwriter.Writer` will correctly be treated as non-TTY). Also change the two `release_download.go` callers passing `os.Stdout` to pass `os.Stderr` (lines 108 and 283). Don't touch `pkg/kots/release/save_test.go:32`. - -**5. Spinner gating in -o json commands** — apply the existing pattern from `cli/cmd/app_hostname_ls.go:80-81` (`showSpinners := outputFormat == "table"`, gate each spinner call). Sites to fix: -- `cli/cmd/app_rm.go:59, 89` — `ActionWithSpinner`, not gated -- `cli/cmd/release_create.go:432, 452` — `ChildActionWithoutSpinner`, not gated. Note the spinners on lines 292/362/387/410/436 are mitigated by `log.Silence()` at line 235, but verify Silence covers `ChildActionWithoutSpinner` — if not, gate those too. - -Do **not** modify `installer_create.go` or `cluster_prepare.go`; they don't support `-o json` today. - -## Guardrails - -- Don't try to fix the broader pattern of ~169 ungated `fmt.Fprintf(r.w, ...)` calls across the CLI. Stay scoped to the five fixes above. -- Don't refactor adjacent code, add comments explaining the fix, or add backwards-compat shims. -- Run `go build ./...`, `go vet ./...`, and `go test ./cli/cmd/... ./pkg/logger/...` before each commit. -- Don't push the branch. Stop when done and report the branch name. - -## Verification - -After implementing, run: - -```bash -go build -o /tmp/replicated ./cli -# Fix #1 + #2: kubeconfig must be clean YAML -/tmp/replicated cluster kubeconfig --name "" --stdout > /tmp/kc.yaml -head -1 /tmp/kc.yaml # must be: apiVersion: v1 - -# Fix #3: lint JSON must parse in an empty directory -mkdir -p /tmp/lint-empty && cd /tmp/lint-empty -/tmp/replicated release lint -o json > out.json 2>err.log -jq -e . out.json # must succeed - -# Fix #5: app rm JSON must parse -/tmp/replicated app rm -o json -f > out.json 2>err.log -jq -e . out.json # must succeed -``` - -A more detailed companion doc with full code snippets and rationale lives at `cli-stdout-pollution-fix-prompt.md` in this repo. This brief is the source of truth for *what* to change; the long doc has *why*. diff --git a/cli-stdout-pollution-fix-prompt.md b/cli-stdout-pollution-fix-prompt.md deleted file mode 100644 index 9b365e51d..000000000 --- a/cli-stdout-pollution-fix-prompt.md +++ /dev/null @@ -1,208 +0,0 @@ -# Task: fix stdout pollution in `replicated` CLI (Shortcut #136974) - -You are working in the `replicated` CLI repo at `/Users/jpwhite/code/replicated` (Go, cobra-based). Implement the five fixes below. Each section names exact file:line references and the change to make. All findings have already been verified by independent research — do not re-verify; just implement. - -## Background - -**Bug**: Commands that emit structured output (kubeconfig YAML, `-o json`) write warning/info text to stdout, corrupting redirected output. Friction-log report: - -> `replicated cluster kubeconfig > kubeconfig.yaml` includes warning lines inline in stdout. The resulting file contains both the kubeconfig YAML and warning text, breaking subsequent `kubectl` calls. - -**Reproduction (cleanest case)**: -```bash -replicated cluster kubeconfig --name "some cluster" --stdout > kubeconfig.yaml -head -1 kubeconfig.yaml -# Output: "Flag --name has been deprecated, use ID_OR_NAME arguments instead" -``` - -The deprecation warning is emitted by cobra to stdout because `cli/cmd/root.go:120` explicitly calls `SetOut(stdout)`. There are also several direct `fmt.Print*` info/error calls in `cluster_kubeconfig.go` and a similar bug in `lint.go` for `-o json` output. - -## PR structure - -All five fixes ship in **one PR** that closes Shortcut #136974. Use a single branch named `justice/sc-136974/cli-stdout-warnings-corrupt-redirected-output`. You may make multiple commits on that branch (one per fix is fine, or group related ones), but the deliverable is one PR. - ---- - -## Fix #1 — Cobra writer should default to stderr - -**File**: `cli/cmd/root.go:120` - -**Current**: -```go -if stdout != nil { - runCmds.rootCmd.SetOut(stdout) -} -``` - -**Change**: Replace `stdout` with `stderr` so cobra's auto-emitted text (deprecation warnings, usage-on-error, any `c.Print*` calls) goes to stderr. This is cobra's default — we are aligning with it. - -```go -if stderr != nil { - runCmds.rootCmd.SetOut(stderr) -} -``` - -**Why this is safe**: -- The codebase has zero direct calls to `cmd.Print`, `cmd.Println`, `cmd.Printf`. Confirmed via grep. -- All command output is written through `r.w` (a tabwriter wrapping stdout) — independent of cobra's writer. -- `runCmds.rootCmd.SetErr(stderr)` is already called on line 117, so error messages are unaffected. -- No tests assert against cobra's writer for usage/help/deprecation text. - -**Behavior change to be aware of**: `replicated --help` will now write help text to stderr instead of stdout. This matches cobra's default. Many users redirect `--help` (e.g. `replicated --help | grep something`) — that will still work because help is shown on the terminal regardless of stream — but a script doing `replicated --help > help.txt` will produce an empty file. If you want to preserve `--help` on stdout while moving usage-on-error to stderr, also add a custom help function: - -```go -runCmds.rootCmd.SetHelpFunc(func(cmd *cobra.Command, args []string) { - cmd.SetOut(stdout) - // call cobra's default help - ... -}) -``` - -Decide which approach you prefer. The minimal one-line change is acceptable; the help-preserving approach is more polite. Document the choice in the commit message. - -## Fix #2 — `cluster_kubeconfig.go` info/error prints - -**File**: `cli/cmd/cluster_kubeconfig.go` - -Three `fmt.Printf`/`fmt.Println` calls leak diagnostic text to stdout. Move them to stderr. - -| Line | Current | Change to | -|------|---------|-----------| -| 126 | `fmt.Printf("kubeconfig written to %s\n", r.args.kubeconfigPath)` | `fmt.Fprintf(os.Stderr, "kubeconfig written to %s\n", r.args.kubeconfigPath)` | -| 183 | `fmt.Printf("failed to remove backup kubeconfig: %s\n", err.Error())` | `fmt.Fprintf(os.Stderr, "failed to remove backup kubeconfig: %s\n", err.Error())` | -| 218 | `fmt.Printf(" ✓ Updated kubernetes context '%s' in '%s'\n", mergedConfig.CurrentContext, kubeconfigPaths[0])` | `fmt.Fprintf(os.Stderr, " ✓ Updated kubernetes context '%s' in '%s'\n", mergedConfig.CurrentContext, kubeconfigPaths[0])` | - -Use `fmt.Fprintf(os.Stderr, ...)` directly — that matches the existing convention in `cli/cmd/` (see `root.go:380, 382-384, 395, 400, 528-530`, `enterprise_portal_preview.go:156, 185, 193, 196, 199, 201`). Do **not** add a stderr field to the `runners` struct. - -**Leave line 111 as-is** — `fmt.Println(string(kubeconfig))` is the `--stdout` data path and correctly belongs on stdout. - -**Bonus fix on line 111**: change `fmt.Println(string(kubeconfig))` to `fmt.Print(string(kubeconfig))` (or `os.Stdout.Write(kubeconfig)`). The kubeconfig payload already ends in `\n`; `Println` adds a second newline. Minor — kubectl tolerates it — but worth fixing while you're in the file. - -**Test impact**: There are no tests for `cluster kubeconfig` in this repo. Verify manually: - -```bash -# After the fix, this file should contain ONLY YAML, no leading deprecation warning: -go build -o /tmp/replicated ./cli -/tmp/replicated cluster kubeconfig --name "" --stdout > /tmp/kc.yaml -head -1 /tmp/kc.yaml # should be: apiVersion: v1 - -# This file should be a valid kubeconfig (currently it contains the success message): -KUBECONFIG=/tmp/test-kc /tmp/replicated cluster kubeconfig 2>/tmp/stderr -cat /tmp/stderr # success message lives here now -``` - ---- - -## Fix #3 — `lint.go` corrupts `-o json` output in auto-discovery mode - -**File**: `cli/cmd/lint.go` - -When `replicated release lint -o json` runs in a directory without a fully-populated `.replicated` config, the auto-discovery branch (gated at line 149: `if autoDiscoveryMode { ... }`) writes informational text to `r.w` (stdout) regardless of output format, breaking JSON parsers. - -**Why existing tests miss it**: `lint_test.go` fixtures all have a populated `.replicated` config, so `autoDiscoveryMode` is false in tests. - -**Offending lines** (all inside the `if autoDiscoveryMode` block at line 149): - -- 150: `fmt.Fprintf(r.w, "No .replicated config found. Auto-discovering lintable resources in current directory...\n\n")` -- 193: `fmt.Fprintf(r.w, "Discovered resources:\n")` -- 194-197: the four `fmt.Fprintf(r.w, " - %d ...")` lines -- 203: `fmt.Fprintf(r.w, "No lintable resources found in current directory.\n")` - -**Fix**: Wrap each of those `fmt.Fprintf(r.w, ...)` calls with `if r.outputFormat == "table" { ... }`. Or, more cleanly, define `showAutoDiscoveryMessages := r.outputFormat == "table"` once at the top of the auto-discovery block and gate each print on it. Other prints in the file (e.g., lines 250, 264, 271, 287, 299, 308, 320, 334, 373, 898) already use the `outputFormat == "table"` gate — match that pattern. - -The auto-discovery **logic itself must still run** — it populates `config` for the rest of the function. Only the prints get gated. - -**Verify after fix**: - -```bash -cd /tmp && mkdir lint-test && cd lint-test -go run /Users/jpwhite/code/replicated/cli release lint -o json > out.json 2>err.log -jq -e . out.json # must succeed (clean JSON) -cat err.log # nothing on stderr is required either, but spurious is fine -``` - ---- - -## Fix #4 — `logger.go` TTY check is hardcoded to `os.Stdout.Fd()` - -**File**: `pkg/logger/logger.go` - -The logger struct holds `w io.Writer`, but its TTY checks at lines **110, 143, 181, 210, 229** all examine `os.Stdout.Fd()` regardless of which writer the caller passed in. Fix the check so it inspects the configured writer. - -**Pattern**: replace each `isatty.IsTerminal(os.Stdout.Fd())` with a helper that checks `l.w`: - -```go -func (l *Logger) isTTY() bool { - if f, ok := l.w.(*os.File); ok { - return isatty.IsTerminal(f.Fd()) - } - return false -} -``` - -Then replace the five call sites. A `tabwriter.Writer` will fail the type assertion and return false (correct — non-TTY behavior). `os.Stdout` and `os.Stderr` will succeed. - -**Also fix the two `release_download.go` callers** that pass `os.Stdout` directly: -- `cli/cmd/release_download.go:108` — change `logger.NewLogger(os.Stdout)` to `logger.NewLogger(os.Stderr)` -- `cli/cmd/release_download.go:283` — same change - -`release download` produces no stdout data (it writes to disk), so progress messages belong on stderr by convention. Don't touch the test file `pkg/kots/release/save_test.go:32`. - -**Test impact**: no existing tests assert on logger output destinations. Verify manually: - -```bash -go run ./cli release download 2>err.log >out.log -# out.log should be empty, err.log contains the action messages -``` - ---- - -## Fix #5 — Audit logger spinner call sites in commands with `-o json` - -**File**: multiple under `cli/cmd/` - -Even with fix #4, the initial `" • "` text from `ActionWithSpinner` and `ActionWithoutSpinner` still prints to whatever writer the logger has. For commands that share their writer with structured output (`r.w` = stdout), this leaks into `-o json` payloads. - -**The architecturally clean fix** is to make the default logger writer in this codebase be stderr. But that's a larger refactor than the ticket calls for. Instead, apply per-call-site gating using the existing pattern at `cli/cmd/app_hostname_ls.go:80-81`: - -```go -showSpinners := outputFormat == "table" -log := logger.NewLogger(r.w) -// ... -if showSpinners { - log.ActionWithSpinner("Fetching app") -} -``` - -**Sites to fix** (commands that support `-o json` and have non-gated logger calls): - -- `cli/cmd/app_rm.go:59` — `ActionWithSpinner` not gated -- `cli/cmd/app_rm.go:89` — `ActionWithSpinner` not gated -- `cli/cmd/release_create.go:432` — `ChildActionWithoutSpinner` not gated (note: spinners on lines 292, 362, 387, 410, 436 are partially mitigated by `log.Silence()` at line 235, but the `ChildActionWithoutSpinner` calls aren't covered by Silence — verify this and gate them) -- `cli/cmd/release_create.go:452` — same as above - -**Do not modify** `installer_create.go` and `cluster_prepare.go` — they don't currently support `-o json`, so leave them alone (this is a "fix only what's needed" instruction; do not add gates for hypothetical future structured-output support). - -**Verify**: -```bash -go run ./cli app rm -o json -f 2>err.log >out.log -jq -e . out.log # must succeed -``` - ---- - -## General implementation guidelines - -- Single PR, single branch (`justice/sc-136974/cli-stdout-warnings-corrupt-redirected-output`). Multiple commits on the branch are fine — group related fixes together. Use Conventional Commits format. -- Don't add comments explaining the fix — the commit message and PR description handle that. -- Don't refactor adjacent code. Don't add backwards-compat shims. Don't add fields to the `runners` struct. -- Run `go build ./...` and `go vet ./...` before each commit. -- Run existing tests in affected packages: `go test ./cli/cmd/... ./pkg/logger/...` -- Don't push the branch unless the user asks. Stop when done and report the branch name. - -## What NOT to do - -- Don't try to "fix everything" — there are ~169 ungated `fmt.Fprintf(r.w, ...)` calls across the CLI for status text. Most are in commands that don't support structured output. Do not try to gate or move them. Stay focused on the five fixes above. -- Don't add a `stderr` field to the `runners` struct. Use `os.Stderr` directly. -- Don't change `cluster_kubeconfig.go:111` to write to stderr — that's the legitimate stdout data path. -- Don't change the version-update banner code (`pkg/version/version.go`) — `PrintIfUpgradeAvailable` already correctly writes to stderr. From a46ff7aa27f7229ffde30edb14d38775a3db1aab Mon Sep 17 00:00:00 2001 From: Justice Perez White Date: Fri, 1 May 2026 10:33:14 -0500 Subject: [PATCH 3/9] fix(cli): route cobra-emitted text to stderr Cobra's auto-emitted output (deprecation warnings, usage-on-error) was going to stdout, corrupting redirected structured output such as `cluster kubeconfig --stdout > kubeconfig.yaml`. Align with cobra's default by setting Out to stderr. Refs sc-136974 --- cli/cmd/root.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/cli/cmd/root.go b/cli/cmd/root.go index 226f41d34..8b954070d 100644 --- a/cli/cmd/root.go +++ b/cli/cmd/root.go @@ -116,8 +116,8 @@ func Execute(rootCmd *cobra.Command, stdin io.Reader, stdout io.Writer, stderr i if stderr != nil { runCmds.rootCmd.SetErr(stderr) } - if stdout != nil { - runCmds.rootCmd.SetOut(stdout) + if stderr != nil { + runCmds.rootCmd.SetOut(stderr) } // Setup PersistentPreRun to handle --debug flag From 5e2f76c147db290868aa46e92469d1c9382ef14a Mon Sep 17 00:00:00 2001 From: Justice Perez White Date: Fri, 1 May 2026 10:33:20 -0500 Subject: [PATCH 4/9] fix(cluster kubeconfig): route diagnostic prints to stderr Move the kubeconfig success, backup-removal failure, and context-updated messages from stdout to stderr so they no longer corrupt the YAML payload when output is redirected. Also switch the --stdout data path from Println to Print to avoid an extra trailing newline. Refs sc-136974 --- cli/cmd/cluster_kubeconfig.go | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/cli/cmd/cluster_kubeconfig.go b/cli/cmd/cluster_kubeconfig.go index 88615e989..2fcf57421 100644 --- a/cli/cmd/cluster_kubeconfig.go +++ b/cli/cmd/cluster_kubeconfig.go @@ -108,7 +108,7 @@ func (r *runners) kubeconfigCluster(_ *cobra.Command, args []string) error { } if r.args.kubeconfigStdout { - fmt.Println(string(kubeconfig)) + fmt.Print(string(kubeconfig)) return nil } @@ -123,7 +123,7 @@ func (r *runners) kubeconfigCluster(_ *cobra.Command, args []string) error { return errors.Wrap(err, "write kubeconfig") } - fmt.Printf("kubeconfig written to %s\n", r.args.kubeconfigPath) + fmt.Fprintf(os.Stderr, "kubeconfig written to %s\n", r.args.kubeconfigPath) return nil } @@ -180,7 +180,7 @@ func (r *runners) kubeconfigCluster(_ *cobra.Command, args []string) error { for _, backupPath := range backupPaths { err := os.Remove(backupPath) if err != nil { - fmt.Printf("failed to remove backup kubeconfig: %s\n", err.Error()) + fmt.Fprintf(os.Stderr, "failed to remove backup kubeconfig: %s\n", err.Error()) } } }() @@ -215,7 +215,7 @@ func (r *runners) kubeconfigCluster(_ *cobra.Command, args []string) error { return errors.Wrap(err, "write kubeconfig") } - fmt.Printf(" ✓ Updated kubernetes context '%s' in '%s'\n", mergedConfig.CurrentContext, kubeconfigPaths[0]) + fmt.Fprintf(os.Stderr, " ✓ Updated kubernetes context '%s' in '%s'\n", mergedConfig.CurrentContext, kubeconfigPaths[0]) return nil } From 9b9100268230ce9a30627f36f3c1f3d2585037e0 Mon Sep 17 00:00:00 2001 From: Justice Perez White Date: Fri, 1 May 2026 10:33:27 -0500 Subject: [PATCH 5/9] fix(release lint): keep auto-discovery messages out of -o json output The auto-discovery branch printed informational text to r.w (stdout) regardless of output format, breaking JSON parsers when running `release lint -o json` outside a configured project. Gate the discovery messages on table format, and have the empty-result early return emit a valid JSON payload so consumers always receive parseable output. Refs sc-136974 --- cli/cmd/lint.go | 34 ++++++++++++++++++++++++---------- 1 file changed, 24 insertions(+), 10 deletions(-) diff --git a/cli/cmd/lint.go b/cli/cmd/lint.go index 3a18d8f24..1b4662d44 100644 --- a/cli/cmd/lint.go +++ b/cli/cmd/lint.go @@ -147,8 +147,12 @@ func (r *runners) runLint(cmd *cobra.Command, args []string) error { autoDiscoveryMode := len(config.Charts) == 0 && len(config.Preflights) == 0 && len(config.Manifests) == 0 if autoDiscoveryMode { - fmt.Fprintf(r.w, "No .replicated config found. Auto-discovering lintable resources in current directory...\n\n") - r.w.Flush() + showAutoDiscoveryMessages := r.outputFormat == "table" + + if showAutoDiscoveryMessages { + fmt.Fprintf(r.w, "No .replicated config found. Auto-discovering lintable resources in current directory...\n\n") + r.w.Flush() + } // Auto-discover Helm charts (for counting and display) chartPaths, err := lint2.DiscoverChartPaths(filepath.Join(".", "**")) @@ -190,18 +194,28 @@ func (r *runners) runLint(cmd *cobra.Command, args []string) error { } // Print what was discovered - fmt.Fprintf(r.w, "Discovered resources:\n") - fmt.Fprintf(r.w, " - %d Helm chart(s)\n", len(chartPaths)) - fmt.Fprintf(r.w, " - %d Preflight spec(s)\n", len(preflightPaths)) - fmt.Fprintf(r.w, " - %d Support Bundle spec(s)\n", len(sbPaths)) - fmt.Fprintf(r.w, " - %d HelmChart manifest(s)\n\n", len(helmChartPaths)) - r.w.Flush() + if showAutoDiscoveryMessages { + fmt.Fprintf(r.w, "Discovered resources:\n") + fmt.Fprintf(r.w, " - %d Helm chart(s)\n", len(chartPaths)) + fmt.Fprintf(r.w, " - %d Preflight spec(s)\n", len(preflightPaths)) + fmt.Fprintf(r.w, " - %d Support Bundle spec(s)\n", len(sbPaths)) + fmt.Fprintf(r.w, " - %d HelmChart manifest(s)\n\n", len(helmChartPaths)) + r.w.Flush() + } // If nothing was found and EC linting is not enabled, exit early. // EC linting runs after this block, so don't bail out when it's enabled. if len(chartPaths) == 0 && len(preflightPaths) == 0 && len(sbPaths) == 0 && !config.ReplLint.Linters.EmbeddedCluster.IsEnabled() { - fmt.Fprintf(r.w, "No lintable resources found in current directory.\n") - r.w.Flush() + if showAutoDiscoveryMessages { + fmt.Fprintf(r.w, "No lintable resources found in current directory.\n") + r.w.Flush() + } + if r.outputFormat == "json" { + output.Summary = r.calculateOverallSummary(output) + if err := print.LintResults(r.outputFormat, r.w, output); err != nil { + return errors.Wrap(err, "failed to print JSON output to stdout") + } + } return nil } } From 1d70e07616e5ca426e75dcade033438aa26c3cb0 Mon Sep 17 00:00:00 2001 From: Justice Perez White Date: Fri, 1 May 2026 10:33:37 -0500 Subject: [PATCH 6/9] fix(logger): inspect configured writer for TTY detection MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Logger TTY checks were hardcoded to os.Stdout.Fd(), so spinners fired even when the logger had been pointed at a non-TTY writer. Replace each call site with an isTTY helper that type-asserts the configured writer. Switch the two `release download` callers from os.Stdout to os.Stderr — the command's data path writes files to disk, so progress messages belong on stderr. Refs sc-136974 --- cli/cmd/release_download.go | 4 ++-- pkg/logger/logger.go | 17 ++++++++++++----- 2 files changed, 14 insertions(+), 7 deletions(-) diff --git a/cli/cmd/release_download.go b/cli/cmd/release_download.go index 80e81cc11..a152ff807 100644 --- a/cli/cmd/release_download.go +++ b/cli/cmd/release_download.go @@ -105,7 +105,7 @@ func (r *runners) releaseDownload(command *cobra.Command, args []string) error { return r.releaseInspect(command, args) } - log := logger.NewLogger(os.Stdout) + log := logger.NewLogger(os.Stderr) // Determine sequence to download var seq int64 @@ -280,7 +280,7 @@ func (r *runners) downloadReleaseArchive(seq int64, dest string) error { } defer os.RemoveAll(tempDir) - log := logger.NewLogger(os.Stdout) + log := logger.NewLogger(os.Stderr) if err := kotsrelease.Save(tempDir, release, log); err != nil { return errors.Wrap(err, "save release to temp dir") } diff --git a/pkg/logger/logger.go b/pkg/logger/logger.go index 808e258c7..e039ef3d1 100644 --- a/pkg/logger/logger.go +++ b/pkg/logger/logger.go @@ -26,6 +26,13 @@ func NewLogger(writer io.Writer) *Logger { } } +func (l *Logger) isTTY() bool { + if f, ok := l.w.(*os.File); ok { + return isatty.IsTerminal(f.Fd()) + } + return false +} + func (l *Logger) Silence() { if l == nil { return @@ -107,7 +114,7 @@ func (l *Logger) ActionWithSpinner(msg string, args ...interface{}) { fmt.Fprintf(l.w, " • ") fmt.Fprintf(l.w, msg, args...) - if isatty.IsTerminal(os.Stdout.Fd()) { + if l.isTTY() { s := spin.New() fmt.Fprintf(l.w, " %s", s.Next()) @@ -140,7 +147,7 @@ func (l *Logger) ChildActionWithSpinner(msg string, args ...interface{}) { fmt.Fprintf(l.w, " • ") fmt.Fprintf(l.w, msg, args...) - if isatty.IsTerminal(os.Stdout.Fd()) { + if l.isTTY() { s := spin.New() fmt.Fprintf(l.w, " %s", s.Next()) @@ -178,7 +185,7 @@ func (l *Logger) FinishChildSpinner() { green.Fprintf(l.w, " ✓") fmt.Fprintf(l.w, " \n") - if isatty.IsTerminal(os.Stdout.Fd()) { + if l.isTTY() { l.spinnerStopCh <- true close(l.spinnerStopCh) } @@ -207,7 +214,7 @@ func (l *Logger) FinishSpinner() { green.Fprintf(l.w, " ✓") fmt.Fprintf(l.w, " \n") - if isatty.IsTerminal(os.Stdout.Fd()) { + if l.isTTY() { l.spinnerStopCh <- true close(l.spinnerStopCh) } @@ -226,7 +233,7 @@ func (l *Logger) FinishSpinnerWithError() { red.Fprintf(l.w, " ✗") fmt.Fprintf(l.w, " \n") - if isatty.IsTerminal(os.Stdout.Fd()) { + if l.isTTY() { l.spinnerStopCh <- true close(l.spinnerStopCh) } From 7708342e6df831bc1fbbf180fd385f2bcc478e3a Mon Sep 17 00:00:00 2001 From: Justice Perez White Date: Fri, 1 May 2026 10:35:54 -0500 Subject: [PATCH 7/9] fix(app rm): suppress spinners when -o json is requested `app rm` shares its logger writer with structured output, so the fetch/delete spinners leaked into the JSON payload. Gate the spinner calls on `outputFormat == \"table\"` to keep the JSON output clean. Refs sc-136974 --- cli/cmd/app_rm.go | 25 +++++++++++++++++++------ 1 file changed, 19 insertions(+), 6 deletions(-) diff --git a/cli/cmd/app_rm.go b/cli/cmd/app_rm.go index 6d3e4fbef..92e5010d8 100644 --- a/cli/cmd/app_rm.go +++ b/cli/cmd/app_rm.go @@ -55,14 +55,21 @@ replicated app delete "Custom App" --output json`, func (r *runners) deleteApp(ctx context.Context, cmd *cobra.Command, appName string, opts deleteAppOpts, outputFormat string) error { log := logger.NewLogger(r.w) + showSpinners := outputFormat == "table" - log.ActionWithSpinner("Fetching App") + if showSpinners { + log.ActionWithSpinner("Fetching App") + } app, err := r.kotsAPI.GetApp(ctx, appName, true) if err != nil { - log.FinishSpinnerWithError() + if showSpinners { + log.FinishSpinnerWithError() + } return errors.Wrap(err, "list apps") } - log.FinishSpinner() + if showSpinners { + log.FinishSpinner() + } apps := []types.AppAndChannels{ { @@ -86,13 +93,19 @@ func (r *runners) deleteApp(ctx context.Context, cmd *cobra.Command, appName str } } - log.ActionWithSpinner("Deleting App") + if showSpinners { + log.ActionWithSpinner("Deleting App") + } err = r.kotsAPI.DeleteKOTSApp(ctx, app.ID) if err != nil { - log.FinishSpinnerWithError() + if showSpinners { + log.FinishSpinnerWithError() + } return errors.Wrap(err, "delete app") } - log.FinishSpinner() + if showSpinners { + log.FinishSpinner() + } return nil } From 6c2f3f0e67b5d712103b86b1340b8c8232f7faea Mon Sep 17 00:00:00 2001 From: Justice Perez White Date: Fri, 1 May 2026 12:16:42 -0500 Subject: [PATCH 8/9] fix(logger): preserve spinner UX when writer wraps stdout The previous patch broke spinners for any logger built from a tabwriter wrapping stdout: the type assertion to *os.File failed, so isTTY() always returned false. Capture the stdout TTY state once in Execute() and let callers thread it through with SetIsTerminal so wrapped writers still spin when stdout is a real terminal. Refs sc-136974 --- cli/cmd/app_hostname_ls.go | 2 +- cli/cmd/app_rm.go | 2 +- cli/cmd/cluster_prepare.go | 2 +- cli/cmd/installer_create.go | 2 +- cli/cmd/release_create.go | 2 +- cli/cmd/root.go | 13 ++++++++++--- cli/cmd/runner.go | 1 + pkg/logger/logger.go | 15 +++++++++++++++ 8 files changed, 31 insertions(+), 8 deletions(-) diff --git a/cli/cmd/app_hostname_ls.go b/cli/cmd/app_hostname_ls.go index 7e755e425..bd881d0c2 100644 --- a/cli/cmd/app_hostname_ls.go +++ b/cli/cmd/app_hostname_ls.go @@ -79,7 +79,7 @@ replicated app hostname ls --app myapp --output json`, func (r *runners) listAppHostnames(ctx context.Context, outputFormat string) error { // Only show spinners for table output showSpinners := outputFormat == "table" - log := logger.NewLogger(r.w) + log := logger.NewLogger(r.w).SetIsTerminal(r.stdoutIsTTY) // Resolve app ID from slug or ID appSlugOrID := r.appSlug diff --git a/cli/cmd/app_rm.go b/cli/cmd/app_rm.go index 92e5010d8..219b58be8 100644 --- a/cli/cmd/app_rm.go +++ b/cli/cmd/app_rm.go @@ -54,7 +54,7 @@ replicated app delete "Custom App" --output json`, } func (r *runners) deleteApp(ctx context.Context, cmd *cobra.Command, appName string, opts deleteAppOpts, outputFormat string) error { - log := logger.NewLogger(r.w) + log := logger.NewLogger(r.w).SetIsTerminal(r.stdoutIsTTY) showSpinners := outputFormat == "table" if showSpinners { diff --git a/cli/cmd/cluster_prepare.go b/cli/cmd/cluster_prepare.go index e0affc2d7..965ad9de3 100644 --- a/cli/cmd/cluster_prepare.go +++ b/cli/cmd/cluster_prepare.go @@ -154,7 +154,7 @@ func (r *runners) prepareCluster(_ *cobra.Command, args []string) error { return errors.New("no app specified") } - log := logger.NewLogger(r.w) + log := logger.NewLogger(r.w).SetIsTerminal(r.stdoutIsTTY) release, err := prepareRelease(r, log) if err != nil { diff --git a/cli/cmd/installer_create.go b/cli/cmd/installer_create.go index 101c4b8ac..38ebf0256 100644 --- a/cli/cmd/installer_create.go +++ b/cli/cmd/installer_create.go @@ -58,7 +58,7 @@ func (r *runners) installerCreate(_ *cobra.Command, _ []string) error { return errors.Errorf("Installer specs are only supported for KOTS applications, app %q has type %q", r.appID, r.appType) } - log := logger.NewLogger(r.w) + log := logger.NewLogger(r.w).SetIsTerminal(r.stdoutIsTTY) if r.args.createInstallerAutoDefaults { log.ActionWithSpinner("Reading Environment") err := r.setKOTSDefaultInstallerParams() diff --git a/cli/cmd/release_create.go b/cli/cmd/release_create.go index 9c8eb2971..b167823d5 100644 --- a/cli/cmd/release_create.go +++ b/cli/cmd/release_create.go @@ -230,7 +230,7 @@ func (r *runners) releaseCreate(cmd *cobra.Command, args []string) (err error) { printIfError(cmd, err) }() - log := logger.NewLogger(r.w) + log := logger.NewLogger(r.w).SetIsTerminal(r.stdoutIsTTY) if r.outputFormat == "json" { // suppress log lines for machine-readable output log.Silence() diff --git a/cli/cmd/root.go b/cli/cmd/root.go index 8b954070d..cb1caa535 100644 --- a/cli/cmd/root.go +++ b/cli/cmd/root.go @@ -9,6 +9,7 @@ import ( "text/tabwriter" "github.com/Masterminds/sprig/v3" + "github.com/mattn/go-isatty" "github.com/pkg/errors" "github.com/replicatedhq/replicated/client" replicatedcache "github.com/replicatedhq/replicated/pkg/cache" @@ -104,11 +105,17 @@ Use "{{.CommandPath}} [command] --help" for more information about a command.{{e func Execute(rootCmd *cobra.Command, stdin io.Reader, stdout io.Writer, stderr io.Writer) error { w := tabwriter.NewWriter(stdout, minWidth, tabWidth, padding, padChar, tabwriter.TabIndent) + stdoutIsTTY := false + if f, ok := stdout.(*os.File); ok { + stdoutIsTTY = isatty.IsTerminal(f.Fd()) + } + // get api client and app ID after flags are parsed runCmds := &runners{ - rootCmd: rootCmd, - stdin: stdin, - w: w, + rootCmd: rootCmd, + stdin: stdin, + w: w, + stdoutIsTTY: stdoutIsTTY, } if runCmds.rootCmd == nil { runCmds.rootCmd = GetRootCmd() diff --git a/cli/cmd/runner.go b/cli/cmd/runner.go index d9f376d06..1f54411a4 100644 --- a/cli/cmd/runner.go +++ b/cli/cmd/runner.go @@ -24,6 +24,7 @@ type runners struct { stdin io.Reader outputFormat string w *tabwriter.Writer + stdoutIsTTY bool rootCmd *cobra.Command args runnerArgs diff --git a/pkg/logger/logger.go b/pkg/logger/logger.go index e039ef3d1..f883b95ba 100644 --- a/pkg/logger/logger.go +++ b/pkg/logger/logger.go @@ -18,6 +18,7 @@ type Logger struct { spinnerArgs []interface{} isSilent bool isVerbose bool + isTerminal *bool } func NewLogger(writer io.Writer) *Logger { @@ -26,7 +27,21 @@ func NewLogger(writer io.Writer) *Logger { } } +// SetIsTerminal lets callers override TTY detection. Useful when the +// logger writes through a wrapper (e.g. tabwriter) that hides the +// underlying file descriptor. +func (l *Logger) SetIsTerminal(isTerminal bool) *Logger { + if l == nil { + return l + } + l.isTerminal = &isTerminal + return l +} + func (l *Logger) isTTY() bool { + if l.isTerminal != nil { + return *l.isTerminal + } if f, ok := l.w.(*os.File); ok { return isatty.IsTerminal(f.Fd()) } From b77d38f48b73fdbdf7c044ddc093be804273f0d3 Mon Sep 17 00:00:00 2001 From: Justice Perez White Date: Fri, 1 May 2026 12:31:13 -0500 Subject: [PATCH 9/9] fix(cli): keep --help on stdout while routing diagnostics to stderr Setting cobra Out to stderr also redirected --help, breaking `replicated --help | grep ...` and `--help > help.txt`. Restore the CLI convention by overriding the root HelpFunc to write to stdout while leaving deprecation warnings and usage-on-error on stderr. Refs sc-136974 --- cli/cmd/root.go | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/cli/cmd/root.go b/cli/cmd/root.go index cb1caa535..866370836 100644 --- a/cli/cmd/root.go +++ b/cli/cmd/root.go @@ -122,10 +122,15 @@ func Execute(rootCmd *cobra.Command, stdin io.Reader, stdout io.Writer, stderr i } if stderr != nil { runCmds.rootCmd.SetErr(stderr) - } - if stderr != nil { runCmds.rootCmd.SetOut(stderr) } + if stdout != nil { + defaultHelpFunc := runCmds.rootCmd.HelpFunc() + runCmds.rootCmd.SetHelpFunc(func(cmd *cobra.Command, args []string) { + cmd.SetOut(stdout) + defaultHelpFunc(cmd, args) + }) + } // Setup PersistentPreRun to handle --debug flag runCmds.rootCmd.PersistentPreRun = func(cmd *cobra.Command, args []string) {