Extract run() int and push os.Exit to the edge

[korya]

Rearchitecture step 2 of 6. Sequence: #54 → 2 → #56 → #48 → #18 → #41.

Problem

die() calls os.Exit from the middle of library-shaped code, so every error path in the program terminates the process:

func die(rc int, format string, args ...interface{}) {
	fmt.Fprintf(os.Stderr, "\nError: "+format, args...)
	os.Exit(rc)
}

It is reached from mustParseLogLevel, mustParseHostMappings, and three sites inside the cobra Run closure. There is no seam between "decide what went wrong" and "terminate" — which means no error path can be exercised without spawning a subprocess. None of them are.

This is the architectural reason coverage sits at 39%, and it is upstream of several filed bugs: there was no cheap place to put validation, so validation didn't get written.

Suggested fix

The standard Go shape — a testable core, with process termination pushed to the edge:

Before:

func main() {
	cmd := &cobra.Command{ /* … */ }   // die() → os.Exit() from anywhere inside
	if err := cmd.ExecuteContext(context.Background()); err != nil {
		die(103, "%s", err)
	}
}

After:

func main() {
	os.Exit(run(os.Args[1:], os.Stdout, os.Stderr))
}

func run(args []string, stdout, stderr io.Writer) int {
	// … returns an exit code; never calls os.Exit
}

die becomes an error value carrying a code, not a call that ends the world:

type exitError struct {
	code int
	msg  string
}

Why this is step 2

It unblocks nearly everything else:

• Test coverage is 39% with no end-to-end CLI tests #48 (end-to-end tests) becomes possible in-process — call run() with args and capture the writers, no subprocess, no build step in the test path.
• Zero assertions is reported as "Cannot perform request" with exit 93 #25 (zero assertions reported as a request failure with exit 93) is a two-line fix once exit codes are values rather than side effects.
• --max-time accepts 0 and negative values without validation #31 (--max-time accepts 0 and negatives) is currently unwritten because validation had nowhere to go.
• All output goes to stderr; stdout is always empty #34 (everything goes to stderr; stdout always empty) requires injectable writers, which this provides.
• Delete viper: 35 of 38 packages to read six environment variables #54 (delete viper) should land first, since removing the global viper singleton removes hidden state from run()'s inputs.

Scope

This is a mechanical restructuring with no behaviour change intended — same exit codes, same messages, same stream for each. Any behaviour change that falls out should be filed separately rather than folded in, so the diff stays reviewable.

Related

• Test coverage is 39% with no end-to-end CLI tests #48, Zero assertions is reported as "Cannot perform request" with exit 93 #25, --max-time accepts 0 and negative values without validation #31, All output goes to stderr; stdout is always empty #34 — all unblocked or made cheap by this
• Delete viper: 35 of 38 packages to read six environment variables #54 — should land before this

[korya]

Closing as wontfix — maintainer decision, and the evidence has moved in its favour since this was filed.

The case for it has largely dissolved

This issue argued for itself almost entirely on what it unblocked. Most of that got unblocked anyway:

Dependent	Outcome
#48 — end-to-end tests	Closed in #57, without this. The suite drives the real binary as a subprocess.
#54 — delete viper	Closed in #59, without this.
#25, #31, #34	Still open, and none of them actually need a run() int.

The last row is the part worth recording. #25 is a two-line change inside the cobra Run closure. #31 is a validation call in the same shape as the environment-value rejection already added in #59 — dief(71, …) at the point of failure. #34 wants injectable writers, which cobra.Command already provides through SetOut/SetErr, no restructuring required.

Notably, #48 landing as a subprocess suite produced better evidence than the in-process run() this issue proposed. It exercises the real binary's real exit codes, which is the actual contract, rather than a function that returns an int and is trusted to correspond.

The premise was also wrong

This is the architectural reason coverage sits at 39%

Coverage is 100.0%. It got there without touching dief.

What the project accepts by closing this

Being explicit so nobody refiles it on the same reasoning. Error paths are covered only by the opt-in end-to-end suite, so the fast loop sees less:

just test        →  45.5% of statements
merged with e2e  → 100.0%

That gap is the residual cost: every error path is verified, but at 10–23s in a suite go test does not run by default, rather than in milliseconds in-process. That is a known and accepted trade, not an oversight.

dief stays. It is nine lines, it does one thing, and the contract it enforces is pinned by the end-to-end suite regardless of where os.Exit is called from.