fix(auxiliary_client): route vertex through the auth_type dispatch - #61853
fix(auxiliary_client): route vertex through the auth_type dispatch#61853haizaar wants to merge 2 commits into
Conversation
Silent regression on any `provider: vertex` deployment:
`resolve_provider_client("vertex", ...)` returned `(None, None)` because
`PROVIDER_REGISTRY.get("vertex")` was None, and the existing
`elif pconfig.auth_type == "vertex":` handler below was unreachable
dead code.
Every auxiliary task on a Vertex-only deployment — `vision_analyze`,
context compression, curator's LLM review pass, `session_search`,
`title_generation`, `web_extract` — fell through to the aggregator
fallback chain (openrouter → nous → local/custom → api-key) and
terminated with:
RuntimeError: No LLM provider configured for
task=<task> provider=auto. Run: hermes setup
Vision was hit hardest: `check_vision_requirements()` uses the same
resolver, so it returned False and `vision_analyze` was stripped from
the model-facing tool schema. Fleets with `OPENROUTER_API_KEY` set
never noticed because Step 3 of `_resolve_auto` caught the miss;
Vertex-only deployments hit it terminally.
Root cause is in `hermes_cli/auth.py`'s auto-extension of
`PROVIDER_REGISTRY` from the provider-plugin catalog:
if _pp.auth_type != "api_key" or not _pp.env_vars:
continue
The vertex `ProviderProfile` declares `auth_type="vertex"` (OAuth2 via
ADC, not a static key) and `env_vars=()`, so both clauses reject it.
Same shape applies to any future non-api_key provider profile
(aws_sdk, oauth_device_code, oauth_external).
Fix: plugin-catalog fallback for `pconfig=None`. When the registry
lookup misses, consult `providers.get_provider_profile(provider)`. If
the profile's `auth_type` is one of the well-known non-api_key
families (`vertex`, `aws_sdk`, `oauth_device_code`, `oauth_external`),
synthesize a `SimpleNamespace(auth_type=<profile.auth_type>)` and let
the existing dispatch branches downstream fire. Genuinely unknown
providers still bail cleanly with the unchanged "unknown provider"
debug log.
No touch to `hermes_cli/auth.py` — keeping `PROVIDER_REGISTRY`
api-key-only preserves invariants elsewhere in the codebase.
Once the fallback is in place, the pre-existing
`elif pconfig.auth_type == "vertex":` handler serves Gemini traffic
via the OpenAI-compat endpoint as it was originally written to do —
no other changes needed here.
Tests: new `tests/agent/test_auxiliary_client_vertex_dispatch.py` (10
cases, all hermetic — mock the credential seams only):
- `TestPluginCatalogFallback` (3): vertex reaches dispatch through
the fallback; aliases (`google-vertex`, `vertex-ai`, `gcp-vertex`)
resolve; genuinely-unknown providers still bail.
- `TestVertexGeminiDispatch` (6): `google/`-prefixed model builds an
OpenAI client with the right base_url + token; no-model default;
bare `gemini-*` still routes to the Gemini handler (Vertex's
404 for the missing publisher stays the loud-fail diagnostic);
credential / token failure paths; async wrapper.
- `TestHistoricalRegression` (1): pins the invariant with `vertex`
forcibly removed from `PROVIDER_REGISTRY`, so a future refactor
of the `auth.py` auto-extension filter cannot silently
reintroduce the bug.
No user-facing behaviour change on non-Vertex deployments.
Duplicate of #56861 (earlier, still open) — both patch the |
teknium1
left a comment
There was a problem hiding this comment.
Thanks for isolating the current registry-miss defect: on main, agent/auxiliary_client.py:4858-4865 returns before the existing Vertex branch at :5012, while hermes_cli/auth.py:455 deliberately excludes the Vertex profile from the API-key registry.
Problems
- The fallback at
agent/auxiliary_client.py:4885-4892accepts allaws_sdkand OAuth profile types, but the downstream implementations are provider-specific: the AWS path is Bedrock-specific (:5048) and OAuth only handles named providers (:5094-5107). Provider profiles are user-overridable (providers/__init__.py:53-62), so constrain this fallback to canonical Vertex handling unless generic dispatch exists. - Cached Vertex clients cannot recover from an expired OAuth token. The auth-retry paths call
_refresh_provider_credentials()(agent/auxiliary_client.py:6743-6773,:7291-7320), but that helper has no Vertex branch (:3488-3558), and auto-route endpoint inference omits Vertex (:3561-3583).
Suggested changes
- Scope the profile fallback to Vertex (including aliases), and add sync/async stale-token refresh coverage plus an
_resolve_auto()regression test.
Automated hermes-sweeper review.
Addresses @teknium1 review feedback on this PR: 1. The plugin-catalog fallback allow-list was too broad. Narrow it from {vertex, aws_sdk, oauth_device_code, oauth_external} to {vertex} only. The downstream branches for the removed auth families are provider-specific rather than generic: - auth_type == aws_sdk builds Bedrock-specific clients (AnthropicBedrockClient / BedrockAuxiliaryClient); a future non-Bedrock aws_sdk profile would misroute here. - auth_type in {oauth_device_code, oauth_external} matches by provider name (nous / openai-codex / xai-oauth) inside the elif chain; any third-party OAuth profile with a novel name falls through to the not-directly-supported branch. Provider profiles are user-overridable (last-writer-wins in providers/__init__.py), so a user plugin re-declaring one of those auth types would land in a branch that cannot build its client. Vertex is the one auth family with genuinely generic downstream dispatch (OAuth2 token + OpenAI-compat endpoint), and its aliases (google-vertex, vertex-ai, gcp-vertex) resolve through the same handler unchanged. 2. Cached Vertex clients could not recover from a stale token. _refresh_provider_credentials() had no vertex branch — auth-retry paths would call it, get False back, and the aux task would fail permanently until process restart. Add a vertex branch that: - clears vertex_adapter._creds_cache so google-auth re-mints, - re-resolves via get_vertex_config() to verify a fresh token can be produced, - evicts cached aux clients so they pick up the new token. 3. Auto-routed aux calls could not infer provider=vertex from the selected clients base URL. Extend _auth_refresh_provider_for_route to recognise both host shapes: - aiplatform.googleapis.com (global location) - {region}-aiplatform.googleapis.com (regional) base_url_host_matches uses strict subdomain-of matching, which would miss the regional case (no dot between region and aiplatform), so match on hostname suffix directly. Tests: extend tests/agent/test_auxiliary_client_vertex_dispatch.py with three new classes covering the requested regression areas: - TestResolveAutoVertex: end-to-end via _resolve_auto Step 1 with _read_main_provider=vertex, _read_main_model=google/gemini-*. Confirms the fallback is reachable through the full aux chain, not just direct resolve_provider_client entry. - TestRefreshProviderCredentialsVertex (3 cases): cache-clear + aux-client eviction on success, False on mint failure, graceful bail when vertex_adapter is unimportable. - TestAuthRefreshProviderRouteVertex (4 cases): global + regional hosts return vertex; look-alike host does not; concrete resolved provider wins over URL inference. Suite green: 18/18 in the vertex-dispatch file, 484/484 across aux client + vertex/bedrock adapter surface via scripts/run_tests.sh.
|
@teknium1 Thanks for the review - both fixed. LMK if you prefer me to remove the long comment in the code. |
What does this PR do?
Fixes a pre-existing bug in
agent/auxiliary_client.py::resolve_provider_clientthat silently disabled every auxiliary task (vision_analyze, context compression,session_search, curator's LLM review pass,title_generation,web_extract) on anyprovider: vertexdeployment.The main-chat surface is unaffected because
hermes_cli/runtime_provider.py::resolve_runtime_providerspecial-cases thevertexname up front. Auxiliary tasks all go throughresolve_provider_clientand inherit the bug.Failure signatures:
vision_analyzeis tool-gated:check_vision_requirements()uses the same resolver, getsNone, returnsFalse, so the tool is stripped from the model-facing schema. The agent narrates its own confusion at not finding the tool.RuntimeError("No LLM provider configured for task=<task> provider=auto. Run: hermes setup").OPENROUTER_API_KEYset, or a Nous auth token cached) don't notice — Step 3 of_resolve_autocatches the miss. Vertex-only deployments hit it terminally.Bug is Vertex-wide, not Anthropic-specific. Reproduces on plain
provider: vertex, default: "google/gemini-3.1-pro-preview"against currentmain.Related Issue
provider: vertex#61852Type of Change
Non-breaking. The change makes an existing (dead-code) handler reachable; nothing that previously worked can regress. Verified with a
TestHistoricalRegressioncase that pins the invariant against future refactors ofhermes_cli/auth.py'sPROVIDER_REGISTRYfilter.Root Cause
Two independent issues stack.
Issue 1 —
PROVIDER_REGISTRYauto-extension filter excludes non-api_key providers.hermes_cli/auth.pyiterates the plugin catalog and adds providers toPROVIDER_REGISTRY, but only ifauth_type == "api_key"andenv_varsis non-empty:The
vertexProviderProfile(plugins/model-providers/vertex/__init__.py) declaresauth_type="vertex"(OAuth2 via ADC — no static key) andenv_vars=(). Both clauses reject it, soPROVIDER_REGISTRY.get("vertex") is None.Issue 2 — the
elif pconfig.auth_type == "vertex":handler is unreachable dead code.agent/auxiliary_client.py::resolve_provider_client:_resolve_auto's Step 1 correctly callsresolve_provider_client(resolved_provider, main_model, …)per its documented intent ("use my main chat model for side tasks as well"). Forvertex, that call returns(None, None), so the chain falls through to Step 3's aggregators — which usually have no credentials on a Vertex-only deployment, and the whole chain terminates with the RuntimeError.Same shape applies to any future non-api_key provider profile.
Changes Made
One hunk in
agent/auxiliary_client.py. Nohermes_cli/auth.pytouch — keepingPROVIDER_REGISTRYapi-key-only preserves invariants elsewhere in the codebase.Plugin-catalog fallback for
pconfig=NoneRight where the
pconfig is Nonebail sits today, add a fallback that consults the plugin catalog directly. If the missing provider resolves viaproviders.get_provider_profile(name)and itsauth_typeis one of the well-known non-api_key families (vertex,aws_sdk,oauth_device_code,oauth_external), synthesize aSimpleNamespace(auth_type=<profile.auth_type>)and let the existing dispatch branches downstream fire. Genuinely unknown providers still bail cleanly with the unchanged "unknown provider" debug log.This makes the existing
elif pconfig.auth_type == "vertex":handler reachable for Vertex users. The handler was already written to build anopenai.OpenAIclient pointed at Vertex's OpenAI-compat endpoint with a fresh OAuth2 bearer token — exactly what Gemini traffic needs. The same fallback also opens the door for any other non-api_key provider profile (aws_sdk / bedrock, oauth_*) to be reached the same way.How to Test
Focused test file:
scripts/run_tests.sh tests/agent/test_auxiliary_client_vertex_dispatch.py -q # ─── 10/10 pass in ~1s.The 10 cases cover:
TestPluginCatalogFallback(3) — vertex reaches dispatch through the fallback; aliases (google-vertex,vertex-ai,gcp-vertex) resolve too; genuinely-unknown providers still bail.TestVertexGeminiDispatch(6) —google/-prefixed model builds anopenai.OpenAIclient with the right base_url + token; no-model default picks up the aux Gemini slug; baregemini-*still routes to the Gemini handler (Vertex's 404 for the missing publisher stays the loud-fail diagnostic); missing-credentials / missing-oauth-token paths bail; async wrapper.TestHistoricalRegression(1) — pins the invariant withvertexforcibly removed fromPROVIDER_REGISTRY, so a future refactor of theauth.pyauto-extension filter cannot silently re-break the aux path.Broader regression sweep (auxiliary + vision + compression + session_search + config bridge + related adapters):
scripts/run_tests.sh tests/agent/test_auxiliary_client_vertex_dispatch.py \ tests/agent/test_auxiliary_client.py \ tests/agent/test_auxiliary_main_first.py \ tests/agent/test_auxiliary_client_azure_foundry.py \ tests/agent/test_auxiliary_client_anthropic_custom.py \ tests/agent/test_auxiliary_client_resolve_dedup.py \ tests/agent/test_vertex_adapter.py \ tests/agent/test_bedrock_adapter.py \ tests/agent/test_bedrock_integration.py \ tests/tools/test_vision_tools.py \ tests/agent/test_context_compressor.py \ tests/tools/test_session_search.py \ tests/agent/test_auxiliary_config_bridge.py \ -q # ─── 13 files, 865/865 tests pass in ~20s.Real-world validation. Deployed on a Vertex-only GCE fleet (NixOS, ADC via VM service-account, both Gemini-on-Vertex and Anthropic-on-Vertex configurations). Pre-fix, every auxiliary task terminated with the
RuntimeErrorabove; post-fix, every task round-trips through the Vertex handler and returns real content in <10s.vision_analyzeon real image uploads via the messaging gateway round-trips cleanly (pre-fix: the tool was silently absent from the model's tool schema).Manual reproduction on a fresh checkout (no Anthropic setup needed — this reproduces the bug on plain Gemini-on-Vertex against
main):Checklist
Code
fix(auxiliary_client): …).scripts/run_tests.shon the affected files (all pass).tests/agent/test_auxiliary_client_vertex_dispatch.py, including aTestHistoricalRegressionthat pins the aux-dispatch invariant against future refactors ofhermes_cli/auth.py.Documentation & Housekeeping
cli-config.yaml.example— N/A.CONTRIBUTING.md/AGENTS.md— N/A.Screenshots / Logs
Pre-fix INFO-level agent.log signature on a Vertex-only deployment (every turn):
Post-fix INFO log line that never fired before the fix: