Backport upstream fixes: qq/cron/web/tools/mcp robustness

opencane/agent/tools/filesystem.py

@@ -1,5 +1,6 @@
 """File system tools: read, write, edit."""
 
+import difflib
 from pathlib import Path
 from typing import Any
 
@@ -153,7 +154,7 @@ async def execute(self, path: str, old_text: str, new_text: str, **kwargs: Any)
             content = file_path.read_text(encoding="utf-8")
 
             if old_text not in content:
-                return "Error: old_text not found in file. Make sure it matches exactly."
+                return self._not_found_message(old_text, content, path)
 
             # Count occurrences
             count = content.count(old_text)
@@ -169,6 +170,39 @@ async def execute(self, path: str, old_text: str, new_text: str, **kwargs: Any)
         except Exception as e:
             return f"Error editing file: {str(e)}"
 
+    @staticmethod
+    def _not_found_message(old_text: str, content: str, path: str) -> str:
+        """Build a helpful error when old_text is not found."""
+        old_lines = old_text.splitlines(keepends=True)
+        lines = content.splitlines(keepends=True)
+        window = max(1, len(old_lines))
+
+        best_ratio = 0.0
+        best_start = 0
+        for idx in range(max(1, len(lines) - window + 1)):
+            chunk = lines[idx : idx + window]
+            ratio = difflib.SequenceMatcher(None, old_lines, chunk).ratio()
+            if ratio > best_ratio:
+                best_ratio = ratio
+                best_start = idx
+
+        if best_ratio >= 0.5:
+            best_chunk = lines[best_start : best_start + window]
+            diff = difflib.unified_diff(
+                old_lines,
+                best_chunk,
+                fromfile="old_text (provided)",
+                tofile=f"{path} (actual, line {best_start + 1})",
+                lineterm="",
+            )
+            return (
+                f"Error: old_text not found in {path}.\n"
+                f"Best match ({best_ratio:.0%} similar) at line {best_start + 1}:\n"
+                + "\n".join(diff)
+            )
+
+        return f"Error: old_text not found in {path}. No similar text found."
+
 
 class ListDirTool(Tool):
     """Tool to list directory contents."""

opencane/agent/tools/mcp.py

@@ -1,13 +1,17 @@
 """MCP client: connects to MCP servers and wraps their tools as native OpenCane tools."""
 
+import asyncio
 from contextlib import AsyncExitStack
 from typing import Any
 
+import httpx
 from loguru import logger
 
 from opencane.agent.tools.base import Tool
 from opencane.agent.tools.registry import ToolRegistry
 
+MCP_TOOL_TIMEOUT = 30
+
 
 def _extract_nullable_branch(options: Any) -> tuple[dict[str, Any], bool] | None:
     """Return the single non-null branch for nullable unions."""
@@ -95,7 +99,14 @@ def parameters(self) -> dict[str, Any]:
 
     async def execute(self, **kwargs: Any) -> str:
         from mcp import types
-        result = await self._session.call_tool(self._original_name, arguments=kwargs)
+        try:
+            result = await asyncio.wait_for(
+                self._session.call_tool(self._original_name, arguments=kwargs),
+                timeout=MCP_TOOL_TIMEOUT,
+            )
+        except asyncio.TimeoutError:
+            logger.warning("MCP tool '{}' timed out after {}s", self._name, MCP_TOOL_TIMEOUT)
+            return f"(MCP tool call timed out after {MCP_TOOL_TIMEOUT}s)"
         parts = []
         for block in result.content:
             if isinstance(block, types.TextContent):
@@ -121,8 +132,11 @@ async def connect_mcp_servers(
                 read, write = await stack.enter_async_context(stdio_client(params))
             elif cfg.url:
                 from mcp.client.streamable_http import streamable_http_client
+                http_client = await stack.enter_async_context(
+                    httpx.AsyncClient(follow_redirects=True, timeout=None)
+                )
                 read, write, _ = await stack.enter_async_context(
-                    streamable_http_client(cfg.url)
+                    streamable_http_client(cfg.url, http_client=http_client)
                 )
             else:
                 logger.warning(f"MCP server '{name}': no command or url configured, skipping")

opencane/agent/tools/web.py

@@ -66,20 +66,28 @@ class WebSearchTool(Tool):
     }
 
     def __init__(self, api_key: str | None = None, max_results: int = 5):
-        self.api_key = api_key or os.environ.get("BRAVE_API_KEY", "")
+        self._config_api_key = api_key
         self.max_results = max_results
 
