Skip to content

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

Open
Agnik47 wants to merge 1 commit into
agentrhq:mainfrom
Agnik47:fix/441-adapter-flag-collision-crash
Open

fix(cli): an adapter arg named json must not crash every command (#441)#442
Agnik47 wants to merge 1 commit into
agentrhq:mainfrom
Agnik47:fix/441-adapter-flag-collision-crash

Conversation

@Agnik47

@Agnik47 Agnik47 commented Aug 25, 2026

Copy link
Copy Markdown
Contributor

Fixes #441.

The bug

configureCommandSurface registers an adapter's own arguments first, then
added the shared options unconditionally:

addOutputFormatOption(command)                       // -f/--format, --json
  .option('--trace <mode>', ..., 'off')
  .option('-v, --verbose', 'Debug output', false);

Commander throws on a duplicate flag, and this runs inside createProgram()
— while the CLI is still being built — so one adapter argument named json
aborted startup for every command. The official LinkedIn plugin ships such
an adapter (plugins/linkedin/thread-snapshot.js:162), so installing it
bricked webcmd. Verified on a clean main worktree at 37036a6 (v0.7.6):

command before after
webcmd --version ok (exits before registration) ok
webcmd list crash ok
webcmd doctor crash ok
webcmd plugin list crash ok
webcmd plugin uninstall linkedin crash ok

plugin uninstall crashing is what made it a dead end: the only way out was
deleting ~/.webcmd/plugins/linkedin by hand. The failure was also an
unhandled exception — a raw Node stack trace instead of a structured webcmd
error, and the process still exited 0.

The fix

Guard every shared option the adapter path registers, the way
ensureOutputFormatOptions in the same file already guarded its own. Both
paths now go through one helper, so they cannot drift apart again:

function addSharedOption(command, flags, description, defaultValue) {
  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;
  ...
}

An adapter that names a flag keeps it; webcmd drops its own rather than
refusing to run. The guard covers --format, --json, --trace,
-v/--verbose, and for browser commands --window, --site-session,
--keep-tabthread-snapshot is the only adapter in the repo that collides
today, but any of those names was equally fatal.

Who owns a shadowed --json

Registering nothing is not sufficient on its own. outputFormatIsExplicit and
requestedOutputFormat keyed off getOptionValueSource('json'), which does
not care who registered the option — so --json on thread-snapshot would
have set the adapter's argument and switched the output format.

Argv preprocessing already resolves this collision in the adapter's favour —
knownCommandOptions seeds the shared flags, then lets adapter args overwrite
them — so format resolution now agrees: --json is read as --format json
only on commands where webcmd actually registered the alias. -f json is
untouched and remains the way to ask for JSON output on such a command.

Help

Help follows the same rule, or it advertises a flag that is not registered and
lists the same flag twice with two different meanings. Before, on this branch,
thread-snapshot --help still printed the alias under Common options; now:

Command options:
  --thread-url <value>   Exact LinkedIn messaging thread URL to open and snapshot
  --max-scrolls [value]  Maximum upward scroll attempts to load older messages  default: 30
  --json [value]         Return only JSON snapshot string in the snapshot_json field  default: false

Common options:
  -f, --format <fmt>  Output format: table, plain, json, yaml, md, csv  default: table
  --trace <mode>      Trace capture: off, on, retain-on-failure  default: off
  -v, --verbose       Debug output  default: false

A command that shadows nothing is unchanged and still lists
--json Alias of --format json. The same filtering is applied to the
structured (--help -f yaml) rendering.

Tests

16 new tests, all failing before this change and passing after (verified by
reverting only src/command-surface.ts and src/command-presentation.ts and
re-running):

  • src/commanderAdapter.test.ts (3) — registering an adapter with a json
    argument does not throw, the adapter keeps the flag, and a sibling command
    that shadows nothing still gets the alias. This is the reported crash at the
    level it actually occurred.
  • src/command-surface.test.ts (7) — registration succeeds for an argument
    named json, format, trace, verbose, window, site-session, or
    keep-tab; every shared option is still registered when nothing collides;
    a shadowed --json reaches the adapter and does not change the output
    format; -f json still does.
  • src/command-presentation.test.ts (4) — shadowed flag listed once with the
    adapter's meaning in text and structured help; other shared options still
    listed; a non-shadowing command unchanged.

vitest run --project unit --project plugin: 46 pre-existing failures on
main (Windows EPERM on fs.symlinkSync, plus hosted/site-memory), 45 on
this branch with no failure that is not also on main. One browser-runner
test appeared in a first run and not a second, and passes in isolation twice —
load-flaky, unrelated to this change. tsc --noEmit clean.

Left for a follow-up

Whether an adapter should be allowed to declare an argument that shadows a
reserved flag at all. This PR makes the collision survivable and gives the
adapter the flag; making webcmd validate reject such names up front is a
separate call, and I did not want to break an adapter that already ships one
in the same change that stops it crashing the CLI.

…ntrhq#441)

`configureCommandSurface` registers an adapter's own arguments first, then
added the shared options unconditionally. Commander throws on a duplicate
flag, and this runs while the CLI is being built, so a single adapter argument
named `json` aborted startup for *every* command — `list`, `doctor`, and the
`plugin uninstall` needed to remove the offending plugin, leaving no recovery
path through the CLI. The official LinkedIn plugin ships such an adapter
(`thread-snapshot`), so installing it bricked webcmd.

Guard every shared option the adapter path registers — `--format`, `--json`,
`--trace`, `-v/--verbose`, and the browser trio — the way
`ensureOutputFormatOptions` in the same file already guarded its own, and
reuse one helper for both so the two paths cannot drift again. An adapter that
names a flag keeps it; webcmd drops its own rather than refusing to run.

Format resolution has to agree about who owns a shadowed `--json`, or the flag
would silently do two things: set the adapter's argument *and* switch the
output format. Argv preprocessing already resolves this collision in the
adapter's favour, so `outputFormatIsExplicit`/`requestedOutputFormat` now
honour `--json` as the format alias only on commands where webcmd registered
it. `-f json` is unaffected and remains the way to ask for JSON output there.

Help follows the same rule: a shared option the adapter shadows is no longer
listed under "Common options", in both the text and structured renderings,
since advertising it would name a flag that is not registered and show the
same flag twice with two different meanings.
@github-actions

Copy link
Copy Markdown
Contributor

🟠 Maintainer review suggested — low confidence

The automated review could not reach a fully supported conclusion.

Limitations

  • The automated review returned an invalid structured result.

This review is advisory and does not block merging.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug]: an adapter arg named json crashes every webcmd command at startup (official linkedin plugin ships one)

1 participant