fixes

Author: a.kaznacheev

ouroboros/llm.py

@@ -7,11 +7,12 @@
 
 from __future__ import annotations
 
+import json
 import logging
 import os
 import re
 import time
-from typing import Any, Dict, List, Optional, Tuple
+from typing import Any, Dict, List, Optional, Set, Tuple
 
 log = logging.getLogger(__name__)
 
@@ -111,24 +112,32 @@ def __init__(
         api_key: Optional[str] = None,
         base_url: str = "https://openrouter.ai/api/v1",
     ):
+        self._api_key_override = api_key
         self._api_key = api_key or os.environ.get("OPENROUTER_API_KEY", "")
         self._base_url = base_url
         self._client = None
+        self._client_api_key: Optional[str] = None
         self._local_client = None
         self._local_port: Optional[int] = None
 
     def _get_client(self):
-        if self._client is None:
+        current_api_key = self._api_key_override
+        if current_api_key is None:
+            current_api_key = os.environ.get("OPENROUTER_API_KEY", "")
+
+        if self._client is None or self._client_api_key != current_api_key:
             from openai import OpenAI
             self._client = OpenAI(
                 base_url=self._base_url,
-                api_key=self._api_key,
+                api_key=current_api_key,
                 max_retries=0,
                 default_headers={
                     "HTTP-Referer": "https://ouroboros.local/",
                     "X-Title": "Ouroboros",
                 },
             )
+            self._client_api_key = current_api_key
+            self._api_key = current_api_key
         return self._client
 
     def _get_local_client(self):