+    def _resolve_api_key(self) -> str:
+        """Resolve API key at call time so env/config refresh is picked up."""
+        return self._config_api_key or os.environ.get("BRAVE_API_KEY", "")
+
     async def execute(self, query: str, count: int | None = None, **kwargs: Any) -> str:
-        if not self.api_key:
-            return "Error: BRAVE_API_KEY not configured"
+        api_key = self._resolve_api_key()
+        if not api_key:
+            return (
+                "Error: Brave Search API key not configured. "
+                "Set BRAVE_API_KEY or configure tools.web.search.apiKey in ~/.opencane/config.json."
+            )
 
         try:
             n = min(max(count or self.max_results, 1), 10)
             async with httpx.AsyncClient() as client:
                 r = await client.get(
                     "https://api.search.brave.com/res/v1/web/search",
                     params={"q": query, "count": n},
-                    headers={"Accept": "application/json", "X-Subscription-Token": self.api_key},
+                    headers={"Accept": "application/json", "X-Subscription-Token": api_key},
                     timeout=10.0
                 )
                 r.raise_for_status()

opencane/channels/qq.py

@@ -31,7 +31,12 @@ def _make_bot_class(channel: "QQChannel") -> "type[botpy.Client]":
 
     class _Bot(botpy.Client):
         def __init__(self):
-            super().__init__(intents=intents)
+            # Disable botpy file handler by default; it can fail under read-only
+            # service environments where cwd is not writable.
+            try:
+                super().__init__(intents=intents, ext_handlers=False)
+            except TypeError:
+                super().__init__(intents=intents)
 
         async def on_ready(self):
             logger.info(f"QQ bot ready: {self.robot.name}")
@@ -55,7 +60,6 @@ def __init__(self, config: QQConfig, bus: MessageBus):
         self.config: QQConfig = config
         self._client: "botpy.Client | None" = None
         self._processed_ids: deque = deque(maxlen=1000)
-        self._bot_task: asyncio.Task | None = None
 
     async def start(self) -> None:
         """Start the QQ bot."""
@@ -71,14 +75,17 @@ async def start(self) -> None:
         bot_class = _make_bot_class(self)
         self._client = bot_class()
 
-        self._bot_task = asyncio.create_task(self._run_bot())
         logger.info("QQ bot started (C2C private message)")
+        await self._run_bot()
 
     async def _run_bot(self) -> None:
         """Run the bot connection with auto-reconnect."""
         while self._running:
             try:
-                await self._client.start(appid=self.config.app_id, secret=self.config.secret)
+                client = self._client
+                if client is None:
+                    break
+                await client.start(appid=self.config.app_id, secret=self.config.secret)
             except Exception as e:
                 logger.warning(f"QQ bot error: {e}")
             if self._running:
@@ -88,11 +95,10 @@ async def _run_bot(self) -> None:
     async def stop(self) -> None:
         """Stop the QQ bot."""
         self._running = False
-        if self._bot_task:
-            self._bot_task.cancel()
+        if self._client:
             try:
-                await self._bot_task
-            except asyncio.CancelledError:
+                await self._client.close()
+            except Exception:
                 pass
         logger.info("QQ bot stopped")
 
@@ -130,5 +136,5 @@ async def _on_message(self, data: "C2CMessage") -> None:
                 content=content,
                 metadata={"message_id": data.id},
             )
-        except Exception as e:
-            logger.error(f"Error handling QQ message: {e}")
+        except Exception:
+            logger.exception("Error handling QQ message")

opencane/cron/service.py

@@ -78,13 +78,19 @@ def __init__(
         on_job: Callable[[CronJob], Coroutine[Any, Any, str | None]] | None = None,
     ):
         self.store_path = store_path
-        self.on_job = on_job  # Callback to execute job, returns response text
+        self.on_job = on_job
         self._store: CronStore | None = None
+        self._last_mtime: float = 0.0
         self._timer_task: asyncio.Task | None = None
         self._running = False
 
     def _load_store(self) -> CronStore:
-        """Load jobs from disk."""
+        """Load jobs from disk and auto-reload when file changes externally."""
+        if self._store and self.store_path.exists():
+            mtime = self.store_path.stat().st_mtime
+            if mtime != self._last_mtime:
+                logger.info("Cron: jobs store modified externally, reloading")
+                self._store = None
         if self._store:
             return self._store
 
@@ -191,6 +197,7 @@ def _save_store(self) -> None:
         }
 
         self.store_path.write_text(json.dumps(data, indent=2))
+        self._last_mtime = self.store_path.stat().st_mtime
 
     async def start(self) -> None:
         """Start the cron service."""
@@ -246,6 +253,9 @@ async def tick():
 
     async def _on_timer(self) -> None:
         """Handle timer tick - run due jobs."""
