fix(providers): use native query-param auth for Gemini model discovery (#62259) - #62267
PRATHAMESH75 wants to merge 2 commits into
Conversation
Duplicate of #42693 (earliest open PR, filed 2026-06-09). Both override |
teknium1
left a comment
There was a problem hiding this comment.
Thanks for tracing the native Gemini discovery path. The current-main premise is valid: providers/base.py:199-214 uses Bearer auth and OpenAI-style data[].id parsing, while the Gemini profile has no override.
Problems
plugins/model-providers/gemini/__init__.py:42-65applies native?key=auth and nativemodels[].nameparsing to every supplied base URL. Main explicitly supports Gemini's/openaicompatibility base URL inagent/transports/chat_completions.py:92-98; retain the base implementation for that branch and add a regression test.plugins/model-providers/gemini/__init__.py:50,68places the API key in the URL and logs the raw caught exception. Do not log an exception value that may contain that URL; log a safe exception type/category instead.
Suggested changes
- Gate the override to native Gemini endpoints and delegate
/openaidiscovery toProviderProfile.fetch_models(). - Replace raw exception logging with non-secret-safe diagnostics and cover the compatibility branch.
Automated hermes-sweeper review.
| the native inference client already uses) and strip the ``models/`` | ||
| prefix each entry's ``name`` carries so IDs match what inference expects. | ||
| """ | ||
| effective_base = (base_url or self.base_url or "").rstrip("/") |
There was a problem hiding this comment.
This override also receives a configured Gemini /openai base URL, which main explicitly treats as OpenAI-compatible. Do not apply native ?key= auth and models[].name parsing to that branch; gate this path to native endpoints and delegate the compatibility endpoint to super().fetch_models(...).
| ] | ||
| return ids or None | ||
| except Exception as exc: | ||
| logger.debug("fetch_models(gemini): %s", exc) |
There was a problem hiding this comment.
The request URL above contains the API key. Avoid logging the raw exception because URL-bearing exception text can expose that key in debug logs; log a safe category such as type(exc).__name__ instead.
514bc92 to
258c715
Compare
|
Addressed both review problems and rebased onto current
Added regression tests for the compat-branch delegation and for the failure path not leaking the key. |
The native Gemini /v1beta endpoint rejects Bearer auth with HTTP 401, so
ProviderProfile.fetch_models()'s OpenAI-style Authorization header returned
None and the /model picker silently fell back to the 4 static fallback
models instead of the 50+ the account can actually call.
Override fetch_models() in GeminiProfile to hit {base_url}/models?key=<key>
(the same query-param auth the native inference client already uses) and
strip the 'models/' prefix each returned name carries, so live discovery
matches inference.
Fixes NousResearch#62259
…act errors Address review: delegate the OpenAI-compat /openai base URL to ProviderProfile.fetch_models (it speaks Bearer + data[].id) instead of forcing native query-param auth, and stop logging the caught exception value — urllib errors embed the request URL, which carries the api_key in the ?key= query param. Log the exception type only. Add regression tests for the compat-branch delegation and the no-key-leak failure path.
28f395a to
be83c1e
Compare
…ng static xfails #120319, #120374 and #120299 are on main, so their probes and every Gap naming them go (the module's own rule); those cells are plain tests now. The two static strict xfails with an open fix PR (#95375 cli resize_scrollback, fix #120321; #62259 Gemini listing, fix #62267/#116509) turned main red the moment the fix merged (XPASS). They now go through _pending_fixes.known_failure: a run-time xfail only while the cell fails with that gap's own message (a turn rendered more than once; an empty live listing), any other failure stays red, and the fix just makes it pass.
…ng static xfails #120319, #120374 and #120299 are on main, so their probes and every Gap naming them go (the module's own rule); those cells are plain tests now. The two static strict xfails with an open fix PR (#95375 cli resize_scrollback, fix #120321; #62259 Gemini listing, fix #62267/#116509) turned main red the moment the fix merged (XPASS). They now go through _pending_fixes.known_failure: a run-time xfail only while the cell fails with that gap's own message (a turn rendered more than once; an empty live listing), any other failure stays red, and the fix just makes it pass.
What does this PR do?
Fixes live model discovery for the Gemini provider.
hermes model/ the/modelpicker showed only the 4 hard-coded fallback models instead of the 50+ the Google API actually exposes.ProviderProfile.fetch_models()hard-codes OpenAI-style auth:Authorization: Bearer <key>against{base_url}/models. Gemini'sbase_urlis the native endpointhttps://generativelanguage.googleapis.com/v1beta, which rejects Bearer auth with HTTP 401 — it requires the key as a?key=query param. So the probe 401'd,fetch_models()returnedNone, and the picker silently fell back to the static list.GeminiProfilenow overridesfetch_models()to hit{base_url}/models?key=<key>— the same query-param auth the native inference client already uses — and strips themodels/prefix each returnednamecarries, so live discovery matches what inference expects. This is Approach A from the issue (native endpoint), chosen because it reuses the native auth model already in place rather than adding a second endpoint path.Related Issue
Fixes #62259
Type of Change
Changes Made
plugins/model-providers/gemini/__init__.py— addGeminiProfile.fetch_models()override: native{base_url}/models?key=<key>query-param auth, parse the{"models": [{"name": "models/..."}]}shape, strip themodels/prefix. ReturnsNoneon no key / no base_url / any error so callers keep the static fallback.tests/providers/test_fetch_models_base_url.py— regression tests: a fake native handler that 401s on Bearer and honours?key=, asserting the prefix is stripped and IDs returned; plus a no-api-key →Nonecase.How to Test
Result:
8 tests passed, 0 failed(2 new + 6 existing). The newtest_native_query_param_auth_strips_prefixfails against the old code path (Bearer → 401 →None) and passes with the fix.Real-endpoint proof from the issue:
Checklist
Code
fix(scope):,feat(scope):, etc.)pytest tests/ -qand all tests pass (ran the affected suite viascripts/run_tests.sh)Documentation & Housekeeping
docs/, docstrings) — or N/A (behavior fix; docstring added on the override)cli-config.yaml.exampleif I added/changed config keys — or N/ACONTRIBUTING.mdorAGENTS.mdif I changed architecture or workflows — or N/Aurllib, no platform-specific codeCredits
Root-cause analysis and the chosen Approach A (native
/v1betaendpoint with?key=query-param auth) come from @Olegever's report in #62259.