improvement(cli): recovery guidance on every parse error and body-shape refusal

packages/sim-cli/src/embed-isolation.test.ts

@@ -117,4 +117,69 @@ describe('embedded CLI output ownership', () => {
     expect(noOptions.stderr).toContain('sim workspaces get takes no options.')
     expect(noOptions.stderr).not.toContain('Options for')
   })
+
+  it('answers a value typed where a flag belongs with the command line that takes it', async () => {
+    const transport = async () => {
+      throw new Error('a parse error must not reach the API')
+    }
+    const [renamed, kbAsPositional, queryAsPositional, siblingFlag, handWritten] =
+      await Promise.all([
+        runEmbeddedCli(['files', 'rename', 'wf_1', 'report.pdf'], { ...identity, transport }),
+        runEmbeddedCli(['knowledge', 'search', 'kb_1', '--query', 'leave'], {
+          ...identity,
+          transport,
+        }),
+        runEmbeddedCli(['knowledge', 'search', 'leave policy', '--kb', 'kb_1'], {
+          ...identity,
+          transport,
+        }),
+        runEmbeddedCli(['tables', 'rows', 'list', 'tbl_1', '--filter', '{}'], {
+          ...identity,
+          transport,
+        }),
+        runEmbeddedCli(['files', 'upload', 'a.png', '--path', 'files/a.png'], {
+          ...identity,
+          transport,
+        }),
+      ])
+    expect(renamed.exitCode).toBe(1)
+    expect(renamed.stderr).toContain('"report.pdf" was passed as a positional; pass it as --name')
+    expect(renamed.stderr).toContain('Example: sim files rename <fileId> --name <value>')
+    expect(kbAsPositional.stderr).toContain('"kb_1" was passed as a positional; pass it as --kb')
+    expect(queryAsPositional.exitCode).toBe(1)
+    expect(queryAsPositional.stderr).toContain('Example: sim knowledge search --kb <value...>')
+    expect(queryAsPositional.stderr).toContain('Options for sim knowledge search:')
+    expect(siblingFlag.stderr).toContain('--filter is an option of:\n')
+    expect(siblingFlag.stderr).toContain(
+      '  sim tables rows query — Query rows with a filter and sort'
+    )
+    expect(handWritten.exitCode).toBe(1)
+    expect(handWritten.stderr).toContain('Options for sim files upload:')
+  })
+
+  it('names the shape a body variant received and the flag that takes it', async () => {
+    const transport = async () => {
+      throw new Error('a shape error must not reach the API')
+    }
+    const [wrapped, oneRow, rowList] = await Promise.all([
+      runEmbeddedCli(['tables', 'rows', 'create', 'tbl_1', '--rows', '{"rows":[{"a":1}]}'], {
+        ...identity,
+        transport,
+      }),
+      runEmbeddedCli(['tables', 'rows', 'create', 'tbl_1', '--rows', '{"a":1}'], {
+        ...identity,
+        transport,
+      }),
+      runEmbeddedCli(['tables', 'rows', 'create', 'tbl_1', '--data', '[{"a":1}]'], {
+        ...identity,
+        transport,
+      }),
+    ])
+    expect(wrapped.exitCode).toBe(1)
+    expect(wrapped.stderr).toContain(`e.g. --rows '[{"column":"value"}]'; got a JSON object`)
+    expect(wrapped.stderr).toContain('Pass the array under its "rows" key itself')
+    expect(oneRow.stderr).toContain('For that shape use --data.')
+    expect(rowList.stderr).toContain('--data must be a JSON object')
+    expect(rowList.stderr).toContain('For that shape use --rows.')
+  })
 })

packages/sim-cli/src/program.ts

