Skip to content

Feat: Honor GOOGLE_GENAI_USE_ENTERPRISE in getExpressModeApiKey() (adk-python parity) - #240

Closed
AmaadMartin wants to merge 8 commits into
mainfrom
feat/express-mode-enterprise-env-parity
Closed

Feat: Honor GOOGLE_GENAI_USE_ENTERPRISE in getExpressModeApiKey() (adk-python parity)#240
AmaadMartin wants to merge 8 commits into
mainfrom
feat/express-mode-enterprise-env-parity

Conversation

@AmaadMartin

@AmaadMartin AmaadMartin commented Jul 29, 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 — no existing issue.
  2. Or, if no issue exists, describe the change:
    Problem: ADK Python has moved the "am I running against the enterprise / Vertex AI surface?" decision off GOOGLE_GENAI_USE_VERTEXAI and onto GOOGLE_GENAI_USE_ENTERPRISE, keeping the old variable alive only as a deprecated fallback (src/google/adk/utils/env_utils.py::is_enterprise_mode_enabled()). ADK JS had no awareness of GOOGLE_GENAI_USE_ENTERPRISE at all: anyone who had already migrated their environment got undefined out of getExpressModeApiKey() and then hit Either (Project ID and Location) or an expressModeApiKey is required. from VertexAiSessionService / VertexAiMemoryBankService, with nothing pointing at the variable name as the cause.

Solution: Add isEnterpriseModeEnabled() next to getBooleanEnvVar in core/src/utils/env_aware_utils.ts, resolving the surface with the same presence-ordered algorithm as Python:

  1. If GOOGLE_GENAI_USE_ENTERPRISE is present, its boolean value decides — GOOGLE_GENAI_USE_VERTEXAI is never consulted and nothing is logged.
  2. Else, if GOOGLE_GENAI_USE_VERTEXAI is present, log a deprecation warning and use its boolean value.
  3. Else, enterprise mode is off.

Precedence is by presence, not truthiness: GOOGLE_GENAI_USE_ENTERPRISE=false means off and must not fall through to a stale GOOGLE_GENAI_USE_VERTEXAI=true. This mirrors Python's if 'GOOGLE_GENAI_USE_ENTERPRISE' in os.environ.

Every place that reads the variable now goes through that helper (review feedback). The first revision converted only getExpressModeApiKey, which left the repo inconsistent: GOOGLE_GENAI_USE_ENTERPRISE=true produced an express-mode key while getGoogleLlmVariant() still reported GEMINI_API and geminiInitParams() still set vertexai: false. Converted call sites:

  • core/src/utils/vertex_ai_utils.tsgetExpressModeApiKey()
  • core/src/utils/variant_utils.tsgetGoogleLlmVariant()
  • core/src/models/google_llm.tsgeminiInitParams(), which ApigeeLlm also routes through

After this change no GOOGLE_GENAI_USE_VERTEXAI read remains outside the helper.

Notes on the details:

  • The deprecation warning is logged once per process. getGoogleLlmVariant() is reached from BaseTool.apiVariant on every request, so warning unconditionally would emit a line per tool per request for everyone still on the legacy variable. Python's warnings.warn(..., DeprecationWarning) is deduplicated by the default filter, so once-per-process is the parity behaviour rather than a shortcut. It costs one module-level flag; the helper's tests reload the module (and the logger it warns to) per test so the flag cannot leak between them.
  • Two places that write the variable are deliberately left alone: dev/src/cli/cli_create.ts (the .env generated by adk create) and dev/src/cli/deploy/deploy_utils.ts (ENV GOOGLE_GENAI_USE_VERTEXAI=1 in the generated Dockerfile). They emit rather than read, so they cannot use the helper, and renaming what they emit is not backwards compatible: the generated image installs @google/adk-devtools@latest but takes @google/adk from the user's copied package.json / node_modules, so the container can run a core that only understands the legacy name — and since geminiInitParams forwards the resolved value to the SDK as an explicit vertexai flag, the SDK's own GOOGLE_GENAI_USE_ENTERPRISE support cannot cover for it. Both keep working unchanged (they take the deprecated path). ADK Python does emit the new name from its scaffolding; happy to follow suit here in a separate change once a release that reads the new variable is out, or to emit both names — let me know which you prefer.
  • Not a breaking change: GOOGLE_GENAI_USE_VERTEXAI still selects the enterprise surface everywhere it did before, it just also logs one deprecation warning. The single intentional behaviour change is that an environment setting both variables to conflicting values now resolves to GOOGLE_GENAI_USE_ENTERPRISE — the same precedence @google/genai (^2.9.0) applies internally.
  • No new module, no new exported package surface (isEnterpriseModeEnabled is package-internal, like getBooleanEnvVar), and no new dependency.

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.

