diff --git a/README.md b/README.md index a4f0f7b8..cd9831a2 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 @@ -113,11 +116,32 @@ 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 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 8c2cf4a0..62a3e300 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, @@ -13,6 +13,7 @@ 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 { 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 +34,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' @@ -77,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] @@ -86,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. @@ -121,11 +150,6 @@ async function cli(args: ParsedArgs) { return } - args._ = args._.map(toCommandWord) - for (const k in args) { - args[k.toLowerCase().replace(/-/g, '_')] = args[k] - } - const useRemoteApiDefs = args['remote_api_defs'] ?? config.get('use_remote_api_defs') @@ -144,17 +168,22 @@ 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 - if (cliFlags.includes(key)) continue - commandParams[key] = v - } + 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']) @@ -236,14 +265,7 @@ 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, '_')}` const params = await interactForCommandParams( { command: selectedCommand, params: commandParams }, @@ -259,8 +281,6 @@ async function cli(args: ParsedArgs) { }) } - const apiPath = `/${selectedCommand.join('/').replace(/-/g, '_')}` - if (apiPath.includes('/events/list') && params.between) { delete params.since } @@ -286,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 ( connectWebview: any, interactivity: Interactivity, @@ -337,6 +413,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/command-spec.ts b/src/lib/command-spec.ts index aa0f386d..f0ef0487 100644 --- a/src/lib/command-spec.ts +++ b/src/lib/command-spec.ts @@ -151,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]) 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..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 => { @@ -51,11 +57,39 @@ 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') }) +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 2005da9c..b5b54e47 100644 --- a/src/lib/util/cli-args.ts +++ b/src/lib/util/cli-args.ts @@ -44,9 +44,27 @@ 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, { - 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. @@ -90,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 923e174c..b785675e 100644 --- a/test/cli.test.ts +++ b/test/cli.test.ts @@ -152,6 +152,116 @@ 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: 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: 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([ + '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'])