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):
    Closes: #issue_number
    Related: #issue_number
  2. Or, if no issue exists, describe the change:
    Problem: A bare adk prints only the version string and exits, so a user who has just installed the CLI learns nothing about web, api_server, create, run, deploy or integration, and has to guess that --help exists.

The cause is in dev/src/cli/cli.ts: the root commander command carried an unconditional action handler.

const program = new Command('ADK CLI');

program
  .addOption(new Option('-v, --version', 'Get ADK CLI version'))
  .action(() => {
    console.log(version);
  });

In commander, when the root command has an action handler and no subcommand matches, that handler runs — so it fired for both adk -v and bare adk, and printed the version in both cases. Commander's own "no subcommand given, show help" path (Command._parseCommand only calls this.help({error: true}) when !this._actionHandler) was therefore unreachable, and the subcommand list was never shown. This also diverges from adk-python, whose root is a click.Group; click.Group defaults no_args_is_help=True, so bare adk prints usage there.

Solution: Branch the root action on the flag rather than printing unconditionally, and print the root help when the flag is absent (one source hunk):

const program = new Command('adk');

program
  .addOption(new Option('-v, --version', 'Get ADK CLI version'))
  .action((options: {version?: boolean}) => {
    if (options.version) {
      console.log(version);
      return;
    }
    program.outputHelp();
  });

Why these specific choices:

  • program.outputHelp(), not console.log(program.helpInformation()). outputHelp() writes through commander's configured output writer, so no new console.log is introduced (the repo guideline forbids adding one) and the output stays redirectable/testable via configureOutput.
  • Not program.help(). That additionally requests an exit, and under exitOverride() it throws a CommanderError with code commander.helpDisplayed, which the existing test harness does not swallow. outputHelp() returns normally, so node exits 0 on its own — no process.exit() is called.
  • The action stays synchronous. dev/src/cli_entrypoint.ts calls .parse(), not .parseAsync().
  • Renaming the program to adk (from 'ADK CLI') is required for the output to be usable: Usage: ADK CLI [options] [command] is not a runnable command, and this text is now what a user sees for a bare invocation. It also fixes every subcommand's usage line (Usage: adk web ..., previously ADK CLI web ...). adk-python prints Usage: adk [OPTIONS] COMMAND [ARGS].... The identically spelled AdkLogger({label: 'ADK CLI'}) two lines above is a separate value and is left alone.

Resulting contract, all verified against the built binary (see E2E below):

invocation stream content exit
adk (no args) stdout usage + full subcommand list, byte-identical to adk --help 0
adk -v / adk --version stdout the version string only 0
adk --help / -h stdout unchanged 0
adk <unknown> stderr commander error 1 (unchanged)

Exit code 0 for the bare invocation is deliberate, so shell wrappers running under set -e that probe for the binary are unaffected. A reviewer comparing SDKs should note this matches adk-python on click < 8.2; click 8.2.0 changed it ("If help is shown because no_args_is_help is enabled ... the exit code is 2 instead of 0") and adk-python pins click>=8.1.8,<9, so bare adk there exits 0 or 2 depending on the resolved click version. That inconsistency is being handled separately on the Python side and is intentionally not imported here — the JS contract is exit 0.

Breaking change analysis: exactly one invocation changes behaviour — bare adk now prints usage instead of the version. A script doing VERSION=$(adk) would break; assessed as negligible and acceptable, since the documented way to get the version is adk --version (unchanged) and the repo contains no such usage. A repo-wide search for binary invocations found only npx adk run / npx adk web in README.md, npx adk ${adkCommand} ... in the generated Dockerfile (dev/src/cli/deploy/deploy_utils.ts), and an explicit-args spawn of dev/dist/esm/cli_entrypoint.js in tests/integration/test_api_server.ts. Nothing invokes adk bare or parses its output. Exit codes are unchanged for every invocation. No API, export or type is added, removed or changed; export function createProgram(): Command is unchanged. No dependency added, removed or bumped — package.json and package-lock.json are untouched.

Deliberately out of scope: the unknown-subcommand message is still error: too many arguments. Expected 0 arguments but got 1. rather than naming the command. Improving it means loosening excess-argument handling on the root and re-implementing the check by hand, which is a separate change; the test added here therefore asserts on the exit code and error code, not on that message string.

