From 036907e32811eae77f36bc870161dde174d8c8b8 Mon Sep 17 00:00:00 2001 From: Teknium <127238744+teknium1@users.noreply.github.com> Date: Thu, 3 Sep 2026 00:08:40 -0700 Subject: [PATCH] refactor(tools): fold single-use locals into call sites in MCP stdio/loop/elicitation paths --- tools/mcp_tool_loop.py | 4 ++-- tools/mcp_tool_sampling.py | 9 +++------ tools/mcp_tool_transport.py | 10 ++++------ 3 files changed, 9 insertions(+), 14 deletions(-) diff --git a/tools/mcp_tool_loop.py b/tools/mcp_tool_loop.py index ff1b69903f..11533e87e0 100644 --- a/tools/mcp_tool_loop.py +++ b/tools/mcp_tool_loop.py @@ -149,10 +149,10 @@ def _run_on_mcp_loop(coro_or_factory, timeout: float = 30): if asyncio.iscoroutine(coro_or_factory): coro_or_factory.close() raise RuntimeError("MCP event loop is not running") - coro = coro_or_factory() if callable(coro_or_factory) else coro_or_factory # run_coroutine_threadsafe copies the LOOP thread's context, so a per-request profile scope # would vanish here; re-establish it inside the task's own context. - coro = _core._wrap_with_dashboard_oauth_flow(_core._wrap_with_home_override(coro)) + coro = _core._wrap_with_dashboard_oauth_flow(_core._wrap_with_home_override( + coro_or_factory() if callable(coro_or_factory) else coro_or_factory)) future = safe_schedule_threadsafe(coro, loop, logger=logger, log_message="MCP scheduling failed") if future is None: raise RuntimeError("MCP event loop unavailable (failed to schedule)") diff --git a/tools/mcp_tool_sampling.py b/tools/mcp_tool_sampling.py index a9ea21ba8c..11a09c388c 100644 --- a/tools/mcp_tool_sampling.py +++ b/tools/mcp_tool_sampling.py @@ -282,8 +282,7 @@ class ElicitationHandler: async def __call__(self, context, params): """SDK elicitation callback (``ElicitationFnT``). Returns ElicitResult or ErrorData.""" self.metrics["requests"] += 1 - # URL-mode (OAuth, payment) needs a browser + notifications/elicitation/complete — not implemented. - if getattr(params, "mode", "form") == "url": + if getattr(params, "mode", "form") == "url": # OAuth/payment: needs a browser + elicitation/complete; unsupported logger.info("MCP server '%s' requested URL-mode elicitation; declining " "(URL-mode elicitation not implemented)", self.server_name) return self._result("decline", "declined") @@ -293,14 +292,12 @@ class ElicitationHandler: # access) — read both or the user approves without seeing the fields. schema = getattr(params, "requestedSchema", None) or getattr(params, "requested_schema", None) or {} logger.info("MCP server '%s' elicitation request: %s", self.server_name, _sanitize_error(message)[:200]) - # Lazy import avoids import-order coupling with early-bootstrap tools.approval. - try: + try: # lazy import inside avoids import-order coupling with early-bootstrap tools.approval invoke_consent = self._consent_thunk(message, _format_elicitation_schema_summary(schema, self.server_name)) except Exception as exc: # pragma: no cover -- defensive logger.error("MCP server '%s' elicitation: approval system unavailable: %s", self.server_name, exc) return self._result("decline", "errors") - # Off-thread: inline, the sync consent flow would freeze the MCP loop and every RPC on it. - try: + try: # off-thread: inline, the sync consent flow would freeze the MCP loop and every RPC on it answer = await asyncio.wait_for( asyncio.to_thread(invoke_consent), timeout=self.timeout + self._OUTER_TIMEOUT_GRACE_SECONDS) except asyncio.TimeoutError: diff --git a/tools/mcp_tool_transport.py b/tools/mcp_tool_transport.py index d313555636..99eef3fd38 100644 --- a/tools/mcp_tool_transport.py +++ b/tools/mcp_tool_transport.py @@ -194,13 +194,12 @@ class MCPServerTransportMixin: if not command: raise ValueError(f"MCP server '{self.name}' has no 'command' in config") command, safe_env = _core._resolve_stdio_command(command, _core._build_safe_env(config.get("env"))) - args = config.get("args", []) - await _osv_malware_preflight(self.name, command, args) + await _osv_malware_preflight(self.name, command, config.get("args", [])) # Parent-death watchdog so kill -9 / crash can't leave the child tree running (POSIX-only). # AFTER the OSV preflight so the check inspects the real package. - command, args = _wrap_command_with_watchdog(command, args) + command, args = _wrap_command_with_watchdog(command, config.get("args", [])) server_params = _core.StdioServerParameters( - command=command, args=args, env=safe_env if safe_env else None, cwd=config.get("cwd"), + command=command, args=args, env=safe_env or None, cwd=config.get("cwd"), # Windows pipes can split non-UTF-8 bytes at chunk boundaries; substitute, don't raise. encoding_error_handler="replace") session_kwargs = self._session_kwargs() @@ -220,8 +219,7 @@ class MCPServerTransportMixin: new_pids = _filter_mcp_children(_core._snapshot_child_pids() - pids_before) if new_pids: self._track_spawned_children(new_pids) - # Tracked on the connection so in-flight calls fail fast when the subprocess dies. - self._stdio_child_pids = set(new_pids) + self._stdio_child_pids = set(new_pids) # so in-flight calls fail fast when the child dies async with _core.ClientSession(read_stream, write_stream, **session_kwargs) as session: # Bound the handshake here: ``connect_timeout`` only bounds the caller's ``.result()``. # A server that never answers ``initialize`` would otherwise hang forever, skip the