feat(vertex): add API key (Express Mode) auth + native API endpoint support - #70663
feat(vertex): add API key (Express Mode) auth + native API endpoint support#70663ddm667 wants to merge 8 commits into
Conversation
alt-glitch
added
type/feature
Related: #29611 pursues Vertex Express API-key support through a |
|
Added 3 more fixes in commit 5b9aa56:
Without these fixes, Vertex was completely missing from the model picker even though credentials were configured. |
|
Added commit 68d147b3 - fix(vertex): add Vertex support to auxiliary resolve_provider_client for vision The Vertex provider (plugin-based, not in auth.py PROVIDER_REGISTRY) was unreachable from resolve_provider_client(), causing all auxiliary tasks (vision, title generation, compression) to fail with Added a dedicated Vertex block before the PROVIDER_REGISTRY path so that GeminiNativeClient is created for all auxiliary tasks — vision processing now works natively through Vertex. |
|
Updated PR with two new commits:
|
|
here are the testing content and image for the commit: |
8eaa3d8 to
552d86d
Compare
|
Rebased on latest Regarding the maintainer comparison with #29611 (extending the built-in
|
teknium1
added
sweeper:risk-compatibility
… config, models, and tests)
🔄 PR Update: Merge Conflict Resolution & Architectural Alignment (
|
GottZ
left a comment
GottZ
left a comment
There was a problem hiding this comment.
This was generated by AI during triage.
Summary
Two PRs address Vertex Express Mode API-key authentication by extending the built-in Vertex provider. #70562 introduced the dual-auth path and broad UI/CLI integration, while #70663 carries that work forward with native generateContent routing, auth-header propagation, auxiliary-client support, model-picker routing, and additional desktop coverage.
Related pull requests
- #70562 [closed]
duplicate— (+840/-145) — n/a: Adds Express Mode credentials, native Vertex routing, model discovery, setup/doctor/dashboard integration, and tests across the built-in Vertex provider. Although closed, it remains relevant as the precursor superseded by #70663; it also overlaps #29611, as noted by the contributor review. - #70663
related— (+1202/-155) — n/a: Extends the same built-in-provider approach with the salvageable core needed for Express Mode: native generateContent endpoint detection, x-goog-api-key propagation, auxiliary-task support, provider routability, and credential UI metadata. The diff still contains contradictory discovery and documentation paths: discover_vertex_models sends the API key as an Authorization bearer token while its own docstring says API-key listing returns 404, and the guide describes an OpenAI-compatible bearer-token endpoint despite the implementation routing through the native endpoint.
Duplicates
#70562 and #70663 implement substantially the same built-in Vertex dual-auth approach; #70562 is the closed precursor superseded by #70663. Both overlap #29611 on Express Mode support, but #29611 uses a separate gemini-vertex plugin architecture rather than extending the built-in vertex runtime.
Suggested consolidation
Author action: split out the part that can merge from #70663. Preserve a focused native transport/authentication cut covering the Express Mode endpoint, x-goog-api-key propagation, model switching, and auxiliary clients; move model discovery, broad desktop/UI changes, curated-model updates, and documentation into follow-ups after reconciling the bearer-token and models.list contradictions. Keep #70562 closed as superseded by #70663, and resolve the built-in-provider versus #29611 plugin architecture explicitly before retaining overlapping implementation work.
Complex graph
flowchart LR
classDef open fill:#dbeafe,stroke:#1d4ed8,color:#1e3a8a
classDef merged fill:#dcfce7,stroke:#15803d,color:#14532d
classDef closed fill:#e5e7eb,stroke:#6b7280,color:#1f2937
classDef unverified fill:#f3f4f6,stroke:#9ca3af,color:#374151
classDef best stroke-width:3px,stroke:#b45309
classDef target stroke-width:3px,stroke:#4338ca
subgraph Dup70562 ["PRs duplicating each other"]
P70562["PR #70562 (closed)"]
P70663["PR #70663 (open)"]
end
class P70562 closed
class P70663 open
class P70663 target
click P70562 "https://github.com/NousResearch/hermes-agent/pull/70562"
click P70663 "https://github.com/NousResearch/hermes-agent/pull/70663"
Graph: solid arrow = fixes / best fix, dashed arrow = partial or unverified (see edge label); boxed group = PRs duplicating each other; amber border = best fix; indigo border = target; gray node = closed (state tag in the node label).
Cross-PR triage: Reviewed 2 pull requests and 0 issues in this complex. Each diff was read against this issue; Assessment working set: 169 kB of PR diffs, 3 kB of issue/PR text, 5 kB of discussion (10 comments), 0 verify verdicts. verdicts reflect diff content, not PR titles. Part of an automated triage batch.
🛠️ Response to Reviewer Feedback (
|
GottZ
left a comment
GottZ
left a comment
There was a problem hiding this comment.
This was generated by AI during triage.
Delta since our previous triage comment
@ddm667 correctly addressed two points from our previous review: the updated diff now selects x-goog-api-key for Express Mode discovery and documents native :generateContent routing through GeminiNativeClient. The remaining discovery inconsistency is narrower but unresolved: the implementation and guide advertise API-key model discovery while the adapter docstring and setup fallback still say models.list is unavailable with API keys.
Changed pull requests
- #70663
related— (+1206/-155) — updated, still needs a focused split: commit0ad8f4d09fixes the previously identified discovery-header and transport-documentation errors, but the diff still contradicts itself on whether API-key model discovery is supported and remains substantially broader than the core Express Mode transport/authentication change.
Suggested consolidation
The consolidation recommendation is unchanged: split the mergeable Express Mode transport/authentication core from discovery and broad UI follow-ups, while explicitly resolving the architectural overlap with #29611.
Complex graph unchanged since our previous triage comment.
Cross-PR triage: Reviewed 2 pull requests and 0 issues in this complex. Each diff was read against this issue; Assessment working set: 169 kB of PR diffs, 3 kB of issue/PR text, 9 kB of discussion (12 comments), 0 verify verdicts. verdicts reflect diff content, not PR titles. Part of an automated triage batch.
🛠️ Response to Reviewer Feedback (
|
GottZ
left a comment
GottZ
left a comment
There was a problem hiding this comment.
This was generated by AI during triage.
Delta since our previous triage comment
@ddm667’s commit 3d579224f corrects the previously flagged discover_vertex_models() docstring and setup-flow fallback, explicitly documenting API-key model discovery. However, the visible diff still contains conflicting claims: agent/vertex_adapter.py describes Express Mode as Authorization: Bearer while the implementation uses x-goog-api-key, and hermes_cli/doctor.py says API-key model listing is unavailable while the adapter, setup flow, plugin, and guide advertise it.
Changed pull requests
- #70663
related— (+1204/-155) — updated, inconsistencies remain: 3d579224f narrows the earlier documentation conflict, but the diff still disagrees on both the Express Mode authentication header and whether API-key model discovery is supported, so the broad implementation is not yet internally coherent.
Suggested consolidation
The previous split-first recommendation is unchanged, consistent with the backlog lane: do not merge #70663 until these remaining contradictions are reconciled, and keep #70562 closed as its superseded duplicate.
Complex graph
flowchart LR
classDef open fill:#dbeafe,stroke:#1d4ed8,color:#1e3a8a
classDef merged fill:#dcfce7,stroke:#15803d,color:#14532d
classDef closed fill:#e5e7eb,stroke:#6b7280,color:#1f2937
classDef unverified fill:#f3f4f6,stroke:#9ca3af,color:#374151
classDef best stroke-width:3px,stroke:#b45309
classDef target stroke-width:3px,stroke:#4338ca
subgraph Dup70562 ["PRs duplicating each other"]
P70562["PR #70562 (closed)"]
P70663["PR #70663 (open)"]
end
class P70562 closed
class P70663 open
class P70663 target
click P70562 "https://github.com/NousResearch/hermes-agent/pull/70562"
click P70663 "https://github.com/NousResearch/hermes-agent/pull/70663"
Graph: solid arrow = fixes / best fix, dashed arrow = partial or unverified (see edge label); boxed group = PRs duplicating each other; amber border = best fix; indigo border = target; gray node = closed (state tag in the node label).
Cross-PR triage: Reviewed 2 pull requests and 0 issues in this complex. Each diff was read against this issue; Assessment working set: 169 kB of PR diffs, 3 kB of issue/PR text, 10 kB of discussion (13 comments), 0 verify verdicts. verdicts reflect diff content, not PR titles. Part of an automated triage batch.
🛠️ Final Text & Comment Reconciliation (
|
GottZ
left a comment
GottZ
left a comment
There was a problem hiding this comment.
This was generated by AI during triage.
Deep review at head f79a814 — not mergeable as-is: the remaining blockers are functional, not textual
First, credit where due: the three reconciliation commits (0ad8f4d09, 3d579224f, f79a814f1) did resolve the previously flagged text contradictions — module docstring, doctor.py, setup flow and guide now consistently describe x-goog-api-key + native routing. The auth_header propagation chain (resolve_runtime_provider → ModelSwitchResult → cli.py → switch_model → default_headers) is coherent, and the 401-refresh path correctly handles both modes including clearing default_headers when switching back to OAuth.
However, a full review at head finds four substantive issues that the earlier text-level passes could not see (details inline):
- OAuth auxiliary regression — the new early Vertex block in
resolve_provider_client()returns(None, None)whenever no API key is set, making the existing OAuth-capableauth_type == "vertex"branch unreachable. OAuth/ADC users lose vision, title generation and compression. - Model discovery calls an endpoint that does not exist — the aiplatform v1 discovery document has no
listmethod underprojects.locations.publishers.models(norpublishers.models), and the parsed response shape is the AI-Studio schema. Production discovery will always 404 →[]→ curated fallback, while setup flow, plugin and guide advertise it. The unit tests mock the invented schema, so they cannot catch this. DEFAULT_REGIONchange breaks existing OAuth users —global→us-central1regresses the documented Gemini 3.x-preview requirement, and Express Mode does not even use the region in its base URL./api/envnow returns every non-password env var unredacted — a global security-surface widening (all providers, not just Vertex) that deserves its own maintainer decision given thesweeper:risk-security-boundarylabel.
Minor findings
- Two comments contain a literal
***where words went missing (likely a redaction tool mangled the source):agent/agent_runtime_helpers.pyL2315 (Bearer *** only works for OAuth2 tokens)) andrun_agent.pyL5285 (*** set this when the key is actually…). Please restore the intended text. create_openai_client()(agent_runtime_helpers.py): the new 4-spaceifkeeps its body at 12-space indentation (legal but confusing) and re-derivesbase_urlafter already using it in the condition.build_vertex_api_key_base_url(project_id, region)ignores both parameters; relatedly,get_vertex_config()hard-fails Express Mode withoutGOOGLE_VERTEX_PROJECTalthough the native base URL contains no project — if discovery is dropped, that hard requirement can likely be relaxed.hermes_cli/models.py(second curated list): the "Entries validated live … as of 2026-07-21 (PR #68767)" comment now also covers newly added, unvalidated entries (gemini-2.5-pro,gemini-2.5-flash).model_setup_flows.pystill previews the OpenAI-compatendpoints/openapiURL in Express Mode, where requests actually go toaiplatform.googleapis.com/v1/publishers/googleviaGeminiNativeClient.- ~190 added lines in
tests/hermes_cli/test_web_server.py(tts/stt schema, copilot, bedrock, catalog tests) are unrelated to this PR and exist neither at the merge base nor on currentmain— they inflate an already broad diff. Where do they come from? - Test name typos: three
test_googole_vertex_*functions (googole→google).
Verification performed locally at head
tests/agent/test_vertex_adapter.py+tests/hermes_cli/test_vertex_provider.py: 39 passed;tests/agent/test_gemini_native_adapter.py: 8 passed;tests/hermes_cli/test_web_server.py -k "env_vars or config_schema or config_defaults": 16 passed (pytest 9.1).- Endpoint claim (finding 2) checked against Google's aiplatform v1 discovery document: methods under
projects.locations.publishers.modelsaregenerateContent,streamGenerateContent,predict,rawPredict,streamRawPredict,serverStreamingPredict,predictLongRunning,fetchPredictOperation,countTokens,computeTokens,embedContent,invoke— nolist;publishers.modelsadditionally has onlyget. - Registry claim (finding 1) checked at head:
hermes_cli/auth.py:445registersvertexwithauth_type="vertex", andtest_vertex_registered_in_provider_registrydocuments thatresolve_provider_client()depends on it.
This strengthens the standing recommendation from our earlier triage: split out the focused Express Mode transport/auth core (adapter dual-auth minus discovery, native URL detection, auth_header propagation, refresh path — that part is close), and keep discovery, the /api/env redaction change, the region-default change and the unrelated tests out until each is resolved on its own merits.
| try: | ||
| from agent.vertex_adapter import get_vertex_config, has_vertex_api_key | ||
|
|
||
| if not has_vertex_api_key(): |
There was a problem hiding this comment.
Functional regression for OAuth/ADC users. This early-return makes the existing OAuth-capable auxiliary path unreachable: vertex is in PROVIDER_REGISTRY (hermes_cli/auth.py:445, auth_type="vertex") — the block comment's premise ("not in PROVIDER_REGISTRY") doesn't hold, and tests/hermes_cli/test_vertex_provider.py::test_vertex_registered_in_provider_registry documents exactly this dependency. On main, a vertex aux request reaches the elif pconfig.auth_type == "vertex": branch further down (which this PR correctly updated to handle both auth modes), but this new block intercepts every vertex alias first and returns (None, None) whenever has_vertex_api_key() is False — so OAuth/ADC users lose vision, title generation and compression.
Suggest deleting this block entirely: the updated registry branch below already covers both API-key and OAuth modes. If the goal was alias coverage (google-vertex, vertex-ai, …), normalize the alias to vertex before the registry lookup instead of gating on the API key.
| host = "aiplatform.googleapis.com" if region == "global" else f"{region}-aiplatform.googleapis.com" | ||
| url = ( | ||
| f"https://{host}/v1/projects/{project_id}/locations/{region}" | ||
| "/publishers/google/models" |
There was a problem hiding this comment.
This endpoint does not exist in the Vertex API surface. The aiplatform v1 discovery document lists no list method under projects.locations.publishers.models (available methods: generateContent, streamGenerateContent, predict, rawPredict, streamRawPredict, predictLongRunning, fetchPredictOperation, countTokens, computeTokens, embedContent, invoke) — and publishers.models has only get plus the predict/generate methods. The parsed response shape below (models array + supportedGenerationMethods) is the AI Studio (generativelanguage.googleapis.com) schema, not a Vertex one.
In production this call will 404 for everyone, return [], and silently fall back to the curated list — while the setup flow, the plugin fetch_models(), and the guide all advertise dynamic discovery as a headline feature. The unit tests cannot catch this: they mock urlopen with the same invented schema the implementation assumes.
Recommend dropping discovery from this PR (which also aligns with the standing split-recommendation) or reimplementing it against an endpoint that actually exists, verified against a real GCP project before re-advertising it.
| # Default region — us-central1 is the most widely available Vertex region. | ||
| # The old default was "global" (required for Gemini 3.x previews via ADC), | ||
| # but API key / Express Mode works best with an explicit region. | ||
| DEFAULT_REGION = "us-central1" |
There was a problem hiding this comment.
Breaking change for existing OAuth/ADC users, with no benefit for Express Mode. The repo's own docs and hermes_cli/models.py state the Gemini 3.x previews are served through the global endpoint and that regional endpoints may 404 them (curated entries were live-validated in global, PR #68767). Express Mode doesn't need this change at all — build_vertex_api_key_base_url() ignores the region entirely. So swapping the shared default only affects the legacy path, negatively: an existing ADC user without an explicit region silently moves from global to us-central1 on upgrade and starts 404ing on preview models.
| # Default region — us-central1 is the most widely available Vertex region. | |
| # The old default was "global" (required for Gemini 3.x previews via ADC), | |
| # but API key / Express Mode works best with an explicit region. | |
| DEFAULT_REGION = "us-central1" | |
| # Default region — "global" is required for the Gemini 3.x previews via | |
| # OAuth2/ADC (regional endpoints may 404 them). Express Mode ignores the | |
| # region in its base URL entirely, so it is unaffected by this default. | |
| DEFAULT_REGION = "global" |
(Reverting also means adjusting test_get_vertex_config_uses_adc_and_default_region and the guide edits that softened the global-region note.)
| return { | ||
| "is_set": bool(value), | ||
| "redacted_value": redact_key(value) if value else None, | ||
| "redacted_value": (redact_key(value) if is_password else value) if value else None, |
There was a problem hiding this comment.
Global security-surface widening, beyond this PR's scope. This flips /api/env from "redact everything" to "return every non-password var in cleartext" — for all providers and env vars, not just the three new Vertex ones (the field is even still named redacted_value). Non-password vars include base-URL-style values that can embed user:pass@ userinfo. The PR is also internally ambivalent about the sensitivity of what it now exposes: agent/vertex_adapter.py's docstring labels GOOGLE_VERTEX_PROJECT "(secret — read at runtime)" while this change displays it unredacted.
Given the sweeper:risk-security-boundary label, this deserves its own maintainer decision rather than riding along here. If the goal is just a readable project/region on the Keys tab, an explicit allowlist for GOOGLE_VERTEX_PROJECT / GOOGLE_VERTEX_LOCATION would be far narrower than unredacting everything.
| "/publishers/google/models" | ||
| ) | ||
| # Use x-goog-api-key for Express Mode API keys and Authorization Bearer for OAuth2 tokens | ||
| if api_key.startswith("AIza") or (has_vertex_api_key() and not api_key.startswith("ya29.")): |
There was a problem hiding this comment.
Key-type sniffing (AIza / ya29. prefixes plus re-reading the env var) is fragile: an OAuth token that doesn't start with ya29. while GOOGLE_VERTEX_API_KEY happens to be set would be sent as x-goog-api-key and fail. Every caller already knows the auth mode — get_vertex_config() returns auth_header — so pass that in as a parameter instead of guessing from the credential's shape.
| "google/gemini-2.5-flash-lite", | ||
| "google/gemini-flash-latest", | ||
| "google/gemini-flash-lite-latest", | ||
| "google/gemini-embedding-001", |
There was a problem hiding this comment.
gemini-embedding-001 is an embedding model — it can't serve the chat completions this picker list feeds. Selecting it would fail at first message.
🛠️ Response to Reviewer Feedback (
|
🛠️ Additional Polish (
|
🛠️ Model List Clean-up (
|
b37cd4c to
353dfce
Compare
…ry client, region, and desktop UI - agent/auxiliary_client.py: restore OAuth2/ADC credential resolution fallback when GOOGLE_VERTEX_API_KEY is unset so auxiliary tasks (vision, compression, titles) work for OAuth users. - agent/vertex_adapter.py: restore DEFAULT_REGION = "global" for Gemini 3.x previews via Vertex OAuth2/ADC; update discover_vertex_models debug logging on HTTP 404; relax hard requirement on GOOGLE_VERTEX_PROJECT in Express Mode. - agent/agent_runtime_helpers.py & run_agent.py: restore docstring Bearer token text and fix 12-space indentation under is_native_gemini_base_url. - hermes_cli/web_server.py: return plain value in redacted_value when is_password is False so public settings (GCP project ID, region, paths) display unredacted in GUI. - hermes_cli/models.py: remove embedding model from Vertex chat completion picker and lead with google/gemini-3.6-flash in static fallback list. - hermes_cli/model_setup_flows.py: update setup flow URL preview for Express Mode. - apps/desktop: widen settings control grid from 22rem to 28rem (448px) so project IDs and long values fit without truncation. - tests: update unit tests in test_vertex_adapter.py, test_vertex_provider.py, and test_web_server.py.
…est discovery docs, focused tests Maintainer review follow-ups on top of the Express Mode API key work: - discover_vertex_models() now takes auth_header (the 3rd element of get_vertex_config()) instead of sniffing AIza/ya29/AQ key prefixes, which is fragile for Express Mode keys and OAuth tokens. The setup flow and plugin pass the mode explicitly. - Stop advertising API-key model discovery: Google's publishers/models list endpoint is not part of the public Express Mode API surface and 404s in every form, so discovery always falls back to the curated catalog. Setup flow text, plugin fetch_models docstring, models.py comment, and the guide now say so. - Drop ~190 lines of unrelated test additions in test_web_server.py (config-schema/tts/stt/copilot/bedrock/catalog tests that exist on neither the merge base nor main). Align the file with main and keep only test_get_env_vars_non_password_fields_unredacted, which covers the PR's /api/env change (project visible, key redacted). - Add regression test for explicit discovery auth-header selection. Verified: 43 vertex tests + web_server env tests pass; live API check (gemini-3.6-flash via native generateContent) returns OK.
eaf60b9 to
25eddd6
Compare
🛠️ Follow-up: all four functional blockers addressed + review hygiene (
|
…ress Mode key
The desktop chat model picker requests model.options with explicit_only=true,
which filters providers through is_provider_explicitly_configured(). That
function only checked env vars when auth_type == "api_key", and the
PROVIDER_REGISTRY vertex entry declared api_key_env_vars=() (it predates
Express Mode). Result: Vertex never counted as explicitly configured and was
silently dropped from the picker even with GOOGLE_VERTEX_API_KEY set.
- auth.py: vertex entry now declares api_key_env_vars=("GOOGLE_VERTEX_API_KEY",);
the env-var check gates on declared vars, not auth_type (vertex's
auth_type stays "vertex" for the dual-auth runtime).
- credential_pool.py: _seed_from_env seeds by declared api_key_env_vars so
env:GOOGLE_VERTEX_API_KEY lands in the pool (step-4 of the gate, and other
pool consumers).
- Regression test: is_provider_explicitly_configured("vertex") flips with
the key set/unset.
Verified end-to-end: list_authenticated_providers() emits the Vertex row and
_filter_explicit_provider_rows keeps it. 112 tests pass.
🐛 Found + fixed: model picker was hiding Vertex entirely (
|
Reviewer verdict (risk-security-boundary): the env-rows change flipped /api/env from redact-everything to returning every non-password var in cleartext — for all providers, not just the three Vertex vars. Fix per the verdict's suggestion: only GOOGLE_VERTEX_PROJECT / GOOGLE_VERTEX_LOCATION show cleartext via an explicit _ENV_CLEARTEXT_ALLOWLIST; everything else, including other providers' base-URL overrides that can embed user:pass@ userinfo, stays redacted. Aligns vertex_adapter.py's docstring (project id is non-secret routing config, not a secret) and extends the env test to pin the non-vertex redaction.
🔒
|
…overy over-claim - _PROVIDER_MODELS["vertex"] now lists only the live-verified Express Mode models (gemini-3.6-flash, gemini-3.5-flash). The unvalidated 2.5-* and -latest alias entries made detect_static_provider_for_model() claim google/gemini-2.5-flash for vertex, breaking the vendor-slug contract in _infer_provider_on_model_change (main's denormalizer test expected openrouter for that slug) — a real regression found by the full suite. - Remove the PR's 2.5-pro/2.5-flash additions from the openapi curated list (unvalidated; the verdict flagged them). - ProviderEntry description no longer advertises "region-specific model discovery" — models.list 404s for Express Mode keys.
✅ Full review-item audit — all verdict items closed + 2 new fixes + main mergeRe-audited every inline comment and every item in the deep-review verdict against current code. Everything is addressed; two previously-missed items surfaced and are fixed: New fixes in this batch:
Verified-again verdict items: OAuth aux regression (fixed, Full affected suites: 204 tests passed, 0 failed (web_server, vertex adapter/provider, models, inventory, credential pool, auth gate). |
5 participants
Summary
Adds dual-auth support for Google Vertex AI — API key (Express Mode) as the primary method, preserving OAuth2/ADC as a fallback.
Changes
1. API Key Auth (Express Mode)
2. Native generateContent Endpoint (Critical Fix)
Express Mode API keys do NOT work with Vertex's OpenAI-compatible /endpoints/openapi/chat/completions endpoint. They only work with the native :generateContent API.
https://aiplatform.googleapis.com/v1/publishers/googlefor the native API3. auth_header Propagation (Another Critical Fix)
The auth_header from resolve_runtime_provider() (e.g. "x-goog-api-key") was being discarded — the API key was sent as Authorization: Bearer instead of x-goog-api-key, causing 401.
4. UI & Diagnostics
Testing