Skip to content

CLI-613 Merge CAG subcommand tree into docs site#448

Draft
mpaladin wants to merge 5 commits into
masterfrom
mp/CLI-613
Draft

CLI-613 Merge CAG subcommand tree into docs site#448
mpaladin wants to merge 5 commits into
masterfrom
mp/CLI-613

Conversation

@mpaladin

Copy link
Copy Markdown
Contributor

Summary

  • Adds build-scripts/docs/dump-cag-tree.ts: downloads the pinned CAG binary (PGP-verified, cached under node_modules/.cache/), invokes tool dump-cli-tree --pretty, and returns the parsed JSON tree
  • Adds build-scripts/docs/merge-cag-tree.ts: exported mergeCagTree(cagTree, allCommands) that stitches CAG subcommands under the existing sonar-context entry, sets isGroup: true, clears the passthrough [action] [args...], maps options to ClidocOption, and assigns requiresAuth flags
  • Updates generate-docs.ts to call await dumpCagTree() + mergeCagTree() after the Commander tree is serialized
  • Adds examples for sonar context guidelines get, sonar context dependencies check, sonar context navigation search-signatures
  • 18 unit tests in tests/unit/build-scripts/docs/merge-cag-tree.test.ts using the captured tests/fixtures/cag-cli-tree.json fixture

Context: sonar context is a Commander passthrough — it just forwards args to the CAG binary. The docs generator walks Commander only, so commands.json currently shows sonar-context with isGroup: false, children: []. This PR bridges the gap via the new tool dump-cli-tree command added to CAG in SonarSource/sonar-context-augmentation#286.

⚠️ Blocked on CAG release: gen:docs will fail until SONAR_CONTEXT_AUGMENTATION_VERSION is bumped to a version that includes dump-cli-tree and bun run fetch:signatures is refreshed. Use CAG_BINARY_PATH=<path-to-dev-build> bun run gen:docs for local development.

After the CAG version is bumped, bun run gen:docs should be re-run to commit updated docs/data/commands.json and docs/llms.txt.

Test plan

  • bun test tests/unit/build-scripts/docs/merge-cag-tree.test.ts — 18 tests pass
  • bun test tests/unit/ — 1578 pass, 0 fail (no regressions)
  • bun run --bun tsc --noEmit — no new type errors (pre-existing picomatch error only)
  • Once CAG PR is merged and version bumped: bun run gen:docs succeeds without override
  • Verify docs/data/commands.json has sonar-context with isGroup: true and all CAG subcommand entries
  • Verify docs/llms.txt contains ### sonar context guidelines get *, ### sonar context navigation search-signatures *, etc.
  • Open docs/commands.html#sonar-context locally — Subcommands panel renders the full tree

sonar context is a Commander passthrough, so the docs generator cannot
discover CAG's clap-based subcommands on its own. Wire the docs build to
download the pinned CAG binary, invoke the new `tool dump-cli-tree --pretty`
command, and stitch the result into commands.json / llms.txt / the HTML tree
under the sonar-context entry.

New files:
- build-scripts/docs/dump-cag-tree.ts  — download, verify, and run dump
- build-scripts/docs/merge-cag-tree.ts — exported mergeCagTree + ClidocCommand types
- tests/fixtures/cag-cli-tree.json     — captured CAG tree fixture
- tests/unit/build-scripts/docs/merge-cag-tree.test.ts — 18 unit tests

Note: docs/data/commands.json and docs/llms.txt are not updated here.
They will be regenerated (bun run gen:docs) once the CAG version in
SONAR_CONTEXT_AUGMENTATION_VERSION is bumped to a release that includes
the dump-cli-tree command.

Set CAG_BINARY_PATH=<path> to use a local build during development.
@netlify

netlify Bot commented Jun 12, 2026

Copy link
Copy Markdown

Deploy Preview for sonarqube-cli canceled.

Name Link
🔨 Latest commit 9fd6eef
🔍 Latest deploy log https://app.netlify.com/projects/sonarqube-cli/deploys/6a2c3755c32ace00080f74e8

@sonarqubecloud

sonarqubecloud Bot commented Jun 12, 2026

Copy link
Copy Markdown

Agentic Analysis: Early Results

Agentic Analysis and Context Augmentation are available on your project. Here are some issues that could have been prevented. Follow the links to learn how to put them into action.

23 issue(s) found across 3 file(s):

