fix(providers): gate plugin api_mode by endpoint (#53054) - #53055
fix(providers): gate plugin api_mode by endpoint (#53054)#53055david-bowiegxw wants to merge 1 commit into
Conversation
…odels anthropic_prompt_cache_policy() only auto-enables caching for third-party Anthropic gateways when the model is Claude-named (is_anthropic_wire and is_claude). Ark speaks the native Anthropic protocol on /api/coding but serves non-Claude families (deepseek-v4, glm-5.2, doubao-seed, kimi-k2, minimax-m3), so it fell through to (False, False) — 0% cache hits, re-billing the full prompt every turn. Same class as NousResearch#17332 (MiniMax's own models); fixed the same way with an explicit allowlist branch under the is_anthropic_wire gate, returning (True, True) for native Anthropic layout. Bundled into this provider PR so adopting Ark gives complete support (routing + caching) in one change; routing depends on the resolver fix in NousResearch#53055. - agent/agent_runtime_helpers.py: add Ark provider/host branch. - tests: TestVolcengineArkAnthropicWire (4 cases incl. OpenAI-wire negative). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
teknium1
left a comment
There was a problem hiding this comment.
Thanks for tracing the profile-to-runtime gap; the current-head premise is valid: ProviderProfile.api_mode exists at providers/base.py:44, while the auto-registration bridge at hermes_cli/auth.py:466-473 does not retain it.
Problems
- The new fallback is only added to the pooled resolver. The explicit API-key route still resolves from config/URL only at
hermes_cli/runtime_provider.py:1493-1501, and the ordinary no-pool API-key route does the same athermes_cli/runtime_provider.py:2039-2062. Those paths still misroute a non-self-describing plugin endpoint. tests/hermes_cli/test_plugin_provider_api_mode.py:152-160mirrors the production bridge while constructingProviderConfig; it does not executeauth.py's registration loop, so it cannot catch a bridge regression.
Suggested changes
- Apply the declared-profile fallback consistently to all three API-key resolver paths, preferably through one helper preserving existing config and URL precedence.
- Add a real discovery → auth registry →
resolve_runtime_providerregression test for the non-self-describing endpoint, including the no-pool path.
Automated hermes-sweeper review.
| @@ -410,6 +410,16 @@ def _resolve_runtime_from_pool_entry( | |||
| detected = _detect_api_mode_for_url(base_url) | |||
| if detected: | |||
| api_mode = detected | |||
| elif pconfig is not None and getattr(pconfig, "api_mode", ""): | |||
There was a problem hiding this comment.
This covers only pooled credentials. _resolve_explicit_runtime and the ordinary no-pool API-key resolver independently apply URL detection and still default to chat_completions; please route all three paths through the same declared-profile fallback.
| auth_type="api_key", | ||
| ) | ||
| # Mirror the bridge in hermes_cli/auth.py | ||
| cfg = ProviderConfig( |
There was a problem hiding this comment.
This duplicates the production bridge rather than invoking it, so the test passes even if auth.py no longer propagates api_mode. Exercise plugin discovery plus the actual auto-registration loop instead.
8be243a to
71f8393
Compare
|
Both points addressed. The branch is rebuilt on current 1. All three API-key resolver paths now share one fallback helper. New 2. The regression test now executes the real chain, not a mirror. Verification: with the fix reverted, the suite fails (1 failed + 6 errors — the bridge drops the declaration and the registration loop isn't callable); with it applied, all 7 pass, and the surrounding suites stay green ( |
|
I incorporated your |
ad55a5c to
e98bd2a
Compare
|
Rebuilt on current What
|
e98bd2a to
2d11d8d
Compare
|
@teknium1 — re-review requested on the new head The pull-request workflows are currently marked This is a clean re-derivation from current The production change is now 13 lines in Verification on current
The PR body now documents how this head relates to #65956 and #52549. In |
2d11d8d to
e2aeb50
Compare
What does this PR do?
Completes the
ProviderProfile.api_modefix on currentmainwithout forcing aplugin's declared transport onto a user-supplied endpoint override.
mainnow bridges profile-only providers throughget_provider()as aProviderDef(source="plugin-profile"), so the old head of this PR is obsolete:the declared transport already reaches
determine_api_mode()and all threeruntime fallback paths. This head is re-derived from current
mainand retainsonly the remaining correctness gap.
Remaining bug on current main
determine_api_mode()currently returns a profile-only provider's transport forevery
base_url, including a different user override. For example, a profilewhose default endpoint is native Anthropic Messages can be redirected to an
OpenAI-compatible
/v1endpoint and still be forced ontoanthropic_messages.That violates the established precedence rule: a different explicit endpoint is
the stronger signal. Recognized hosts remain handled by the host-mandated tier;
an unknown override should retain the conservative
chat_completionsdefault.Fix
determine_api_mode(), apply asource="plugin-profile"transport onlywhen
base_urlis absent or matches the profile's declared endpoint.config, Nous dual-wire routing, and unknown-provider defaults unchanged.
providers/README.mdto describe the current profile-to-runtime path.The endpoint-gating design is adopted from @samfoy's #65956.
Regression coverage
The test installs a real provider plugin beneath the isolated test
HERMES_HOMEbefore the first auth import, then exercises the productionchain:
plugin on disk → discovery →
ProviderProfileregistration → auth registryextension →
get_provider()→determine_api_mode()→ runtime resolution.Coverage includes:
base_urloverride gating and trailing-slash equivalence;Mutation check: removing the new endpoint gate makes the override regression
test fail (
anthropic_messagesinstead ofchat_completions).Verification
Relationship to the overlapping PRs
that scope should be reviewed separately from this endpoint-gating fix.
mainalreadycontains the profile-only
get_provider()bridge this PR now builds on, sothis focused head does not reintroduce that broader identity change.
Fixes #53054.