+        # Reload on each tick so external edits (CLI/file) are honored promptly.
+        self._store = None
+        self._load_store()
         if not self._store:
             return
 

tests/test_cron_service.py

@@ -139,3 +139,30 @@ def test_compute_next_run_uses_reference_now_ms_for_cron() -> None:
 
     assert next_run is not None
     assert next_run > now_ms
+
+
+@pytest.mark.asyncio
+async def test_running_service_honors_external_disable(tmp_path) -> None:
+    store_path = tmp_path / "cron" / "jobs.json"
+    called: list[str] = []
+
+    async def _on_job(job) -> None:  # type: ignore[no-untyped-def]
+        called.append(job.id)
+
+    service = CronService(store_path, on_job=_on_job)
+    job = service.add_job(
+        name="external-disable",
+        schedule=CronSchedule(kind="every", every_ms=200),
+        message="hello",
+    )
+    await service.start()
+    try:
+        external = CronService(store_path)
+        updated = external.enable_job(job.id, enabled=False)
+        assert updated is not None
+        assert updated.enabled is False
+
+        await asyncio.sleep(0.35)
+        assert called == []
+    finally:
+        service.stop()

tests/test_filesystem_tools_workspace.py

@@ -78,3 +78,23 @@ async def test_allowed_dir_check_rejects_startswith_path_bypass(tmp_path: Path)
 
     assert result.startswith("Error: Path ")
     assert "outside allowed directory" in result
+
+
+@pytest.mark.asyncio
+async def test_edit_file_tool_not_found_reports_best_match(tmp_path: Path) -> None:
+    workspace = tmp_path / "workspace"
+    workspace.mkdir(parents=True)
+    target = workspace / "docs" / "plan.md"
+    target.parent.mkdir(parents=True)
+    target.write_text("alpha line\nbeta line\n", encoding="utf-8")
+
+    edit_tool = EditFileTool(workspace=workspace)
+    result = await edit_tool.execute(
+        path="docs/plan.md",
+        old_text="alpha lin\nbeta line\n",
+        new_text="patched",
+    )
+
+    assert "Best match" in result
+    assert "similar" in result
+    assert "old_text (provided)" in result

tests/test_mcp_tool.py

@@ -1,5 +1,9 @@
+import asyncio
+import sys
 from types import SimpleNamespace
 
+import pytest
+
 from opencane.agent.tools.mcp import MCPToolWrapper
 
 
@@ -64,3 +68,64 @@ def test_wrapper_normalizes_nullable_property_anyof() -> None:
         "description": "optional name",
         "nullable": True,
     }
+
+
+@pytest.mark.asyncio
+async def test_wrapper_execute_times_out_slow_mcp_call(monkeypatch: pytest.MonkeyPatch) -> None:
+    class _FakeTypes:
+        class TextContent:
+            def __init__(self, text: str):
+                self.text = text
+
+    monkeypatch.setitem(sys.modules, "mcp", SimpleNamespace(types=_FakeTypes))
+    monkeypatch.setattr("opencane.agent.tools.mcp.MCP_TOOL_TIMEOUT", 0.01)
+
+    class _SlowSession:
+        async def call_tool(self, _name: str, arguments: dict):  # type: ignore[no-untyped-def]
+            del arguments
+            await asyncio.sleep(0.1)
+            return SimpleNamespace(content=[])
+
+    wrapper = MCPToolWrapper(
+        _SlowSession(),
+        "test",
+        SimpleNamespace(
+            name="demo",
+            description="demo tool",
+            inputSchema={"type": "object", "properties": {}},
+        ),
+    )
+
+    result = await wrapper.execute()
+    assert "timed out" in result
+
+
+@pytest.mark.asyncio
+async def test_wrapper_execute_renders_text_blocks(monkeypatch: pytest.MonkeyPatch) -> None:
+    class _FakeTypes:
+        class TextContent:
+            def __init__(self, text: str):
+                self.text = text
+
+    monkeypatch.setitem(sys.modules, "mcp", SimpleNamespace(types=_FakeTypes))
+
+    class _Session:
+        async def call_tool(self, _name: str, arguments: dict):  # type: ignore[no-untyped-def]
+            del arguments
+            return SimpleNamespace(
+                content=[_FakeTypes.TextContent("line1"), {"type": "other"}]
+            )
+
+    wrapper = MCPToolWrapper(
+        _Session(),
+        "test",
+        SimpleNamespace(
+            name="demo",
+            description="demo tool",
+            inputSchema={"type": "object", "properties": {}},
+        ),
+    )
+
+    result = await wrapper.execute()
+    assert "line1" in result
+    assert "{'type': 'other'}" in result