@@ -9,6 +9,7 @@ import { attachSecretCommands } from './commands/secrets'
 import { telemetryCommand } from './commands/telemetry'
 import { OUTPUT_FORMATS } from './config/index'
 import {
+  addParseErrorGuidanceToTree,
   assertNoReservedProgramFlags,
   buildGeneratedCommands,
   refuseHelpAfterUnknownCommand,
@@ -211,6 +212,7 @@ export function buildProgram(
   )
 
   refuseHelpAfterUnknownCommand(program)
+  addParseErrorGuidanceToTree(program)
   assertNoReservedProgramFlags(program)
 
   return program

packages/sim-cli/src/runtime/build.ts

@@ -1,4 +1,4 @@
-import { Command } from 'commander'
+import { Command, type Option } from 'commander'
 import { CLI_CONTRACT } from '../contract/commands'
 import type { CommandSpec, CommandVariantSpec } from '../contract/types'
 import { V2_OPERATIONS, type V2OperationName } from '../generated/v2-api'
@@ -93,46 +93,133 @@ function looksLikeAnId(token: string): boolean {
   )
 }
 
+const MISSING_OPTION_ERROR = /^error: required option '(.+?)' not specified/
+const EXCESS_ARGUMENTS_ERROR = /^error: too many arguments/
+
+interface CommandInternals {
+  _actionHandler?: unknown
+  _hidden?: boolean
+}
+
+function longFlagSyntax(option: Option): string {
+  return option.flags.replace(/^-[A-Za-z], /, '')
+}
+
+/** The command line with its positionals and every required flag, as `--help` would read it. */
+function usageExample(command: Command): string {
+  const parts = [commandPath(command), argumentSyntax(command)]
+  for (const option of command.options) if (option.mandatory) parts.push(longFlagSyntax(option))
+  return parts.filter(Boolean).join(' ')
+}
+
+function optionList(command: Command): string {
+  const flags = command.options.filter((option) => !option.hidden).map((option) => option.flags)
+  return flags.length > 0
+    ? `Options for ${commandPath(command)}: ${flags.join(', ')}`
+    : `${commandPath(command)} takes no options.`
+}
+
+/** Operands past the declared positionals: the values a caller typed where a flag was wanted. */
+function extraOperands(command: Command): string[] {
+  const declared = command.registeredArguments.length
+  if (command.registeredArguments.some((argument) => argument.variadic)) return []
+  return command.args.filter((arg) => !arg.startsWith('-')).slice(declared)
+}
+
 /**
- * Appends a worked example to the parse errors a positional argument causes,
- * and the command's own flags to an unknown option.
+ * The visible sibling commands that do declare a flag this one does not, each
+ * with its description, so a read never picks a sibling that deletes.
+ */
+function siblingsDeclaring(command: Command, flag: string): string[] {
+  const siblings = command.parent?.commands ?? []
+  return siblings
+    .filter(
+      (sibling) =>
+        sibling !== command &&
+        !(sibling as Command & CommandInternals)._hidden &&
+        sibling.options.some(
+          (option) => !option.hidden && (option.long === flag || option.short === flag)
+        )
+    )
+    .map((sibling) => `  ${commandPath(sibling)} — ${sibling.description().split('\n', 1)[0]}`)
+}
+
+/**
+ * Appends recovery guidance to commander's parse errors, so the retry needs no
+ * `--help` round.
  *
- * Covers the argument being absent and the argument being swallowed as an
- * option because its id opens with a dash; the second needs the `--` escape,
- * which commander never mentions. A guessed flag gets the real list: commander
- * suggests only a near spelling, so `--row-ids` for `--row` got a bare error
- * and a caller had to look the command up before retrying.
+ * - A missing positional or required flag gets the full command line, and a
+ *   value typed as a positional where the flag was wanted (`files rename
+ *   <id> new.pdf`, `knowledge search <kbId>`) is named against that flag.
+ * - Too many positionals get the same command line and the flag list.
+ * - An unknown option gets the command's own flags, and the sibling commands
+ *   that do take it: `tables rows list --filter` belongs to `tables rows
+ *   query`, which commander's near-spelling suggestion never reaches.
+ * - An id swallowed as an option because it opens with a dash gets the `--`
+ *   escape, which commander never mentions.
  */
