Skip to content

refactor(cmd): inject output stream - #171

Open
synackd wants to merge 1 commit into
stack/coverage/07-command-and-client-cleanupfrom
stack/coverage/08-output-and-error-injection
Open

synackd wants to merge 1 commit into
stack/coverage/07-command-and-client-cleanupfrom
stack/coverage/08-output-and-error-injection

Conversation

@synackd

@synackd synackd commented Oct 5, 2026 •

Copy link
Copy Markdown
Collaborator

Description

Context

This is branch 8 of 16 in a stacked series (stack/coverage/01-exit-code-contract → stack/coverage/16-convention-checks) that replaces ochami's package-global CLI state with an invocation-scoped runtime, adds a regression test suite, makes the tests safe to run in parallel under Go's race detector, and enforces the test conventions in CI. It is stacked on stack/coverage/07-command-and-client-cleanup and merges before stack/coverage/09-http-client-foundations. Review each branch's own diff.

Branches

  • stack/coverage/01-exit-code-contract
  • stack/coverage/02-client-contract-tests
  • stack/coverage/03-command-e2e-and-interactive
  • stack/coverage/04-public-config
  • stack/coverage/05-injected-boundaries
  • stack/coverage/06-coverage-regressions
  • stack/coverage/07-command-and-client-cleanup
  • stack/coverage/08-output-and-error-injection ← THIS BRANCH
  • stack/coverage/09-http-client-foundations
  • stack/coverage/10-configfile-and-rcs-hardening
  • stack/coverage/11-typed-batches-and-context
  • stack/coverage/12-local-command-options
  • stack/coverage/13-runtime-core-and-command-migration
  • stack/coverage/14-test-parallelization-and-race-detection
  • stack/coverage/15-final-coverage-closure
  • stack/coverage/16-convention-checks

Summary

Route command output through the CLI's injected output stream (cli.Ios.Out()) instead of process-global stdout: service output, configuration display, version information, cloud-init data, and prompts. Callers and tests can capture output without replacing os.Stdout, and the command test harness no longer swaps it.

dialWebSocket returns the error from parsing a malformed console URI instead of ignoring it and dereferencing a nil URL, and ConnectConsole returns a failure to restore the terminal after an interactive session instead of discarding it.

.golangci.yml turns on errcheck's check-blank, so errors discarded with _ = are reported, and exempts the command flag getters (Flags().GetBool etc.), whose errors can't occur for flags registered with the matching type on the same command. Tests that discarded errors now check them. New discover-static and BSS error-arm tests live in _errors_test.go files, and pkg/discover, pkg/format, and pkg/xname get new tests.

Why

Writing to process-global stdout prevents capturing output per invocation and makes output assertions depend on swapping operating-system globals.

Review notes

Check that every migrated command writes to the injected stream. No output format or wording change is intended. Write failures are still ignored, as before.

Checklist

  • My code follows the style guidelines of this project
  • I have added/updated comments where needed
  • I have added tests that prove my fix is effective or my feature works
  • I have run make test (or equivalent) locally and all tests pass
  • I have updated the relevant documentation (CLI examples, man pages, README, other docs, etc.)
  • DCO Sign-off: All commits are signed off (git commit -s) with my real name and email
  • REUSE Compliance:
    • Each new/modified source file has SPDX copyright and license headers
    • Any non-commentable files include a <filename>.license sidecar
    • All referenced licenses are present in the LICENSES/ directory

Type of Change

  • Bug fix
  • New feature
  • Breaking change
  • Documentation update
  • Dependency update
  • Build system/CI

@synackd
synackd added this pull request to stack #180 October 5, 2026 22:27
@synackd synackd added this to the v0.11.0 milestone Oct 5, 2026
@synackd
synackd force-pushed the stack/coverage/08-output-and-error-injection branch from 1f99598 to 53cb61a Compare October 5, 2026 22:43
@synackd
synackd marked this pull request as ready for review October 6, 2026 16:50
Route command output through the CLI's injected output stream
(cli.Ios.Out()) instead of process-global stdout, across service output,
configuration display, version information, cloud-init data, and command
prompts, so callers and tests can observe output without replacing
operating-system globals; the command test harness no longer swaps
os.Stdout. dialWebSocket returns the error from parsing a malformed
console URI instead of ignoring it and dereferencing a nil URL, and
ConnectConsole returns a failure to restore the terminal after an
interactive session instead of discarding it.

Turn on errcheck's check-blank, so errors discarded with `_ =` are
reported, and exempt the command flag getters (Flags().GetBool etc.):
every flag read back this way is registered with the matching type on
the same command, so their errors can't occur. Tests that discarded
errors now check them.

Add discover-static and BSS error-arm tests in companion
_errors_test.go files, and tests for pkg/discover, pkg/format, and
pkg/xname.

Signed-off-by: Devon Bautista <17506592+synackd@users.noreply.github.com>
@synackd
synackd force-pushed the stack/coverage/08-output-and-error-injection branch from 53cb61a to 4b83e26 Compare October 8, 2026 04:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant