Replace viper with an explicit environment lookup

[korya]

Problem

Viper contributes 35 of the module's 38 non-stdlib packages to read six environment variables.

It is used for nothing but AutomaticEnv — no config files, no remote providers, no watching — and for that the module graph carries fsnotify, afero, mapstructure, pelletier/go-toml, locafero, sourcegraph/conc, gotenv, cast and yaml. It also produced a split-brain: six options were read through viper and thirteen straight off cobra, so HTTP_ASSERT_INSECURE worked while HTTP_ASSERT_REQUEST silently did nothing, with no rule a reader could infer.

Solution

Replace viper with one 25-line function over the flag set, and name the six environment-backed options in a single list.

	Before	After
Non-stdlib packages	38	3 (cobra, pflag, mousetrap)
Total packages	235	193
Binary	9.93 MB	9.01 MB
Coverage	99.3%	100.0%

Four commits, ordered so the risky part is provable

#	Commit	Behaviour change	Test edits
1	Pin value-level environment semantics	none	+32 subtests
2	Replace viper with an explicit environment lookup	none	zero ← the proof
3	Reject unparseable environment values	yes, deliberate	edits the cases from #1 that pinned silence
4	Delete the unused logWarn and logError helpers	none	comment only

Commit 2 touches no test file. All 233 subtests pass unedited, including the 32 added in commit 1 specifically to cover environment parsing. That is the evidence the swap changed nothing — not an assertion in a commit message.

Commit 1 exists because the existing matrix asks "did this option take effect?" — a boolean — so it could not see how a value was parsed, only that it was. Empty variables, unparseable values, non-canonical booleans, precedence on five of six options, and log levels other than error had no coverage at all, and those are precisely the paths a viper replacement turns on.

Why the environment list stays at six

Preserving --maphost's existing behaviour requires splitting one variable into several values on whitespace (viper's semantics, verified against the library). Applied uniformly — as issue #54 originally proposed — that corrupts every option whose values contain spaces:

'X-Foo: bar baz'              -> strings.Fields -> ['X-Foo:', 'bar', 'baz']
'Cache-Control: max-age=3600' -> strings.Fields -> ['Cache-Control:', 'max-age=3600']

Header values routinely contain spaces; host mappings never do. Uniformity would trade one inconsistency for a silent value-corrupting footgun across ten flags. The inconsistency is instead fixed where it actually hurt — it is now a named list in the source and a table in the README, rather than an accident of which flags happened to get a BindPFlag line. #54 has been updated to record this rather than leaving the issue describing something we deliberately did not build.

The one deliberate behaviour change

A typo in HTTP_ASSERT_MAX_TIME used to disable the request timeout. The value was cast to the type's zero value, and a zero http.Client.Timeout means no timeout at all — so a monitoring check could hang indefinitely, in a tool whose entire purpose is enforcing a deadline. Likewise HTTP_ASSERT_VERBOSE=yes quietly turned verbosity off, because yes is not a Go boolean.

Before:

$ HTTP_ASSERT_MAX_TIME=abc http-assert --assert-ok https://api.example.com
[+] PASSED          # timeout silently removed
$ echo $?
0

After:

$ HTTP_ASSERT_MAX_TIME=abc http-assert --assert-ok https://api.example.com
Error: Invalid value for HTTP_ASSERT_MAX_TIME="abc": strconv.ParseInt: parsing "abc": invalid syntax
$ echo $?
71

Exit code 71 matches how --log-level and --maphost already report bad input. Empty variables are unaffected and still count as unset. This lands in its own commit, and the test diff is its record.

No visual change — this is a headless CLI.

Other Changes

• go.mod is now enforced tidy. Nothing checked that it matched the imports, which is exactly how viper survived as a requirement long after it stopped earning its place. A tidy-check recipe runs go mod tidy -diff — it reports what it would change and exits non-zero without modifying either file — wired into pre-commit rather than added as a separate CI step, so local runs and CI check the same thing through the same command. Verified in both directions: an unused require makes just pre-commit exit 1.

• Coverage reaches 100.0%. logWarn and logError had no callers and were kept alive with //nolint:unused, silencing a linter that was right. Their four statements were the only ones in the repo no test could reach. The LWarn level stays, since --log-level warn is documented flag surface.

• README documents that an empty variable counts as unset, that unparseable values are rejected, and which boolean spellings are accepted.

• Silences the TLS handshake warnings the test server logged whenever a case deliberately connected without --insecure — the expected result of those cases, not a failure.

Related:

• Closes Delete viper: 35 of 38 packages to read six environment variables #54

🤖 Generated with Claude Code