BLUF
Every structured-output helper in internal/output writes to os.Stdout directly. That makes command output untestable without mutating a process global, and it's the reason the SIP credential write-failure tests had to swap os.Stdout to simulate a broken pipe (see #20). Migrating to io.Writer + cmd.OutOrStdout() is the right long-term shape. Zero user-visible change, so this is pure hygiene — but it's 91 call sites, so it wants its own PR series.
Why now
Surfaced while fixing a Windows CI failure in #20. The test there injected a failing stdout by pointing os.Stdout at an O_RDONLY file — which fails on Unix but succeeds on Windows, so the test broke on one platform only. We fixed it by using an already-closed *os.File (ErrClosed is returned by Go's portable internal/poll layer, so it's platform-independent), which kept the change to one test file and zero production edits.
That's a fine fix, but the underlying friction remains: testing output requires global mutation, and cmd.OutOrStdout() is the convention cobra exists to provide. The repo already half-uses it — cmd/vcp, cmd/recording, cmd/message, and cmd/bxml all pass cmd.OutOrStdout() to fmt.Fprintf, while every output.* call bypasses it.
Scope
|
Count |
Direct output.Stdout* call sites |
91 |
| Files |
63 |
| Packages |
18 |
cmd-less local wrappers needing signature changes |
3 |
The wrappers that don't currently take a *cobra.Command:
cmd/sip/emit(format, plain, data) — 15 callerscmd/quickstart/printResult(r) — 3 callerscmd/auth/emitJSON(v) — 2 callers
Most direct call sites are tail-position return output.StdoutAuto(format, plain, result) inside a RunE, so cmd is already in scope. Those are mechanical.
The actual risk
Not the edit count — it's that the three helpers have subtly different semantics:
StdoutAuto — flattens when plain, flattens for tableStdoutPlain — flattens, always JSONStdoutPlainList — flattens and normalizes single items to arrays, specifically to prevent single-item ambiguity in list commands
A wrong-helper swap silently changes a command's JSON shape. Auditing golden-test coverage across all 18 packages is the real work here, not the mechanical edits.
Proposed sequencing
Each step provably behavior-preserving, rather than big-bang:
- Add
Print*(w io.Writer, ...) variants and reimplement the existing Stdout* helpers as one-line delegates passing os.Stdout. Pure addition, no call sites touched, trivially reviewable.
- Migrate package-by-package, one PR per package or small group, golden tests as the guard.
- Add a lint rule banning new
output.Stdout* outside internal/output, so the migration can't regress while it's in flight.
Out of scope
Changing any command's output shape. This is a plumbing change — if a golden test needs updating, that's a bug in the migration, not an expected diff.
BLUF
Every structured-output helper in
internal/outputwrites toos.Stdoutdirectly. That makes command output untestable without mutating a process global, and it's the reason the SIP credential write-failure tests had to swapos.Stdoutto simulate a broken pipe (see #20). Migrating toio.Writer+cmd.OutOrStdout()is the right long-term shape. Zero user-visible change, so this is pure hygiene — but it's 91 call sites, so it wants its own PR series.Why now
Surfaced while fixing a Windows CI failure in #20. The test there injected a failing stdout by pointing
os.Stdoutat anO_RDONLYfile — which fails on Unix but succeeds on Windows, so the test broke on one platform only. We fixed it by using an already-closed*os.File(ErrClosedis returned by Go's portableinternal/polllayer, so it's platform-independent), which kept the change to one test file and zero production edits.That's a fine fix, but the underlying friction remains: testing output requires global mutation, and
cmd.OutOrStdout()is the convention cobra exists to provide. The repo already half-uses it —cmd/vcp,cmd/recording,cmd/message, andcmd/bxmlall passcmd.OutOrStdout()tofmt.Fprintf, while everyoutput.*call bypasses it.Scope
output.Stdout*call sitescmd-less local wrappers needing signature changesThe wrappers that don't currently take a
*cobra.Command:cmd/sip/emit(format, plain, data)— 15 callerscmd/quickstart/printResult(r)— 3 callerscmd/auth/emitJSON(v)— 2 callersMost direct call sites are tail-position
return output.StdoutAuto(format, plain, result)inside aRunE, socmdis already in scope. Those are mechanical.The actual risk
Not the edit count — it's that the three helpers have subtly different semantics:
StdoutAuto— flattens whenplain, flattens fortableStdoutPlain— flattens, always JSONStdoutPlainList— flattens and normalizes single items to arrays, specifically to prevent single-item ambiguity in list commandsA wrong-helper swap silently changes a command's JSON shape. Auditing golden-test coverage across all 18 packages is the real work here, not the mechanical edits.
Proposed sequencing
Each step provably behavior-preserving, rather than big-bang:
Print*(w io.Writer, ...)variants and reimplement the existingStdout*helpers as one-line delegates passingos.Stdout. Pure addition, no call sites touched, trivially reviewable.output.Stdout*outsideinternal/output, so the migration can't regress while it's in flight.Out of scope
Changing any command's output shape. This is a plumbing change — if a golden test needs updating, that's a bug in the migration, not an expected diff.