Fix: map the ADK log level to a legal gcloud verbosity in deploy cloud_run - #698
Open
AmaadMartin wants to merge 2 commits into
Open
Fix: map the ADK log level to a legal gcloud verbosity in deploy cloud_run#698AmaadMartin wants to merge 2 commits into
AmaadMartin wants to merge 2 commits into
Conversation
added 2 commits
August 5, 2026 22:13
…d_run gcloud's --verbosity accepts critical|debug|error|info|none|warning. The ADK CLI accepts debug|info|warn|error. `adk deploy cloud_run --log_level warn` forwarded 'warn' verbatim, so gcloud rejected the argument and the whole deploy failed before it reached GCP. Translate at the single point of use. The generated Dockerfile keeps the raw ADK level, because the ADK server inside the container speaks the ADK vocabulary.
The toGcloudVerbosity wrapper had one caller and wrapped a constant
declared directly above it. Inlining the lookup at the call site matches
the shape cli.ts:47 already uses for the same problem.
The fallback test asserted the result was a member of a hardcoded list
that contained the literal the preceding line already pinned, so the
assertion could not fail. The toBe('info') assertion above it pins the
fallback on its own.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Please ensure you have read the contribution guide before creating a pull request.
Link to Issue or Description of Change
Link to an existing issue (if applicable):
Or, if no issue exists, describe the change:
Problem:
adk deploy cloud_run --log_level warnalways fails. The ADK CLI acceptsdebug|info|warn|error, but gcloud's--verbosityacceptscritical|debug|error|info|none|warning.prepareGCloudArgumentsforwarded the ADK level verbatim, so gcloud rejected--verbosity warnduring argument parsing and the deploy aborted before it reached GCP. The user only sawFailed to deploy to Cloud Run: Command failed with exit code 2, which does not name the log level as the cause.Solution: I translate the ADK log level to a gcloud verbosity at the single point of use. The lookup falls back to
infofor any value outside the ADK vocabulary, so the argv can never carry an illegal verbosity. I chose a fallback over a throw for two reasons: a throw replaces one broken deploy with a different broken deploy, and the ADK CLI already resolves an unrecognized log level toINFOeverywhere else (cli.ts:47). The translation happens at the gcloud boundary only, so the generated Dockerfile keeps the raw ADK level for the ADK server inside the container.Notes for the reviewer:
--log_level warning,criticalandnoneare not ADK log levels, but they are legal gcloud verbosities, so today they reach gcloud unchanged. They now fall back toinfo. This is intentional: those values already resolve toINFOfor the ADK server itself, both locally and in the container, so the gcloud verbosity and the agent log level now agree instead of diverging.debug,infoanderrorproduce byte-identical argv.dev/src/cli/deploy/cli_deploy_cloud_run.tsordev/src/cli/cli.ts(Fix: merge user --labels into the single gcloud labels flag in adk deploy cloud_run #696, Fix: expose --otel_to_cloud on adk deploy cloud_run #694, Fix: forward --session_service_uri and --artifact_service_uri into the Cloud Run Dockerfile #588, Fix: document the gcloud pass-through of adk deploy cloud_run in --help #587, Fix: validate --log_level against a closed vocabulary and stop discarding LogLevel.DEBUG #592, Fix: derive cloud_run pass-through gcloud args from the commander parser, not raw process.argv #447, Fix: honor a caller-supplied appName in deployToCloudRun (|| / ternary precedence) #354, Fix: pin the generated Dockerfile to an explicit Node LTS major #540, Fix: exit non-zero when adk deploy fails #455). None of them adds or edits a--verbosityline, so this branch is cut frommainrather than stacked. Fix: validate --log_level against a closed vocabulary and stop discarding LogLevel.DEBUG #592 is the closest neighbour: it validates--log_levelincli.ts. It is complementary, not overlapping, becausewarnis a valid ADK level that gcloud still rejects.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_deploy_cloud_run_test.ts-> 34 passed. I added 7 cases and edited none of the 27 existing ones.Coverage. The new code (the table at
cli_deploy_cloud_run.ts:60-69and the call site at:104) has 100% line and branch coverage; both sides of the??are exercised. The file as a whole reports 95.53% statements / 89.13% branches. The shortfall is entirely pre-existing uncovered code (lines 74, 119, 152-155, 187, 191) that this change does not touch.Proof the tests can fail. I ran each new test against mutated source and confirmed it fails.
options.logLevel.toLowerCase()on the--verbositylineexpected 'warn' to be 'warning',expected 'trace' to be 'info'options.logLevelexpected '\nFROM node:lts-alpine...' to contain '--log_level='warn'''debug': 'critical'expected 'critical' to be 'debug'?? 'info'fallbackexpected undefined to be 'info'The second mutation is the one worth calling out: it is the in-place variant that looks idiomatic here, because the function already mutates
options.projectandoptions.region. It would leak the gcloud spelling into the container command line, and that test is the guard against it.Manual End-to-End (E2E) Tests:
The failure is in argv construction, and gcloud validates
--verbosityduring argument parsing, before authentication. So this reproduces offline with no GCP project. Against Google Cloud SDK 579.0.0:The full choice list comes from the same SDK:
gcloud version --verbosity=bogusreportsValid choices are [critical, debug, error, info, none, warning].To exercise the CLI end to end:
npm install && npm run build, thenadk deploy cloud_run --project any-project --region us-central1 --log_level warn ./path/to/agent. Before this change the deploy aborts on the rejected argument; after it, gcloud accepts the argv and proceeds.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.