From 62840537e5fcd1de9351c736390905aac8f67f69 Mon Sep 17 00:00:00 2001 From: AmirX0 Date: Tue, 18 Aug 2026 00:47:14 +0000 Subject: [PATCH] Bind dedupe LLM credentials to the model instead of extra_args A dedicated dedupe model (STRIX_DEDUPE_MODEL) carried its own DEDUPE_LLM_API_KEY / DEDUPE_LLM_API_BASE through ModelSettings.extra_args. LitellmModel already passes api_key= to litellm.acompletion() explicitly and then splats extra_args into the same call, so the key arrived twice and every dedupe request died before it was sent: TypeError: litellm.main.acompletion() got multiple values for keyword argument 'api_key' The OpenAI route was broken the same way: chat.completions.create() has no api_key parameter at all. Credentials now ride on the model rather than on the request. StrixProvider takes optional api_key/api_base, forwarding them to MultiProvider for the OpenAI route and binding them onto the LitellmModel for the LiteLLM route, which is what those constructor arguments are for. dedupe_model_provider() builds that provider, so a dedicated dedupe model keeps its endpoint and key separate from the main model's process-wide config exactly as before. Fixes #1095 --- strix/config/models.py | 35 ++++++++++++++++- strix/interface/main.py | 14 ++----- strix/report/dedupe.py | 38 +++++++++--------- tests/test_dedupe_model.py | 80 ++++++++++++++++++++++++++++++++------ 4 files changed, 127 insertions(+), 40 deletions(-) diff --git a/strix/config/models.py b/strix/config/models.py index e632bb06..82231209 100644 --- a/strix/config/models.py +++ b/strix/config/models.py @@ -445,12 +445,43 @@ def _response_usage(usage: Usage | None) -> ResponseUsage | None: ) +def _bind_route_credentials(model: Model, api_key: str | None, api_base: str | None) -> Model: + """Bind a route's own credential and endpoint to a LiteLLM model instance. + + ``LitellmModel`` reads both off itself and forwards them to + ``litellm.acompletion`` as explicit keyword arguments, so the same values + passed through ``ModelSettings.extra_args`` would reach that call twice and + raise ``got multiple values for keyword argument 'api_key'``. The OpenAI + route is covered by ``MultiProvider``'s own client settings instead. + """ + if api_key is None and api_base is None: + return model + + from agents.extensions.models.litellm_model import LitellmModel + + if isinstance(model, LitellmModel): + if api_key is not None: + model.api_key = api_key + if api_base is not None: + model.base_url = api_base + return model + + class StrixProvider(MultiProvider): """Route any non-OpenAI prefix through LiteLLM with the prefix preserved, so users type ``deepseek/deepseek-chat`` rather than ``litellm/deepseek/deepseek-chat``. + + ``api_key``/``api_base`` scope a credential and endpoint to the models this + provider hands out, for routes that must not inherit the process-wide LLM + config. Omit them to keep using the global configuration. """ + def __init__(self, *, api_key: str | None = None, api_base: str | None = None) -> None: + super().__init__(openai_api_key=api_key, openai_base_url=api_base) + self._api_key = api_key + self._api_base = api_base + def _resolve_prefixed_model( self, *, @@ -482,7 +513,9 @@ class StrixProvider(MultiProvider): reasoning_effort=llm.reasoning_effort, ) else: - model = super().get_model(model_name) + model = _bind_route_credentials( + super().get_model(model_name), self._api_key, self._api_base + ) if llm.disable_streaming: model = _NonStreamingModel(model) # The wrapper emits its single event only once the whole request diff --git a/strix/interface/main.py b/strix/interface/main.py index 06966f4c..b04387bc 100644 --- a/strix/interface/main.py +++ b/strix/interface/main.py @@ -127,12 +127,10 @@ def _subscription_error_hint(exc: BaseException) -> str | None: async def warm_up_llm(show_model_warning: bool = True) -> None: - from agents.model_settings import ModelSettings from agents.models.interface import ModelTracing from strix.config.models import ( RECOMMENDED_MODEL_NAMES, - StrixProvider, configure_sdk_model_defaults, is_known_openai_bare_model, is_recommended_or_frontier_model, @@ -209,15 +207,14 @@ async def warm_up_llm(show_model_warning: bool = True) -> None: logger.info("LLM warm-up succeeded for model %s", (llm.model or "").strip()) if settings.dedupe.model: - from strix.report.dedupe import _dedupe_extra_args + from strix.report.dedupe import dedupe_model_provider dedupe_model = settings.dedupe.model.strip() raw_model = dedupe_model - deduper = StrixProvider().get_model(dedupe_model) - deduper_extra = _dedupe_extra_args(settings.dedupe) # A dedicated dedupe model may route to another provider, which must - # never receive the main endpoint's headers; it has its own - # DEDUPE_LLM_EXTRA_HEADERS. + # never receive the main endpoint's credentials or headers; it has + # its own DEDUPE_LLM_* settings, bound to the model by the provider. + deduper = dedupe_model_provider(settings.dedupe).get_model(dedupe_model) deduper_settings = make_model_settings( None, model_name=dedupe_model, @@ -226,9 +223,6 @@ async def warm_up_llm(show_model_warning: bool = True) -> None: extra_headers=settings.dedupe.extra_headers, has_tools=False, ) - if deduper_extra: - merged = {**(deduper_settings.extra_args or {}), **deduper_extra} - deduper_settings = deduper_settings.resolve(ModelSettings(extra_args=merged)) await asyncio.wait_for( deduper.get_response( system_instructions="You are a helpful assistant.", diff --git a/strix/report/dedupe.py b/strix/report/dedupe.py index 1cc0a66a..0a13cdf8 100644 --- a/strix/report/dedupe.py +++ b/strix/report/dedupe.py @@ -7,7 +7,6 @@ import logging import re from typing import TYPE_CHECKING, Any -from agents.model_settings import ModelSettings from agents.models.interface import ModelTracing from openai.types.responses import ResponseOutputMessage @@ -22,6 +21,7 @@ from strix.report.state import get_global_report_state if TYPE_CHECKING: from agents.items import ModelResponse + from agents.model_settings import ModelSettings from strix.config.settings import DedupeSettings @@ -29,30 +29,36 @@ if TYPE_CHECKING: logger = logging.getLogger(__name__) -def _dedupe_extra_args(dedupe: DedupeSettings) -> dict[str, str]: - """Per-call credential + endpoint for the dedupe model. +def dedupe_model_provider(dedupe: DedupeSettings) -> StrixProvider: + """Provider carrying the dedupe model's own credential + endpoint. Provider env vars and the global base URL are process-wide, so a shared-provider dedupe key or a distinct dedupe endpoint can't be installed globally without clobbering (or being clobbered by) the main model's - config. Passing them per call keeps the two apart. Only applies when a - dedicated dedupe model is configured. + config. Binding them to the dedupe model keeps the two apart. They must + ride on the model rather than on ``ModelSettings.extra_args``: LiteLLM + already receives an explicit ``api_key`` on every call, so an ``extra_args`` + copy arrives as a duplicate keyword argument and the call fails before it + is sent. Only applies when a dedicated dedupe model is configured. """ if not dedupe.model: - return {} - extra: dict[str, str] = {} - if dedupe.api_key and dedupe.api_key.strip(): - extra["api_key"] = dedupe.api_key.strip() - if dedupe.api_base and dedupe.api_base.strip(): - extra["api_base"] = dedupe.api_base.strip() - return extra + return StrixProvider() + return StrixProvider( + api_key=_stripped_or_none(dedupe.api_key), + api_base=_stripped_or_none(dedupe.api_base), + ) + + +def _stripped_or_none(value: str | None) -> str | None: + stripped = (value or "").strip() + return stripped or None def _dedupe_model_settings( dedupe: DedupeSettings, model_name: str, request_timeout: float | None ) -> ModelSettings: llm = load_settings().llm - settings = make_model_settings( + return make_model_settings( dedupe.reasoning_effort, model_name=model_name, force_required_tool_choice=False, @@ -64,10 +70,6 @@ def _dedupe_model_settings( extra_headers=dedupe.extra_headers if dedupe.model else llm.extra_headers, has_tools=False, ) - extra = _dedupe_extra_args(dedupe) - if extra: - settings = settings.resolve(ModelSettings(extra_args=extra)) - return settings DEDUPE_SYSTEM_PROMPT = """You are an expert vulnerability report deduplication judge. @@ -371,7 +373,7 @@ async def check_duplicate( configure_sdk_model_defaults(settings) resolved_model = model_name.strip() - model = StrixProvider().get_model(resolved_model) + model = dedupe_model_provider(dedupe).get_model(resolved_model) response = await model.get_response( system_instructions=DEDUPE_SYSTEM_PROMPT, input=user_msg, diff --git a/tests/test_dedupe_model.py b/tests/test_dedupe_model.py index b17946e9..0455214d 100644 --- a/tests/test_dedupe_model.py +++ b/tests/test_dedupe_model.py @@ -5,9 +5,13 @@ from __future__ import annotations import json from typing import TYPE_CHECKING +from agents.extensions.models.litellm_model import LitellmModel +from agents.model_settings import ModelSettings +from agents.models.interface import ModelTracing + from strix.config import loader from strix.config.settings import DedupeSettings -from strix.report.dedupe import _dedupe_model_settings +from strix.report.dedupe import _dedupe_model_settings, dedupe_model_provider if TYPE_CHECKING: @@ -16,12 +20,31 @@ if TYPE_CHECKING: import pytest -def test_dedupe_key_sent_per_call_not_via_global_env() -> None: +def _litellm_route(dedupe: DedupeSettings, model_name: str) -> LitellmModel: + model = dedupe_model_provider(dedupe).get_model(model_name) + while not isinstance(model, LitellmModel): + model = model._inner # type: ignore[attr-defined] + return model + + +def test_dedupe_key_bound_to_model_not_via_global_env() -> None: dedupe = DedupeSettings(STRIX_DEDUPE_MODEL="deepseek/cheap", DEDUPE_LLM_API_KEY="dedupe-key") + # The key rides on the dedupe model, so a shared-provider main key can't + # clobber it (and vice versa) through the global provider env var. + assert _litellm_route(dedupe, "deepseek/cheap").api_key == "dedupe-key" + + +def test_dedupe_credentials_never_ride_on_extra_args() -> None: + # LiteLLM already receives an explicit api_key on every call; a copy in + # extra_args reaches acompletion() twice and fails the call outright. + dedupe = DedupeSettings( + STRIX_DEDUPE_MODEL="deepseek/cheap", + DEDUPE_LLM_API_KEY="dedupe-key", + DEDUPE_LLM_API_BASE="https://dedupe.example/v1", + ) settings = _dedupe_model_settings(dedupe, "deepseek/cheap", 300) - # The key rides on the request, so a shared-provider main key can't clobber - # it (and vice versa) through the global provider env var. - assert (settings.extra_args or {})["api_key"] == "dedupe-key" + assert "api_key" not in (settings.extra_args or {}) + assert "api_base" not in (settings.extra_args or {}) def test_dedupe_settings_omit_api_key_when_unset() -> None: @@ -29,19 +52,54 @@ def test_dedupe_settings_omit_api_key_when_unset() -> None: settings = _dedupe_model_settings(dedupe, "deepseek/cheap", 300) assert "api_key" not in (settings.extra_args or {}) assert "api_base" not in (settings.extra_args or {}) + assert _litellm_route(dedupe, "deepseek/cheap").api_key is None -def test_dedupe_endpoint_sent_per_call() -> None: +def test_dedupe_endpoint_bound_to_model() -> None: dedupe = DedupeSettings( - STRIX_DEDUPE_MODEL="openai/cheap", + STRIX_DEDUPE_MODEL="deepseek/cheap", DEDUPE_LLM_API_KEY="dedupe-key", DEDUPE_LLM_API_BASE="https://dedupe.example/v1", ) - settings = _dedupe_model_settings(dedupe, "openai/cheap", 300) - # A distinct dedupe endpoint rides on the request instead of the + # A distinct dedupe endpoint rides on the dedupe model instead of the # process-wide base URL, so it can't clobber the main model's endpoint. - assert (settings.extra_args or {})["api_base"] == "https://dedupe.example/v1" - assert (settings.extra_args or {})["api_key"] == "dedupe-key" + route = _litellm_route(dedupe, "deepseek/cheap") + assert route.base_url == "https://dedupe.example/v1" + assert route.api_key == "dedupe-key" + + +def test_fallback_dedupe_model_keeps_global_credentials() -> None: + # Without a dedicated dedupe model the main model's global config applies. + route = _litellm_route(DedupeSettings(DEDUPE_LLM_API_KEY="dedupe-key"), "deepseek/cheap") + assert route.api_key is None + assert route.base_url is None + + +async def test_dedupe_call_does_not_duplicate_litellm_api_key() -> None: + """Regression for #1095: the dedupe call reached litellm with two api_key values.""" + dedupe = DedupeSettings( + STRIX_DEDUPE_MODEL="deepseek/cheap", + DEDUPE_LLM_API_KEY="dedupe-key", + DEDUPE_LLM_API_BASE="https://dedupe.example/v1", + ) + model = dedupe_model_provider(dedupe).get_model("deepseek/cheap") + settings = _dedupe_model_settings(dedupe, "deepseek/cheap", 300) + settings = settings.resolve( + ModelSettings(extra_args={**(settings.extra_args or {}), "mock_response": "OK"}) + ) + response = await model.get_response( + system_instructions="You are a helpful assistant.", + input="Reply with just 'OK'.", + model_settings=settings, + tools=[], + output_schema=None, + handoffs=[], + tracing=ModelTracing.DISABLED, + previous_response_id=None, + conversation_id=None, + prompt=None, + ) + assert response.output def test_dedicated_dedupe_model_uses_own_headers_not_main() -> None: