feat(help): support JSON output for --help flags - #1337
Conversation
Rewrite flag-based `--help --json` requests to the dedicated `help` command in argv preprocessing, so `sentry --help --json` and `sentry <command> --help --json` emit the same structured JSON as `sentry help --json`. Agents and tooling reach for `--help` first, but Stricli handles it internally and ignores `--json`. Only fires when both `--help` and `--json` are present before any `--` escape; a bare `--help` keeps Stricli's text usage output unchanged. `--fields` is carried through for field selection. Fixes #1265
|
|
fix-ci: attempt 1 — the failure is |
|
Marked ready for review. All CI is green — Unit Tests, E2E Tests, Lint & Typecheck, and the security/CodeQL/semgrep checks all passed (the earlier E2E failure was a flaky bundle-build race that cleared on re-run). Self-review found nothing to change. Flagging that the risk-scoring workflow labeled this |
Two issues in scanHelpJsonToken when a --help --json request carried other flags: - A dropped value flag kept its spaced value, so `--org acme` / `--limit 5` pushed `acme` / `5` into the command path and resolved the wrong command (or a not-found error). Value flags now drop their spaced value too; known boolean flags (`--verbose`) still leave the following token as a real path segment. - `--fields` unconditionally consumed the next token, so `--fields --json` swallowed `--json` and the rewrite never fired. It now only takes a spaced value when the next token isn't a flag. Reuses the existing GLOBAL_FLAGS metadata to tell boolean from value-taking flags. Added tests for value-flag dropping, boolean-flag path retention, and the --fields swallow case.
scanHelpJsonToken treated any --flag=value token as possibly value-taking and dropped the following token as its spaced value. Since isBooleanFlagToken looks up the full name including =value, an =-form flag never matched a known boolean and its next token was discarded, so `--org=acme issue list --help --json` resolved the wrong help path. An =-form flag carries its value inline and never consumes a following token, so skip the drop-next logic when the token contains =. Added a test covering --org=acme / --limit=5 before the command path.
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 2 potential issues.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 0d4f5d6. Configure here.
| } | ||
| if (!token.startsWith("-")) { | ||
| scan.commandPath.push(token); | ||
| return 1; |
There was a problem hiding this comment.
Positionals leak into help path
Medium Severity
rewriteHelpJsonRequest treats every non-flag token as a command path segment, so positionals like an issue id are forwarded into help --json …. introspectCommand then rejects the extra segment and returns a not-found JSON error instead of help for the leaf command. Stricli’s bare --help ignores those positionals, so adding --json regresses those invocations from usable text help to an error.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit 0d4f5d6. Configure here.
There was a problem hiding this comment.
Leaving this as-is by design. The rewrite is scoped to command paths (sentry [<group> <command>] --help --json) — the forms agents actually use to discover a command's shape. When a positional like an issue id is present, forwarding it produces a structured JSON Command not found error (exit 60), which is the documented, machine-readable behavior for the JSON/agent use case this feature targets; it doesn't crash or hang. Making <command> <positional> --help --json fall back to the leaf command's help would require running route-tree resolution inside the argv preprocessor (argv-hoist.ts is currently a dependency-light string transform on the hot preprocessArgv path). Given this PR is already risk: high, I'd rather not couple the preprocessor to the command graph here — happy to do it as a follow-up if maintainers want the fallback.
| for (let i = 0; i < argv.length; ) { | ||
| // Tokens after -- are positional/pass-through — a --help there is not ours. | ||
| if (argv[i] === "--") { | ||
| return null; |
There was a problem hiding this comment.
Escape aborts valid help rewrite
Low Severity
On encountering --, rewriteHelpJsonRequest returns null immediately even when --help and --json were already seen earlier in argv. The documented rule is only that those flags must appear before the escape; a later -- should stop scanning, not discard a rewrite that already qualified.
Reviewed by Cursor Bugbot for commit 0d4f5d6. Configure here.
There was a problem hiding this comment.
This is intentional. Returning null on any pre--- scan hitting -- keeps the rule simple and conservative: a -- escape means everything after it is opaque pass-through, and I'd rather defer to Stricli's normal handling than commit to a JSON rewrite when an escape is in play (documented in the PR as -- tool --help --json not being rewritten). In practice --help --json ... -- combined with a trailing escape is not a form agents use for help discovery, so the extra complexity to salvage it isn't worth the risk on this path. Leaving as-is.
Stricli treats `-h` as a built-in alias for `--help`, but scanHelpJsonToken only matched the literal `--help` token, so `sentry -h --json` fell through to Stricli's text usage instead of producing JSON. Recognize `-h` alongside `--help` and add tests.
|
Jared, would this be easier if we made a patch to Stricli itself (like we did for the |
|
The `-H` patch works because it just deletes an alias registration from Stricli's scanner — a self-contained edit with no dependency on our code. `--help --json` is a different shape: the structured output comes from our own `introspectAllCommands`/`introspectCommand` in `src/lib/help.ts`, which a vendored There's also a maintenance angle: the patch is against The preprocessor keeps it in normal TS next to the existing That said — if you'd rather push the |
I'd rather have this upstream but in a simpler way: just support "top-level args" concept so we don't need any pre-processing. This applies to everything in that preprocessor file like |
|
Agreed, that's the better shape — a "top-level flags" allow-list in the scanner ( One nuance to sequence around: How do you want to sequence it? Options I see:
I lean toward (1) since the refactor touches every global flag and wants its own review, but happy to do (2) if you'd rather not ship the preprocessor at all. Your call. |
|
Go with 1. That said I'd argue we should allow modifying Feel free to submit PRs to upstream both for this and |
|
Sounds good. Filed #1339 for the top-level-flags refactor and captured your terminal-flag note there — This PR is unchanged and ready as the interim fix — leaving the merge call to you given the |


sentry help --jsonreturned structured help, but the--helpforms agents reach for first (sentry --help --json,sentry issue --help --json,sentry issue list --help --json) fell through to Stricli's built-in text usage and ignored--json.This rewrites a flag-based
--help --jsonrequest to the dedicatedhelpcommand during argv preprocessing (preprocessArgv), so all--helpforms now emit the same JSON assentry help --json. The rewrite only fires when both--helpand--jsonare present before any--escape — a bare--helpkeeps Stricli's text usage unchanged — and--fieldsis carried through for field selection.Testing
pnpm exec vitest run test/lib/argv-hoist.test.ts test/lib/argv-hoist.property.test.ts test/commands/help.test.ts(89 passed)tsc --noEmitclean,biome checkclean on changed filessentry --help --json,sentry issue --help --json, andsentry issue list --help --jsonproduce valid JSON identical to thehelpcommand; bare--helpstill shows text usage; invalid command returns a JSON error (exit 60);-- tool --help --jsonafter an escape is not rewrittenCloses #1265