diff --git a/agent/auxiliary_client.py b/agent/auxiliary_client.py index 64768963ec5b..1eb35249665c 100644 --- a/agent/auxiliary_client.py +++ b/agent/auxiliary_client.py @@ -4940,9 +4940,35 @@ def _finalize(resolved_provider: str, sync_client: Any, default_model: Optional[ main_provider, ) else: + # Custom endpoints (``custom`` / ``custom:``) carry no + # built-in base_url/api_key — resolve_provider_client("custom") + # would return None ("no endpoint credentials found") and the + # whole chain would fall through to the aggregators, breaking + # vision for every user on a custom provider that has no + # separate ``auxiliary.vision`` block. Recover the live main + # endpoint that ``set_runtime_main()`` recorded for this turn so + # Step 1 can build a working client. + rpc_base_url = None + rpc_api_key = None + rpc_api_mode = resolved_api_mode + if main_provider == "custom" or main_provider.startswith("custom:"): + if _RUNTIME_MAIN_BASE_URL: + rpc_base_url = _RUNTIME_MAIN_BASE_URL + rpc_api_key = _RUNTIME_MAIN_API_KEY or None + rpc_api_mode = resolved_api_mode or _RUNTIME_MAIN_API_MODE or None + else: + # No live runtime recorded (non-gateway caller): fall + # back to resolving the configured custom endpoint. + custom_base, custom_key, custom_mode = _resolve_custom_runtime() + if custom_base: + rpc_base_url = custom_base + rpc_api_key = custom_key + rpc_api_mode = resolved_api_mode or custom_mode or None rpc_client, rpc_model = resolve_provider_client( main_provider, vision_model, - api_mode=resolved_api_mode, + api_mode=rpc_api_mode, + explicit_base_url=rpc_base_url, + explicit_api_key=rpc_api_key, is_vision=True) if rpc_client is not None: logger.info( diff --git a/scripts/release.py b/scripts/release.py index 1b3d3a672fe2..4099a6bfbe16 100755 --- a/scripts/release.py +++ b/scripts/release.py @@ -1388,6 +1388,7 @@ "holynn@placeholder.local": "holynn-q", "agent@hermes.local": "jacdevos", "sunsky.lau@gmail.com": "liuhao1024", + "suninrain086@gmail.com": "suninrain086", # PR #57651 salvage of #50685 (vision custom-endpoint creds) "mohamed.origami@gmail.com": "mohamedorigami-jpg", # PR #32117 (cron storage root anchor; #32091) "58446328+sherman-yang@users.noreply.github.com": "sherman-yang", # PR #32788 (cron per-job MCP merge; #23997) "rob@rbrtbn.com": "rbrtbn", diff --git a/tests/agent/test_auxiliary_main_first.py b/tests/agent/test_auxiliary_main_first.py index 94181d468d4d..0b8b0a04462a 100644 --- a/tests/agent/test_auxiliary_main_first.py +++ b/tests/agent/test_auxiliary_main_first.py @@ -543,6 +543,124 @@ def test_explicit_provider_override_still_wins(self): mock_strict.assert_called_once_with("nous", None) +# ── Vision — custom provider endpoint credential passthrough ──────────────── + + +class TestResolveVisionCustomProvider: + """Custom-endpoint mains must forward base_url/api_key to Step 1. + + Regression: a ``custom:`` main provider resolves to the bare + runtime provider id ``"custom"``. ``resolve_provider_client("custom")`` + has no built-in endpoint, so without forwarding the live base_url/api_key + it returns ``(None, None)`` and vision falls through to OpenRouter / Nous, + which an offline / aggregator-less user has never configured — breaking + vision entirely with ``No LLM provider configured for task=vision + provider=auto``. The fix recovers the live endpoint that + ``set_runtime_main()`` recorded for the turn. + """ + + def test_custom_main_forwards_runtime_endpoint(self, monkeypatch): + """custom main with recorded runtime endpoint → Step 1 builds a client.""" + import agent.auxiliary_client as aux + + monkeypatch.setattr(aux, "_RUNTIME_MAIN_BASE_URL", "https://my.endpoint.example/v1") + monkeypatch.setattr(aux, "_RUNTIME_MAIN_API_KEY", "sk-runtime-key") + monkeypatch.setattr(aux, "_RUNTIME_MAIN_API_MODE", "anthropic_messages") + + with patch( + "agent.auxiliary_client._read_main_provider", return_value="custom", + ), patch( + "agent.auxiliary_client._read_main_model", return_value="claude-opus-4-8", + ), patch( + "agent.auxiliary_client._resolve_task_provider_model", + return_value=("auto", None, None, None, None), + ), patch( + "agent.auxiliary_client.resolve_provider_client" + ) as mock_resolve: + mock_client = MagicMock() + mock_resolve.return_value = (mock_client, "claude-opus-4-8") + + from agent.auxiliary_client import resolve_vision_provider_client + + provider, client, model = resolve_vision_provider_client() + + assert provider == "custom" + assert client is mock_client + assert model == "claude-opus-4-8" + # The endpoint credentials recorded for the turn MUST be forwarded, + # otherwise resolve_provider_client("custom") returns (None, None). + kwargs = mock_resolve.call_args.kwargs + assert kwargs.get("explicit_base_url") == "https://my.endpoint.example/v1" + assert kwargs.get("explicit_api_key") == "sk-runtime-key" + assert kwargs.get("is_vision") is True + + def test_custom_prefixed_main_forwards_runtime_endpoint(self, monkeypatch): + """A ``custom:`` provider id also forwards the runtime endpoint.""" + import agent.auxiliary_client as aux + + monkeypatch.setattr(aux, "_RUNTIME_MAIN_BASE_URL", "https://named.example/v1") + monkeypatch.setattr(aux, "_RUNTIME_MAIN_API_KEY", "sk-named") + monkeypatch.setattr(aux, "_RUNTIME_MAIN_API_MODE", "") + + with patch( + "agent.auxiliary_client._read_main_provider", + return_value="custom:copilot-gateway", + ), patch( + "agent.auxiliary_client._read_main_model", return_value="claude-opus-4-8", + ), patch( + "agent.auxiliary_client._resolve_task_provider_model", + return_value=("auto", None, None, None, None), + ), patch( + "agent.auxiliary_client.resolve_provider_client" + ) as mock_resolve: + mock_client = MagicMock() + mock_resolve.return_value = (mock_client, "claude-opus-4-8") + + from agent.auxiliary_client import resolve_vision_provider_client + + provider, client, model = resolve_vision_provider_client() + + assert provider == "custom:copilot-gateway" + assert client is mock_client + kwargs = mock_resolve.call_args.kwargs + assert kwargs.get("explicit_base_url") == "https://named.example/v1" + assert kwargs.get("explicit_api_key") == "sk-named" + assert kwargs.get("is_vision") is True + + def test_custom_main_no_runtime_falls_back_to_configured_endpoint(self, monkeypatch): + """No recorded runtime endpoint → resolve the configured custom endpoint.""" + import agent.auxiliary_client as aux + + monkeypatch.setattr(aux, "_RUNTIME_MAIN_BASE_URL", "") + monkeypatch.setattr(aux, "_RUNTIME_MAIN_API_KEY", "") + monkeypatch.setattr(aux, "_RUNTIME_MAIN_API_MODE", "") + + with patch( + "agent.auxiliary_client._read_main_provider", return_value="custom", + ), patch( + "agent.auxiliary_client._read_main_model", return_value="claude-opus-4-8", + ), patch( + "agent.auxiliary_client._resolve_task_provider_model", + return_value=("auto", None, None, None, None), + ), patch( + "agent.auxiliary_client._resolve_custom_runtime", + return_value=("https://configured.example/v1", "sk-configured", "chat_completions"), + ), patch( + "agent.auxiliary_client.resolve_provider_client" + ) as mock_resolve: + mock_client = MagicMock() + mock_resolve.return_value = (mock_client, "claude-opus-4-8") + + from agent.auxiliary_client import resolve_vision_provider_client + + provider, client, model = resolve_vision_provider_client() + + assert client is mock_client + kwargs = mock_resolve.call_args.kwargs + assert kwargs.get("explicit_base_url") == "https://configured.example/v1" + assert kwargs.get("explicit_api_key") == "sk-configured" + + # ── Constant cleanup ──────────────────────────────────────────────────────── diff --git a/tests/tools/test_vision_tools.py b/tests/tools/test_vision_tools.py index 98bdd2276e10..73cc672a6ed3 100644 --- a/tests/tools/test_vision_tools.py +++ b/tests/tools/test_vision_tools.py @@ -261,6 +261,56 @@ async def test_falls_back_to_default_model(self): # (the centralized call_llm router picks the default) assert model is None + @pytest.mark.asyncio + async def test_config_yaml_model_takes_priority_over_env(self): + """config.yaml auxiliary.vision.model should be preferred over env var.""" + with ( + patch( + "tools.vision_tools.vision_analyze_tool", new_callable=AsyncMock + ) as mock_tool, + patch( + "tools.vision_tools._should_use_native_vision_fast_path", + return_value=False, + ), + patch( + "hermes_cli.config.load_config", + return_value={"auxiliary": {"vision": {"model": "qwen3.7-plus"}}}, + ), + patch.dict(os.environ, {"AUXILIARY_VISION_MODEL": "env-model"}), + ): + mock_tool.return_value = json.dumps({"result": "ok"}) + await _handle_vision_analyze( + {"image_url": "https://example.com/img.png", "question": "test"} + ) + call_args = mock_tool.call_args + model = call_args[0][2] # third positional arg + assert model == "qwen3.7-plus" + + @pytest.mark.asyncio + async def test_env_var_used_when_config_missing_model(self): + """Env var should be used when config.yaml has no auxiliary.vision.model.""" + with ( + patch( + "tools.vision_tools.vision_analyze_tool", new_callable=AsyncMock + ) as mock_tool, + patch( + "tools.vision_tools._should_use_native_vision_fast_path", + return_value=False, + ), + patch( + "hermes_cli.config.load_config", + return_value={"auxiliary": {"vision": {}}}, + ), + patch.dict(os.environ, {"AUXILIARY_VISION_MODEL": "fallback-model"}), + ): + mock_tool.return_value = json.dumps({"result": "ok"}) + await _handle_vision_analyze( + {"image_url": "https://example.com/img.png", "question": "test"} + ) + call_args = mock_tool.call_args + model = call_args[0][2] + assert model == "fallback-model" + def test_empty_args_graceful(self): """Missing keys should default to empty strings, not raise.""" with patch( diff --git a/tools/vision_tools.py b/tools/vision_tools.py index 23273483ede5..6b67abec9773 100644 --- a/tools/vision_tools.py +++ b/tools/vision_tools.py @@ -1356,7 +1356,18 @@ async def _handle_vision_analyze(args: Dict[str, Any], **kw: Any) -> str: "Fully describe and explain everything about this image, then answer the " f"following question:\n\n{question}" ) - model = os.getenv("AUXILIARY_VISION_MODEL", "").strip() or None + # Prefer config.yaml auxiliary.vision.model; env var is a legacy override. + model = None + try: + from hermes_cli.config import cfg_get, load_config + _cfg = load_config() + _vmodel = cfg_get(_cfg, "auxiliary", "vision", "model") + if _vmodel: + model = str(_vmodel).strip() or None + except Exception: + pass + if not model: + model = os.getenv("AUXILIARY_VISION_MODEL", "").strip() or None return await vision_analyze_tool(image_url, full_prompt, model) @@ -1718,7 +1729,19 @@ def _handle_video_analyze(args: Dict[str, Any], **kw: Any) -> Awaitable[str]: "including visual content, motion, audio cues, text overlays, and scene " f"transitions. Then answer the following question:\n\n{question}" ) - model = os.getenv("AUXILIARY_VIDEO_MODEL", "").strip() or os.getenv("AUXILIARY_VISION_MODEL", "").strip() or None + # Prefer config.yaml auxiliary.video.model (falling back to vision); + # env vars are a legacy override. + model = None + try: + from hermes_cli.config import cfg_get, load_config + _cfg = load_config() + _vmodel = cfg_get(_cfg, "auxiliary", "video", "model") or cfg_get(_cfg, "auxiliary", "vision", "model") + if _vmodel: + model = str(_vmodel).strip() or None + except Exception: + pass + if not model: + model = os.getenv("AUXILIARY_VIDEO_MODEL", "").strip() or os.getenv("AUXILIARY_VISION_MODEL", "").strip() or None return video_analyze_tool(video_url, full_prompt, model)