fix(cli): an adapter arg named json must not crash every command (#441)

src/command-presentation.test.ts

@@ -1,6 +1,7 @@
 import { describe, expect, it } from 'vitest';
 import { Strategy, type CliCommand } from './registry.js';
 import {
+  commandHelpData,
   commandListPresentation,
   commandListRows,
   filterCommandsByTag,
@@ -167,3 +168,41 @@ describe('shared command presentation', () => {
       .toEqual(['issue-list', 'issues']);
   });
 });
+
+describe('help for a command that shadows a shared flag', () => {
+  const shadowing = toPresentableCommand({
+    site: 'demo',
+    name: 'snapshot',
+    access: 'read',
+    description: 'Snapshot a thread',
+    browser: false,
+    args: [{ name: 'json', type: 'bool', default: false, help: 'Return only the snapshot string' }],
+  });
+
+  it('lists the shadowed flag once, with the adapter’s meaning', () => {
+    const help = formatCommandHelp(shadowing);
+    expect(help).toContain('Return only the snapshot string');
+    // Advertising the alias would name a flag that is no longer registered.
+    expect(help).not.toContain('Alias of --format json');
+  });
+
+  it('still lists the other shared options', () => {
+    const help = formatCommandHelp(shadowing);
+    expect(help).toContain('Common options:');
+    expect(help).toContain('-f, --format <fmt>');
+  });
+
+  it('omits the shadowed flag from structured help too', () => {
+    const data = commandHelpData(shadowing) as { common_options: Array<{ name: string }> };
+    expect(data.common_options.map((option) => option.name)).not.toContain('json');
+    expect(data.common_options.map((option) => option.name)).toContain('format');
+  });
+
+  it('leaves a command that shadows nothing unchanged', () => {
+    const plain = toPresentableCommand({
+      site: 'demo', name: 'search', access: 'read', description: 'Search',
+      browser: false, args: [{ name: 'limit', type: 'int', default: 10 }],
+    });
+    expect(formatCommandHelp(plain)).toContain('Alias of --format json');
+  });
+});

src/command-presentation.ts

@@ -406,31 +406,55 @@ export function siteHelpData(site: string, commands: readonly PresentableCommand
 }
 
 export function commandHelpData(command: PresentableCommand): Record<string, unknown> {
+  const shadowed = shadowedCommonOptions(command);
+  const unshadowed = <T extends { name: string }>(options: readonly T[]): T[] =>
+    options.filter((option) => !shadowed.has(option.name));
   return {
     site: command.site,
     ...compactCommand(command),
-    common_options: COMMON_OPTIONS.map(compactCommonOption),
-    ...(command.browser ? { browser_common_options: BROWSER_COMMON_OPTIONS.map(compactCommonOption) } : {}),
+    common_options: unshadowed(COMMON_OPTIONS).map(compactCommonOption),
+    ...(command.browser ? { browser_common_options: unshadowed(BROWSER_COMMON_OPTIONS).map(compactCommonOption) } : {}),
     output_formats: ['table', 'plain', 'yaml', 'json', 'md', 'csv'],
   };
 }
 
-export function formatCommonOptionsHelp(): string {
-  const rows = COMMON_OPTIONS.map((option) => {
-    const details: string[] = [option.help];
-    if ('default' in option) details.push(`default: ${option.default}`);
-    if ('choices' in option) details.push(`choices: ${option.choices.join(', ')}`);
-    return [option.flags, details.join('  ')] as [string, string];
-  });
+/**
+ * Names of shared options this command shadows with an argument of its own.
+ *
+ * An adapter argument named after a shared flag keeps that flag — webcmd skips
+ * registering its own (see `addSharedOption` in command-surface.ts). Help has
+ * to skip it too, or it advertises a flag that is not registered and lists the
+ * same flag twice with two different meanings.
+ */
+function shadowedCommonOptions(command?: PresentableCommand): Set<string> {
+  if (!command) return new Set();
+  return new Set(commandOptions(command).map((arg) => arg.name));
+}
+
+function formatCommonOptionRows(
+  options: typeof COMMON_OPTIONS | typeof BROWSER_COMMON_OPTIONS,
+  command?: PresentableCommand,
+): Array<[string, string]> {
+  const shadowed = shadowedCommonOptions(command);
+  return options
+    .filter((option) => !shadowed.has(option.name))
+    .map((option) => {
+      const details: string[] = [option.help];
+      if ('default' in option) details.push(`default: ${option.default}`);
+      if ('choices' in option) details.push(`choices: ${option.choices.join(', ')}`);
+      return [option.flags, details.join('  ')] as [string, string];
+    });
+}
+
+export function formatCommonOptionsHelp(command?: PresentableCommand): string {
+  const rows = formatCommonOptionRows(COMMON_OPTIONS, command);
+  if (rows.length === 0) return '';
   return ['Common options:', ...formatRows(rows)].join('\n');
 }
 
-export function formatBrowserCommonOptionsHelp(): string {
-  const rows = BROWSER_COMMON_OPTIONS.map((option) => {
-    const details: string[] = [option.help];
-    if ('choices' in option) details.push(`choices: ${option.choices.join(', ')}`);
-    return [option.flags, details.join('  ')] as [string, string];
-  });
+export function formatBrowserCommonOptionsHelp(command?: PresentableCommand): string {
+  const rows = formatCommonOptionRows(BROWSER_COMMON_OPTIONS, command);
+  if (rows.length === 0) return '';
   return ['Browser common options:', ...formatRows(rows)].join('\n');
 }
 
@@ -473,8 +497,12 @@ export function formatCommandHelp(command: PresentableCommand): string {
   ] as [string, string]);
   if (optionRows.length) lines.push('Command options:', ...formatRows(optionRows), '');
 
-  lines.push(formatCommonOptionsHelp(), '');
-  if (command.browser) lines.push(formatBrowserCommonOptionsHelp(), '');
+  const commonOptionsHelp = formatCommonOptionsHelp(command);
+  if (commonOptionsHelp) lines.push(commonOptionsHelp, '');
+  if (command.browser) {
+    const browserOptionsHelp = formatBrowserCommonOptionsHelp(command);
+    if (browserOptionsHelp) lines.push(browserOptionsHelp, '');
+  }
 
   const meta = [
     `Access: ${command.access}`,

src/command-surface.test.ts

@@ -494,3 +494,65 @@ describe('complete Commander structural grammar and precedence parity', () => {
     expect(captureSharedSurface(argv)).toEqual(captureReferenceSurface(argv));
   });
 });
+
+describe('shared options an adapter shadows', () => {
+  // linkedin/thread-snapshot ships exactly this: an argument named `json`.
+  // Registering webcmd's own `--json` alias on top of it made Commander throw
+  // while the CLI was still being built, so every command crashed at startup
+  // — including the `plugin uninstall` needed to remove the plugin.
+  const shadowing = {
+    command: 'demo/snapshot',
+    browser: true,
+    defaultFormat: 'table',
+    args: [
+      { name: 'thread-url', required: true, type: 'string' },
+      { name: 'json', type: 'boolean', default: false },
+    ],
+  } satisfies CommandSurfaceMetadata;
+
+  it.each(['json', 'format', 'trace', 'verbose', 'window', 'site-session', 'keep-tab'])(
+    'registers a command whose argument is named %s without throwing',
+    (name) => {
+      const metadataFor = {
+        command: `demo/${name}-arg`,
+        browser: true,
+        args: [{ name, type: 'boolean', default: false }],
+      } satisfies CommandSurfaceMetadata;
+      expect(() => configureCommandSurface(new Command('demo'), metadataFor)).not.toThrow();
+    },
+  );
+
+  it('registers every shared option when nothing collides', () => {
+    const command = new Command('demo');
+    configureCommandSurface(command, { command: 'demo/plain', browser: true, args: [] });
+    const flags = command.options.map((option) => option.long);
+    expect(flags).toEqual(expect.arrayContaining([
+      '--format', '--json', '--trace', '--verbose', '--window', '--site-session', '--keep-tab',
+    ]));
+  });
+
+  it('keeps the adapter argument rather than webcmd’s alias when they collide', () => {
+    const command = new Command('demo');
+    configureCommandSurface(command, shadowing);
+    const jsonOptions = command.options.filter((option) => option.long === '--json');
+    expect(jsonOptions).toHaveLength(1);
+    expect(jsonOptions[0]!.description).not.toBe('Alias of --format json');
+  });
+
+  it('does not treat a shadowed --json as a request for JSON output', () => {
+    // Argv preprocessing already resolves this collision in the adapter's
+    // favour, so format resolution has to agree or --json means two things.
+    expect(parseCommandSurface(shadowing, ['--thread-url', 'https://example.com/t/1', '--json']))
+      .toMatchObject({ format: 'table', formatExplicit: false });
+  });
+
+  it('passes a shadowed --json through to the adapter', () => {
+    const parsed = parseCommandSurface(shadowing, ['--thread-url', 'https://example.com/t/1', '--json']);
+    expect(parsed.args.json).toBe(true);
+  });
+
+  it('still honours -f json on a command that shadows --json', () => {
+    expect(parseCommandSurface(shadowing, ['--thread-url', 'https://example.com/t/1', '-f', 'json']))
+      .toMatchObject({ format: 'json', formatExplicit: true });
+  });
+});

src/command-surface.ts

@@ -1,4 +1,4 @@
-import { Command, CommanderError } from 'commander';
+import { Command, CommanderError, Option } from 'commander';
 import { ArgumentError, CliError, EXIT_CODES, type ErrorEnvelope } from './errors.js';
 import type { Arg, CliCommand, CommandArgs } from './registry.js';
 
@@ -225,15 +225,16 @@ export function configureCommandSurface(command: Command, metadata: CommandSurfa
     else command.option(flag, arg.help ?? '');
   }
 
-  addOutputFormatOption(command)
-    .option('--trace <mode>', `Trace capture: ${TRACE_MODES.join(', ')}`, 'off')
-    .option('-v, --verbose', 'Debug output', false);
+  // Every shared option below is guarded: the adapter's own arguments are
+  // registered first, and any of these names may collide with one of them.
+  addOutputFormatOption(command);
+  addSharedOption(command, '--trace <mode>', `Trace capture: ${TRACE_MODES.join(', ')}`, 'off');
+  addSharedOption(command, '-v, --verbose', 'Debug output', false);
 
   if (metadata.browser) {
-    command
-      .option('--window <mode>', `Browser window mode: ${BROWSER_WINDOW_MODES.join(' or ')} (default: background)`)
-      .option('--site-session <mode>', `Adapter site session lifecycle: ${SITE_SESSION_MODES.join(' or ')}`)
-      .option('--keep-tab <bool>', 'Keep the browser tab lease after the command finishes');
+    addSharedOption(command, '--window <mode>', `Browser window mode: ${BROWSER_WINDOW_MODES.join(' or ')} (default: background)`);
+    addSharedOption(command, '--site-session <mode>', `Adapter site session lifecycle: ${SITE_SESSION_MODES.join(' or ')}`);
+    addSharedOption(command, '--keep-tab <bool>', 'Keep the browser tab lease after the command finishes');
   }
 }
 
@@ -441,11 +442,64 @@ export function resolveOutputFormat(raw: string | undefined): OutputFormat | nul
   }
 }
 
+/** Long and short flags already registered on `command`. */
+function registeredFlags(command: Command): Set<string> {
+  const flags = new Set<string>();
+  for (const option of command.options) {
+    if (option.short) flags.add(option.short);
+    if (option.long) flags.add(option.long);
+  }
+  return flags;
+}
+
+/**
+ * Commands where webcmd — not the adapter — owns `--json`.
+ *
+ * An adapter may declare an argument named `json`, in which case the flag
+ * means whatever that adapter says it means and must not be read as
+ * `--format json`. Argv preprocessing already resolves the collision this way
+ * (`knownCommandOptions` lets adapter args overwrite the shared entries), so
+ * format resolution has to agree, or `--json` would silently do two things.
+ */
+const WEBCMD_OWNS_JSON_ALIAS = new WeakSet<Command>();
+
+/**
+ * Add a shared option unless the command already declares one of its flags.
+ *
+ * Commander throws on a duplicate flag, and these options are registered
+ * while the CLI is being built, so one adapter argument named after a shared
+ * flag used to abort startup for *every* command — including the
+ * `plugin uninstall` needed to remove the offending plugin. An adapter that
+ * names a flag keeps it; webcmd drops its own rather than refusing to run.
+ */
+function addSharedOption(
+  command: Command,
+  flags: string,
+  description: string,
+  defaultValue?: unknown,
+): boolean {
+  const option = new Option(flags, description);
+  const taken = registeredFlags(command);
+  if ((option.short && taken.has(option.short)) || (option.long && taken.has(option.long))) return false;
+  if (defaultValue !== undefined) option.default(defaultValue);
+  command.addOption(option);
+  return true;
+}
+
 /** Register `-f/--format` plus the `--json` alias on one command. */
 export function addOutputFormatOption(command: Command, defaultFormat = 'table'): Command {
-  return command
-    .option('-f, --format <fmt>', OUTPUT_FORMAT_HELP, defaultFormat)
-    .option('--json', JSON_FORMAT_ALIAS_HELP, false);
+  const taken = registeredFlags(command);
+  if (!taken.has('--format')) {
+    command.option(
+      taken.has('-f') ? '--format <fmt>' : '-f, --format <fmt>',
+      OUTPUT_FORMAT_HELP,
+      defaultFormat,
+    );
+  }
+  if (addSharedOption(command, '--json', JSON_FORMAT_ALIAS_HELP, false)) {
+    WEBCMD_OWNS_JSON_ALIAS.add(command);
+  }
+  return command;
 }
 
 /**
@@ -465,27 +519,27 @@ export function addOutputFormatOption(command: Command, defaultFormat = 'table')
 export function ensureOutputFormatOptions(command: Command): void {
   for (const child of command.commands) {
     if (child.commands.length === 0 && (child as Command & { _allowUnknownOption?: boolean })._allowUnknownOption !== true) {
-      const flags = new Set<string>();
-      for (const option of child.options) {
-        if (option.short) flags.add(option.short);
-        if (option.long) flags.add(option.long);
-      }
-      if (!flags.has('--format')) {
-        child.option(flags.has('-f') ? '--format <fmt>' : '-f, --format <fmt>', OUTPUT_FORMAT_HELP, 'table');
-      }
-      if (!flags.has('--json')) child.option('--json', JSON_FORMAT_ALIAS_HELP, false);
+      addOutputFormatOption(child);
     }
     ensureOutputFormatOptions(child);
   }
 }
 
+/**
+ * True when `--json` on this command is webcmd's format alias rather than an
+ * adapter argument that happens to be named `json`.
+ */
+function jsonAliasPassed(command: Command): boolean {
+  return WEBCMD_OWNS_JSON_ALIAS.has(command) && command.getOptionValueSource('json') === 'cli';
+}
+
 export function outputFormatIsExplicit(command: Command): boolean {
-  return command.getOptionValueSource('format') === 'cli' || command.getOptionValueSource('json') === 'cli';
+  return command.getOptionValueSource('format') === 'cli' || jsonAliasPassed(command);
 }
 
 /** Resolve `--json` onto `--format json` unless `--format` was also passed. */
 export function requestedOutputFormat(command: Command, format: unknown): unknown {
-  return command.getOptionValueSource('json') === 'cli' && command.getOptionValueSource('format') !== 'cli'
+  return jsonAliasPassed(command) && command.getOptionValueSource('format') !== 'cli'
     ? 'json'
     : format;
 }

src/commanderAdapter.test.ts

@@ -508,3 +508,46 @@ describe('commanderAdapter error envelope output', () => {
     stderrSpy.mockRestore();
   });
 });
+
+describe('registering an adapter that shadows a shared flag', () => {
+  // The reported crash: registration happens while the CLI is being built, so
+  // one plugin command with an argument named `json` aborted startup for every
+  // command — `list`, `doctor`, and even `plugin uninstall`.
+  const shadowing: CliCommand = {
+    site: 'linkedin',
+    name: 'thread-snapshot',
+    access: 'read',
+    description: 'Snapshot a thread',
+    browser: true,
+    args: [
+      { name: 'thread-url', required: true, help: 'Thread URL' },
+      { name: 'json', type: 'bool', default: false, help: 'Return only the snapshot string' },
+    ],
+    func: vi.fn(),
+  };
+
+  it('registers without throwing', () => {
+    const program = new Command();
+    const siteCmd = program.command('linkedin');
+    expect(() => registerCommandToProgram(siteCmd, shadowing)).not.toThrow();
+  });
+
+  it('leaves the adapter owning the flag', () => {
+    const program = new Command();
+    const siteCmd = program.command('linkedin');
+    registerCommandToProgram(siteCmd, shadowing);
+    const registered = siteCmd.commands.find((child) => child.name() === 'thread-snapshot')!;
+    const jsonOptions = registered.options.filter((option) => option.long === '--json');
+    expect(jsonOptions).toHaveLength(1);
+    expect(jsonOptions[0]!.description).toBe('Return only the snapshot string');
+  });
+
+  it('does not disturb sibling commands that shadow nothing', () => {
+    const program = new Command();
+    const siteCmd = program.command('linkedin');
+    registerCommandToProgram(siteCmd, shadowing);
+    registerCommandToProgram(siteCmd, { ...shadowing, name: 'timeline', args: [] });
+    const timeline = siteCmd.commands.find((child) => child.name() === 'timeline')!;
+    expect(timeline.options.map((option) => option.long)).toContain('--json');
+  });
+});