diff --git a/CHANGELOG.md b/CHANGELOG.md index 545139c9..ea463b0d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3,6 +3,12 @@ ## [Unreleased] +## [v0.51.369] — 2026-06-12 — Release MH (WebUI streaming honors runtime target model/base_url) + +### Fixed + +- **Provider OpenCode-Go (and similar pooled providers) no longer 404 with "model not found" when selected via the WebUI model picker (#3895).** WebUI streaming used the *configured* provider base URL instead of the runtime provider's per-model-normalized URL, which duplicated a `/v1` path segment for OpenCode-Go and produced a 404. The target model is now threaded into runtime provider resolution (including the 401 credential self-heal retries), and the runtime-normalized base URL is preferred — but only when it points at the same scheme+host+port as the configured one, so an explicit `providers..base_url` override at a different endpoint (e.g. an LM Studio LAN address or an OpenRouter mirror) is still honored. (#3895) + ## [v0.51.368] — 2026-06-12 — Release MG (bind active-profile cookie to auth session) ### Security diff --git a/api/streaming.py b/api/streaming.py index 88c3cf49..d1ed3583 100644 --- a/api/streaming.py +++ b/api/streaming.py @@ -237,6 +237,74 @@ def _resolve_custom_provider_runtime_overrides( return resolved_provider, resolved_api_key, resolved_base_url +def _same_base_url_endpoint(url_a: str, url_b: str) -> bool: + """True if two base URLs point at the same scheme+host+port endpoint. + + Used to decide whether a runtime base_url is just a normalized form of the + configured one (e.g. OpenCode-Go's ``/v1`` de-duplication on the same host) + versus a genuinely different endpoint (an explicit ``providers..base_url`` + override at a different host/port that must be preserved). Path/query are + intentionally ignored — the normalization #3895 fixes is path-only. + """ + from urllib.parse import urlsplit + try: + a = urlsplit((url_a or "").strip()) + b = urlsplit((url_b or "").strip()) + except Exception: + return False + _default_port = {"http": 80, "https": 443} + a_host = (a.hostname or "").lower() + b_host = (b.hostname or "").lower() + a_scheme = (a.scheme or "").lower() + b_scheme = (b.scheme or "").lower() + a_port = a.port or _default_port.get(a_scheme) + b_port = b.port or _default_port.get(b_scheme) + return bool(a_host) and a_host == b_host and a_scheme == b_scheme and a_port == b_port + + +def _runtime_preferred_base_url( + runtime_provider: dict | None, + resolved_provider: str | None, + configured_base_url: str | None, +) -> str | None: + """Prefer the runtime-normalized base_url, but never override an explicit + configured endpoint that points somewhere genuinely different. + + The #3895 bug was that WebUI used the *configured* base_url (which can carry a + duplicated ``/v1``) instead of the runtime provider's per-model-normalized + base_url, 404ing OpenCode-Go. But blindly preferring the runtime URL would + clobber a legitimate ``providers..base_url`` override (e.g. LM Studio at a + LAN IP, an OpenRouter mirror). So: + - no runtime URL -> keep configured + - no configured URL -> use runtime (all we have) + - named ``custom:`` endpoint -> configured wins (then runtime as fallback) + - same scheme+host+port -> runtime wins (it's the normalized/corrected + form of the same endpoint — the #3895 case) + - different endpoint -> configured override wins (no regression) + """ + runtime_base_url = None + if isinstance(runtime_provider, dict): + runtime_base_url = runtime_provider.get("base_url") + if not runtime_base_url: + return configured_base_url + if not configured_base_url: + return runtime_base_url + + provider_id = str( + resolved_provider + or (runtime_provider or {}).get("provider") + or "" + ).strip().lower() + if provider_id.startswith("custom:"): + return configured_base_url or runtime_base_url + + # An explicit configured override at a DIFFERENT endpoint must be preserved; + # only prefer the runtime URL when it's the same endpoint (path-normalized). + if _same_base_url_endpoint(configured_base_url, runtime_base_url): + return runtime_base_url + return configured_base_url + + def _is_fallback_lifecycle_message(kind: str, message: str) -> bool: """Return True if an agent lifecycle status should surface as a fallback warning.""" k = str(kind or '').strip().lower() @@ -4592,7 +4660,7 @@ def _replace_session_db_in_kwargs(agent_kwargs, state_db_path): def _attempt_credential_self_heal( - provider_id, session_id, _agent_lock_ref, + provider_id, session_id, _agent_lock_ref, *, target_model=None, ): """Try to silently refresh credentials after a 401/auth error (#1401). @@ -4640,6 +4708,7 @@ def _attempt_credential_self_heal( _new_rt = resolve_runtime_provider_with_anthropic_env_lock( resolve_runtime_provider, requested=provider_id, + target_model=target_model, ) logger.info( @@ -5879,6 +5948,7 @@ def _run_agent_streaming( resolved_model, resolved_provider, resolved_base_url = resolve_model_provider( model_with_provider_context(model, provider_context) ) + configured_base_url = resolved_base_url # Resolve API key via Hermes runtime provider (matches gateway behaviour). # Pass the resolved provider so non-default providers get their own credentials. @@ -5889,12 +5959,14 @@ def _run_agent_streaming( _rt = resolve_runtime_provider_with_anthropic_env_lock( resolve_runtime_provider, requested=resolved_provider, + target_model=resolved_model, ) resolved_api_key = _rt.get("api_key") if not resolved_provider: resolved_provider = _rt.get("provider") - if not resolved_base_url: - resolved_base_url = _rt.get("base_url") + resolved_base_url = _runtime_preferred_base_url( + _rt, resolved_provider, configured_base_url + ) except Exception as _e: print(f"[webui] WARNING: resolve_runtime_provider failed: {_e}", flush=True) @@ -6764,6 +6836,7 @@ def _run_agent_streaming( _heal_result = None _heal_rt = _attempt_credential_self_heal( resolved_provider or '', session_id, _agent_lock, + target_model=resolved_model, ) if _heal_rt is not None: logger.info('[webui] self-heal: retrying stream after credential refresh') @@ -6772,8 +6845,9 @@ def _run_agent_streaming( resolved_api_key = _heal_rt.get('api_key') if not resolved_provider: resolved_provider = _heal_rt.get('provider') - if not resolved_base_url: - resolved_base_url = _heal_rt.get('base_url') + resolved_base_url = _runtime_preferred_base_url( + _heal_rt, resolved_provider, configured_base_url + ) resolved_provider, resolved_api_key, resolved_base_url = _resolve_custom_provider_runtime_overrides( resolved_provider, resolved_api_key, resolved_base_url ) @@ -7716,6 +7790,7 @@ def _run_agent_streaming( # ── Credential self-heal on 401 (#1401) ── _heal_rt = _attempt_credential_self_heal( resolved_provider or '', session_id, _agent_lock, + target_model=resolved_model, ) if _heal_rt is not None: logger.info('[webui] self-heal (except path): retrying stream after credential refresh') @@ -7725,8 +7800,9 @@ def _run_agent_streaming( resolved_api_key = _heal_rt.get('api_key') if not resolved_provider: resolved_provider = _heal_rt.get('provider') - if not resolved_base_url: - resolved_base_url = _heal_rt.get('base_url') + resolved_base_url = _runtime_preferred_base_url( + _heal_rt, resolved_provider, configured_base_url + ) resolved_provider, resolved_api_key, resolved_base_url = _resolve_custom_provider_runtime_overrides( resolved_provider, resolved_api_key, resolved_base_url ) diff --git a/tests/test_issue3895_opencode_go_model_picker.py b/tests/test_issue3895_opencode_go_model_picker.py new file mode 100644 index 00000000..177511e9 --- /dev/null +++ b/tests/test_issue3895_opencode_go_model_picker.py @@ -0,0 +1,304 @@ +"""Regression tests for OpenCode-Go model-picker runtime routing (#3895).""" + +import queue +import sys +import threading +import types +from unittest import mock + +import api.config +import api.oauth +import api.streaming as streaming + + +def test_runtime_preferred_base_url_uses_runtime_value_for_pooled_provider(): + assert streaming._runtime_preferred_base_url( + {"provider": "opencode-go", "base_url": "https://opencode.example.com/api"}, + "opencode-go", + "https://opencode.example.com/api/v1", + ) == "https://opencode.example.com/api" + + +def test_runtime_preferred_base_url_keeps_custom_config_base_url(): + assert streaming._runtime_preferred_base_url( + {"provider": "custom:opencode-proxy", "base_url": "https://runtime.example.com"}, + "custom:opencode-proxy", + "https://config.example.com/v1", + ) == "https://config.example.com/v1" + + +def test_runtime_preferred_base_url_uses_runtime_for_custom_provider_without_config(): + assert streaming._runtime_preferred_base_url( + {"provider": "custom:opencode-proxy", "base_url": "https://runtime.example.com"}, + "custom:opencode-proxy", + None, + ) == "https://runtime.example.com" + + +def test_runtime_preferred_base_url_preserves_different_endpoint_config_override(): + """Codex CORE regression guard: an explicit providers..base_url override + at a DIFFERENT host/port must NOT be clobbered by the runtime default. Only a + same-endpoint runtime URL (the #3895 /v1-dedup case) should win.""" + # LM Studio pinned at a LAN IP — must be preserved over the runtime localhost default. + assert streaming._runtime_preferred_base_url( + {"provider": "lmstudio", "base_url": "http://localhost:1234/v1"}, + "lmstudio", + "http://10.0.0.5:1234/v1", + ) == "http://10.0.0.5:1234/v1" + # OpenRouter mirror override preserved over the runtime canonical host. + assert streaming._runtime_preferred_base_url( + {"provider": "openrouter", "base_url": "https://openrouter.ai/api/v1"}, + "openrouter", + "https://my-mirror.example.com/api/v1", + ) == "https://my-mirror.example.com/api/v1" + + +def test_runtime_preferred_base_url_prefers_runtime_for_same_endpoint_path_normalization(): + """The #3895 case: same scheme+host+port, runtime just strips a duplicated + path segment — runtime (corrected) form wins.""" + assert streaming._runtime_preferred_base_url( + {"provider": "opencode-go", "base_url": "https://opencode.example.com/zen/go"}, + "opencode-go", + "https://opencode.example.com/zen/go/v1", + ) == "https://opencode.example.com/zen/go" + # Same host different port = different endpoint → configured wins. + assert streaming._runtime_preferred_base_url( + {"provider": "someprov", "base_url": "https://host.example.com:8080/v1"}, + "someprov", + "https://host.example.com:9090/v1", + ) == "https://host.example.com:9090/v1" + + +def test_streaming_passes_target_model_and_prefers_runtime_base_url(monkeypatch): + captured = {} + + class FakeSession: + def __init__(self): + self.session_id = "sess-3895" + self.title = "OpenCode test" + self.workspace = "/tmp" + self.model = "glm-5.1" + self.messages = [] + self.personality = None + self.input_tokens = 0 + self.output_tokens = 0 + self.estimated_cost = None + self.tool_calls = [] + self.active_stream_id = None + self.pending_user_message = None + self.pending_attachments = [] + self.pending_started_at = None + + def save(self, touch_updated_at=True): + self._saved = touch_updated_at + + def compact(self): + return { + "session_id": self.session_id, + "title": self.title, + "workspace": self.workspace, + "model": self.model, + "created_at": 0, + "updated_at": 0, + "pinned": False, + "archived": False, + "project_id": None, + "profile": None, + "input_tokens": self.input_tokens, + "output_tokens": self.output_tokens, + "estimated_cost": self.estimated_cost, + "personality": self.personality, + } + + class CapturingAgent: + def __init__( + self, + model=None, + provider=None, + base_url=None, + api_key=None, + platform=None, + quiet_mode=False, + enabled_toolsets=None, + fallback_model=None, + session_id=None, + session_db=None, + stream_delta_callback=None, + reasoning_callback=None, + tool_progress_callback=None, + clarify_callback=None, + **kwargs, + ): + captured["init_kwargs"] = { + "model": model, + "provider": provider, + "base_url": base_url, + "api_key": api_key, + "session_id": session_id, + "session_db": session_db, + } + self.session_id = session_id + self.context_compressor = None + self.session_prompt_tokens = 0 + self.session_completion_tokens = 0 + self.session_estimated_cost_usd = None + self.reasoning_config = None + self.ephemeral_system_prompt = None + self._last_error = None + + def run_conversation(self, **kwargs): + captured["run_kwargs"] = kwargs + return { + "messages": [ + {"role": "user", "content": kwargs["persist_user_message"]}, + {"role": "assistant", "content": "ok"}, + ] + } + + def interrupt(self, _message): + captured["interrupted"] = _message + + fake_session = FakeSession() + fake_stream_id = "stream-3895" + fake_session.active_stream_id = fake_stream_id + fake_queue = queue.Queue() + fake_runtime_module = types.ModuleType("hermes_cli.runtime_provider") + resolve_runtime_provider = mock.Mock( + return_value={ + "provider": "opencode-go", + "base_url": "https://opencode.example.com/api", + "api_key": "rt-key", + } + ) + fake_runtime_module.resolve_runtime_provider = resolve_runtime_provider + fake_hermes_cli = types.ModuleType("hermes_cli") + fake_hermes_cli.runtime_provider = fake_runtime_module + fake_hermes_state = types.ModuleType("hermes_state") + fake_hermes_state.SessionDB = mock.Mock(return_value=object()) + + def fake_runtime_lock(resolver, **kwargs): + return resolver(**kwargs) + + monkeypatch.setattr( + api.oauth, + "resolve_runtime_provider_with_anthropic_env_lock", + fake_runtime_lock, + ) + monkeypatch.setattr(streaming, "get_session", lambda _session_id: fake_session) + monkeypatch.setattr(streaming, "_get_ai_agent", lambda: CapturingAgent) + monkeypatch.setattr( + streaming, + "resolve_model_provider", + lambda *_args, **_kwargs: ( + "glm-5.1", + "opencode-go", + "https://opencode.example.com/api/v1", + ), + ) + monkeypatch.setattr("api.config.get_config", lambda: {}) + monkeypatch.setattr("api.config._resolve_cli_toolsets", lambda *_args, **_kwargs: []) + monkeypatch.setitem(sys.modules, "hermes_cli", fake_hermes_cli) + monkeypatch.setitem(sys.modules, "hermes_cli.runtime_provider", fake_runtime_module) + monkeypatch.setitem(sys.modules, "hermes_state", fake_hermes_state) + + try: + streaming.STREAMS[fake_stream_id] = fake_queue + streaming._run_agent_streaming( + session_id=fake_session.session_id, + msg_text="hello from picker", + model="glm-5.1", + workspace="/tmp", + stream_id=fake_stream_id, + ) + finally: + streaming.STREAMS.pop(fake_stream_id, None) + streaming.AGENT_INSTANCES.pop(fake_stream_id, None) + + resolve_runtime_provider.assert_called_once_with( + requested="opencode-go", + target_model="glm-5.1", + ) + assert captured["init_kwargs"]["provider"] == "opencode-go" + assert captured["init_kwargs"]["base_url"] == "https://opencode.example.com/api" + assert captured["init_kwargs"]["api_key"] == "rt-key" + + +def test_runtime_provider_lock_wrapper_forwards_target_model(): + calls = {} + + def fake_resolver(**kwargs): + calls["kwargs"] = kwargs + return {"provider": kwargs.get("requested"), "base_url": None, "api_key": None} + + result = api.oauth.resolve_runtime_provider_with_anthropic_env_lock( + fake_resolver, + requested="opencode-go", + target_model="glm-5.1", + ) + + assert result["provider"] == "opencode-go" + assert calls["kwargs"] == { + "requested": "opencode-go", + "target_model": "glm-5.1", + } + + +def test_attempt_credential_self_heal_passes_target_model(monkeypatch): + calls = {} + fake_runtime_module = types.ModuleType("hermes_cli.runtime_provider") + + def fake_resolve_runtime_provider(**kwargs): + calls["resolver_kwargs"] = kwargs + return { + "provider": kwargs.get("requested"), + "base_url": "https://opencode.example.com/api", + "api_key": "rt-key", + } + + fake_runtime_module.resolve_runtime_provider = fake_resolve_runtime_provider + + def fake_runtime_lock(resolver, **kwargs): + calls["lock_kwargs"] = kwargs + return resolver(**kwargs) + + closed = [] + monkeypatch.setattr(api.oauth, "read_auth_json", lambda: {"providers": {"opencode-go": {}}}) + monkeypatch.setattr( + api.oauth, + "resolve_runtime_provider_with_anthropic_env_lock", + fake_runtime_lock, + ) + monkeypatch.setattr( + api.config, + "SESSION_AGENT_CACHE", + {"sess-3895": object()}, + ) + monkeypatch.setattr(api.config, "SESSION_AGENT_CACHE_LOCK", threading.Lock()) + monkeypatch.setattr(api.config, "invalidate_credential_pool_cache", lambda provider_id: calls.setdefault("invalidated", []).append(provider_id)) + monkeypatch.setattr( + streaming, + "_close_cached_agent_entry_at_session_boundary", + lambda session_id, entry: closed.append((session_id, entry)), + ) + monkeypatch.setitem(sys.modules, "hermes_cli.runtime_provider", fake_runtime_module) + + result = streaming._attempt_credential_self_heal( + "opencode-go", + "sess-3895", + None, + target_model="glm-5.1", + ) + + assert result["base_url"] == "https://opencode.example.com/api" + assert calls["lock_kwargs"] == { + "requested": "opencode-go", + "target_model": "glm-5.1", + } + assert calls["resolver_kwargs"] == { + "requested": "opencode-go", + "target_model": "glm-5.1", + } + assert calls["invalidated"] == ["opencode-go"] + assert len(closed) == 1 + assert closed[0][0] == "sess-3895" + assert api.config.SESSION_AGENT_CACHE == {} diff --git a/tests/test_sprint42.py b/tests/test_sprint42.py index b2d6381e..6f76a8e0 100644 --- a/tests/test_sprint42.py +++ b/tests/test_sprint42.py @@ -256,7 +256,11 @@ class TestRuntimeRouteInjection(unittest.TestCase): stream_id=fake_stream_id, ) - resolve_runtime_provider.assert_called_once_with(requested="openai-codex") + # #4022: the resolver is now called with the target model too so per-model + # base_url normalization (e.g. OpenCode-Go /v1 stripping) is applied. + resolve_runtime_provider.assert_called_once_with( + requested="openai-codex", target_model="gpt-5.4" + ) init_kwargs = captured["init_kwargs"] self.assertEqual(init_kwargs["api_mode"], "codex_responses") self.assertEqual(init_kwargs["acp_command"], "codex")