mirror of
https://github.com/AstrBotDevs/AstrBot
synced 2026-07-15 17:30:13 +08:00
fix: resolve anyio compliance and type checking issues
- Fix orchestrator to use anyio.get_cancelled_exc_class() instead of anyio.CancelledError - Fix tests to properly check for anyio compliance (not violations) - Add type annotations for MCP exception fallbacks in registry.py - Remove unused type: ignore comment in mcp/tool.py - All 111 tests pass - uvx ty check passes - ruff check passes
This commit is contained in:
@@ -37,7 +37,7 @@ class BaseAstrbotAcpClient(ABC):
|
||||
...
|
||||
|
||||
@abstractmethod
|
||||
async def connect_to_unix_socket(self, path: str) -> None:
|
||||
async def connect_to_unix_socket(self, socket_path: str) -> None:
|
||||
"""Connect via Unix domain socket."""
|
||||
...
|
||||
|
||||
|
||||
@@ -11,8 +11,8 @@ import asyncio
|
||||
import json
|
||||
from typing import Any
|
||||
|
||||
from astrbot._internal.abc.lsp.base_astrbot_lsp_client import BaseAstrbotLspClient
|
||||
from astrbot import logger
|
||||
from astrbot._internal.abc.lsp.base_astrbot_lsp_client import BaseAstrbotLspClient
|
||||
|
||||
log = logger
|
||||
|
||||
|
||||
@@ -142,13 +142,13 @@ class McpClient(BaseAstrbotMcpClient):
|
||||
self.active: bool = True
|
||||
self.tools: list[mcp.Tool] = []
|
||||
self.server_errlogs: list[str] = []
|
||||
self.running_event = asyncio.Event()
|
||||
self.running_event = anyio.Event()
|
||||
self.process_pid: int | None = None
|
||||
|
||||
# Store connection config for reconnection
|
||||
self._mcp_server_config: dict | None = None
|
||||
self._mcp_server_config: McpServerConfig | None = None
|
||||
self._server_name: str | None = None
|
||||
self._reconnect_lock = asyncio.Lock() # Lock for thread-safe reconnection
|
||||
self._reconnect_lock = anyio.Lock() # Lock for thread-safe reconnection
|
||||
self._reconnecting: bool = False # For logging and debugging
|
||||
|
||||
async def connect(self) -> None:
|
||||
|
||||
@@ -19,7 +19,7 @@ class MCPTool(FunctionTool):
|
||||
|
||||
def __init__(
|
||||
self,
|
||||
mcp_tool: "mcp.types.Tool", # type: ignore[name-defined]
|
||||
mcp_tool: "mcp.types.Tool",
|
||||
mcp_client: "McpClient",
|
||||
mcp_server_name: str,
|
||||
**kwargs: Any,
|
||||
|
||||
@@ -7,9 +7,10 @@ and runs the main event loop that dispatches messages between components.
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
import asyncio
|
||||
from typing import Any
|
||||
|
||||
import anyio
|
||||
|
||||
from astrbot import logger
|
||||
from astrbot._internal.abc.base_astrbot_orchestrator import BaseAstrbotOrchestrator
|
||||
from astrbot._internal.protocols.abp.client import AstrbotAbpClient
|
||||
@@ -71,13 +72,14 @@ class AstrbotOrchestrator(BaseAstrbotOrchestrator):
|
||||
self._running = True
|
||||
log.info("AstrbotOrchestrator run loop started.")
|
||||
|
||||
# TODO: Replace asyncio.sleep loop with proper task scheduling using anyio.
|
||||
# The ASYNC110 warning is valid - this should use asyncio.Event.wait() or
|
||||
# a task group with cancellation, but keeping as-is for now to allow
|
||||
# periodic health checks during development. Final implementation should:
|
||||
# - Use anyio.TaskGroup for managing periodic tasks
|
||||
# - Implement proper cancellation via Event.wait()
|
||||
# - Avoid busy-waiting with sleep intervals
|
||||
stop_event = anyio.Event()
|
||||
|
||||
def set_stop() -> None:
|
||||
stop_event.set()
|
||||
|
||||
# Store the callback for cleanup
|
||||
self._stop_callback = set_stop
|
||||
|
||||
try:
|
||||
while self._running:
|
||||
# TODO: Periodic tasks:
|
||||
@@ -86,12 +88,15 @@ class AstrbotOrchestrator(BaseAstrbotOrchestrator):
|
||||
# - Check ACP client connections
|
||||
# - Process any pending star notifications
|
||||
|
||||
await asyncio.sleep(5)
|
||||
# Wait for 5 seconds or until shutdown is called
|
||||
with anyio.move_on_after(5):
|
||||
await stop_event.wait()
|
||||
|
||||
except asyncio.CancelledError:
|
||||
except anyio.get_cancelled_exc_class():
|
||||
log.info("Orchestrator run loop cancelled.")
|
||||
finally:
|
||||
self._running = False
|
||||
self._stop_callback = None
|
||||
log.info("AstrbotOrchestrator run loop stopped.")
|
||||
|
||||
async def register_star(self, name: str, star_instance: Any) -> None:
|
||||
|
||||
@@ -34,11 +34,11 @@ try:
|
||||
)
|
||||
except ImportError:
|
||||
DEFAULT_MCP_CONFIG: dict[str, Any] = {}
|
||||
MCPAllServicesFailedError = Exception
|
||||
MCPInitError = Exception
|
||||
MCPInitSummary = dict
|
||||
MCPInitTimeoutError = TimeoutError
|
||||
MCPShutdownTimeoutError = TimeoutError
|
||||
MCPAllServicesFailedError: type[Exception] = Exception
|
||||
MCPInitError: type[Exception] = Exception
|
||||
MCPInitSummary: type[dict] = dict
|
||||
MCPInitTimeoutError: type[TimeoutError] = TimeoutError
|
||||
MCPShutdownTimeoutError: type[TimeoutError] = TimeoutError
|
||||
|
||||
ENABLE_MCP_TIMEOUT_ENV = "ASTRBOT_MCP_TIMEOUT_ENABLED"
|
||||
MCP_INIT_TIMEOUT_ENV = "ASTRBOT_MCP_INIT_TIMEOUT"
|
||||
|
||||
@@ -7,6 +7,7 @@ Transport: stdio | SSE | streamable_http
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
import anyio
|
||||
import asyncio
|
||||
from typing import Any
|
||||
from unittest.mock import AsyncMock, MagicMock, patch
|
||||
@@ -92,7 +93,7 @@ class TestMcpClientReconnect:
|
||||
def test_reconnect_lock_exists(self, mcp_client):
|
||||
"""MCP client should have a reconnect lock."""
|
||||
assert mcp_client._reconnect_lock is not None
|
||||
assert isinstance(mcp_client._reconnect_lock, asyncio.Lock)
|
||||
assert isinstance(mcp_client._reconnect_lock, anyio.Lock)
|
||||
|
||||
def test_reconnecting_flag_initial_false(self, mcp_client):
|
||||
"""_reconnecting flag should be False initially."""
|
||||
@@ -100,26 +101,24 @@ class TestMcpClientReconnect:
|
||||
|
||||
|
||||
class TestMcpClientUsesAsyncioNotAnyio:
|
||||
"""TEST REQUIREMENT: Document asyncio vs anyio violations.
|
||||
"""TEST REQUIREMENT: Document asyncio vs anyio compliance.
|
||||
|
||||
Per openspec directive: "异步库使用anyio" (Use anyio as async library).
|
||||
|
||||
The MCP client implementation currently uses asyncio primitives:
|
||||
- asyncio.Lock instead of anyio.Lock
|
||||
- asyncio.Future instead of anyio.Future
|
||||
- asyncio.Event instead of anyio.Event
|
||||
|
||||
This is a VIOLATION of the architecture requirement.
|
||||
The MCP client implementation should use anyio primitives:
|
||||
- anyio.Lock instead of asyncio.Lock
|
||||
- anyio.Future instead of asyncio.Future
|
||||
- anyio.Event instead of asyncio.Event
|
||||
"""
|
||||
|
||||
def test_mcp_client_uses_asyncio_lock(self):
|
||||
"""VIOLATION: MCP client uses asyncio.Lock instead of anyio.Lock."""
|
||||
"""COMPLIANCE: MCP client uses anyio.Lock, not asyncio.Lock."""
|
||||
from astrbot._internal.protocols.mcp.client import McpClient
|
||||
|
||||
client = McpClient()
|
||||
|
||||
# Check if _reconnect_lock is asyncio.Lock (violation)
|
||||
assert isinstance(client._reconnect_lock, asyncio.Lock), (
|
||||
# Check that _reconnect_lock is anyio.Lock (compliant)
|
||||
assert isinstance(client._reconnect_lock, anyio.Lock), (
|
||||
"MCP client should use anyio.Lock instead of asyncio.Lock "
|
||||
"per the 'async_library: Use anyio' directive in openspec"
|
||||
)
|
||||
@@ -135,27 +134,27 @@ class TestMcpClientUsesAsyncioNotAnyio:
|
||||
# Note: Futures are created in call_star_tool, not at init
|
||||
|
||||
def test_orchestrator_run_loop_uses_asyncio_sleep(self):
|
||||
"""VIOLATION: Orchestrator uses asyncio.sleep instead of anyio."""
|
||||
"""COMPLIANCE: Orchestrator uses anyio.sleep, not asyncio.sleep."""
|
||||
from astrbot._internal.runtime.orchestrator import AstrbotOrchestrator
|
||||
|
||||
import inspect
|
||||
|
||||
source = inspect.getsource(AstrbotOrchestrator.run_loop)
|
||||
|
||||
assert "asyncio.sleep" in source, (
|
||||
assert "asyncio.sleep" not in source, (
|
||||
"Orchestrator should use anyio.sleep instead of asyncio.sleep "
|
||||
"per the 'async_library: Use anyio' directive in openspec"
|
||||
)
|
||||
|
||||
def test_orchestrator_run_loop_handles_asyncio_cancelled_error(self):
|
||||
"""VIOLATION: Orchestrator catches asyncio.CancelledError not anyio."""
|
||||
"""COMPLIANCE: Orchestrator catches anyio.CancelledError, not asyncio."""
|
||||
from astrbot._internal.runtime.orchestrator import AstrbotOrchestrator
|
||||
|
||||
import inspect
|
||||
|
||||
source = inspect.getsource(AstrbotOrchestrator.run_loop)
|
||||
|
||||
assert "asyncio.CancelledError" in source, (
|
||||
assert "asyncio.CancelledError" not in source, (
|
||||
"Orchestrator should catch anyio.CancelledError not asyncio.CancelledError "
|
||||
"per the 'async_library: Use anyio' directive in openspec"
|
||||
)
|
||||
|
||||
@@ -154,11 +154,11 @@ class TestAstrbotOrchestrator:
|
||||
|
||||
source = inspect.getsource(orchestrator.run_loop)
|
||||
|
||||
# Check if asyncio.sleep is used (VIOLATION)
|
||||
# Check that asyncio.sleep is NOT used (COMPLIANCE)
|
||||
uses_asyncio_sleep = "asyncio.sleep" in source
|
||||
|
||||
# This assertion documents the violation
|
||||
assert uses_asyncio_sleep, (
|
||||
# This assertion documents compliance
|
||||
assert not uses_asyncio_sleep, (
|
||||
"run_loop should use anyio.sleep instead of asyncio.sleep "
|
||||
"per the 'async_library: Use anyio' directive in openspec"
|
||||
)
|
||||
|
||||
Reference in New Issue
Block a user