New coverage, per call site:

  • core/test/utils/env_aware_utils_test.ts — the helper itself: neither variable set, the new variable enabled, both precedence directions, set-but-empty (proves precedence is by presence, not truthiness), the deprecated fallback when enabled and when disabled (both warn), and warn-once across repeated reads.
  • core/test/utils/variant_utils_test.tsgetGoogleLlmVariant() honours the new variable, and a disabled new variable beats an enabled legacy one.
  • core/test/models/google_llm_test.ts — the same two cases through geminiInitParams().
  • core/test/utils/vertex_ai_utils_test.ts — the same two cases through getExpressModeApiKey(), plus the set-but-empty case.

All 13 new cases fail against the unmodified core/src and pass with it (verified by reverting core/src and re-running). New lines and branches in env_aware_utils.ts are at 100% statements / branches / functions / lines.

Pre-existing tests were all kept, and edited only where this change makes them non-deterministic or where they were vacuous — disclosed explicitly:

  • variant_utils_test.ts, vertex_ai_utils_test.ts, google_llm_test.ts and apigee_llm_test.ts now clear GOOGLE_GENAI_USE_ENTERPRISE in setup/teardown. Without that, an ambient value of the new variable — including '' or false — silently flips those suites, because precedence is by presence. variant_utils_test.ts also moves from replacing process.env wholesale to per-key assignment for the same reason; no assertion changed.
  • Two vertex_ai_utils_test.ts cases gained a GOOGLE_API_KEY value, which strengthens assertions that previously passed for the wrong reason (they returned undefined because no key existed, not because the surface was off).
npx vitest run --project unit:core \
  core/test/utils/env_aware_utils_test.ts \
  core/test/utils/variant_utils_test.ts \
  core/test/utils/vertex_ai_utils_test.ts \
  core/test/models/google_llm_test.ts \
  core/test/models/apigee_llm_test.ts \
  core/test/sessions/vertex_ai_session_service_test.ts \
  core/test/memory/vertex_ai_memory_bank_service_test.ts
#   175 passed

npm run build          # ok
npm run lint           # ok
npm run format:check   # ok

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

The helper does no I/O, so the unit tests drive it with real environment variables and no mocks. To observe it by hand against the built package (npm run build first) — each command below was run on this branch and the output shown is the output produced:

RUN="node --input-type=module -e
  import {getExpressModeApiKey} from './core/dist/esm/utils/vertex_ai_utils.js';
  import {getGoogleLlmVariant} from './core/dist/esm/utils/variant_utils.js';
  console.log(getExpressModeApiKey(), getGoogleLlmVariant());"

# 1. New variable: express-mode key resolved, VERTEX_AI surface, no warning.
GOOGLE_GENAI_USE_ENTERPRISE=true GOOGLE_API_KEY=demo-key $RUN
# demo-key VERTEX_AI

# 2. Deprecated variable: same behaviour, plus ONE warning even though two
#    separate call sites read the variable in this process.
GOOGLE_GENAI_USE_VERTEXAI=true GOOGLE_API_KEY=demo-key $RUN
# WARN: [ADK] GOOGLE_GENAI_USE_VERTEXAI is deprecated, please use GOOGLE_GENAI_USE_ENTERPRISE instead
# demo-key VERTEX_AI

# 3. Explicit opt-out beats a stale deprecated variable, and stays silent.
GOOGLE_GENAI_USE_ENTERPRISE=false GOOGLE_GENAI_USE_VERTEXAI=true GOOGLE_API_KEY=demo-key $RUN
# undefined GEMINI_API