Collision check (required before implementing): gh pr list --repo AmaadMartin/adk-js --state open --limit 1000 returned 496 open PRs. Filtering titles/branches for cli|usage|help|version|bare|invocation|command|subcommand|no-args and then diffing every candidate that touches dev/src/cli/cli.ts (#595, #592, #587, #585, #451, #385, #358, #433, #289) showed none of them touch the root program construction (lines 199–215) — their hunks are at lines 13–29, 95–160, 336–520. Greps of those diffs for new Command('ADK CLI'), outputHelp, options.version and console.log(version) found only helpInformation() in unrelated tests (#592, #358) and an outputHelp mention in a comment about addHelpText on the deploy cloud_run subcommand (#587). No PR lands or overlaps this change, so this branches from main.

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.

Added a new describe('bare invocation', ...) block in the existing dev/test/cli/cli_test.ts. No existing test, mock or hook was modified or deleted — in particular the existing describe('command: version') block is byte-for-byte unchanged, since it is the regression signal that --version still works. The new block installs capturing writeOut/writeErr writers via program.configureOutput() in a nested beforeEach (nested hooks run after the outer one, so program is already built and exitOverride() already applied), because outputHelp() writes through commander's output writer rather than console.log.

  1. should print usage listing every subcommand and request no exit — asserts stdout contains Usage: adk, Commands: and each of web, api_server, create, run, deploy, integration; that stderr stayed empty; that console.log was not called with the version; and that nothing was thrown. "Nothing thrown" is a sound proxy for exit 0: with exitOverride() installed commander requests every non-zero exit by throwing, so no throw means no exit requested, and the real process exit code is additionally confirmed 0 by the E2E run below.
  2. should print the version without usage for -v and --version — for both flags, asserts console.log was called with the version exactly twice and that the commander writers stayed empty, i.e. no usage block leaked into the --version path.
  3. should exit non-zero for an unknown subcommand — asserts err.code === 'commander.excessArguments', err.exitCode === 1, and that no help was dumped to stdout on the error path.
$ npx vitest run --project unit:dev dev/test/cli/cli_test.ts
 ✓ dev/test/cli/cli_test.ts (27 tests) 65ms
 Test Files  1 passed (1)   Tests  27 passed (27)

All 24 pre-existing cases still pass unmodified.

Coverage of the new code. Measured with --coverage.include=dev/src/cli/cli.ts and read out of coverage-final.json for the changed region: every new statement is hit, and both sides of the one new branch are executed — the if (options.version) true-arm 4 times (cases 2 and the untouched existing version test) and the program.outputHelp() arm once (case 1). That is 100% line and branch coverage of the new code. Whole-file coverage is 95.36% statements / 74.41% branches, unchanged in substance from before — the uncovered remainder is pre-existing subcommand wiring this change does not touch.

Proof the tests can fail (each mutation applied to dev/src/cli/cli.ts alone, with the new tests left in place, then reverted):

mutation result
Revert the fix: restore the unconditional .action(() => { console.log(version); }) case 1 FAILS — AssertionError: expected '' to contain 'Usage: adk' (26 passed, 1 failed)
Revert the rename only: new Command('adk')new Command('ADK CLI') case 1 FAILS — AssertionError: expected 'Usage: ADK CLI [options] [command]\n\…' to contain 'Usage: adk'
Delete the version branch (if (options.version) { console.log(version); return; }), leaving only outputHelp() case 2 FAILS and the pre-existing command: version test FAILS — expected "log" to be called with arguments: [ '1.0.0-test' ] (25 passed, 2 failed)

Case 3 (unknown subcommand) is a guard-rail test: it passes before and after this change by design, so it is not claimed as new signal for the fix. It is not vacuous, though — it pins that the root command's excess-argument handling was not widened, and it fails if that invariant is broken: adding program.allowExcessArguments() to the root makes it fail with expected commander to reject an unknown subcommand (26 passed, 1 failed).

The helper driving cases 1–3 is typed with commander's exported CommanderError rather than a hand-rolled {code?: string; exitCode?: number} shape, so code and exitCode are the real required fields and the assertions need no optional chaining; the possibly-undefined result is narrowed with expect.fail(...), which also turns "commander did not throw at all" into a proper assertion failure rather than an undefined mismatch.

No integration test was added, deliberately: the behaviour is fully determined by createProgram(), which these unit tests drive against the real commander instance (only leaf action targets such as AdkApiServer are mocked). A build-dependent spawn test for a help string would add no signal.

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

From the repo root, npm run build, then invoke the exact file dev/package.json maps bin.adk to:

$ node dev/dist/esm/cli_entrypoint.js
Usage: adk [options] [command]

Options:
  -v, --version                      Get ADK CLI version
  -h, --help                         display help for command

Commands:
  web [options] [agents_dir]         Start ADK web server
  api_server [options] [agents_dir]  Start ADK API server
  create [options] [agent]           Creates a new agent
  run [options] <agent>              Runs agent
  deploy                             Deploy agent
  integration                        Run ADK integration and conformance tests
$ echo $?
0

$ node dev/dist/esm/cli_entrypoint.js --version
1.5.0
$ echo $?
0

$ node dev/dist/esm/cli_entrypoint.js -v
1.5.0

$ node dev/dist/esm/cli_entrypoint.js bogus
error: too many arguments. Expected 0 arguments but got 1.   # on stderr
$ echo $?
1

$ node dev/dist/esm/cli_entrypoint.js web --help | head -1
Usage: adk web [options] [agents_dir]                        # was "ADK CLI web ..."

Postcondition check — bare output is byte-identical to --help:

$ node dev/dist/esm/cli_entrypoint.js > bare.out
$ node dev/dist/esm/cli_entrypoint.js --help > help.out
$ diff bare.out help.out && echo IDENTICAL
IDENTICAL

Also run on the pushed commit: npm run build (succeeds), npm run lint (clean), npx prettier --check on both changed files (clean). npm run ts:check reports 281 errors, but the identical 281 are present on the base commit with the change stashed (error sets diffed and identical) — this branch adds none.

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.

Amaad Martin added 2 commits August 3, 2026 16:39
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. Branch the handler on the --version flag
and fall through to the root help, matching adk-python, where the click
group defaults to no_args_is_help.

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.
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