Skip to content

Fix: apply the resolved log level in every command that advertises it - #929

Open
AmaadMartin wants to merge 1 commit into
fix/cli-verbose-log-level-precedencefrom
fix/cli-apply-log-level-in-all-commands
Open

Fix: apply the resolved log level in every command that advertises it#929
AmaadMartin wants to merge 1 commit into
fix/cli-verbose-log-level-precedencefrom
fix/cli-apply-log-level-in-all-commands

Conversation

@AmaadMartin

@AmaadMartin AmaadMartin commented Aug 11, 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: adk deploy cloud_run, deploy agent_engine and deploy reasoning_engine advertise -v/--verbose and --log_level, but their actions never resolve either flag. The ADK core logger therefore keeps its default level on every deploy. Separately, the CLI's own AdkLogger is never level-adjusted on any command, so it stays at INFO whatever the user asks for.

Solution: applyLogLevel() resolves the level once and applies it to both loggers the CLI owns: the ADK core logger and the CLI's own AdkLogger. All seven commands that advertise the options now call it, including the two deploy actions that had no call at all. cli.ts only emits logger.error today, so no message it prints changes yet; the level is now honest for any record the CLI logs.

Stacked on #700. The collision check found #592 and #700 already on this file. #592 fixes the || that discarded LogLevel.DEBUG; #700 makes --verbose a --log_level debug shortcut and wires integration conformance. Neither touches the CLI's own logger or the deploy actions, so this branch stacks on #700 instead of competing with it. --verbose precedence is inherited from #700 and not changed here.

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 — 111 passed. A new log level wiring block spies on AdkLogger.prototype.setLogLevel and asserts both loggers for all seven command surfaces, at --verbose, --log_level error and the no-flag default.

One test covers the error path. It makes deployToCloudRun reject, runs deploy cloud_run --log_level error, and asserts the message still reaches the console. This pins the invariant that the strictest selectable level does not swallow deploy failures.

Proof each new test can fail. I ran each mutation against the new tests:

Mutation Result
Delete logger.setLogLevel(logLevel) from applyLogLevel 21 failures, expected "setLogLevel" to be called with arguments: [ +0 ]
Delete applyLogLevel(logger, options) from the cloud_run action 4 failures on the deploy cloud_run cases
Delete applyLogLevel(logger, options) from the agent_engine action 6 failures on the agent_engine and reasoning_engine cases
Change AdkLogger.error's gate to this.logLevel >= LogLevel.ERROR 1 failure, expected '' to contain 'Error deploying agent: permission denied'

Coverage. dev/src/cli/cli.ts moves from 97.13% lines / 77.27% branches to 97.59% / 80.43%. Every line this change adds is covered. The remaining gap is five pre-existing catch blocks this change does not touch.

Unrelated failures. dev/test/cli/cli_create_test.ts fails one case, and npm run ts:check reports 313 errors in 53 files. Both are identical on the stack base with my commit stashed, so neither comes from this change.

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

Build with npm install && npm run build, then create an agent directory inside the checkout so @google/adk resolves.

  1. node dev/dist/esm/cli_entrypoint.js deploy agent_engine <agent> --project <p> --region us-central1 --temp_folder <tmp> — the deploy runs the new applyLogLevel call and still fails with the expected Artifact Registry repository is not specified message.
  2. The same command with --log_level error — the error message still prints, which is the invariant the unit test pins.
  3. node dev/dist/esm/cli_entrypoint.js integration conformance --verbose --agents_dir <dir> --tests_dir <dir> — the run completes and reports its agents and tests.

CI is absent on this PR: the base is fix/cli-verbose-log-level-precedence, and the test workflow triggers on pull_request into main. I validated the pushed commit locally instead: the targeted vitest run above, npx eslint dev/src/cli/cli.ts dev/test/cli/cli_test.ts (clean), and npx prettier --check on both files (clean).

A complexity review asked me to inline getLogLevelFromOptions, which this change had reduced to a single caller. applyLogLevel now holds that one expression and its comment. The ?? is still pinned: changing it back to || fails 25 tests, including applies --log_level debug from #592.

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.

The deploy commands never resolved --verbose / --log_level at all, so the
ADK core logger stayed at its default. The CLI's own AdkLogger was never
level-adjusted on any command.

applyLogLevel() resolves the level once and applies it to both loggers the
CLI owns. All seven commands that advertise the options call it.
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