forked from Rockachopa/Timmy-time-dashboard
Merge pull request '[loop-cycle-9] fix: thinking engine skips MCP tools to avoid cancel-scope errors (#72)' (#74) from fix/thinking-mcp-cancel-scope into main
This commit is contained in:
@@ -192,6 +192,8 @@ def create_timmy(
|
||||
db_file: str = "timmy.db",
|
||||
backend: str | None = None,
|
||||
model_size: str | None = None,
|
||||
*,
|
||||
skip_mcp: bool = False,
|
||||
) -> TimmyAgent:
|
||||
"""Instantiate the agent — Ollama or AirLLM, same public interface.
|
||||
|
||||
@@ -199,6 +201,10 @@ def create_timmy(
|
||||
db_file: SQLite file for Agno conversation memory (Ollama path only).
|
||||
backend: "ollama" | "airllm" | "auto" | None (reads config/env).
|
||||
model_size: AirLLM size — "8b" | "70b" | "405b" | None (reads config).
|
||||
skip_mcp: If True, omit MCP tool servers (Gitea, filesystem).
|
||||
Use for background tasks (thinking, QA) where MCP's
|
||||
stdio cancel-scope lifecycle conflicts with asyncio
|
||||
task cancellation.
|
||||
|
||||
Returns an Agno Agent or backend-specific agent — all expose
|
||||
print_response(message, stream).
|
||||
@@ -253,8 +259,10 @@ def create_timmy(
|
||||
if toolkit:
|
||||
tools_list.append(toolkit)
|
||||
|
||||
# Add MCP tool servers (lazy-connected on first arun())
|
||||
if use_tools:
|
||||
# Add MCP tool servers (lazy-connected on first arun()).
|
||||
# Skipped when skip_mcp=True — MCP's stdio transport uses anyio cancel
|
||||
# scopes that conflict with asyncio background task cancellation (#72).
|
||||
if use_tools and not skip_mcp:
|
||||
try:
|
||||
from timmy.mcp_tools import create_filesystem_mcp_tools, create_gitea_mcp_tools
|
||||
|
||||
|
||||
@@ -821,19 +821,16 @@ class ThinkingEngine:
|
||||
async def _call_agent(self, prompt: str) -> str:
|
||||
"""Call Timmy's agent to generate a thought.
|
||||
|
||||
Uses a separate session_id to avoid polluting user chat history.
|
||||
Creates a lightweight agent with skip_mcp=True to avoid the cancel-scope
|
||||
errors that occur when MCP stdio transports are spawned inside asyncio
|
||||
background tasks (#72). The thinking engine doesn't need Gitea or
|
||||
filesystem tools — it only needs the LLM.
|
||||
"""
|
||||
try:
|
||||
from timmy.session import chat
|
||||
from timmy.agent import create_timmy
|
||||
|
||||
return await chat(prompt, session_id="thinking")
|
||||
except Exception:
|
||||
# Fallback: create a fresh agent
|
||||
from timmy.agent import create_timmy
|
||||
|
||||
agent = create_timmy()
|
||||
run = await agent.arun(prompt, stream=False)
|
||||
return run.content if hasattr(run, "content") else str(run)
|
||||
agent = create_timmy(skip_mcp=True)
|
||||
run = await agent.arun(prompt, stream=False)
|
||||
return run.content if hasattr(run, "content") else str(run)
|
||||
|
||||
def _store_thought(self, content: str, seed_type: str) -> Thought:
|
||||
"""Persist a thought to SQLite."""
|
||||
|
||||
@@ -302,3 +302,42 @@ def test_create_timmy_no_extra_kwargs():
|
||||
f"Unknown Agent kwargs {invalid} — verify they exist in agno "
|
||||
f"before adding to VALID_AGENT_KWARGS"
|
||||
)
|
||||
|
||||
|
||||
# ── skip_mcp flag (#72) ─────────────────────────────────────────────────────
|
||||
|
||||
|
||||
def test_create_timmy_skip_mcp_omits_mcp_tools():
|
||||
"""create_timmy(skip_mcp=True) must not add MCP tool servers."""
|
||||
with (
|
||||
patch("timmy.agent.Agent"),
|
||||
patch("timmy.agent.Ollama"),
|
||||
patch("timmy.agent.SqliteDb"),
|
||||
patch("timmy.mcp_tools.create_gitea_mcp_tools") as mock_gitea_mcp,
|
||||
patch("timmy.mcp_tools.create_filesystem_mcp_tools") as mock_fs_mcp,
|
||||
):
|
||||
from timmy.agent import create_timmy
|
||||
|
||||
create_timmy(skip_mcp=True)
|
||||
|
||||
# MCP factory functions should never be called
|
||||
mock_gitea_mcp.assert_not_called()
|
||||
mock_fs_mcp.assert_not_called()
|
||||
|
||||
|
||||
def test_create_timmy_default_includes_mcp_tools():
|
||||
"""create_timmy() without skip_mcp should attempt MCP tool creation."""
|
||||
with (
|
||||
patch("timmy.agent.Agent"),
|
||||
patch("timmy.agent.Ollama"),
|
||||
patch("timmy.agent.SqliteDb"),
|
||||
patch("timmy.mcp_tools.create_gitea_mcp_tools", return_value=None) as mock_gitea_mcp,
|
||||
patch("timmy.mcp_tools.create_filesystem_mcp_tools", return_value=None) as mock_fs_mcp,
|
||||
):
|
||||
from timmy.agent import create_timmy
|
||||
|
||||
create_timmy(skip_mcp=False)
|
||||
|
||||
# MCP factories should be called when skip_mcp is False
|
||||
mock_gitea_mcp.assert_called_once()
|
||||
mock_fs_mcp.assert_called_once()
|
||||
|
||||
@@ -842,3 +842,46 @@ def test_thinking_chain_api_404(client):
|
||||
"""GET /thinking/api/{bad_id}/chain should return 404."""
|
||||
response = client.get("/thinking/api/nonexistent/chain")
|
||||
assert response.status_code == 404
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# _call_agent uses skip_mcp=True (#72)
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_call_agent_uses_skip_mcp(tmp_path):
|
||||
"""_call_agent must create_timmy(skip_mcp=True) to avoid cancel-scope errors."""
|
||||
engine = _make_engine(tmp_path)
|
||||
|
||||
mock_agent = AsyncMock()
|
||||
mock_run = AsyncMock()
|
||||
mock_run.content = "thought output"
|
||||
mock_agent.arun.return_value = mock_run
|
||||
|
||||
with patch("timmy.agent.create_timmy", return_value=mock_agent) as mock_factory:
|
||||
result = await engine._call_agent("test prompt")
|
||||
|
||||
mock_factory.assert_called_once_with(skip_mcp=True)
|
||||
mock_agent.arun.assert_awaited_once_with("test prompt", stream=False)
|
||||
assert result == "thought output"
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_call_agent_does_not_use_session_chat(tmp_path):
|
||||
"""_call_agent should NOT go through session.chat() (which uses the singleton
|
||||
with MCP tools). It creates its own agent directly."""
|
||||
engine = _make_engine(tmp_path)
|
||||
|
||||
mock_agent = AsyncMock()
|
||||
mock_run = AsyncMock()
|
||||
mock_run.content = "direct agent"
|
||||
mock_agent.arun.return_value = mock_run
|
||||
|
||||
with (
|
||||
patch("timmy.agent.create_timmy", return_value=mock_agent),
|
||||
patch("timmy.session.chat", new_callable=AsyncMock) as mock_session_chat,
|
||||
):
|
||||
await engine._call_agent("prompt")
|
||||
|
||||
mock_session_chat.assert_not_awaited()
|
||||
|
||||
Reference in New Issue
Block a user