Rule File Line Message
typescript:S109 build-scripts/docs/dump-cag-tree.ts 135 No magic number: 0o755.
typescript:S109 build-scripts/docs/dump-cag-tree.ts 185 No magic number: 200.
typescript:S109 build-scripts/docs/dump-cag-tree.ts 186 No magic number: 200.
typescript:S3358 build-scripts/docs/merge-cag-tree.ts 100 Extract this nested ternary operation into an independent statement.
typescript:S4325 tests/unit/build-scripts/docs/merge-cag-tree.test.ts 71 This assertion is unnecessary since it does not change the type of the expression.
typescript:S4325 tests/unit/build-scripts/docs/merge-cag-tree.test.ts 76 This assertion is unnecessary since it does not change the type of the expression.
typescript:S4325 tests/unit/build-scripts/docs/merge-cag-tree.test.ts 81 This assertion is unnecessary since it does not change the type of the expression.
typescript:S4325 tests/unit/build-scripts/docs/merge-cag-tree.test.ts 98 This assertion is unnecessary since it does not change the type of the expression.
typescript:S4325 tests/unit/build-scripts/docs/merge-cag-tree.test.ts 99 This assertion is unnecessary since it does not change the type of the expression.
typescript:S4325 tests/unit/build-scripts/docs/merge-cag-tree.test.ts 105 This assertion is unnecessary since it does not change the type of the expression.
typescript:S4325 tests/unit/build-scripts/docs/merge-cag-tree.test.ts 107 This assertion is unnecessary since it does not change the type of the expression.
typescript:S109 tests/unit/build-scripts/docs/merge-cag-tree.test.ts 107 No magic number: 3.
typescript:S4325 tests/unit/build-scripts/docs/merge-cag-tree.test.ts 108 This assertion is unnecessary since it does not change the type of the expression.
typescript:S109 tests/unit/build-scripts/docs/merge-cag-tree.test.ts 108 No magic number: 3.
typescript:S4325 tests/unit/build-scripts/docs/merge-cag-tree.test.ts 113 This assertion is unnecessary since it does not change the type of the expression.
typescript:S4325 tests/unit/build-scripts/docs/merge-cag-tree.test.ts 114 This assertion is unnecessary since it does not change the type of the expression.
typescript:S4325 tests/unit/build-scripts/docs/merge-cag-tree.test.ts 115 This assertion is unnecessary since it does not change the type of the expression.
typescript:S4325 tests/unit/build-scripts/docs/merge-cag-tree.test.ts 126 This assertion is unnecessary since it does not change the type of the expression.
typescript:S4325 tests/unit/build-scripts/docs/merge-cag-tree.test.ts 139 This assertion is unnecessary since it does not change the type of the expression.
typescript:S4325 tests/unit/build-scripts/docs/merge-cag-tree.test.ts 145 This assertion is unnecessary since it does not change the type of the expression.
typescript:S4325 tests/unit/build-scripts/docs/merge-cag-tree.test.ts 149 This assertion is unnecessary since it does not change the type of the expression.
typescript:S4325 tests/unit/build-scripts/docs/merge-cag-tree.test.ts 150 This assertion is unnecessary since it does not change the type of the expression.
typescript:S4325 tests/unit/build-scripts/docs/merge-cag-tree.test.ts 155 This assertion is unnecessary since it does not change the type of the expression.

Analyzed by SonarQube Agentic Analysis in 2.7 s

@hashicorp-vault-sonar-prod

hashicorp-vault-sonar-prod Bot commented Jun 12, 2026

Copy link
Copy Markdown

CLI-613

Comment thread build-scripts/docs/merge-cag-tree.ts Outdated
Comment thread build-scripts/docs/dump-cag-tree.ts Outdated
Comment thread build-scripts/docs/dump-cag-tree.ts Outdated
…ctly

Address Gitar review:
- Clean up downloaded .tar.gz/.asc in a `finally` block so failed verifications
  don't leave unverified archives behind and successful runs don't waste cache.
- Guard `JSON.parse` of the binary's stdout with an actionable error that
  includes stdout/stderr snippets.
- Treat absence of `value_name` as the boolean-flag signal (CAG strips it
  upstream). Render variadic options (`num_args: "<min>+"`) with a trailing
  ellipsis (e.g. `--categories <CATEGORIES>...`).
- Refresh fixture from updated CAG dump (boolean flags no longer carry
  value_name) and add regression tests for boolean/variadic rendering.