# 4. Neither variable set.
GOOGLE_API_KEY=demo-key $RUN
# undefined GEMINI_API

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.

AmaadMartin and others added 8 commits July 28, 2026 14:48
* docs: document minimum supported Node.js version in README

Add a short prerequisite note under the Installation section stating that
ADK for TypeScript requires Node.js 18 or newer, so new users know which
Node.js runtime they need before running npm install @google/adk.

The version reflects the mandated fallback: no engines.node field is
declared in any package.json in the repo.

* docs: reference current Node.js LTS instead of a fixed version

Node.js 18 is EOL and any hard-coded minimum version goes stale over time.
Reword the installation prerequisite to point readers at the current Node.js
LTS releases, which stays accurate without future edits.

Addresses PR review feedback on #526.

---------

Co-authored-by: Amaad Martin <amaadmartin@google.com>
…#536)

`ToolAuthHandler` accepted an `authCredential` and then never read it. When
no auth response was present it went straight to `requestCredential()`, so a
credential handed to `OpenAPIToolset`/`RestApiTool` at construction time was
ignored and the tool returned `{pending: true}` on every call. For `apiKey`,
`http` and `serviceAccount` schemes nothing ever resolves that request — no
user interaction is involved — so the tool could never complete.

Fall back to the configured credential when there is no auth response, which
mirrors `_get_auth_response() or self.auth_credential` in adk-python.

