From 121bfb88435a07e39db519bc067bea829120d4b9 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 4 Aug 2026 05:28:36 +0000 Subject: [PATCH 1/3] feat: Add --page-cursor support to every paginated command 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 Claude-Session: https://claude.ai/code/session_01QQvXm83ki7Mn9dXGhHrK7i --- README.md | 17 ++++++++ src/lib/command-spec.test.ts | 55 +++++++++++++++++++++++++- src/lib/command-spec.ts | 4 +- src/lib/completion/completion.test.ts | 2 +- src/lib/get-request-parameters.ts | 39 ++++++++++++++++++ src/lib/interact-for-command-params.ts | 3 +- src/lib/render-help.ts | 4 ++ src/lib/util/cli-args.test.ts | 16 ++++++++ src/lib/util/cli-args.ts | 5 ++- test/cli.test.ts | 21 ++++++++++ test/fixtures/blueprint.ts | 2 + 11 files changed, 162 insertions(+), 6 deletions(-) create mode 100644 src/lib/get-request-parameters.ts diff --git a/README.md b/README.md index 64520119..8f689395 100644 --- a/README.md +++ b/README.md @@ -113,6 +113,23 @@ seam devices list > devices.json seam devices list | jq '.devices[].device_id' ``` +### Pagination + +Every command that paginates accepts `--page-cursor` to select a page of +results, alongside `--limit` for the size of that page. Each response reports +its `pagination`, whose `next_page_cursor` is the cursor for the page after it. + +```bash +# The first page, and the cursor for the next one +seam devices list --limit 2 | jq '.pagination.next_page_cursor' + +# The page after it +seam devices list --limit 2 --page-cursor "$CURSOR" +``` + +A cursor is opaque: pass it back exactly as it was reported, and do not build +one yourself. Run `seam --help` to see whether a command paginates. + ### JSON Request params may be piped or redirected in as a JSON object. Params given as diff --git a/src/lib/command-spec.test.ts b/src/lib/command-spec.test.ts index 2a8e0af1..56c07263 100644 --- a/src/lib/command-spec.test.ts +++ b/src/lib/command-spec.test.ts @@ -37,7 +37,58 @@ test('command spec: includes commands handled by the CLI itself', () => { test('command spec: turns parameters into kebab-case flags', () => { expect( findCommand(spec, ['devices', 'list'])?.flags.map(({ long }) => long), - ).toEqual(['device-type', 'is-managed', 'limit']) + ).toEqual(['device-type', 'is-managed', 'limit', 'page-cursor']) +}) + +test('command spec: offers --page-cursor on every paginated command', () => { + const pageCursor = findCommand(spec, ['devices', 'list'])?.flags.find( + ({ long }) => long === 'page-cursor', + ) + expect(pageCursor).toMatchObject({ + isRequired: false, + takesValue: true, + values: [], + }) + expect(pageCursor?.description).toContain('next_page_cursor') + + // An endpoint that does not paginate has no page to ask for. + expect( + findCommand(spec, ['devices', 'unmanaged', 'get'])?.flags.map( + ({ long }) => long, + ), + ).not.toContain('page-cursor') +}) + +test('command spec: keeps the documented --page-cursor when there is one', () => { + const documented = getCommandSpec({ + routes: [ + { + endpoints: [ + { + path: '/devices/list', + title: 'List Devices', + description: '', + hasPagination: true, + request: { + parameters: [ + { + name: 'page_cursor', + description: 'The cursor as the definitions describe it.', + format: 'string', + isRequired: false, + }, + ], + }, + }, + ], + }, + ], + } as unknown as Parameters[0]) + + const flags = findCommand(documented, ['devices', 'list'])?.flags ?? [] + expect(flags.filter(({ long }) => long === 'page-cursor')).toMatchObject([ + { description: 'The cursor as the definitions describe it.' }, + ]) }) test('command spec: carries whether a flag is required', () => { @@ -48,7 +99,7 @@ test('command spec: carries whether a flag is required', () => { findCommand(spec, ['devices', 'list'])?.flags.map( ({ isRequired }) => isRequired, ), - ).toEqual([false, false, false]) + ).toEqual([false, false, false, false]) }) test('command spec: collects values for enum and boolean flags', () => { diff --git a/src/lib/command-spec.ts b/src/lib/command-spec.ts index aa0f386d..e18fc096 100644 --- a/src/lib/command-spec.ts +++ b/src/lib/command-spec.ts @@ -1,5 +1,7 @@ import type { Blueprint } from '@seamapi/blueprint' +import { getRequestParameters } from './get-request-parameters.js' + type Endpoint = Blueprint['routes'][number]['endpoints'][number] type Parameter = Endpoint['request']['parameters'][number] @@ -266,7 +268,7 @@ const toCommandDefinition = (endpoint: Endpoint): CommandDefinition => { ? firstSentence(description) : toPlainText(endpoint.title), description, - flags: [...endpoint.request.parameters] + flags: getRequestParameters(endpoint) .map(toCommandFlag) .filter((flag) => flag.long == null || isSafeToken(flag.long)) .sort((a, b) => compare(a.long ?? a.short, b.long ?? b.short)), diff --git a/src/lib/completion/completion.test.ts b/src/lib/completion/completion.test.ts index f35cd478..9f172167 100644 --- a/src/lib/completion/completion.test.ts +++ b/src/lib/completion/completion.test.ts @@ -32,7 +32,7 @@ test('bash completion: dispatches on the command path', () => { expect(script).toContain('complete -F _seam_completion seam') expect(script).toContain("'devices') echo 'list unmanaged' ;;") expect(script).toContain( - "'devices list') echo '--device-type --is-managed --limit' ;;", + "'devices list') echo '--device-type --is-managed --limit --page-cursor' ;;", ) expect(script).toContain( "'devices list --device-type') echo 'august_lock schlage_lock' ;;", diff --git a/src/lib/get-request-parameters.ts b/src/lib/get-request-parameters.ts new file mode 100644 index 00000000..affcf8e2 --- /dev/null +++ b/src/lib/get-request-parameters.ts @@ -0,0 +1,39 @@ +import type { Endpoint, Parameter } from '@seamapi/blueprint' + +/** The parameter that selects a page of results on a paginated endpoint. */ +export const pageCursorParameterName = 'page_cursor' + +const pageCursorParameter: Parameter = { + name: pageCursorParameterName, + description: + "Identifies the specific page of results to return, obtained from the previous page's next_page_cursor.", + format: 'string', + jsonType: 'string', + isRequired: false, + isDeprecated: false, + deprecationMessage: '', + isUndocumented: false, + undocumentedMessage: '', + isDraft: false, + draftMessage: '', + hasDefault: false, +} + +/** + * The request parameters for an endpoint, including `page_cursor` for every + * endpoint that paginates. + * + * Every paginated endpoint takes a cursor, yet the API definitions do not + * always document one, so add it wherever it is missing: without it there is + * no way to ask for any page but the first. + */ +export const getRequestParameters = (endpoint: Endpoint): Parameter[] => { + const { parameters } = endpoint.request + + if (!endpoint.hasPagination) return parameters + if (parameters.some(({ name }) => name === pageCursorParameterName)) { + return parameters + } + + return [...parameters, pageCursorParameter] +} diff --git a/src/lib/interact-for-command-params.ts b/src/lib/interact-for-command-params.ts index 4923496e..5d089516 100644 --- a/src/lib/interact-for-command-params.ts +++ b/src/lib/interact-for-command-params.ts @@ -1,4 +1,5 @@ import { getCommandBlueprintDef } from './get-command-blueprint-def.js' +import { getRequestParameters } from './get-request-parameters.js' import { interactForBlueprintObject } from './interact-for-blueprint-object.js' import type { ContextHelpers } from './types.js' @@ -15,7 +16,7 @@ export const interactForCommandParams = async ( { command: args.command, params: args.params, - parameters: endpoint.request.parameters, + parameters: getRequestParameters(endpoint), }, ctx, ) diff --git a/src/lib/render-help.ts b/src/lib/render-help.ts index a7656037..5a54b01c 100644 --- a/src/lib/render-help.ts +++ b/src/lib/render-help.ts @@ -66,6 +66,10 @@ const examples = [ name: "seam access-codes create {bold --code} '1234' {bold --name} 'My Code'", summary: 'Create an access code.', }, + { + name: 'seam devices list {bold --page-cursor} $NEXT_PAGE_CURSOR', + summary: 'List the next page of devices.', + }, { name: 'seam devices list > devices.json', summary: 'Write the response to a file as JSON.', diff --git a/src/lib/util/cli-args.test.ts b/src/lib/util/cli-args.test.ts index cb91d3fa..052cea1d 100644 --- a/src/lib/util/cli-args.test.ts +++ b/src/lib/util/cli-args.test.ts @@ -51,6 +51,22 @@ test('parseCliArgs: interactivity flags do not consume the next argument', () => expect(args._).toEqual(['devices', 'get']) }) +test('parseCliArgs: reads a page cursor exactly as given', () => { + // Cursors are opaque, so anything that looks like a number must survive. + expect( + parse(['devices', 'list', '--page-cursor', '0755'])['page_cursor'], + ).toBe('0755') + expect( + parse(['devices', 'list', '--page-cursor', '1e5'])['page_cursor'], + ).toBe('1e5') + expect( + parse(['devices', 'list', '--page-cursor=eyJrIjoxfQ=='])['page_cursor'], + ).toBe('eyJrIjoxfQ==') + expect( + parse(['devices', 'list', '--page_cursor', '0755'])['page_cursor'], + ).toBe('0755') +}) + test('toArgName: renders a parameter as its argument', () => { expect(toArgName('device_id')).toBe('--device-id') expect(toArgName('code')).toBe('--code') diff --git a/src/lib/util/cli-args.ts b/src/lib/util/cli-args.ts index 2005da9c..f7456059 100644 --- a/src/lib/util/cli-args.ts +++ b/src/lib/util/cli-args.ts @@ -46,7 +46,10 @@ export class NonInteractiveError extends Error { export const parseCliArgs = (argv: string[]): ParsedArgs => parseArgs(argv, { - string: ['code'], + // A page cursor is opaque, so keep it exactly as given: read as a number + // it would lose leading zeroes and turn exponent notation into a digit + // string, naming a page the API never issued. + string: ['code', 'page-cursor', 'page_cursor'], boolean: ['non-interactive', 'interactive', 'json'], // Deliberately not aliased to -n, which is reserved for a future // --dry-run flag. diff --git a/test/cli.test.ts b/test/cli.test.ts index 923e174c..d6738c01 100644 --- a/test/cli.test.ts +++ b/test/cli.test.ts @@ -152,6 +152,27 @@ test('cli: params given as flags win over params piped in', async () => { expect(requests[0]?.body).toEqual({ limit: 5 }) }) +test('cli: sends a page cursor as the opaque string it is', async () => { + requests = [] + const { exitCode } = await runCli([ + 'devices', + 'list', + '--page-cursor', + '0755', + ]) + + expect(exitCode).toBe(0) + expect(requests[0]?.path).toBe('/devices/list') + expect(requests[0]?.body).toEqual({ page_cursor: '0755' }) +}) + +test('cli: documents --page-cursor for a paginated command', async () => { + const { stdout, exitCode } = await runCli(['devices', 'list', '--help']) + + expect(exitCode).toBe(0) + expect(stdout).toContain('--page-cursor') +}) + test('cli: does not send cli flags as request params', async () => { requests = [] await runCli(['devices', 'list', '--json', '-y']) diff --git a/test/fixtures/blueprint.ts b/test/fixtures/blueprint.ts index 802c6095..46f3e064 100644 --- a/test/fixtures/blueprint.ts +++ b/test/fixtures/blueprint.ts @@ -13,6 +13,8 @@ export const testBlueprint = { title: 'List Devices', description: 'Returns a list of all [devices](https://docs.seam.co). Results are paginated.', + // Paginated, yet the definitions document no page_cursor parameter. + hasPagination: true, request: { parameters: [ { From dea400d654f4aa6ce061c165f273126cd4cd8100 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 4 Aug 2026 05:42:09 +0000 Subject: [PATCH 2/3] fix: Report an unknown argument instead of sending it 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 Claude-Session: https://claude.ai/code/session_01QQvXm83ki7Mn9dXGhHrK7i --- README.md | 5 ++- src/bin/cli.ts | 65 +++++++++++++++++++++++++++++------ src/lib/util/cli-args.test.ts | 20 ++++++++++- src/lib/util/cli-args.ts | 29 ++++++++++++++++ test/cli.test.ts | 61 ++++++++++++++++++++++++++++++++ 5 files changed, 167 insertions(+), 13 deletions(-) diff --git a/README.md b/README.md index 8f689395..e147cf17 100644 --- a/README.md +++ b/README.md @@ -85,7 +85,10 @@ seam devices list --non-interactive # Fails with: Missing required parameter for /locks/unlock_door: --device-id seam locks unlock-door --non-interactive -MY_DOOR=$(seam devices get --name "Front Door" --id-only) +# Fails with: Unknown parameter for /devices/list: --limitt +seam devices list --limitt 5 + +MY_DOOR=$(seam devices get --name "Front Door" | jq -r '.device.device_id') # Unlock a lock seam locks unlock-door --device-id $MY_DOOR diff --git a/src/bin/cli.ts b/src/bin/cli.ts index 5fd2395b..de715ebd 100644 --- a/src/bin/cli.ts +++ b/src/bin/cli.ts @@ -13,6 +13,8 @@ import { } from 'lib/completion/index.js' import { getConfigStore } from 'lib/config/index.js' import { getApiBlueprint } from 'lib/get-api-blueprint.js' +import { getCommandBlueprintDef } from 'lib/get-command-blueprint-def.js' +import { getRequestParameters } from 'lib/get-request-parameters.js' import { getResponseKey } from 'lib/get-response-key.js' import { getServer } from 'lib/get-server.js' import { interactForActionAttemptPoll } from 'lib/interact-for-action-attempt-poll.js' @@ -33,6 +35,9 @@ import { type Interactivity, NonInteractiveError, parseCliArgs, + toGivenArgName, + toParameterName, + UsageError, } from 'lib/util/cli-args.js' import { canPrompt, prompt } from 'lib/util/prompt.js' import { readStdinJson } from 'lib/util/read-stdin-json.js' @@ -122,8 +127,17 @@ async function cli(args: ParsedArgs) { } args._ = args._.map(toCommandWord) - for (const k in args) { - args[k.toLowerCase().replace(/-/g, '_')] = args[k] + + // Argument keys name parameters however they are written, so normalize each + // one to the name the API gives it. Replace the key rather than adding the + // normalized form alongside it, or an argument would be sent twice: once as + // written and once as the API names it. + for (const key of Object.keys(args)) { + if (key === '_') continue + const name = toParameterName(key) + if (name === key) continue + args[name] = args[key] + delete args[key] } const use_remote_api_defs = @@ -144,16 +158,17 @@ async function cli(args: ParsedArgs) { const isNonInteractive = ctx.interactivity === 'non-interactive' - for (const k in args) { - if (k === '_') continue - const v = args[k] - delete args[k] - const key = k.replace(/-/g, '_') - args[key] = v + // Params given as arguments, kept apart from the params read from stdin so + // that only the arguments are held to what the command accepts. + const argParams: Record = {} + for (const [key, value] of Object.entries(args)) { + if (key === '_') continue if (cliFlags.includes(key)) continue - commandParams[key] = v + argParams[key] = value } + Object.assign(commandParams, argParams) + const selectedCommand = await interactForCommandSelection(args._, ctx) if (isEqual(selectedCommand, ['login'])) { if (args['server']) { @@ -245,6 +260,30 @@ async function cli(args: ParsedArgs) { }) } + const apiPath = `/${selectedCommand.join('/').replace(/-/g, '_')}` + + // An argument the command does not accept is a mistake, not a param: report + // it rather than sending it, since the request would either fail somewhere + // less obvious or quietly ignore what was asked for. + const accepted = new Set( + getRequestParameters(getCommandBlueprintDef(selectedCommand, ctx)).map( + ({ name }) => name, + ), + ) + const unknownArgs = Object.keys(argParams).filter((key) => !accepted.has(key)) + if (unknownArgs.length > 0) { + throw new UsageError( + `Unknown ${ + unknownArgs.length === 1 ? 'parameter' : 'parameters' + } for ${apiPath}: ${unknownArgs.map(toGivenArgName).join(' ')}`, + { + hint: `Run 'seam ${selectedCommand.join( + ' ', + )} --help' to see the parameters it accepts.`, + }, + ) + } + const params = await interactForCommandParams( { command: selectedCommand, params: commandParams }, ctx, @@ -259,8 +298,6 @@ async function cli(args: ParsedArgs) { }) } - const apiPath = `/${selectedCommand.join('/').replace(/-/g, '_')}` - if (apiPath.includes('/events/list') && params.between) { delete params.since } @@ -337,6 +374,12 @@ run(process.argv.slice(2)).catch((e: unknown) => { const output = getOutput() process.exitCode = 1 + if (e instanceof UsageError) { + output.error(chalk.red(e.message)) + if (e.hint !== '') output.error(e.hint) + return + } + if (e instanceof NonInteractiveError) { output.error(chalk.red(e.message)) return diff --git a/src/lib/util/cli-args.test.ts b/src/lib/util/cli-args.test.ts index 052cea1d..8c6ca1a5 100644 --- a/src/lib/util/cli-args.test.ts +++ b/src/lib/util/cli-args.test.ts @@ -1,7 +1,13 @@ import type { ParsedArgs } from 'minimist' import { expect, test } from 'vitest' -import { getInteractivity, parseCliArgs, toArgName } from './cli-args.js' +import { + getInteractivity, + parseCliArgs, + toArgName, + toGivenArgName, + toParameterName, +} from './cli-args.js' // The CLI normalizes argument keys before checking them. const parse = (argv: string[]): ParsedArgs => { @@ -72,6 +78,18 @@ test('toArgName: renders a parameter as its argument', () => { expect(toArgName('code')).toBe('--code') }) +test('toParameterName: reads an argument key as the parameter it names', () => { + expect(toParameterName('page-cursor')).toBe('page_cursor') + expect(toParameterName('page_cursor')).toBe('page_cursor') + expect(toParameterName('PAGE-CURSOR')).toBe('page_cursor') + expect(toParameterName('limit')).toBe('limit') +}) + +test('toGivenArgName: renders a one letter key as a short argument', () => { + expect(toGivenArgName('n')).toBe('-n') + expect(toGivenArgName('page_cursor')).toBe('--page-cursor') +}) + test('getInteractivity: never prompts without a terminal', () => { expect( getInteractivity(parse(['devices', 'list']), { canPrompt: false }), diff --git a/src/lib/util/cli-args.ts b/src/lib/util/cli-args.ts index f7456059..b5b54e47 100644 --- a/src/lib/util/cli-args.ts +++ b/src/lib/util/cli-args.ts @@ -44,6 +44,21 @@ export class NonInteractiveError extends Error { override name = 'NonInteractiveError' } +/** + * Thrown when the arguments do not name something the CLI can run. + */ +export class UsageError extends Error { + override name = 'UsageError' + + /** What to run instead, reported after the message. */ + readonly hint: string + + constructor(message: string, { hint = '' }: { hint?: string } = {}) { + super(message) + this.hint = hint + } +} + export const parseCliArgs = (argv: string[]): ParsedArgs => parseArgs(argv, { // A page cursor is opaque, so keep it exactly as given: read as a number @@ -93,3 +108,17 @@ export const getInteractivity = ( */ export const toArgName = (parameterName: string): string => `--${parameterName.replace(/_/g, '-')}` + +/** + * Read an argument key as the parameter it names, e.g., `--page-cursor`, + * `--page_cursor`, and `--PAGE-CURSOR` all name `page_cursor`. + */ +export const toParameterName = (argKey: string): string => + argKey.toLowerCase().replace(/-/g, '_') + +/** + * Render an argument key as the argument it was given as, naming a one letter + * key as the short form it can only have been written as, e.g., `-n`. + */ +export const toGivenArgName = (argKey: string): string => + argKey.length === 1 ? `-${argKey}` : toArgName(argKey) diff --git a/test/cli.test.ts b/test/cli.test.ts index d6738c01..a34497ce 100644 --- a/test/cli.test.ts +++ b/test/cli.test.ts @@ -173,6 +173,67 @@ test('cli: documents --page-cursor for a paginated command', async () => { expect(stdout).toContain('--page-cursor') }) +test('cli: reports an unknown argument rather than sending it', async () => { + requests = [] + const { stdout, stderr, exitCode } = await runCli([ + 'devices', + 'list', + '--limitt', + '5', + ]) + + expect(exitCode).toBe(1) + expect(stdout).toBe('') + expect(stderr).toContain('Unknown parameter for /devices/list: --limitt') + expect(stderr).toContain("Run 'seam devices list --help'") + expect(requests).toHaveLength(0) +}) + +test('cli: names every unknown argument at once', async () => { + requests = [] + const { stderr, exitCode } = await runCli([ + 'devices', + 'list', + '--limitt', + '5', + '--pagecursor', + 'abc', + ]) + + expect(exitCode).toBe(1) + expect(stderr).toContain( + 'Unknown parameters for /devices/list: --limitt --pagecursor', + ) + expect(requests).toHaveLength(0) +}) + +test('cli: reports an unknown short argument as the short form', async () => { + requests = [] + const { stderr, exitCode } = await runCli(['devices', 'list', '-n']) + + expect(exitCode).toBe(1) + expect(stderr).toContain('Unknown parameter for /devices/list: -n') + expect(requests).toHaveLength(0) +}) + +test('cli: sends an argument once, however it is written', async () => { + requests = [] + const { exitCode } = await runCli(['devices', 'list', '--LIMIT', '5']) + + expect(exitCode).toBe(0) + expect(requests[0]?.body).toEqual({ limit: 5 }) +}) + +test('cli: does not hold params read from stdin to the command', async () => { + requests = [] + const { exitCode } = await runCli(['devices', 'list'], { + input: JSON.stringify({ limit: 2, nope: true }), + }) + + expect(exitCode).toBe(0) + expect(requests[0]?.body).toEqual({ limit: 2, nope: true }) +}) + test('cli: does not send cli flags as request params', async () => { requests = [] await runCli(['devices', 'list', '--json', '-y']) From 2c4148dd8720242b12d2fd7c3ba338310d7e6c29 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 4 Aug 2026 16:44:15 +0000 Subject: [PATCH 3/3] fix: Report an unknown argument to any command, not just an endpoint 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 Claude-Session: https://claude.ai/code/session_01QQvXm83ki7Mn9dXGhHrK7i --- README.md | 4 + src/bin/cli.ts | 151 ++++++++++++++++--------- src/lib/command-spec.test.ts | 55 +-------- src/lib/command-spec.ts | 16 ++- src/lib/completion/completion.test.ts | 2 +- src/lib/get-request-parameters.ts | 39 ------- src/lib/interact-for-command-params.ts | 3 +- test/cli.test.ts | 28 +++++ test/fixtures/blueprint.ts | 2 - 9 files changed, 144 insertions(+), 156 deletions(-) delete mode 100644 src/lib/get-request-parameters.ts diff --git a/README.md b/README.md index e147cf17..4b1dd72d 100644 --- a/README.md +++ b/README.md @@ -138,6 +138,10 @@ one yourself. Run `seam --help` to see whether a command paginates. Request params may be piped or redirected in as a JSON object. Params given as arguments win over params read from stdin. +An argument the command does not accept is an error, so a typo is reported +rather than sent. Params read from stdin are passed through as given, so +anything the API itself accepts may be sent that way. + ```bash # Read params from a file seam locks unlock-door < params.json diff --git a/src/bin/cli.ts b/src/bin/cli.ts index de715ebd..1bcb78da 100644 --- a/src/bin/cli.ts +++ b/src/bin/cli.ts @@ -5,7 +5,7 @@ import { isDeepStrictEqual as isEqual } from 'node:util' import chalk from 'chalk' import type { ParsedArgs } from 'minimist' -import { getCommandSpec } from 'lib/command-spec.js' +import { findLocalCommand, getCommandSpec } from 'lib/command-spec.js' import { completionShells, isCompletionShell, @@ -14,7 +14,6 @@ import { import { getConfigStore } from 'lib/config/index.js' import { getApiBlueprint } from 'lib/get-api-blueprint.js' import { getCommandBlueprintDef } from 'lib/get-command-blueprint-def.js' -import { getRequestParameters } from 'lib/get-request-parameters.js' import { getResponseKey } from 'lib/get-response-key.js' import { getServer } from 'lib/get-server.js' import { interactForActionAttemptPoll } from 'lib/interact-for-action-attempt-poll.js' @@ -82,6 +81,29 @@ async function cli(args: ParsedArgs) { return } + args._ = args._.map(toCommandWord) + + // Argument keys name parameters however they are written, so normalize each + // one to the name the API gives it. Replace the key rather than adding the + // normalized form alongside it, or an argument would be sent twice: once as + // written and once as the API names it. + for (const key of Object.keys(args)) { + if (key === '_') continue + const name = toParameterName(key) + if (name === key) continue + args[name] = args[key] + delete args[key] + } + + // Params given as arguments, kept apart from the params read from stdin so + // that only the arguments are held to what the command accepts. + const argParams: Record = {} + for (const [key, value] of Object.entries(args)) { + if (key === '_') continue + if (cliFlags.includes(key)) continue + argParams[key] = value + } + if (args._[0] === 'completion') { const shell = args._[1] @@ -91,6 +113,8 @@ async function cli(args: ParsedArgs) { return } + assertKnownArgs(argParams, ['completion', shell]) + // Completions always come from the cached API definitions so that they // can be generated without logging in. They may lag the definitions // served by Seam when config use-remote-api-defs is enabled. @@ -126,20 +150,6 @@ async function cli(args: ParsedArgs) { return } - args._ = args._.map(toCommandWord) - - // Argument keys name parameters however they are written, so normalize each - // one to the name the API gives it. Replace the key rather than adding the - // normalized form alongside it, or an argument would be sent twice: once as - // written and once as the API names it. - for (const key of Object.keys(args)) { - if (key === '_') continue - const name = toParameterName(key) - if (name === key) continue - args[name] = args[key] - delete args[key] - } - const use_remote_api_defs = args['remote_api_defs'] ?? config.get('use_remote_api_defs') @@ -158,18 +168,22 @@ async function cli(args: ParsedArgs) { const isNonInteractive = ctx.interactivity === 'non-interactive' - // Params given as arguments, kept apart from the params read from stdin so - // that only the arguments are held to what the command accepts. - const argParams: Record = {} - for (const [key, value] of Object.entries(args)) { - if (key === '_') continue - if (cliFlags.includes(key)) continue - argParams[key] = value - } - Object.assign(commandParams, argParams) const selectedCommand = await interactForCommandSelection(args._, ctx) + + // Hit 'back' on a top-level command path, so we start again + if (selectedCommand.slice(-1)[0] === '[Back]') { + return await cli({ + ...args, + _: [], + }) + } + + // Check the arguments before the command acts on any of them, so a mistake + // is reported rather than half applied. + assertKnownArgs(argParams, selectedCommand, ctx) + if (isEqual(selectedCommand, ['login'])) { if (args['server']) { config.set('server', args['server']) @@ -251,39 +265,8 @@ async function cli(args: ParsedArgs) { commandParams['accepted_providers'].split(',') } - // Hit 'back' on a top-level command path, so we start again - const lastCommandPath = selectedCommand.slice(-1)[0] - if (lastCommandPath === '[Back]') { - return await cli({ - ...args, - _: [], - }) - } - const apiPath = `/${selectedCommand.join('/').replace(/-/g, '_')}` - // An argument the command does not accept is a mistake, not a param: report - // it rather than sending it, since the request would either fail somewhere - // less obvious or quietly ignore what was asked for. - const accepted = new Set( - getRequestParameters(getCommandBlueprintDef(selectedCommand, ctx)).map( - ({ name }) => name, - ), - ) - const unknownArgs = Object.keys(argParams).filter((key) => !accepted.has(key)) - if (unknownArgs.length > 0) { - throw new UsageError( - `Unknown ${ - unknownArgs.length === 1 ? 'parameter' : 'parameters' - } for ${apiPath}: ${unknownArgs.map(toGivenArgName).join(' ')}`, - { - hint: `Run 'seam ${selectedCommand.join( - ' ', - )} --help' to see the parameters it accepts.`, - }, - ) - } - const params = await interactForCommandParams( { command: selectedCommand, params: commandParams }, ctx, @@ -323,6 +306,62 @@ async function cli(args: ParsedArgs) { const toCommandWord = (arg: string): string => arg.toLowerCase().replace(/_/g, '-') +/** + * Report any argument the command does not accept, rather than acting on it. + * An unrecognized argument is a mistake: forwarded to the API it would fail + * somewhere less obvious or be quietly ignored, and on a command the CLI + * handles itself it would go nowhere at all. + * + * Only arguments are checked. Params read from stdin are passed through as + * given, so a caller may send whatever the API itself accepts. + * + * `ctx` is only needed to look up an endpoint's parameters, so commands the + * CLI declares itself can be checked before any blueprint is loaded. + */ +const assertKnownArgs = ( + argParams: Record, + command: string[], + ctx?: ContextHelpers, +): void => { + const local = findLocalCommand(command) + + let accepted: Set + if (local != null) { + accepted = new Set( + local.flags.flatMap(({ long }) => + long == null ? [] : [toParameterName(long)], + ), + ) + } else if (ctx != null) { + accepted = new Set( + getCommandBlueprintDef(command, ctx).request.parameters.map( + ({ name }) => name, + ), + ) + } else { + throw new Error(`No definition for command seam ${command.join(' ')}`) + } + + const unknown = Object.keys(argParams).filter((key) => !accepted.has(key)) + if (unknown.length === 0) return + + // Name an endpoint command by its path, as missing params are named, and a + // command the CLI handles itself by the words that run it. + const target = + local == null + ? `/${command.join('/').replace(/-/g, '_')}` + : command.join(' ') + + throw new UsageError( + `Unknown ${ + unknown.length === 1 ? 'parameter' : 'parameters' + } for ${target}: ${unknown.map(toGivenArgName).join(' ')}`, + { + hint: `Run 'seam ${command.join(' ')} --help' to see what it accepts.`, + }, + ) +} + const handleConnectWebviewResponse = async ( connect_webview: any, interactivity: Interactivity, diff --git a/src/lib/command-spec.test.ts b/src/lib/command-spec.test.ts index 56c07263..2a8e0af1 100644 --- a/src/lib/command-spec.test.ts +++ b/src/lib/command-spec.test.ts @@ -37,58 +37,7 @@ test('command spec: includes commands handled by the CLI itself', () => { test('command spec: turns parameters into kebab-case flags', () => { expect( findCommand(spec, ['devices', 'list'])?.flags.map(({ long }) => long), - ).toEqual(['device-type', 'is-managed', 'limit', 'page-cursor']) -}) - -test('command spec: offers --page-cursor on every paginated command', () => { - const pageCursor = findCommand(spec, ['devices', 'list'])?.flags.find( - ({ long }) => long === 'page-cursor', - ) - expect(pageCursor).toMatchObject({ - isRequired: false, - takesValue: true, - values: [], - }) - expect(pageCursor?.description).toContain('next_page_cursor') - - // An endpoint that does not paginate has no page to ask for. - expect( - findCommand(spec, ['devices', 'unmanaged', 'get'])?.flags.map( - ({ long }) => long, - ), - ).not.toContain('page-cursor') -}) - -test('command spec: keeps the documented --page-cursor when there is one', () => { - const documented = getCommandSpec({ - routes: [ - { - endpoints: [ - { - path: '/devices/list', - title: 'List Devices', - description: '', - hasPagination: true, - request: { - parameters: [ - { - name: 'page_cursor', - description: 'The cursor as the definitions describe it.', - format: 'string', - isRequired: false, - }, - ], - }, - }, - ], - }, - ], - } as unknown as Parameters[0]) - - const flags = findCommand(documented, ['devices', 'list'])?.flags ?? [] - expect(flags.filter(({ long }) => long === 'page-cursor')).toMatchObject([ - { description: 'The cursor as the definitions describe it.' }, - ]) + ).toEqual(['device-type', 'is-managed', 'limit']) }) test('command spec: carries whether a flag is required', () => { @@ -99,7 +48,7 @@ test('command spec: carries whether a flag is required', () => { findCommand(spec, ['devices', 'list'])?.flags.map( ({ isRequired }) => isRequired, ), - ).toEqual([false, false, false, false]) + ).toEqual([false, false, false]) }) test('command spec: collects values for enum and boolean flags', () => { diff --git a/src/lib/command-spec.ts b/src/lib/command-spec.ts index e18fc096..f0ef0487 100644 --- a/src/lib/command-spec.ts +++ b/src/lib/command-spec.ts @@ -1,7 +1,5 @@ import type { Blueprint } from '@seamapi/blueprint' -import { getRequestParameters } from './get-request-parameters.js' - type Endpoint = Blueprint['routes'][number]['endpoints'][number] type Parameter = Endpoint['request']['parameters'][number] @@ -153,6 +151,18 @@ export const findGroup = ( ): CommandGroup | undefined => spec.groups.find((group) => isSamePath(group.path, path)) +/** + * The definition of a command the CLI handles itself, or `undefined` when the + * path is an endpoint or no command at all. + * + * Unlike {@link findCommand} this needs no blueprint, since these commands are + * declared by the CLI rather than derived from the API definitions. + */ +export const findLocalCommand = ( + path: string[], +): CommandDefinition | undefined => + localCommands.find((command) => isSamePath(command.path, path)) + const isSamePath = (a: string[], b: string[]): boolean => a.length === b.length && a.every((word, index) => word === b[index]) @@ -268,7 +278,7 @@ const toCommandDefinition = (endpoint: Endpoint): CommandDefinition => { ? firstSentence(description) : toPlainText(endpoint.title), description, - flags: getRequestParameters(endpoint) + flags: [...endpoint.request.parameters] .map(toCommandFlag) .filter((flag) => flag.long == null || isSafeToken(flag.long)) .sort((a, b) => compare(a.long ?? a.short, b.long ?? b.short)), diff --git a/src/lib/completion/completion.test.ts b/src/lib/completion/completion.test.ts index 9f172167..f35cd478 100644 --- a/src/lib/completion/completion.test.ts +++ b/src/lib/completion/completion.test.ts @@ -32,7 +32,7 @@ test('bash completion: dispatches on the command path', () => { expect(script).toContain('complete -F _seam_completion seam') expect(script).toContain("'devices') echo 'list unmanaged' ;;") expect(script).toContain( - "'devices list') echo '--device-type --is-managed --limit --page-cursor' ;;", + "'devices list') echo '--device-type --is-managed --limit' ;;", ) expect(script).toContain( "'devices list --device-type') echo 'august_lock schlage_lock' ;;", diff --git a/src/lib/get-request-parameters.ts b/src/lib/get-request-parameters.ts deleted file mode 100644 index affcf8e2..00000000 --- a/src/lib/get-request-parameters.ts +++ /dev/null @@ -1,39 +0,0 @@ -import type { Endpoint, Parameter } from '@seamapi/blueprint' - -/** The parameter that selects a page of results on a paginated endpoint. */ -export const pageCursorParameterName = 'page_cursor' - -const pageCursorParameter: Parameter = { - name: pageCursorParameterName, - description: - "Identifies the specific page of results to return, obtained from the previous page's next_page_cursor.", - format: 'string', - jsonType: 'string', - isRequired: false, - isDeprecated: false, - deprecationMessage: '', - isUndocumented: false, - undocumentedMessage: '', - isDraft: false, - draftMessage: '', - hasDefault: false, -} - -/** - * The request parameters for an endpoint, including `page_cursor` for every - * endpoint that paginates. - * - * Every paginated endpoint takes a cursor, yet the API definitions do not - * always document one, so add it wherever it is missing: without it there is - * no way to ask for any page but the first. - */ -export const getRequestParameters = (endpoint: Endpoint): Parameter[] => { - const { parameters } = endpoint.request - - if (!endpoint.hasPagination) return parameters - if (parameters.some(({ name }) => name === pageCursorParameterName)) { - return parameters - } - - return [...parameters, pageCursorParameter] -} diff --git a/src/lib/interact-for-command-params.ts b/src/lib/interact-for-command-params.ts index 5d089516..4923496e 100644 --- a/src/lib/interact-for-command-params.ts +++ b/src/lib/interact-for-command-params.ts @@ -1,5 +1,4 @@ import { getCommandBlueprintDef } from './get-command-blueprint-def.js' -import { getRequestParameters } from './get-request-parameters.js' import { interactForBlueprintObject } from './interact-for-blueprint-object.js' import type { ContextHelpers } from './types.js' @@ -16,7 +15,7 @@ export const interactForCommandParams = async ( { command: args.command, params: args.params, - parameters: getRequestParameters(endpoint), + parameters: endpoint.request.parameters, }, ctx, ) diff --git a/test/cli.test.ts b/test/cli.test.ts index a34497ce..b785675e 100644 --- a/test/cli.test.ts +++ b/test/cli.test.ts @@ -189,6 +189,34 @@ test('cli: reports an unknown argument rather than sending it', async () => { expect(requests).toHaveLength(0) }) +test('cli: reports an unknown argument to a command it handles itself', async () => { + const { stdout, stderr, exitCode } = await runCli([ + 'select', + 'server', + '--serverr', + 'https://example.com', + ]) + + expect(exitCode).toBe(1) + expect(stdout).toBe('') + expect(stderr).toContain('Unknown parameter for select server: --serverr') + expect(stderr).toContain("Run 'seam select server --help'") +}) + +test('cli: reports an unknown argument to a command taking none', async () => { + const { stderr, exitCode } = await runCli(['completion', 'bash', '--shell']) + + expect(exitCode).toBe(1) + expect(stderr).toContain('Unknown parameter for completion bash: --shell') +}) + +test('cli: takes the arguments a command it handles itself accepts', async () => { + const { stdout, exitCode } = await runCli(['completion', 'bash']) + + expect(exitCode).toBe(0) + expect(stdout).toContain('complete -F _seam_completion seam') +}) + test('cli: names every unknown argument at once', async () => { requests = [] const { stderr, exitCode } = await runCli([ diff --git a/test/fixtures/blueprint.ts b/test/fixtures/blueprint.ts index 46f3e064..802c6095 100644 --- a/test/fixtures/blueprint.ts +++ b/test/fixtures/blueprint.ts @@ -13,8 +13,6 @@ export const testBlueprint = { title: 'List Devices', description: 'Returns a list of all [devices](https://docs.seam.co). Results are paginated.', - // Paginated, yet the definitions document no page_cursor parameter. - hasPagination: true, request: { parameters: [ {