Comment thread build-scripts/docs/merge-cag-tree.ts
Address Gitar suggestion: CAG emits `default: "false"` for boolean flags that
initialize to false. Surfacing this as `defaultValue` in the generated docs
would advertise a stringified "false" as a meaningful default. Only carry
defaultValue forward when the option actually accepts a value.
@gitar-bot

gitar-bot Bot commented Jun 12, 2026

Copy link
Copy Markdown
Code Review ✅ Approved 4 resolved / 4 findings

Integrates the CAG subcommand tree into the documentation site by parsing the CLI output and merging it into the existing Commander structure. Resolved issues regarding boolean flag rendering, artifact cleanup, and unsafe JSON parsing.

✅ 4 resolved
Quality: Boolean CAG flags rendered as value-taking options in docs

📄 build-scripts/docs/merge-cag-tree.ts:75-89 📄 tests/fixtures/cag-cli-tree.json:18-23 📄 tests/fixtures/cag-cli-tree.json:83-88 📄 tests/fixtures/cag-cli-tree.json:127-141
mapCagOptions/buildCagFlags decide whether an option takes a value purely from the presence of value_name: type: o.value_name ? 'string' : 'boolean' and const valuePart = opt.value_name ? ' <${opt.value_name}>' : ''. However the captured CAG dump (tests/fixtures/cag-cli-tree.json) includes a value_name even for pure boolean flags — e.g. --json has value_name: "JSON" (lines 18-23, 40-46), and stop --all has value_name: "ALL" (lines 83-88). As a result the generated docs/llms.txt will show misleading usage like --json <JSON> and --all <ALL> for what are actually boolean switches. The unit tests don't catch this because buildCagFlags/mapCagOptions are exercised with hand-written options that omit value_name for booleans, not with the real fixture shape. Consider treating known boolean flags (or a CAG-provided indicator such as num_args: 0/missing) as booleans, or coordinating with the CAG dump-cli-tree output to distinguish flags from value options. Additionally, num_args: "1+" (variadic, e.g. --categories, lines 127-152) is dropped entirely, so multi-value options are documented as single-value.

Quality: dump-cag-tree leaves downloaded .tar.gz/.asc artifacts in cache

📄 build-scripts/docs/dump-cag-tree.ts:94-108
resolveCagBinary downloads the archive to ${binaryPath}.tar.gz and the signature to ${archivePath}.asc, but never removes them after extracting the binary. The sibling implementation in src/cli/commands/_common/install/context-augmentation.ts (lines 123-138) deliberately rmSyncs both files in a finally/catch so failed verifications don't leave stale archives behind and successful installs don't waste disk. Here, if PGP verification or extraction throws, the downloaded .tar.gz/.asc persist in the cache dir, and on success they remain alongside the extracted binary. Consider cleaning these up (e.g. in a finally) to mirror the install path and avoid leaving partial/unverified downloads on disk.

Edge Case: JSON.parse of CAG stdout is unguarded

📄 build-scripts/docs/dump-cag-tree.ts:160-172
dumpCagTree returns JSON.parse(result.stdout) as CagCliTree with no try/catch. If the binary exits 0 but emits non-JSON (e.g. a deprecation/warning line on stdout, or a future CAG build whose dump-cli-tree output format drifts), the build fails with a raw SyntaxError: Unexpected token... that gives no indication it came from the CAG tree dump. Given the surrounding code goes to lengths to produce actionable errors (signature mismatch, missing binary), consider wrapping the parse to surface a clear message including a snippet of stdout/stderr.

Quality: Boolean CAG flags now carry a string "false" defaultValue

📄 build-scripts/docs/merge-cag-tree.ts:105-117
mapCagOptions sets defaultValue: o.default unconditionally (build-scripts/docs/merge-cag-tree.ts:114). With the fixture/CAG change that now emits default: "false" for zero-arg boolean flags (e.g. --json, --all, --fqn-stdin), every boolean option in the generated docs will render a defaultValue of the string "false". This is functionally harmless but produces noisy/odd output: a boolean flag documented with defaultValue: "false" (a string, not a boolean) where previously it was undefined. Consider clearing/normalizing the default for non-value (boolean) options so the docs don't advertise a stringified "false" default for flags that take no value. The new mapCagOptions unit tests also don't assert defaultValue, so this behavior is untested.

Options

Auto-apply is off → Gitar will not commit updates to this branch.
Display: compact → Showing less information.

Comment with these commands to change:

Auto-apply Compact
gitar auto-apply:on         
gitar display:verbose         

Was this helpful? React with 👍 / 👎 | Gitar

@sonarqubecloud

Copy link
Copy Markdown

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.

1 participant