@@ -273,9 +282,82 @@ def _chat_local(
         choices = resp_dict.get("choices") or [{}]
         msg = (choices[0] if choices else {}).get("message") or {}
 
+        if not msg.get("tool_calls") and msg.get("content") and clean_tools:
+            allowed_tool_names = {
+                str(t.get("function", {}).get("name", "")).strip()
+                for t in clean_tools
+                if isinstance(t, dict)
+            }
+            msg = self._parse_tool_calls_from_content(msg, allowed_tool_names)
+
         usage["cost"] = 0.0
         return msg, usage
 
+    @staticmethod
+    def _parse_tool_calls_from_content(
+        msg: Dict[str, Any],
+        allowed_tool_names: Optional[Set[str]] = None,
+    ) -> Dict[str, Any]:
+        """Parse <tool_call> XML tags from content into structured tool_calls.
+
+        Works around llama-cpp-python not parsing Qwen/Hermes-style tool calls
+        (https://github.com/abetlen/llama-cpp-python/issues/1784).
+        """
+        content = str(msg.get("content", "") or "")
+        stripped = content.strip()
+        if not stripped:
+            return msg
+
+        # Safety: only upgrade the response when it consists solely of
+        # one or more <tool_call> blocks. If the model mixed prose with
+        # examples or explanations, leave it as plain text.
+        full_pattern = re.compile(
+            r"^(?:\s*<tool_call>\s*\{.*?\}\s*</tool_call>\s*)+$",
+            re.DOTALL,
+        )
+        if not full_pattern.fullmatch(stripped):
+            return msg
+
+        matches = re.findall(r"<tool_call>\s*(\{.*?\})\s*</tool_call>", stripped, re.DOTALL)
+        if not matches:
+            return msg
+
+        allowed = {name for name in (allowed_tool_names or set()) if name}
+        tool_calls = []
+        for i, raw in enumerate(matches):
+            try:
+                obj = json.loads(raw)
+                if not isinstance(obj, dict):
+                    raise ValueError("tool_call payload must be an object")
+                name = str(obj.get("name", "")).strip()
+                args = obj.get("arguments", {})
+                if not name:
+                    raise ValueError("tool_call missing function name")
+                if allowed and name not in allowed:
+                    raise ValueError(f"unknown tool '{name}'")
+                if not isinstance(args, dict):
+                    raise ValueError("tool_call arguments must be an object")
+                tool_calls.append({
+                    "id": f"call_local_{i}",
+                    "type": "function",
+                    "function": {
+                        "name": name,
+                        "arguments": json.dumps(args),
+                    },
+                })
+            except (json.JSONDecodeError, ValueError) as exc:
+                log.warning("Rejected local <tool_call> block: %s (%s)", raw[:200], exc)
+                return msg
+
+        if not tool_calls:
+            return msg
+
+        msg = dict(msg)
+        msg["tool_calls"] = tool_calls
+        msg["content"] = None
+        log.info("Parsed %d local tool call(s) from text output", len(tool_calls))
+        return msg
+
     @staticmethod
     def _truncate_messages_for_context(
         messages: List[Dict[str, Any]], ctx_len: int, max_tokens: int,

ouroboros/local_model.py

@@ -427,7 +427,19 @@ def test_tool_calling(self) -> Dict[str, Any]:
                 max_tokens=256,
             )
             msg = resp.choices[0].message if resp.choices else None
-            if msg and msg.tool_calls:
+            tool_calls = list(getattr(msg, "tool_calls", None) or []) if msg else []
+            if msg and not tool_calls and getattr(msg, "content", None):
+                from ouroboros.llm import LLMClient
+
+                parsed = LLMClient._parse_tool_calls_from_content(
+                    {
+                        "content": msg.content,
+                        "tool_calls": [],
+                    },
+                    {"get_time"},
+                )
+                tool_calls = parsed.get("tool_calls") or []
+            if tool_calls:
                 result["tool_call_ok"] = True
             else:
                 result["details"] = "Model returned text instead of tool_call"

tests/test_llm_client_refresh.py

@@ -0,0 +1,54 @@
+import os
+import sys
+import types
+import unittest
+from unittest.mock import patch
+
+
+sys.path.insert(0, os.path.join(os.path.dirname(__file__), ".."))
+
+
+class _FakeOpenAI:
+    created = []
+
+    def __init__(self, **kwargs):
+        self.kwargs = kwargs
+        type(self).created.append(self)
+
+
+class TestLlmClientRefresh(unittest.TestCase):
+    def setUp(self):
+        _FakeOpenAI.created.clear()
+
+    def test_runtime_client_refreshes_when_env_key_changes(self):
+        from ouroboros.llm import LLMClient
+
+        fake_openai = types.SimpleNamespace(OpenAI=_FakeOpenAI)
+        with patch.dict(sys.modules, {"openai": fake_openai}):
+            with patch.dict(os.environ, {"OPENROUTER_API_KEY": ""}, clear=False):
+                client = LLMClient()
+                first = client._get_client()
+
+            with patch.dict(os.environ, {"OPENROUTER_API_KEY": "sk-or-new-key"}, clear=False):
+                second = client._get_client()
+
+        self.assertIsNot(first, second)
+        self.assertEqual(len(_FakeOpenAI.created), 2)
+        self.assertEqual(_FakeOpenAI.created[0].kwargs["api_key"], "")
+        self.assertEqual(_FakeOpenAI.created[1].kwargs["api_key"], "sk-or-new-key")
+
+    def test_explicit_api_key_does_not_track_env_changes(self):
+        from ouroboros.llm import LLMClient
+
+        fake_openai = types.SimpleNamespace(OpenAI=_FakeOpenAI)
+        with patch.dict(sys.modules, {"openai": fake_openai}):
+            with patch.dict(os.environ, {"OPENROUTER_API_KEY": ""}, clear=False):
+                client = LLMClient(api_key="explicit-key")
+                first = client._get_client()
+
+            with patch.dict(os.environ, {"OPENROUTER_API_KEY": "sk-or-new-key"}, clear=False):
+                second = client._get_client()
+
+        self.assertIs(first, second)
+        self.assertEqual(len(_FakeOpenAI.created), 1)
+        self.assertEqual(_FakeOpenAI.created[0].kwargs["api_key"], "explicit-key")

tests/test_local_tool_parsing.py

@@ -0,0 +1,87 @@
+import json
+import os
+import sys
+import unittest
+
+
+sys.path.insert(0, os.path.join(os.path.dirname(__file__), ".."))
+
+
+class TestLocalToolCallParsing(unittest.TestCase):
+    def test_parses_pure_tool_call_blocks(self):
+        from ouroboros.llm import LLMClient
+
+        msg = {
+            "content": """
+<tool_call>
+{"name": "repo_read", "arguments": {"path": "README.md"}}
+</tool_call>
+<tool_call>
+{"name": "repo_write", "arguments": {"path": "notes.txt", "content": "hello"}}
+</tool_call>
+""",
+            "tool_calls": [],
+        }
+
+        parsed = LLMClient._parse_tool_calls_from_content(
+            msg,
+            {"repo_read", "repo_write"},
+        )
+
+        self.assertEqual(len(parsed["tool_calls"]), 2)
+        self.assertIsNone(parsed["content"])
+        self.assertEqual(parsed["tool_calls"][0]["function"]["name"], "repo_read")
+        self.assertEqual(
+            json.loads(parsed["tool_calls"][0]["function"]["arguments"]),
+            {"path": "README.md"},
+        )
+
+    def test_rejects_mixed_prose_and_tool_calls(self):
+        from ouroboros.llm import LLMClient
+
+        msg = {
+            "content": """
+Sure, I will use the tool now.
+
+<tool_call>
+{"name": "repo_read", "arguments": {"path": "README.md"}}
+</tool_call>
+""",
+            "tool_calls": [],
+        }
+
+        parsed = LLMClient._parse_tool_calls_from_content(msg, {"repo_read"})
+
+        self.assertEqual(parsed, msg)
+
+    def test_rejects_unknown_tool_names(self):
+        from ouroboros.llm import LLMClient
+
+        msg = {
+            "content": """
+<tool_call>
+{"name": "repo_delete_everything", "arguments": {}}
+</tool_call>
+""",
+            "tool_calls": [],
+        }
+
+        parsed = LLMClient._parse_tool_calls_from_content(msg, {"repo_read"})
+
+        self.assertEqual(parsed, msg)
+
+    def test_rejects_non_object_arguments(self):
+        from ouroboros.llm import LLMClient
+
+        msg = {
+            "content": """
+<tool_call>
+{"name": "repo_read", "arguments": "README.md"}
+</tool_call>
+""",
+            "tool_calls": [],
+        }
+
+        parsed = LLMClient._parse_tool_calls_from_content(msg, {"repo_read"})
+
+        self.assertEqual(parsed, msg)

tests/test_settings_ui_guards.py

@@ -0,0 +1,35 @@
+import os
+import pathlib
+import sys
+import unittest
+
+
+sys.path.insert(0, os.path.join(os.path.dirname(__file__), ".."))
+
+REPO = pathlib.Path(__file__).resolve().parents[1]
+
+
+class TestSettingsUiGuards(unittest.TestCase):
+    def test_save_checks_http_status(self):
+        source = (REPO / "web/modules/settings.js").read_text(encoding="utf-8")
+        self.assertIn("if (!resp.ok) throw new Error(data.error || `HTTP ${resp.status}`);", source)
+
+    def test_save_does_not_overwrite_masked_secrets(self):
+        source = (REPO / "web/modules/settings.js").read_text(encoding="utf-8")
+        self.assertIn("if (orKey && !orKey.includes('...')) body.OPENROUTER_API_KEY = orKey;", source)
+        self.assertIn("if (oaiKey && !oaiKey.includes('...')) body.OPENAI_API_KEY = oaiKey;", source)
+        self.assertIn("if (antKey && !antKey.includes('...')) body.ANTHROPIC_API_KEY = antKey;", source)
+        self.assertIn("if (ghToken && !ghToken.includes('...')) body.GITHUB_TOKEN = ghToken;", source)
+
+    def test_masked_secret_inputs_clear_on_focus(self):
+        source = (REPO / "web/modules/settings.js").read_text(encoding="utf-8")
+        self.assertIn("if (input.value.includes('...')) input.value = '';", source)
+
+    def test_models_section_explains_local_switching(self):
+        source = (REPO / "web/modules/settings.js").read_text(encoding="utf-8")
+        self.assertIn("These fields are cloud model IDs.", source)
+        self.assertIn("through the GGUF server configured above.", source)
+
+    def test_save_reloads_settings_after_success(self):
+        source = (REPO / "web/modules/settings.js").read_text(encoding="utf-8")
+        self.assertIn("await loadSettings();", source)