From 00b85b71dd6abe553ea59cd8d5a7e6e0fdb72e4a Mon Sep 17 00:00:00 2001 From: krypticmouse Date: Fri, 29 May 2026 01:10:25 +0000 Subject: [PATCH] fix(agents): unwrap wrapped engines to derive opencode provider URL (+ clear guard) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The real `jarvis ask --agent opencode` path passes an InstrumentedEngine (telemetry wrapper) whose underlying engine — and its `_host` — lives at `_inner`. The base-URL derivation only checked the top object, so it returned "", no provider was registered, and opencode 500'd on `openjarvis/`. Direct construction with a raw engine masked this; only the CLI path exposed it. - _derive_openai_base_url now unwraps up to 6 wrapper layers (`_inner`/`_engine`/`_wrapped`) to find `_host`/`base_url`. - run() resolves the provider/model spec up front and, when it genuinely can't (no base URL + bare model name), returns a clear actionable error instead of letting opencode 500. Tests: wrapper-unwrap derivation + unresolvable-provider guard. 20 passed, ruff clean. Co-Authored-By: Claude Opus 4.7 (1M context) --- src/openjarvis/agents/opencode.py | 66 ++++++++++++++++++++--------- tests/agents/test_opencode_agent.py | 13 ++++++ 2 files changed, 60 insertions(+), 19 deletions(-) diff --git a/src/openjarvis/agents/opencode.py b/src/openjarvis/agents/opencode.py index 1f3b1cc3..08f55448 100644 --- a/src/openjarvis/agents/opencode.py +++ b/src/openjarvis/agents/opencode.py @@ -42,18 +42,32 @@ def _derive_openai_base_url(engine: Any) -> str: """Best-effort OpenAI-compatible base URL for an OpenJarvis engine. HTTP engines (Ollama, vLLM, llama.cpp, SGLang, LM Studio, …) expose a - ``_host`` and serve an OpenAI-compatible API at ``/v1``. Returns "" - when it cannot be derived (caller then requires an explicit base URL or a - pre-configured opencode provider). + ``_host`` and serve an OpenAI-compatible API at ``/v1``. Engines are + often wrapped (e.g. ``InstrumentedEngine`` for telemetry stores the real + engine at ``_inner``), so we unwrap a few layers to find the host. Returns + "" when it cannot be derived (caller then needs an explicit base URL or a + ``provider/model`` that opencode already knows). """ - for attr in ("openai_base_url", "base_url"): - val = getattr(engine, attr, "") - if val: - return str(val).rstrip("/") - host = getattr(engine, "_host", "") or getattr(engine, "host", "") - if host: - host = str(host).rstrip("/") - return host if host.endswith("/v1") else f"{host}/v1" + seen: set[int] = set() + cur = engine + for _ in range(6): + if cur is None or id(cur) in seen: + break + seen.add(id(cur)) + for attr in ("openai_base_url", "base_url"): + val = getattr(cur, attr, "") + if val: + return str(val).rstrip("/") + host = getattr(cur, "_host", "") or getattr(cur, "host", "") + if host: + host = str(host).rstrip("/") + return host if host.endswith("/v1") else f"{host}/v1" + # Unwrap common wrapper attributes (InstrumentedEngine -> _inner, etc.) + cur = ( + getattr(cur, "_inner", None) + or getattr(cur, "_engine", None) + or getattr(cur, "_wrapped", None) + ) return "" @@ -288,6 +302,27 @@ class OpenCodeAgent(BaseAgent): ) -> AgentResult: """Run a coding task through opencode and return the assistant result.""" self._emit_turn_start(input) + + # Resolve which opencode provider/model to address. Fail clearly rather + # than letting opencode 500 on an unregistered provider. + if self._provider_base_url: + model_spec = {"providerID": self._provider_id, "modelID": self._model_id} + elif "/" in self._model_id: + prov, _, mid = self._model_id.partition("/") + model_spec = {"providerID": prov, "modelID": mid} + else: + self._emit_turn_end(turns=1, error=True) + return AgentResult( + content=( + f"OpenCodeAgent could not determine an opencode provider for " + f"model {self._model_id!r}: no OpenAI-compatible base URL could " + f"be derived from the engine. Pass provider_base_url=..., or use " + f"a 'provider/model' that opencode already knows." + ), + turns=1, + metadata={"error": True}, + ) + try: self._ensure_server() except RuntimeError as exc: @@ -306,16 +341,9 @@ class OpenCodeAgent(BaseAgent): body: dict = { "agent": self._agent, + "model": model_spec, "parts": [{"type": "text", "text": input}], } - if self._provider_base_url or "/" not in self._model_id: - body["model"] = { - "providerID": self._provider_id, - "modelID": self._model_id, - } - else: - prov, _, mid = self._model_id.partition("/") - body["model"] = {"providerID": prov, "modelID": mid} resp = c.post(f"/session/{session_id}/message", json=body) resp.raise_for_status() diff --git a/tests/agents/test_opencode_agent.py b/tests/agents/test_opencode_agent.py index 33f9f485..8ca5f2a1 100644 --- a/tests/agents/test_opencode_agent.py +++ b/tests/agents/test_opencode_agent.py @@ -71,6 +71,11 @@ class TestDeriveBaseUrl: eng = SimpleNamespace(base_url="http://y/v1") assert _derive_openai_base_url(eng) == "http://y/v1" + def test_unwraps_wrapper_engine(self): + # InstrumentedEngine wraps the real engine at `_inner`; must unwrap. + wrapped = SimpleNamespace(_inner=SimpleNamespace(_host="http://localhost:11434")) + assert _derive_openai_base_url(wrapped) == "http://localhost:11434/v1" + def test_none_when_unknown(self): assert _derive_openai_base_url(SimpleNamespace()) == "" @@ -136,6 +141,14 @@ class TestRunGracefulDegradation: assert res.metadata.get("error") is True assert "opencode" in res.content.lower() + def test_unresolvable_provider_fails_clearly(self, tmp_path): + # No derivable base URL + bare model name -> clear error, not a 500 + # (and we fail before spawning a server). + agent = OpenCodeAgent(SimpleNamespace(), "qwen3:8b", workspace=str(tmp_path)) + res = agent.run("do something") + assert res.metadata.get("error") is True + assert "could not determine" in res.content.lower() + class _FakeResp: def __init__(self, data):