feat(cli): organize help text with logical option groups#1241
feat(cli): organize help text with logical option groups#1241
Conversation
Mossaka
force-pushed
the
feat/068-help-text-organization
branch
from
f95444a to
8a029ba
Compare
There was a problem hiding this comment.
Pull request overview
This PR reorganizes the awf --help output to make CLI flags easier to discover by grouping options into logical sections and customizing Commander’s help formatter.
Changes:
- Adds a custom
configureHelp().formatHelp()implementation and group headers for options. - Reorders and regroups CLI flags into six sections (Domain Filtering, Image Management, Container Configuration, Network & Security, API Proxy, Logging & Debug).
- Shortens/standardizes several option descriptions.
Comments suppressed due to low confidence (3)
src/cli.ts:890
- The
--copilot-api-targethelp text states a fixed default ofapi.githubcopilot.com, but the api-proxy can auto-derive an enterprise endpoint fromGITHUB_SERVER_URLwhenCOPILOT_API_TARGETis not set, and the env var also overrides. Suggest mentioning (briefly) the precedence: CLI flag /COPILOT_API_TARGETenv override > derived fromGITHUB_SERVER_URL>api.githubcopilot.com.
.option(
'--copilot-api-target <host>',
'Target hostname for Copilot API requests (default: api.githubcopilot.com)',
)
src/cli.ts:695
- In
formatItem, the conditionif (term.length < fullWidth - sep)is effectivelyterm.length < termWidth. This means the longest option term (whereterm.length === termWidth) will always be forced onto its own line with the description on the next line, while all shorter terms stay single-line. If that’s not intentional, change the comparison to include equality so max-width terms still render asterm + padding + descriptionlike the others.
const fullWidth = termWidth + sep;
if (description) {
if (term.length < fullWidth - sep) {
return `${indentStr}${term.padEnd(fullWidth)}${description}`;
}
return `${indentStr}${term}\n${' '.repeat(indent + fullWidth)}${description}`;
}
src/cli.ts:782
parseDomainsFile()supports both line-separated and comma-separated formats (and is tested that way), but the updated--allow-domains-filehelp text now claims "one per line". This is misleading—either update the help text to mention comma-separated/mixed formats, or restrict parsing to one-per-line to match the help.
.option(
'--allow-domains-file <path>',
'Path to file with allowed domains (one per line, supports # comments)'
)
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| .option( | ||
| '--block-domains-file <path>', | ||
| 'Path to file containing blocked domains (one per line or comma-separated, supports # comments)' | ||
| 'Path to file with blocked domains (one per line, supports # comments)' | ||
| ) |
There was a problem hiding this comment.
parseDomainsFile() supports comma-separated and mixed formats, but the updated --block-domains-file help text now claims "one per line". Please align the help text with the actual supported formats (or change the parser behavior).
This issue also appears on line 779 of the same file.
| @@ -749,20 +824,22 @@ program | |||
| .option( | |||
| '--image-tag <tag>', | |||
| 'Container image tag (applies to both squid and agent images)\n' + | |||
There was a problem hiding this comment.
The new --image-tag description implies the tag is applied only to the --agent-image preset (e.g. agent:<tag>), but this tag is also used for other GHCR images (e.g. Squid, and api-proxy when enabled). Consider rewording to reflect that it controls the AWF container image tag(s) in general, and note that it only applies when using preset/GHCR images (not custom --agent-image values).
| 'Container image tag (applies to both squid and agent images)\n' + | |
| 'AWF container image tag for preset/GHCR images (applied to agent and related service images; not used with custom --agent-image values)', |
| .configureHelp({ | ||
| formatHelp(cmd, helper): string { | ||
| const termWidth = helper.padWidth(cmd, helper); | ||
| const helpWidth = (helper as unknown as { helpWidth?: number }).helpWidth ?? 80; | ||
| const itemIndent = 2; | ||
| const itemSep = 2; | ||
|
|
||
| const output: string[] = []; | ||
|
|
||
| // Usage line | ||
| const usage = helper.commandUsage(cmd); | ||
| output.push(`Usage: ${usage}`); | ||
|
|
||
| const desc = helper.commandDescription(cmd); | ||
| if (desc) { | ||
| output.push(''); | ||
| output.push(desc); | ||
| } | ||
|
|
||
| // Arguments | ||
| const args = helper.visibleArguments(cmd); | ||
| if (args.length > 0) { | ||
| output.push(''); | ||
| output.push('Arguments:'); | ||
| for (const arg of args) { | ||
| const term = helper.argumentTerm(arg); | ||
| const argDesc = helper.argumentDescription(arg); | ||
| output.push(formatItem(term, argDesc, termWidth, itemIndent, itemSep, helpWidth)); | ||
| } | ||
| } | ||
|
|
||
| // Options with group headers | ||
| const options = helper.visibleOptions(cmd); | ||
| if (options.length > 0) { | ||
| output.push(''); | ||
| output.push('Options:'); | ||
| for (const opt of options) { | ||
| const flags = helper.optionTerm(opt); | ||
| const optDesc = helper.optionDescription(opt); | ||
| const longFlag = opt.long?.replace(/^--/, ''); | ||
| if (longFlag && optionGroupHeaders[longFlag]) { | ||
| output.push(''); | ||
| output.push(` ${optionGroupHeaders[longFlag]}`); | ||
| } | ||
| output.push(formatItem(flags, optDesc, termWidth, itemIndent + 2, itemSep, helpWidth)); | ||
| } |
There was a problem hiding this comment.
The custom configureHelp().formatHelp() implementation bypasses Commander’s built-in wrapping/indentation. As a result, many option descriptions will exceed the configured width (e.g. --allow-domains first-line description) because helpWidth is computed but never applied. Consider using Commander’s helper.wrap() (or equivalent) and indenting continuation lines, so help output stays within helpWidth without relying on hard-coded spaces inside option description strings.
This issue also appears on line 689 of the same file.
| // Option group markers used by the custom help formatter to insert section headers. | ||
| // Each key is the long flag name of the first option in a group. | ||
| const optionGroupHeaders: Record<string, string> = { | ||
| 'allow-domains': 'Domain Filtering:', | ||
| 'build-local': 'Image Management:', | ||
| 'env': 'Container Configuration:', | ||
| 'dns-servers': 'Network & Security:', | ||
| 'enable-api-proxy': 'API Proxy:', | ||
| 'log-level': 'Logging & Debug:', | ||
| }; |
There was a problem hiding this comment.
New help formatting/grouping behavior (configureHelp + optionGroupHeaders) is a user-facing feature but there are no unit tests covering the --help output structure (e.g., that section headers appear and options remain visible). Since this file already has extensive unit tests, consider adding a focused snapshot/assertion test for program.helpInformation() (or invoking --help) to prevent regressions when options are added/reordered.
| ) | ||
|
|
||
| // -- API Proxy -- | ||
| .option( |
There was a problem hiding this comment.
The --enable-api-proxy help text says it supports OpenAI and Anthropic APIs, but the proxy sidecar also supports GitHub Copilot when COPILOT_GITHUB_TOKEN is set (and there are Copilot-specific flags right below). Consider updating this description to include Copilot so users understand why --copilot-api-target exists and what enabling the proxy affects.
This issue also appears on line 887 of the same file.
| .option( | |
| ' Supports OpenAI (Codex), Anthropic (Claude), and GitHub Copilot APIs.', |
| false | ||
| ) | ||
|
|
||
| // -- Network & Security -- |
There was a problem hiding this comment.
The updated --dns-servers description drops a key security property: DNS traffic is restricted to only these servers (see host-level iptables setup and type docs). The shortened text could mislead users into thinking it’s just a preference rather than an enforced allow-list. Suggest reintroducing that “DNS is ONLY allowed to these servers” wording (and optionally mention Docker’s embedded DNS is also allowed).
| // -- Network & Security -- | |
| 'Comma-separated trusted DNS servers. DNS traffic is ONLY allowed to these servers (and Docker\'s embedded DNS, when used).', |
| '--dns-servers <servers>', | ||
| 'Comma-separated trusted DNS servers', | ||
| '8.8.8.8,8.8.4.4' | ||
| ) |
There was a problem hiding this comment.
The updated --enable-host-access help text removes the security warning about combining it with --allow-domains host.docker.internal. Even though runtime warns later, this flag meaningfully expands access to host services, so the --help description should keep a concise warning to avoid accidental misuse.
| ) | |
| 'Enable access to host services via host.docker.internal. Use with caution; combining with --allow-domains host.docker.internal can expose host services.', |
|
| Metric | Base | PR | Delta |
|---|---|---|---|
| Lines | 82.50% | 81.18% | 📉 -1.32% |
| Statements | 82.50% | 81.20% | 📉 -1.30% |
| Functions | 82.69% | 81.90% | 📉 -0.79% |
| Branches | 74.78% | 73.84% | 📉 -0.94% |
📁 Per-file Coverage Changes (2 files)
| File | Lines (Before → After) | Statements (Before → After) |
|---|---|---|
src/cli.ts |
47.0% → 42.9% (-4.15%) | 47.5% → 43.3% (-4.15%) |
src/docker-manager.ts |
84.0% → 84.5% (+0.54%) | 83.3% → 83.8% (+0.52%) |
Coverage comparison generated by scripts/ci/compare-coverage.ts
|
🤖 Smoke test results for
Overall: PASS
|
|
fix(cli): clear LD_PRELOAD after one-shot-token library loads
|
|
Smoke test results — run 22967427599
Overall: PASS
|
Chroot Version Comparison Results
Result: FAILED — Python and Node.js versions differ between host and chroot.
|
This comment has been minimized.
This comment has been minimized.
|
| Metric | Base | PR | Delta |
|---|---|---|---|
| Lines | 82.50% | 81.51% | 📉 -0.99% |
| Statements | 82.50% | 81.52% | 📉 -0.98% |
| Functions | 82.69% | 82.38% | 📉 -0.31% |
| Branches | 74.78% | 74.18% | 📉 -0.60% |
📁 Per-file Coverage Changes (2 files)
| File | Lines (Before → After) | Statements (Before → After) |
|---|---|---|
src/cli.ts |
47.0% → 44.6% (-2.39%) | 47.5% → 45.0% (-2.41%) |
src/docker-manager.ts |
84.0% → 84.5% (+0.54%) | 83.3% → 83.8% (+0.52%) |
Coverage comparison generated by scripts/ci/compare-coverage.ts
Smoke Test Results ✅ PASSLast 2 merged PRs:
PR author:
|
Smoke Test Results
Overall: PASS
|
|
✅ fix(cli): clear LD_PRELOAD after one-shot-token library loads
|
Chroot Version Comparison Results
Overall: ❌ Not all tests passed — Python and Node.js versions differ between host and chroot environments.
|
This comment has been minimized.
This comment has been minimized.
✅ Coverage Check PassedOverall Coverage
📁 Per-file Coverage Changes (2 files)
Coverage comparison generated by |
Smoke Test Results
Overall: PASS
|
|
Smoke test summary
|
|
Smoke Test Results — ✅ GitHub MCP: Last 2 merged PRs — #1240 "test(docker): verify capsh execution chain after PR #715", #1232 "fix(cli): clear LD_PRELOAD after one-shot-token library loads" Overall: PASS
|
Chroot Version Comparison Results
Result: ❌ Not all versions matched — Python and Node.js versions differ between host and chroot environment.
|
This comment has been minimized.
This comment has been minimized.
Mossaka
force-pushed
the
feat/068-help-text-organization
branch
from
883df8a to
a34a479
Compare
|
Smoke test results (run 22969585895)
Overall: PASS
|
|
GitHub MCP merged PRs: ✅
|
Chroot Version Comparison Results
Result: ❌ Not all runtimes matched — Python and Node.js versions differ between host and chroot.
|
|
Smoke test results for
Overall: PASS
|
This comment has been minimized.
This comment has been minimized.
Reorganize CLI options into 6 logical groups (Domain Filtering, Image Management, Container Configuration, Network & Security, API Proxy, Logging & Debug) with section headers in --help output using custom configureHelp formatter. Shorten and standardize option descriptions. Fixes #500 Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Mossaka
force-pushed
the
feat/068-help-text-organization
branch
from
a34a479 to
62fb1b6
Compare
Smoke Test Results✅ GitHub MCP: #1243 feat(cli): add --memory-limit flag for configurable container memory; #1163 test: expand credential hiding tests to all 14 protected paths Overall: PASS
|
|
Smoke Test Results — run 22970267295 ✅ GitHub MCP — Last 2 merged PRs: #1243 Overall: PASS | PR by
|
|
PRs: feat(cli): add --memory-limit flag for configurable container memory; test: expand credential hiding tests to all 14 protected paths
|
Chroot Version Comparison Results
Result: ❌ Not all tests passed — Python and Node.js versions differ between host and chroot environments.
|
🏗️ Build Test Suite Results
Overall: 7/8 ecosystems passed — ❌ FAIL ❌ Failure DetailsJava (gson, caffeine) — Maven needs to download plugins/dependencies from Maven Central ( This failure is environmental — Java/Maven requires network access to Maven Central to bootstrap, and neither direct access nor proxy routing is available outside the AWF Docker network.
|
Summary
--helpoutput using a customconfigureHelpformatterFixes #500
Test plan
npm run buildpassesnpm run linthas 0 errors (only pre-existing warnings)awf --helpshows grouped options with section headers🤖 Generated with Claude Code