From 2f1609a86c52410a13056215b8e3457f7cd5b5ae Mon Sep 17 00:00:00 2001 From: kshitijk4poor <82637225+kshitijk4poor@users.noreply.github.com> Date: Mon, 7 Sep 2026 02:44:09 +0530 Subject: [PATCH] refactor: sms AIOHTTP_AVAILABLE flag; ElicitationHandler call_context defaults to a no-op thunk; drop stale TYPE_CHECKING/type-ignore in two tests MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Self-review follow-ups on the F821 sweep: - plugins/platforms/sms/adapter.py: the optional-import block now sets AIOHTTP_AVAILABLE like the homeassistant / webhook / whatsapp_cloud adapters, and both call sites test the flag. Removes the `if not aiohttp is not None:` double negation left by inlining `_aiohttp_available()`. - tools/mcp_tool_sampling.py: `call_context` defaults to `lambda: None` so the use site is a single call instead of an Optional guard; the only None caller was a test. The `from __future__ import annotations` was noise (`Context` is a runtime import). Comment names the actual cycle (mcp_tool_server_run imports this module). - gateway/platforms/helpers.py: drop the `from __future__ import annotations` — the only MessageEvent annotations are attribute-target locals, which are never evaluated. - tests/gateway/test_telegram_audio_vs_voice.py, test_video_context_note.py: module-level `from gateway.run import GatewayRunner` like the ~100 sibling files; the TYPE_CHECKING block + `# type: ignore[name-defined]` were contradicting each other. (tests/e2e/conftest.py and test_feishu.py keep TYPE_CHECKING deliberately: they stub telegram/discord before importing, and FeishuAdapter is gated on optional lark_oapi.) Mutation check: neutralising the thunk read (`captured = None`) fails test_captured_context_is_replayed_in_consent_call; restored → 14/14 green. ty on the three touched production files vs origin/main: 0 new, 6 resolved. --- gateway/platforms/helpers.py | 2 -- plugins/platforms/sms/adapter.py | 6 ++++-- tests/gateway/test_telegram_audio_vs_voice.py | 10 ++-------- tests/gateway/test_video_context_note.py | 10 ++-------- tests/tools/test_mcp_elicitation.py | 2 +- tools/mcp_tool_sampling.py | 9 ++++----- 6 files changed, 13 insertions(+), 26 deletions(-) diff --git a/gateway/platforms/helpers.py b/gateway/platforms/helpers.py index db2e4694c9..15d8b2fe11 100644 --- a/gateway/platforms/helpers.py +++ b/gateway/platforms/helpers.py @@ -2,8 +2,6 @@ stripping, thread participation tracking, GFM table → bullets, mention-pattern compilation, and fence-aware markdown chunking.""" -from __future__ import annotations - import json import logging import re diff --git a/plugins/platforms/sms/adapter.py b/plugins/platforms/sms/adapter.py index c4ef780084..d17d4e1191 100644 --- a/plugins/platforms/sms/adapter.py +++ b/plugins/platforms/sms/adapter.py @@ -30,7 +30,9 @@ from gateway.platforms._shared import get_scoped_secret as _get_scoped_secret try: import aiohttp from aiohttp import web + AIOHTTP_AVAILABLE = True except ImportError: # optional ([messaging] extra) + AIOHTTP_AVAILABLE = False aiohttp = None # type: ignore[assignment] web = None # type: ignore[assignment] @@ -75,7 +77,7 @@ def _new_session(**kwargs): def check_sms_requirements() -> bool: """Check if SMS adapter dependencies are available.""" - return aiohttp is not None and bool( + return AIOHTTP_AVAILABLE and bool( _get_scoped_secret("TWILIO_ACCOUNT_SID") and _get_scoped_secret("TWILIO_AUTH_TOKEN")) @@ -292,7 +294,7 @@ def _strip_markdown_for_sms(message: str) -> str: async def _standalone_send(pconfig, chat_id, message, *, thread_id=None, media_files=None, force_document=False): """Out-of-process SMS delivery via the Twilio REST API (standalone_sender_fn contract).""" auth_token = getattr(pconfig, "api_key", None) or _get_scoped_secret("TWILIO_AUTH_TOKEN", "") - if not aiohttp is not None: + if not AIOHTTP_AVAILABLE: return {"error": "aiohttp not installed. Run: pip install aiohttp"} account_sid = _get_scoped_secret("TWILIO_ACCOUNT_SID", "") from_number = os.getenv("TWILIO_PHONE_NUMBER", "") diff --git a/tests/gateway/test_telegram_audio_vs_voice.py b/tests/gateway/test_telegram_audio_vs_voice.py index 7b351a1e4d..7031b8a68d 100644 --- a/tests/gateway/test_telegram_audio_vs_voice.py +++ b/tests/gateway/test_telegram_audio_vs_voice.py @@ -18,17 +18,11 @@ import pytest from gateway.config import GatewayConfig, Platform from gateway.platforms.event import MessageEvent, MessageType +from gateway.run import GatewayRunner from gateway.session import SessionSource -from typing import TYPE_CHECKING - -if TYPE_CHECKING: - from gateway.run import GatewayRunner - - -def _make_runner(stt_enabled: bool = True) -> "GatewayRunner": # type: ignore[name-defined] - from gateway.run import GatewayRunner +def _make_runner(stt_enabled: bool = True) -> GatewayRunner: runner = GatewayRunner.__new__(GatewayRunner) runner.config = GatewayConfig(stt_enabled=stt_enabled) runner.adapters = {} diff --git a/tests/gateway/test_video_context_note.py b/tests/gateway/test_video_context_note.py index 69dc17d7a9..8319acc00b 100644 --- a/tests/gateway/test_video_context_note.py +++ b/tests/gateway/test_video_context_note.py @@ -6,17 +6,11 @@ import pytest from gateway.config import GatewayConfig, Platform from gateway.platforms.event import MessageEvent, MessageType +from gateway.run import GatewayRunner from gateway.session import SessionSource -from typing import TYPE_CHECKING - -if TYPE_CHECKING: - from gateway.run import GatewayRunner - - -def _make_runner() -> "GatewayRunner": # type: ignore[name-defined] - from gateway.run import GatewayRunner +def _make_runner() -> GatewayRunner: runner = GatewayRunner.__new__(GatewayRunner) runner.config = GatewayConfig() runner.adapters = {} diff --git a/tests/tools/test_mcp_elicitation.py b/tests/tools/test_mcp_elicitation.py index d9dddb5313..edc05bb4da 100644 --- a/tests/tools/test_mcp_elicitation.py +++ b/tests/tools/test_mcp_elicitation.py @@ -254,7 +254,7 @@ class TestElicitationHandlerContextBridge: call) the handler must still invoke the consent router -- just without the contextvar replay. Otherwise CLI/TUI sessions, which don't set HERMES_SESSION_PLATFORM, would break.""" - handler = ElicitationHandler("pay", {"timeout": 5}, call_context=None) + handler = ElicitationHandler("pay", {"timeout": 5}) params = _form_params() with patch("tools.approval_prompt.request_elicitation_consent", return_value="accept") as m: diff --git a/tools/mcp_tool_sampling.py b/tools/mcp_tool_sampling.py index 4ce8c96629..89a5276d41 100644 --- a/tools/mcp_tool_sampling.py +++ b/tools/mcp_tool_sampling.py @@ -1,8 +1,6 @@ """MCP client-side handlers for server-initiated requests: sampling (sampling/createMessage, text and tool-use results) and elicitation.""" -from __future__ import annotations - import asyncio import functools import json @@ -255,12 +253,13 @@ class ElicitationHandler: _ANSWER_RESULTS = {"accept": ("accept", "accepted"), "cancel": ("cancel", "errors")} def __init__(self, server_name: str, config: dict, - call_context: Optional[Callable[[], Optional[Context]]] = None): + call_context: Callable[[], Optional[Context]] = lambda: None): self.server_name = server_name # 5 min mirrors the gateway approval default so async surfaces (Telegram, Slack) can respond. self.timeout = _safe_numeric(config.get("timeout", 300), 300, float) # Returns the owning MCPServerTask's contextvars snapshot for the in-flight tool call (None - # between calls). A thunk, not the task: the task module imports this one. + # between calls). A thunk, not the task: mcp_tool_server_run imports this module, so + # MCPServerTask cannot be named here. self._call_context = call_context self.metrics = {"requests": 0, "accepted": 0, "declined": 0, "errors": 0} @@ -281,7 +280,7 @@ class ElicitationHandler: consent = functools.partial(request_elicitation_consent, message, description, timeout_seconds=int(self.timeout), surface=f"mcp-elicitation/{self.server_name}") - captured = self._call_context() if self._call_context else None + captured = self._call_context() return consent if captured is None else (lambda: captured.copy().run(consent)) async def __call__(self, context, params):