Skip to content

Fix: print usage for a bare adk invocation instead of only the version - #597

Open
AmaadMartin wants to merge 2 commits into
mainfrom
fix/adk-js-bare-invocation-usage
Open

Fix: print usage for a bare adk invocation instead of only the version#597
AmaadMartin wants to merge 2 commits into
mainfrom
fix/adk-js-bare-invocation-usage

Conversation

@AmaadMartin

@AmaadMartin AmaadMartin commented Aug 3, 2026

Copy link
Copy Markdown
Owner

Please ensure you have read the contribution guide before creating a pull request.

Link to Issue or Description of Change

  1. Link to an existing issue (if applicable):
    N/A
  2. Or, if no issue exists, describe the change:
    Problem: The root commander command has an unconditional action handler that prints the version. Bare adk therefore answers with a version string and never lists its subcommands, so the CLI has no discovery affordance at its own front door. Every generated usage line also reads Usage: ADK CLI web ..., which names no runnable command.

Solution: The handler now branches on the --version flag and falls through to program.outputHelp(). The help goes to stdout and the process exits 0, because a bare adk is a discovery request rather than a usage error. I also renamed the program to adk, so every usage line names the real binary.

Why stdout and exit 0

adk exits 0 with help on stdout, but adk deploy still exits 1 with help on stderr. The asymmetry is deliberate:

  • The root must keep an action handler, because that handler serves -v/--version. Printing help and returning normally is the natural expression there, and it exits 0.
  • The groups deliberately have no action handler, because adding one switches off commander's unknown-subcommand detection. Their exit-1 behaviour follows from that choice.
  • gh (0), npm (1) and git (1) all print the usage block on stdout for a bare invocation. The ecosystem is split on the exit code but not on the stream. Exit 0 is also the less breaking option, since bare adk exits 0 today.

Rejected alternative: stderr and exit 1, for symmetry with the groups. Please push back if you prefer it.

outputHelp() is load-bearing. help() raises a CommanderError through commander's exit handling, and help({error: true}) also redirects to stderr and exits 1.

Notes for the reviewer

Testing Plan

Please describe the tests that you ran to verify your changes. This is required for all PRs that are not small documentation or typo fixes.
Unit Tests:
[x] I have added or updated unit tests for my change.
[x] All unit tests pass locally.

npx vitest run --project unit:dev dev/test/cli/cli_test.ts -> 29/29 pass.

One new describe('bare invocation') block with three cases: bare adk prints the usage and all six subcommands and no version; -v and --version print only the version; an unknown subcommand exits 1 with nothing on stdout. The block sits before describe('command: version') to avoid the insertion point another in-flight branch uses. No existing test, mock or hook was modified or deleted, and the diff on the test file is additions only.

Coverage of the new code is 100% line and branch, read from coverage-final.json: both arms of the one new branch are hit, 4 times (version) and 1 time (bare).

Mutation proof. Every new test fails against mutated source:

mutation result
if (true || options.version), i.e. today's behaviour 1 failed: expected '' to contain 'Usage: adk'
if (false && options.version) 2 failed: expected "log" to be called with arguments: [ '1.0.0-test' ] (the new test and the pre-existing version test)
revert the rename to new Command('ADK CLI') 1 failed: expected 'Usage: ADK CLI [options] [command]\n…' to contain 'Usage: adk'
program.allowExcessArguments(true) on the root 1 failed: expected commander to reject an unknown subcommand

Manual End-to-End (E2E) Tests:
Please provide instructions on how to manually test your changes, including any necessary setup or configuration.

npm install && npm run build
E=dev/dist/esm/cli_entrypoint.js
node $E 2>/dev/null            # usage block on stdout, exit 0
node $E -v                     # 1.6.0, exit 0
node $E --version              # 1.6.0, exit 0
node $E bogus                  # error on stderr, exit 1, stdout empty
node $E deploy                 # deploy usage on stderr, exit 1 (unchanged)
node $E web --help | head -1   # Usage: adk web [options] [agents_dir]

All six behave as listed. The bare output is byte-identical to --help, and it survives 2>/dev/null, which proves it goes to stdout.

Also run on this commit: npm run lint clean, npm run format:check clean, npx tsc --noEmit reports 0 errors.

Checklist

[x] I have read the CONTRIBUTING.md document.
[x] I have performed a self-review of my own code.
[x] I have commented my code, particularly in hard-to-understand areas.
[x] I have added tests that prove my fix is effective or that my feature works.
[x] New and existing unit tests pass locally with my changes.

CI note

The first windows run failed on tests/integration/app_loader/app_loader_test.ts ("discover apps vs agents"), which timed out against its own 40s budget. That test is a known flake and this branch does not touch it: the diff is two CLI files, ubuntu and macos passed the same commit, and 3138 of 3139 tests passed on windows. Four open PRs already target this flake, including #247 ("stop app_loader discovery test timing out on Windows/macOS CI"). The job passed on re-run, and all three platforms are now green.

Amaad Martin added 2 commits August 8, 2026 10:47
The root commander command had an unconditional action handler that
printed the version, so bare `adk` answered with a version string and
never listed its subcommands. A first-time user then has no way to
discover the tool from its own front door. Branch the handler on the
--version flag and fall through to the root help.

The help goes to stdout and the process still exits 0, because a bare
`adk` is a discovery request rather than a usage error. `outputHelp()`
is what keeps that contract; `help()` would raise a CommanderError.

Also rename the program from 'ADK CLI' to 'adk' so the usage line the
user now sees names the actual binary (and every subcommand's usage line
reads 'adk web ...' rather than 'ADK CLI web ...').
…rError

Replace the hand-rolled {code?, exitCode?} structural type with the class
commander exports for exactly this. Both fields are required on the real
type, so the unknown-subcommand assertions no longer need optional
chaining; narrow the possibly-undefined result with expect.fail so a
missing throw reports as a real assertion failure.
@AmaadMartin
AmaadMartin force-pushed the fix/adk-js-bare-invocation-usage branch from 11341be to 43ad09c Compare August 8, 2026 17:53
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