feat: Reject unknown arguments and keep page cursors opaque

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 <command> --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

src/bin/cli.ts

@@ -5,14 +5,15 @@ 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,
   renderCompletion,
 } 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<string, any> = {}
+  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<string, any>,
+  command: string[],
+  ctx?: ContextHelpers,
+): void => {
+  const local = findLocalCommand(command)
+
+  let accepted: Set<string>
+  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

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])
 

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.',

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 }),

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)