-function addParseErrorGuidance(command: Command): Command {
-  const outputError = command.configureOutput().outputError
-  if (!outputError) throw new Error('Commander output formatter is not configured')
-
-  command.configureOutput({
-    outputError: (message, write) => {
-      outputError(message, write)
-
-      if (message.startsWith('error: missing required argument ')) {
-        const syntax = argumentSyntax(command)
-        const example = syntax ? `${commandPath(command)} ${syntax}` : commandPath(command)
-        write(`Example: ${example}\n`)
-        return
-      }
+function parseErrorGuidance(command: Command, message: string): string[] {
+  if (message.startsWith('error: missing required argument ')) {
+    return [`Example: ${usageExample(command)}`]
+  }
 
-      const token = UNKNOWN_OPTION_TOKEN.exec(message)?.[1]
-      if (!token) return
-      if (looksLikeAnId(token) && command.registeredArguments.length > 0) {
-        write(`Example: ${commandPath(command)} -- ${token}\n`)
-        return
-      }
-      const flags = command.options.filter((option) => !option.hidden).map((option) => option.flags)
-      write(
-        flags.length > 0
-          ? `Options for ${commandPath(command)}: ${flags.join(', ')}\n`
-          : `${commandPath(command)} takes no options.\n`
-      )
-    },
-  })
-  return command
+  const missingOption = MISSING_OPTION_ERROR.exec(message)?.[1]
+  if (missingOption) {
+    const extra = extraOperands(command)
+    const flag = missingOption.split(' ', 1)[0]
+    const moved =
+      extra.length === 1
+        ? [`"${extra[0]}" was passed as a positional; pass it as ${flag} "${extra[0]}".`]
+        : []
+    return [...moved, `Example: ${usageExample(command)}`]
+  }
+
+  if (EXCESS_ARGUMENTS_ERROR.test(message)) {
+    return [`Example: ${usageExample(command)}`, optionList(command)]
+  }
+
+  const token = UNKNOWN_OPTION_TOKEN.exec(message)?.[1]
+  if (!token) return []
+  if (looksLikeAnId(token) && command.registeredArguments.length > 0) {
+    return [`Example: ${commandPath(command)} -- ${token}`]
+  }
+  const flag = token.split('=', 1)[0]
+  const owners = siblingsDeclaring(command, flag)
+  return [
+    optionList(command),
+    ...(owners.length > 0 ? [`${flag} is an option of:`, ...owners] : []),
+  ]
+}
+
+/**
+ * Wraps the command's own `error`, which every commander parse failure on it
+ * goes through. The output configuration cannot carry this: commands made with
+ * `.command()` share their parent's configuration object, so a formatter set on
+ * one would answer for all of them.
+ */
+function addParseErrorGuidance(command: Command): void {
+  const error = command.error.bind(command)
+  command.error = (message, options) => {
+    const guidance = parseErrorGuidance(command, message)
+    return error(guidance.length > 0 ? `${message}\n${guidance.join('\n')}` : message, options)
+  }
+}
+
+/**
+ * Attaches {@link addParseErrorGuidance} to every command that parses its own
+ * arguments, generated or attached by hand (`files upload`, `secrets set`), so
+ * a hand-written command does not answer a guessed flag with a bare error.
+ * Pure dispatchers below the root are left to commander's unknown-command handling.
+ */
+export function addParseErrorGuidanceToTree(program: Command): void {
+  const walk = (command: Command): void => {
+    const parsesOwnArguments =
+      command.commands.length === 0 ||
+      Boolean((command as Command & CommandInternals)._actionHandler)
+    if (parsesOwnArguments || !command.parent) addParseErrorGuidance(command)
+    for (const child of command.commands) walk(child)
+  }
+  walk(program)
 }
 
 /**
@@ -374,7 +461,7 @@ function configureOperation(
 }
 
 function buildLeaf(operation: V2OperationName, spec: CommandSpec, leafName: string): Command {
-  return addParseErrorGuidance(configureOperation(new Command(leafName), operation, spec))
+  return configureOperation(new Command(leafName), operation, spec)
 }
 
 /**

packages/sim-cli/src/runtime/request.ts

@@ -1,13 +1,47 @@
 import { closeSync, existsSync, fstatSync, openSync, readSync } from 'node:fs'
 import { CLI_CONTRACT } from '../contract/commands'
-import type { CommandSpec, FlagSpec } from '../contract/types'
+import type { BodyVariantSpec, CommandSpec, FlagSpec } from '../contract/types'
 import { embedStore } from '../embed-context'
 import { V2_OPERATIONS, type V2OperationName } from '../generated/v2-api'
 import { type QueryValue, SimApiError } from '../http/client'
 import { embeddedFileContent } from '../transfer/local-file'
 import { camel, kebab } from './derive'
 import type { OperationSpec } from './types'
 
+function jsonKind(value: unknown): string {
+  if (Array.isArray(value)) return 'a JSON array'
+  if (value === null) return 'null'
+  return typeof value === 'object' ? 'a JSON object' : `a JSON ${typeof value}`
+}
+
+/**
+ * The refusal for a body variant given the wrong JSON shape, stating what came
+ * in and the spelling that takes it. `--rows '{"rows":[...]}'` and `--rows`
+ * with a single row were the recurring misses; neither value is reshaped,
+ * because guessing which part the caller meant is the server's write to make.
+ */
+function bodyVariantShapeError(
+  variant: BodyVariantSpec,
+  parsed: unknown,
+  variants: readonly BodyVariantSpec[]
+): string {
+  const flag = `--${variant.name}`
+  const example = variant.kind === 'array' ? `'[{"column":"value"}]'` : `'{"column":"value"}'`
+  const base = `${flag} must be a JSON ${variant.kind}, e.g. ${flag} ${example}; got ${jsonKind(parsed)}.`
+  if (variant.kind === 'array' && parsed && typeof parsed === 'object' && !Array.isArray(parsed)) {
+    if (Array.isArray((parsed as Record<string, unknown>)[variant.property])) {
+      return `${base} Pass the array under its "${variant.property}" key itself, not the object wrapping it.`
+    }
+  }
+  const other = variants.find(
+    (candidate) =>
+      candidate !== variant && candidate.kind === (Array.isArray(parsed) ? 'array' : 'object')
+  )
+  return other && parsed && typeof parsed === 'object'
+    ? `${base} For that shape use --${other.name}.`
+    : base
+}
+
 /** One request field, as the generator describes it. */
 export interface FieldSpec {
   kind: 'string' | 'number' | 'integer' | 'boolean' | 'enum' | 'array' | 'object' | 'unknown'
@@ -803,7 +837,7 @@ export async function buildRequest(
           (!parsed || typeof parsed !== 'object' || Array.isArray(parsed))) ||
         (variant.kind === 'array' && !Array.isArray(parsed))
       ) {
-        throw new SimApiError(`--${variant.name} must be a JSON ${variant.kind}`, 0)
+        throw new SimApiError(bodyVariantShapeError(variant, parsed, commandSpec.bodyVariants), 0)
       }
       return boundedRequest({
         params,

scripts/check-unused-exports.baseline.json

@@ -5908,7 +5908,6 @@
   "packages/sim-cli/src/config/index.ts#normalizeEndpoint",
   "packages/sim-cli/src/config/index.ts#telemetryStatePath",
   "packages/sim-cli/src/config/paths.ts#configDir",
-  "packages/sim-cli/src/contract/types.ts#BodyVariantSpec",
   "packages/sim-cli/src/contract/types.ts#PathFlagSpec",
   "packages/sim-cli/src/http/client.ts#OAuthRefresher",
   "packages/sim-cli/src/output/terminal-text.ts#artPad",