fix(cli): report invalid flag values as usage errors - #106
Merged
Conversation
Four flags accepted values they could not honour, and said so late or not at all. --header was parsed only while building a client, so a reserved or malformed value came back as a runtime error with a "run with --debug" hint, and commands that build no client accepted it silently. It is now checked before the command runs, like --jq and -o. --start-time was sent to the server verbatim, so `--start-time yesterday` became a server-side rejection rather than a usage error naming RFC 3339. `list-users --type ""` satisfied MarkFlagRequired, and `--type 'org#'` built a filter with an empty relation; both went out as degenerate filters and came back as a bare 400. --limit and --max-results write the same variable, so passing both silently let whichever came last win. They now conflict explicitly.
10 tasks
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
1 participant
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.
📝 Summary
Four flags accepted values they couldn't honour, and reported the problem late, badly, or not at all. All four now fail as usage errors (exit 2) before any request goes out.
--headerwas only parsed while constructing a client, so a reserved or malformed value surfaced as a runtime error (exit 1) with arun with -d/--debug for more detailhint — and commands that build no client accepted garbage silently. It's a bad invocation, so it's now validated inPersistentPreRunEalongside--jqand-o.tuples changes --start-timepromised RFC 3339 in its help but was passed to the server verbatim, so--start-time yesterdaybecame a server-side rejection instead of a local usage error. Compare--consistency, which has always been checked locally against its enumerated values.query list-users --type—MarkFlagRequiredis satisfied by--type "", and--type 'org#'parsed into a filter with an empty relation. Both reached the server as degenerate filters and came back as a bare 400. (Validation also moved ahead of client construction, matching the sibling commands.)--limit/--max-resultsare two flags bound to one variable acrossstores list,model list,tuples readandtuples changes, so passing both silently let whichever appeared last on the command line win. They now conflict explicitly.🏷 Type of change
Each rejects an invocation that previously "worked" by silently doing something other than what was asked.
✅ How to verify
New tests:
TestBadHeaderIsAUsageError(drives the real binary throughofga version, which builds no client, so it only passes if the header is checked up front),TestListUsersRejectsDegenerateTypeFilters,TestRejectFlagAliasConflict, andTestParseTimestamp.📋 Checklist
--help/ examples) if behavior changed — help text already documented the expected formats; this enforces themmake checkpasses locally