refactor(moa): unify slot provider-identity on the single call_llm chokepoint - #89
Merged
Conversation
|
Review Complete Risk: 🟡 Medium (35/100) — 1 medium finding · 99 LOC across 2 files Files Reviewed: 2 By Severity:
The PR refactors MoA loop slot resolution, removing a duplicate provider whitelist from moa_loop.py and delegating all identity preservation to _preserve_provider_with_base_url. One defense-in-depth gap noted: the hardcoded fallback set in auxiliary_client.py omits 'bedrock', which was previously covered by the removed whitelist. Files Reviewed (2 files) |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
MoA slot provider-identity is now decided in one place.
_slot_runtimeforwards the resolved endpoint for every slot;_resolve_task_provider_modelis the single chokepoint that keeps a first-class provider's identity instead of collapsing an explicitbase_urlto the genericcustomroute.This retires the hand-maintained name-preservation whitelist in
agent/moa_loop.pythat had to be extended by hand for every provider with custom auth/signing — the exact drift that produced the recent anthropic (NousResearch#54609) and bedrock (NousResearch#54912) breakages.Root cause of the duplication
Two guards were enforcing the same invariant:
moa_loop.py::_slot_runtime— a fixed set{nous, anthropic, openai-codex, xai-oauth, bedrock}that returned bareprovider+modelsocall_llmnever saw abase_urlto collapse.auxiliary_client.py::_resolve_task_provider_model—_preserve_provider_with_base_url(provider), a provider-catalog capability check that keeps identity for any first-class provider even when abase_urlis present.The second is strictly more general (auto-tracks the catalog); the first was a manual denylist of the collapse that drifted out of sync.
Change
_slot_runtimeforwardsbase_url/api_key/api_modeunconditionally; whitelist removed._slot_runtime→_resolve_task_provider_modelpreserves identity) against the real resolver, instead of the whitelist's bare-return shape.Why it's behaviorally safe
The five providers'
resolve_provider_clientbranches re-resolve their own credentials by name and ignore a forwardedbase_url/api_key:openai-codex→_read_codex_access_token()(Responses wrap + Cloudflare headers)xai-oauth→_build_xai_oauth_aux_client()bedrock(aws_sdk) → boto3 credential chain + SigV4 signing; forwarded placeholder"aws-sdk"key is ignoredanthropic→_try_anthropic()(OAuth-token detection → Bearer +anthropic-beta)nous→ catalog api_key provider, re-resolves the same endpointValidation
_slot_runtime→_resolve_task_provider_modelfor openai-codex / xai-oauth / bedrock / anthropic / nous (+openrouter control)customtests/run_agent/test_moa_loop_mode.pytests/agent/test_auxiliary_client.pyInfographic
Mirror-of: NousResearch#55991
NousResearch#55991