fix(registry): add vertex to PROVIDER_REGISTRY so auxiliary clients can use ADC - #66707
fix(registry): add vertex to PROVIDER_REGISTRY so auxiliary clients can use ADC#66707xxiaoxiong wants to merge 1 commit into
Conversation
…an use ADC (NousResearch#66674) Vertex AI ADC (auth_type="vertex") was unreachable for auxiliary clients because the auto-extend loop in PROVIDER_REGISTRY skips non-api-key providers. Mirroring the bedrock approach by adding a static "vertex" entry with empty api_key_env_vars and auth_type="vertex" lets resolve_provider_client() reach the existing ADC token branch at agent/auxiliary_client.py:5215 instead of returning (None, None).
tonydwb
left a comment
There was a problem hiding this comment.
Code Review Summary
Verdict: Comment
+8/-0 adds vertex to PROVIDER_REGISTRY so auxiliary clients can use ADC (Application Default Credentials). Small registry addition. No concerns.
Reviewed by Hermes Agent
|
Thanks for tracing this to the auxiliary registry boundary. The current-main premise is confirmed: Problems
Suggested changes
This is an automated hermes-sweeper review. |
SummaryTwenty-three PRs address or reference the Vertex integration complex, spanning auxiliary ADC reachability, misleading credential errors, model normalization, doctor validation, picker visibility, and CLI overlay resolution. For #66707's reported cause, the relevant diffs either register Vertex so the existing auxiliary OAuth/ADC branch becomes reachable or provide alternative profile/direct dispatch mechanisms; the same registry repair is recorded as integrated from #56688 through #70282. Related pull requests
DuplicatesRegistry/reachability cluster: #66707, #61933, #62444, and closed #62700 overlap the landed registry outcome of #56688, while #56861/#61853 use the competing ProviderProfile-fallback mechanism and closed #62440 uses a direct branch. Normalization: #56805/#56806 overlap #56809; message-only errors: #56863 duplicates #56842; overlays: closed #57541 overlaps #57559 and #56688; closed #68249 duplicates #56688. Suggested consolidationClose #66707 as already implemented on main: contributor follow-up on #56688 identifies cluster PR #70282 commit 9b868f6 as the integration of the same auth_type="vertex" registry repair, so #66707's +8-line diff no longer adds a distinct fix. This departs from the keep_open review on #66707 only because that review evaluated the missing-regression-test PR in isolation before accounting for the recorded landed implementation; close overlapping registry alternatives #61933 and #62444 as duplicates of the landed #56688 outcome, while preserving #62444's recorded best-fix status as evidence that its mechanism correctly addresses #61852/#66674, and keep the distinct #56809, #56861, #56939/#57154/#59679, #57399, #57559, and #61853 salvage paths open for their separately reviewed deltas. Complex graphflowchart 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
I56687(["issue #56687 (closed)"])
I56810(["issue #56810 (open)"])
I61852(["issue #61852 (open)"])
I66674(["issue #66674 (open)"])
subgraph Dup61933 ["PRs duplicating each other"]
P61933["PR #61933 (open)"]
P62700["PR #62700 (closed)"]
P66707["PR #66707 (open)"]
end
P66707 -.->|partial| I56687
P66707 -.->|partial| I56810
P66707 -->|fixes| I61852
P66707 -->|fixes| I66674
class I56687 closed
class I56810 open
class I61852 open
class I66674 open
class P61933 open
class P62700 closed
class P66707 open
class P66707 target
click I56687 "https://github.com/NousResearch/hermes-agent/issues/56687"
click I56810 "https://github.com/NousResearch/hermes-agent/issues/56810"
click I61852 "https://github.com/NousResearch/hermes-agent/issues/61852"
click I66674 "https://github.com/NousResearch/hermes-agent/issues/66674"
click P61933 "https://github.com/NousResearch/hermes-agent/pull/61933"
click P62700 "https://github.com/NousResearch/hermes-agent/pull/62700"
click P66707 "https://github.com/NousResearch/hermes-agent/pull/66707"
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 23 pull requests and 10 issues in this complex. Each diff was read against this issue; Assessment working set: 134 kB of PR diffs, 100 kB of issue/PR text, 40 kB of discussion (60 comments), 86 verify verdicts. verdicts reflect diff content, not PR titles. Part of an automated triage batch. |
|
Closing stale PR — superseded by upstream work / no longer relevant. |
Summary
Fixes #66674.
The auxiliary client (
title_generation,context_compression,memory_flush) cannot use Vertex AI over ADC, even when the main chat agent works fine. Settingauxiliary.<task>.provider: vertexalways falls back to OpenRouter/Nous with the spurious error:Root cause
resolve_provider_client()(agent/auxiliary_client.py, ~L4849) guards with:There is a correct Vertex/ADC branch a bit further down (
agent/auxiliary_client.py:5215), but it is dead code becausevertexis never inPROVIDER_REGISTRY:vertexexists only as a plugin (plugins/model-providers/vertex/) withauth_type="vertex"andenv_vars=().hermes_cli/auth.py:~L452skips non-api_keyproviders:vertexis skipped.Contrast
bedrock(auth_type="aws_sdk",api_key_env_vars=()) — it is hardcoded into the staticPROVIDER_REGISTRYathermes_cli/auth.py:429, so its analogous aux branch is reachable.Fix
Mirror
bedrock: add a staticvertexentry toPROVIDER_REGISTRYwithauth_type="vertex"and emptyapi_key_env_vars. The existingelif pconfig.auth_type == "vertex":branch atagent/auxiliary_client.py:5215then becomes reachable and usesget_vertex_config()to mint an ADC token, identical to the main agent's path.Verification
The auto-extend loop does not overwrite the static entry (it only adds new plugin providers, not replaces).
Checklist
origin/mainbedrockexactly (sameProviderConfigshape)vertexplugin's ownaliasesare still auto-extended via the_PROVIDER_ALIASESloopNotes
This is a low-risk 8-line change to a single dict. No new tests added because exercising
resolve_provider_client(vertex)end-to-end requires GCP ADC, which CI cannot provide. Manual verification above confirms the unblocking.Maintainer consideration:
vertexis the third non-api_keyprovider (afterbedrockand copilot-acp-style OAuth). If more ADC providers appear, a follow-up could relax the auto-extend predicate to includeauth_type in {"vertex", "aws_sdk"}and remove the need for static entries — but that is a larger refactor and out of scope for this defect.