fix(cmd): validate --format before any network call (CONTRACT §4 spirit)#39
Merged
Conversation
Each data-producing subcommand (activity, intraday, measurements, sleep, workouts) was instantiating client.New() and hitting the Withings API before calling validateFormat() on --format. An invalid value like --format obviously-not-a-format would only fail after the HTTP round-trip, so the compat/formats.flagValidationIsHermetic subtest was passing for the wrong reason (network error masking the parse failure under the no-net proxy env). Move validateFormat() to the top of each RunE, before client.New(). Now an unknown --format exits non-zero with an empty stdout and the real "unknown --format" message on stderr, with no auth load and no dial attempt — verified per-subcommand under HTTP_PROXY=127.0.0.1:1. Refs: quantcli/common QUA-30, CONTRACT §4 (output format). Co-Authored-By: Paperclip <noreply@paperclip.ing>
DTTerastar
approved these changes
May 19, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes QUA-30. Follow-up to #36.
Why
PR #36 wired
withings-export-cliintocompat/formats, including theflagValidationIsHermeticsubtest. It passed, but for the wrong reason: each data-producing subcommand'sRunEcalledclient.New()and the Withings HTTP path beforevalidateFormat(). Under the compat test'sHTTP_PROXY=127.0.0.1:1env that HTTP round-trip failed with a dial-timeout, so the binary exited non-zero — making the subtest happy even though it never proved a parse-time failure was network-free.The contract's §4 spirit (and the framework comment on the subtest) is "a flag-validation failure must not have already opened a connection by the time it exits non-zero." Today's withings behavior violates that. This PR closes the gap.
What
In each of
cmd/{activity,intraday,measurements,sleep,workouts}.go, the first statement ofRunEis now:The previous late-binding
validateFormatcall near the codec switch is removed; theformatvariable from the top is reused there. No other logic moves. No new dependencies. No changes to flag declarations, output shapes, or the help string.Acceptance check (per the QUA-30 description)
Built the binary and ran every data-producing subcommand under the same no-network proxy env the compat suite uses:
Every subcommand now exits with:
1Error: unknown --format "obviously-not-a-format" (use markdown, json, or csv)No dial-timeout error, no auth error — the parse fails first and that's all that runs.
Local
go test -tags=compat -run TestContractFormats ./...still green: all 30 subtests (6 contract subtests × 5 subcommands) PASS, including the now-genuinely-hermeticFlagValidationIsHermetic.go vet ./...andgo test ./...clean.Re: the optional WITHINGS_API_BASE drop
The QUA-30 description proposed optionally dropping
WITHINGS_API_BASEfrom the compat-test runner once parse failure was hermetic. Skipping that here — the happy-path subtests (JSONIsArray,CSVHasHeader,DefaultIsMarkdown) run the binary withoutHTTP_PROXYoverride, and they still need the stub origin viaWITHINGS_API_BASEto reach a successful HTTP response. Removing it would break the data-path subtests. The hermetic claim is now enforced at the code level by the validate-first ordering, not by the env shape.🤖 Generated with Claude Code