Also narrow what gets written to session state. The credential store exists
to avoid repeating work that either cannot be repeated (an auth response is
readable once) or is expensive (an exchange costs a round trip). A static
credential that needed no exchange is neither, so it is no longer persisted —
that would only copy the developer's secret into the session store.
…hon parity (#542)

* Feat: add LoadMcpResourceTool and MCPToolset resource access

Port adk-python's LoadMcpResourceTool to adk-js for cross-language parity.

- Add listResources/getResourceInfo/readResource to MCPToolset, following
  the existing create -> try -> closeSession-in-finally session idiom.
- Add LoadMcpResourceTool (mirrors the in-repo LoadArtifactsTool idiom):
  declares load_mcp_resource({resource_names}), and processLlmRequest injects
  resolved resource contents (text + base64 binary, no decode step) into the
  LlmRequest.
- Export the tool from core/src/index.ts (@google/adk public API).

* test: cover LoadMcpResourceTool and MCPToolset resource access

Add full unit coverage (100% line + branch of the new code):

- load_mcp_resource_tool_test.ts: init, declaration, runAsync (incl. default),
  list injection (incl. empty + swallowed list errors), text/binary/unknown
  content, base64 blob passthrough + default mime type, swallowed read errors,
  and all no-op guard paths (non-matching/absent function response, missing
  parts).
- mcp_toolset_test.ts: listResources/getResourceInfo/readResource happy paths
  and error paths (unknown name, missing URI), plus session-cleanup assertions
  for success and failure (closeSession in finally, no leaked sessions).

* test(e2e): exercise LoadMcpResourceTool against a real MCP server

Add a no-mock end-to-end test that spawns a real MCP server over stdio
(mcp_resource_server.mjs, exposing a text and a binary resource) and drives
the real MCPToolset + LoadMcpResourceTool: listing/resolving/reading resources
and injecting their contents (text + base64 binary) into an LlmRequest.

---------

Co-authored-by: Amaad Martin <amaadmartin@google.com>
* feat(tools): add ExampleTool for few-shot examples

Port adk-python's ExampleTool to adk-js. The tool accepts a static
Example[] or a BaseExampleProvider and, on each outgoing LLM request,
appends a few-shot <EXAMPLES> block (built via buildExampleSi from the
latest user query) to the system instruction. It is never declared to
the model (mirrors PreloadMemoryTool) and is a no-op when no user text
is present. Exported from the public @google/adk API.

* test(tools): cover ExampleTool unit and end-to-end paths

Add Vitest coverage for ExampleTool: static list and provider paths,
model-style passthrough, no-op branches (missing user content, empty
parts, text-less first part), runAsync throwing, and the public export.
Includes an end-to-end block that drives processLlmRequest through a
real Context/InvocationContext (no mocks). 100% line/branch coverage of
the new tool.

* refactor(tools): apply simplicity audit feedback

Use a constructor parameter property for `examples` (repo convention),
and drop the redundant provider end-to-end test whose only unique aspect
was a spy — keeping the no-mock e2e block strictly mock-free. The
provider selection path stays fully covered by the unit tests; the tool
retains 100% line/branch coverage.

---------

Co-authored-by: Amaad Martin <amaadmartin@google.com>
* feat(agents): support clone() for RoutedAgent

RoutedAgent derives its routing targets from config.agents rather than
subAgents, so the inherited BaseAgent.clone() rebuilt the agent from the
already-parented originals and threw "already has a parent agent".

Add a RoutedAgent.clone() override that deep-clones the routing targets
(via a private cloneRoutingTargets helper) and passes them through the
agents override, so super.clone() rebuilds the constructor with fresh,
detached copies that are re-parented onto the clone. The array-vs-record
shape and record keys are preserved so the clone routes identically, and
parent-override rejection plus the detached-root guarantee are still
enforced by the base implementation.

Remove the now-obsolete "documented limitation" test (and its unused
RoutedAgent import) from base_agent_test; positive coverage lives in
routed_agent_test.

* test(agents): cover RoutedAgent.clone()

Add a clone describe suite exercising the new override and the
cloneRoutingTargets helper: array and record forms, deep-clone and
re-parenting of targets, originals left untouched, functional routing on
the clone (record form), verbatim agents override, non-agents overrides,
and parentAgent-override rejection. Includes a no-mock end-to-end case
that clones a RoutedAgent whose targets are real LlmAgents.

---------

Co-authored-by: Amaad Martin <amaadmartin@google.com>
* Feat(tools): add SSRF-safe load_web_page tool for adk-python parity

Ports the adk-python load_web_page tool to adk-js. Fetches a URL and
returns its extracted, readable text, hardened against SSRF:

- only http/https schemes are fetched
- localhost-style hostnames and hosts resolving to non-global IPs
  (private, loopback, link-local, shared/CGNAT, reserved, multicast,
  IPv4-mapped IPv6) are rejected before any connection
- redirects are never followed (redirect: 'manual')
- a configurable timeout (default 30s) bounds every request
- expected failures return the parity string "Failed to fetch url: <url>"
  instead of throwing

Exposes loadWebPage(), the LOAD_WEB_PAGE FunctionTool, and the
LoadWebPageOptions type via the @google/adk public API.

* Refactor(tools): inline single-use failure prefix in load_web_page

Addresses simplicity-audit feedback: the FAILURE_PREFIX constant had a
single caller, so its literal is inlined into failedToFetchMessage, which
remains the sole formatter of the parity failure string.

---------

Co-authored-by: Amaad Martin <amaadmartin@google.com>
Express mode key resolution only recognized GOOGLE_GENAI_USE_VERTEXAI, so
environments already migrated to GOOGLE_GENAI_USE_ENTERPRISE got undefined and
then failed with "Either (Project ID and Location) or an expressModeApiKey is
required."

Resolve enterprise mode by presence, matching adk-python's
is_enterprise_mode_enabled(): GOOGLE_GENAI_USE_ENTERPRISE decides whenever it is
set (even to a falsy value), GOOGLE_GENAI_USE_VERTEXAI is only consulted when
the new variable is absent and logs a deprecation warning.
Drop cases that re-tested getBooleanEnvVar's value parser (already covered by
env_aware_utils_test.ts) and fold the deprecation-warning assertions into the
existing GOOGLE_GENAI_USE_VERTEXAI tests instead of duplicating them. Coverage
of vertex_ai_utils.ts stays at 100% statements/branches/functions/lines.
@AmaadMartin
AmaadMartin force-pushed the feat/express-mode-enterprise-env-parity branch from 94426d3 to 02839db Compare July 29, 2026 18:05
@AmaadMartin

Copy link
Copy Markdown
Owner Author

Automated: ported to upstream as google#569.

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.

2 participants