fix(auxiliary): route minimax-oauth provider through dedicated helper - #36779
fix(auxiliary): route minimax-oauth provider through dedicated helper#36779VIPKaiser wants to merge 1 commit into
Conversation
resolve_provider_client() had early branches for the nous, openai-codex,
and xai-oauth OAuth providers, but no branch for minimax-oauth. When a
user configured auxiliary.<task>.provider = minimax-oauth (as is
recommended for MiniMax OAuth subscribers), the call fell through to the
generic auth_type dispatch, which only handles oauth_device_code /
oauth_external — not the oauth_minimax auth_type that MiniMax uses. The
fallback logged an unhandled-auth-type warning and returned (None, None),
which the caller then translated into the misleading:
RuntimeError: Provider 'minimax-oauth' is set in config.yaml but no
API key was found. Set the MINIMAX-OAUTH_API_KEY environment variable,
or switch to a different provider with `hermes model`.
…even though the user was correctly authenticated.
This commit:
1. Adds a _build_minimax_oauth_aux_client() helper that pulls a fresh
access token from build_minimax_oauth_token_provider() on every
request (MiniMax issues short-lived tokens, ~15min TTL, so a static
string captured at client construction would expire mid-session).
2. Adds an early branch in resolve_provider_client() that routes
provider='minimax-oauth' through the new helper — parallel to the
existing nous / openai-codex / xai-oauth branches.
3. Adds a defensive elif pconfig.auth_type == 'oauth_minimax' arm in
the generic dispatch as belt-and-braces protection against future
refactors that might remove the early branch.
4. Rewrites the Anthropic-Messages base URL to the OpenAI-compatible
/v1/chat/completions path so auxiliary tasks can use the bare OpenAI
client (the main agent path uses the Anthropic transport, but
auxiliaries work fine over the OpenAI route and that's what the
existing minimax config.yaml examples show).
Tests:
tests/agent/test_auxiliary_client_minimax_oauth.py — 8 cases covering
the happy path, not-logged-in, callable-token refresh, and regression
on the misleading 'no API key' error message.
Live verification:
resolve_provider_client('minimax-oauth', 'MiniMax-M3') now returns a
working OpenAI client (base_url=https://api.minimax.io/v1/), and a
call_llm() round-trip with the same prompt the title_generator uses
produces a real ChatCompletion response.
mxnstrexgl
left a comment
There was a problem hiding this comment.
🤖 Automated PR Review
Security Scan
- ✓ No hardcoded secrets, injection sinks, unsafe deserialization, or dependency red flags found by this automated scan.
Code Quality
- ✓ No blocking code-quality issues found by this automated scan.
Summary
Status: APPROVE — security findings: 0, quality suggestions: 0.
Automated review; raw diff content intentionally omitted.
|
I verified this branch locally while triaging #38685. Focused checks pass in an isolated worktree: python -m pytest tests/agent/test_auxiliary_client_minimax_oauth.py -q
# 8 passed
python -m ruff check agent/auxiliary_client.py tests/agent/test_auxiliary_client_minimax_oauth.py
# All checks passed
git diff --check origin/main...HEAD
# passedThis PR looks like the existing fix coverage for #38685 / #21521 from the focused regression angle. GitHub currently reports no checks on the branch, but the local focused verification is clean. |
teknium1
left a comment
There was a problem hiding this comment.
Thanks for isolating a real auxiliary-routing gap. Current main still registers minimax-oauth as oauth_minimax (hermes_cli/auth.py:301-310), while resolve_provider_client() only routes oauth_device_code and oauth_external (agent/auxiliary_client.py:5106-5119).
Problems
- The proposed raw OpenAI
/v1route bypasses the current MiniMax OAuth transport path. The provider declaresanthropic_messages(plugins/model-providers/minimax/__init__.py:82-92), and the callable credential is designed forbuild_anthropic_client()'s per-request bearer hook (hermes_cli/auth.py:7779-7823,agent/anthropic_adapter.py:745-751). - The source-inspection regression test is implementation-coupled rather than proving the authenticated client and transport behavior.
Suggested changes
- Port the route onto current
agent/auxiliary_client.py, usingresolve_minimax_oauth_runtime_credentials(as_token_provider=True)plusbuild_anthropic_client()andAnthropicAuxiliaryClient. - Replace the source-grep assertion with a mocked behavioral routing/transport test.
This is an automated hermes-sweeper review.
| # ``hermes model -> MiniMax (OAuth)``. | ||
| if provider == "minimax-oauth": | ||
| client, default = _build_minimax_oauth_aux_client(model) | ||
| if client is None: |
There was a problem hiding this comment.
Please preserve the current MiniMax OAuth transport rather than returning a raw OpenAI /v1 client here. Current main declares this provider anthropic_messages and uses build_anthropic_client() to install the callable credential as a per-request bearer hook (hermes_cli/auth.py:7779-7823; agent/anthropic_adapter.py:745-751). Build and return the Anthropic auxiliary adapter from that established path instead.
Problem
Users with MiniMax OAuth authentication (configured via
hermes model -> MiniMax (OAuth)) cannot use it for auxiliary tasks (compression, title_generation, web_extract, etc.). Anyauxiliary.<task>.provider: minimax-oauthconfig inconfig.yamlproduces:This is misleading: the user is correctly authenticated; the failure is in the dispatch.
Root cause
resolve_provider_client()inagent/auxiliary_client.pyhas early branches for the other OAuth providers:…but no branch for
minimax-oauth. So requests fall through to the genericpconfig.auth_type in {"oauth_device_code", "oauth_external"}arm, which doesn't listoauth_minimax(the auth_type the MiniMax ProviderConfig uses). It hits the finalunhandled auth_type oauth_minimaxwarning, returns(None, None), and the caller raises the misleading error.Fix
_build_minimax_oauth_aux_client()— plumbs the MiniMax OAuth token provider callable (build_minimax_oauth_token_provider()) into the OpenAI client so a fresh access token is minted per request (MiniMax issues short-lived tokens, ~15min TTL).resolve_provider_client()— parallel to the nous/openai-codex/xai-oauth branches, routesprovider='minimax-oauth'to the new helper.elif pconfig.auth_type == 'oauth_minimax'arm — belt-and-braces: if someone refactors out the early branch in the future, the auth_type fallback still works./anthropicendpoint works over/v1/chat/completionstoo, so auxiliary tasks can use the bare OpenAI client (matching the existingminimaxapi-key path).Tests
8 new test cases in
tests/agent/test_auxiliary_client_minimax_oauth.py:test_returns_client_when_logged_in— happy pathtest_returns_client_with_model_fallback— caller passes no modeltest_returns_none_when_not_logged_in— clean (None, None) when auth.json has no MiniMax entrytest_returns_none_when_creds_resolution_raises— AuthError handledtest_dispatches_to_minimax_oauth_helper— confirm early-branch routingtest_returns_none_cleanly_when_helper_returns_none— no false-positive errortest_oauth_minimax_auth_type_branch_exists— defensive source-grep on the auth_type fallbacktest_does_not_raise_api_key_error_when_logged_in— regression test for the original misleading error stringAll 388 auxiliary/minimax tests pass; the 5 unrelated failures elsewhere in
tests/agent/are pre-existing (verified by running on stashed/clean main).Live verification
End-to-end call to MiniMax via the new auxiliary path returns a real ChatCompletion.
Risk
auxiliary_client.pyis on the hot path for every auxiliary call (compression, web_extract, title_generation, curator, profile_describer, kanban_decomposer, triage_specifier, approval, mcp, skills_hub, vision). The new code is contained to one early branch and one helper function, both gated onprovider == 'minimax-oauth'. The defensive auth_type arm only fires for the same provider./anthropic→/v1base_url rewrite is wrapped in a try/except so a non-matching base URL silently keeps the original (and would surface the same Anthropic 404 the user might have been seeing before).Checklist