Normalize output-format parsing and rendering across builtin commands

[Sneh30]

What

Fixes #172: unify -f/--format parsing and rendering so every builtin command accepts and honors the same set of output formats, rejects unsupported values, and emits consistent structured output.

Single source of truth

• src/command-surface.ts now owns the canonical formats (table, plain, json, yaml, md, csv), the aliases yml → yaml and markdown → md, the shared help text, and the parseOutputFormat/resolveOutputFormat validators (case-insensitive).
• All -f/--format flags (builtin commands, auth status/refresh, adapter/hosted surfaces) advertise the same format list.

Behavior fixes

• plugin list -f yaml now emits real YAML (previously a table); empty plugin list -f json emits [] instead of human guidance.
• skills list, external list, plugin catalog list/add, plugin search, convention-audit, and auth status/refresh now honor all six formats with the correct explicit/implicit semantics (table in TTY, YAML outside TTY unless -f is given).
• webcmd list, skills list, convention-audit, plugin commands, and auth reject unsupported formats (e.g. -f xml) with a usage error (exit 2) instead of silently rendering a table.
• Aliases (yml, markdown) and case (-f YAML) are accepted and normalized everywhere.

plugin list note

plugin list intentionally keeps its human-friendly grouped listing as its table rendering — whenever the effective format is table (the default or an explicit -f table), and regardless of TTY. Use -f json or -f yaml for machine-readable output. This is documented in docs/cli-reference.mdx and skills/webcmd-usage/SKILL.md.

Hosted mode

Hosted list, profile, and plugin search now validate and normalize -f/--format through the same parseOutputFormat path as the local CLI, so unknown formats fail with a usage error and aliases/case are handled consistently there too.

Breaking change

-f/--format values other than table, plain, json, yaml, md, or csv now fail with a usage error (exit code 2) instead of silently rendering a table. This applies to builtin commands and adapter commands (e.g. webcmd hackernews top -f xml), and now also to hosted list, profile, and plugin search. The PR commit carries a BREAKING CHANGE: footer so release-please emits the changelog entry automatically.

Docs

• docs/cli-reference.mdx and skills/webcmd-usage/SKILL.md updated to reflect the shared format behavior, the plugin list table rendering, rejection of unknown formats, aliases, and plugin list -f json → [].

Testing

• npm test: 5195 passed, 1 skipped (16 new tests).
• npm run build OK; check:hosted-contract contract bytes unchanged; check:typed-error-lint 0 new; check:silent-column-drop 0 new.

[github-actions]

🟠 Maintainer review suggested — low confidence

The automated review could not reach a fully supported conclusion.

This review is advisory and does not block merging.

[rajarshidattapy]

Thanks for the PR!

A few changes I'd like before this lands:

1. Please squash the changes into a single commit.

2. plugin list doesn't currently behave the way the PR description claims.

   The PR body says plugin commands now follow "table in TTY, YAML outside TTY unless -f is given," but src/cli.ts:3177-3196 falls back to the human-readable grouped output whenever the resolved format is table—whether implicit or explicit. In a non-TTY with no -f, it still prints the pretty list rather than YAML. The fmtExplicit you're threading through at src/cli.ts:3184 only reaches renderOutput on non-table formats, so it never affects this path.

   I suspect this is intentional (the existing e2e assertion for "No plugins installed" also suggests that), which is fine. If so, please update the PR body and docs/cli-reference.mdx to describe the actual behavior. If it isn't intentional, gate the human-output path on fmt === 'table' && fmtExplicit and update the corresponding e2e test.

3. The breaking change is broader than just builtin commands.

   parseOutputFormat is also used by configureCommandSurface (src/command-surface.ts:143,165) and commanderAdapter.ts:100, so making it throw means every adapter command now hard-fails on an unknown format. For example, webcmd hackernews top -f xml previously rendered a table but now exits with code 2.

   I think this is the right behavior and aligns with Normalize output-format parsing and rendering #172's acceptance criteria, but the removed comment in parseOutputFormat shows the previous fallback was intentional, while the PR title scopes the change to builtins. Please call this out explicitly with:

   • a breaking-change note in the PR body, and
   • a CHANGELOG entry,

   so users relying on unknown -f values in scripts aren't surprised.

   It would also be good to confirm hosted mode behaves the same way. hosted/runner.ts appears to have picked up the updated help text, but I don't see the validator wired in there.

4. webcmd list still doesn't pass fmtExplicit (src/cli.ts:815-820), even though Normalize output-format parsing and rendering #172 explicitly calls out "derive fmtExplicit from Commander's option-value source everywhere."

   It's currently harmless because commandListPresentation returns displayLines for table output and never reaches the renderer. However, the other seven handlers were updated, and leaving this one out makes it look like an oversight. I'd either pass it through for consistency or leave a short comment explaining why it's intentionally unnecessary.

Nits

• resolveOutputFormat (src/command-surface.ts:271) currently returns string | null, even though parseOutputFormat has already narrowed the type. It should return OutputFormat | null.
• It also writes directly to console.error and sets process.exitCode instead of going through the existing CliError path. The behavior matches Commander's standard lowercase error: output, so I wouldn't block on it, but it does introduce a second error-handling path into a file that already imports CliError.

The tests look well targeted. I especially liked inverting the existing command-surface/commanderAdapter assertions instead of adding new ones, and the empty-result e2e cases are the ones that actually pin #172.

[Sneh30]

Thanks for the thorough review — all the feedback has been addressed. On the squash request, I'd like to keep the branch as-is if that's acceptable; the commits are already logically separated (feature, per-family fixes, tests, docs) and the review comments are covered by four small follow-ups. If you'd prefer a single commit in main, squash/rebase merge on the maintainer side achieves that without rewriting the branch. Happy to squash if it's a hard requirement.

Point-by-point:

1. plugin list behavior — This is intentional; the human-readable grouped listing is plugin list's table rendering, regardless of whether table is implicit or explicit and regardless of TTY. I've corrected the PR body and updated docs/cli-reference.mdx and skills/webcmd-usage/SKILL.md to state that explicitly and to point scripts at -f json/-f yaml for machine-readable output. (plugin list -f yaml still emits real YAML and empty -f json still emits [].)

2. Breaking change scope — Agreed it's the right behavior per Normalize output-format parsing and rendering #172. The PR body now carries a BREAKING CHANGE section, and the new hosted commit includes a BREAKING CHANGE: footer so release-please emits the changelog entry automatically. I've also wired the validator into hosted mode: list, profile, and plugin search now go through the same parseOutputFormat path as the local CLI, so webcmd list -f xml, webcmd profile list -f xml, and webcmd plugin search x -f xml fail with a usage error (exit 2) instead of rendering a table. (The hosted main command path already validated via parseCommandSurface; only those three custom surfaces were missed.)

3. webcmd list fmtExplicit — Now passed through to the renderer for consistency with the other handlers (src/cli.ts).

Nits:

• resolveOutputFormat now returns OutputFormat | null (src/command-surface.ts).
• On the console.error/process.exitCode path: I kept it as-is since the reviewer noted it's non-blocking and matches Commander's standard lowercase error: output; happy to route it through CliError if preferred.

Tests: npm test 5195 passed / 1 skipped (4 new hosted tests covering rejection and alias/case normalization across the three surfaces), build green, check:hosted-contract bytes unchanged, check:typed-error-lint and check:silent-column-drop 0 new.