Reject unknown arguments and keep page cursors opaque - #598
Draft
razor-x wants to merge 3 commits into
Draft
Conversation
Paginated commands take a page cursor to select a page of results, but the CLI only offered one where the API definitions happened to document the parameter, and minimist read the value as a number. Derive the flag from the endpoint's own pagination instead, so every paginated command offers --page-cursor in its help, its completions, and its interactive prompt. Read the value as a string so an opaque cursor survives verbatim rather than losing a leading zero or being rewritten from exponent notation. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01QQvXm83ki7Mn9dXGhHrK7i
An argument the command does not accept was forwarded to the API as a param, so a typo either failed somewhere less obvious or was quietly ignored, and the request went out either way. Hold the arguments to what the endpoint accepts and name every one it does not, pointing at the command's own help. Params read from stdin are left as they are: only the arguments are checked. Normalizing an argument key now replaces it rather than adding the normalized form alongside it, which had sent --LIMIT as both LIMIT and limit. Drop the README's --id-only from seam devices get, which no version of the CLI has ever implemented. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01QQvXm83ki7Mn9dXGhHrK7i
Extend the check to the commands the CLI handles itself, which took arguments they did not accept and silently did nothing with them, e.g. seam logout --force or a misspelled seam login --toekn. Their flags are already declared in the command spec, so look them up there. Arguments are now read and collected before the command runs, so completion can be checked too, and hitting 'back' is handled before the check rather than after it. Drop get-request-parameters. Every endpoint the definitions mark as paginated already documents page_cursor, so synthesizing one only added a description that can drift from upstream, keyed on a flag that is false for endpoints that do paginate. If the definitions ever omit a cursor that belongs in @seamapi/types, not in a patch here. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01QQvXm83ki7Mn9dXGhHrK7i
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.
Summary
Two fixes to how the CLI reads arguments, found while adding
--page-cursorsupport.--page-cursorturned out to already be a normal request parameter on every paginated endpoint, so no flag needed adding. What was broken was the value:minimistread it as a number, and unrecognized flags were forwarded to the API instead of being reported.A page cursor is read as an opaque string
parseCliArgsonly forcedcodeto a string, so a cursor that looks numeric was silently corrupted:--page-cursor 0755755"0755"--page-cursor 1e5100000"1e5"--page-cursor 1234512345"12345"A cursor is opaque, so this named a page the API never issued, and failed invisibly.
page-cursorandpage_cursornow joincodein minimist'sstringlist.An unknown argument is an error
Anything not recognized as a CLI flag was previously added to the request params and sent, so a typo either failed somewhere less obvious or was quietly ignored — and the request went out either way.
The check runs after the command path is resolved and before the command acts on anything, so nothing is sent and nothing is half applied. It covers both kinds of command:
seam logout --force,seam completion bash --shell, and a misspelledseam login --toeknare now reported instead of ignored.Params read from stdin are deliberately not checked — they are passed through as given, so anything the API itself accepts can still be sent that way:
Multiple unknowns are named at once, and a one-letter key is reported as the short form it must have been written as (
-n, not--n).--helpis handled before the check, so it always works.Along the way
--LIMIT 5sent bothLIMIT: 5andlimit: 5. It now replaces the key, which is also what lets the new check see one argument instead of two.--id-onlyfrom the README.seam devices get --name "Front Door" --id-onlyonly ever appeared to work becauseid_onlywas forwarded and ignored; no version of the CLI implements it. Replaced with| jq -r '.device.device_id', matching the piping idiom used elsewhere in the README.pagination.next_page_cursor, plus a help example.Notes for review
omitUndocumented: true, so a real-but-undocumented param is now rejected as unknown. The stdin passthrough is the escape hatch. Say the word if it should consult the unfiltered definitions instead.page_cursorbut are not markedhasPagination, because that flag is derived from the response schema declaringpagination:/locks/list,/noise_sensors/list,/thermostats/list. They are device-list aliases with the sameresponseKeyandresourceTypeas/devices/list, which is marked paginated. That looks like an upstream inconsistency in@seamapi/typesand is not addressed here.Testing
116 tests pass;
typecheck,lint,prettier, andbuildare clean. Beyond the unit and end-to-end tests added here, the argument handling was smoke-tested against a fake server using the real API definitions to confirm the check does not over-reject:--page-cursorondevices listandacs users list,events list --between(with thesincedefault still dropped),--accepted-providers august,schlagesplitting to an array,devices get --name, dotted--custom-metadata.foo barkeeping its nested object,--json/--no-json, and every command the CLI handles itself.🤖 Generated with Claude Code
https://claude.ai/code/session_01QQvXm83ki7Mn9dXGhHrK7i