Skip to content

Commit f89fa9e

Browse files
swissmoclaude
andcommitted
fix(component): give the loopback MCP session an actual generous timeout
llm_api._mcp_session handed the SDK Home Assistant's shared httpx client (helpers.httpx_client.get_async_client) so the SDK would never build its own client mid-event-loop (blocking CA-bundle load). But the SDK applies no timeout of its own when a caller-provided client is passed, and HA's shared client is built with no explicit timeout=, so it silently carried httpx's own hardcoded 5-second default — capping every tool call at 5s of read-idle no matter how generous _CALL_TOOL_TIMEOUT_SECONDS (300s) / _LIST_TOOLS_TIMEOUT_SECONDS (10s) looked. _mcp_session now builds its own short-lived client per session instead, with an explicit generous timeout and verify=False (the loopback URL is always plain http://127.0.0.1 and never negotiates TLS, so skipping verification is not a security relaxation — it just avoids the same blocking SSL-context setup the shared-client approach was working around). Found investigating a ~60s Assist-pipeline hang; ruled out as root cause for that specific report, but a real, independent bug regardless. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
1 parent 45824cd commit f89fa9e

3 files changed

Lines changed: 77 additions & 55 deletions

File tree

custom_components/ha_mcp_tools/llm_api.py

Lines changed: 49 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -50,15 +50,14 @@
5050
import importlib
5151
import logging
5252
from collections.abc import AsyncIterator, Iterable
53-
from contextlib import asynccontextmanager
53+
from contextlib import AsyncExitStack, asynccontextmanager
5454
from dataclasses import dataclass
5555
from typing import TYPE_CHECKING, Any, cast
5656

5757
import voluptuous as vol
5858
from homeassistant.core import HomeAssistant
5959
from homeassistant.exceptions import HomeAssistantError
6060
from homeassistant.helpers import llm
61-
from homeassistant.helpers.httpx_client import get_async_client
6261
from voluptuous_openapi import convert_to_voluptuous
6362

6463
from .const import (
@@ -226,47 +225,63 @@ async def async_probe_mcp_sdk(hass: HomeAssistant) -> bool:
226225
@asynccontextmanager
227226
async def _mcp_session(
228227
url: str,
229-
http_client: Any = None,
230228
) -> AsyncIterator[tuple[ClientSession, mcp_types.InitializeResult]]:
231229
"""Open an initialized MCP session against the loopback server.
232230
233231
Imports resolve from ``sys.modules`` — :func:`async_probe_mcp_sdk` did the
234232
real (blocking) import on the executor before the API was registered.
235233
236-
``http_client`` is Home Assistant's shared httpx client
237-
(``helpers.httpx_client.get_async_client``). Passing it is what keeps
238-
this loop-safe: without it the SDK constructs its own httpx client per
239-
session, whose SSL setup loads the CA bundle SYNCHRONOUSLY inside HA's
240-
event loop (live-found — HA's blocking-call monitor flagged this exact
241-
line). HA's shared client is built against the process-cached SSL
242-
context, and the SDK does not close caller-owned clients (HA core's mcp
243-
integration relies on the same contract).
234+
Builds a throwaway httpx client scoped to this one session rather than
235+
reusing Home Assistant's shared one (``helpers.httpx_client.
236+
get_async_client`` — the prior approach): the SDK applies NO timeout of
237+
its own when a caller-provided client is passed, so whatever timeout
238+
THAT client happens to carry becomes the real wire-level ceiling. HA's
239+
shared client is built with no explicit ``timeout=``, so it silently
240+
carries httpx's own hardcoded 5-second default — capping every tool call
241+
at 5 seconds of read-idle no matter how generous
242+
``_CALL_TOOL_TIMEOUT_SECONDS`` / ``_LIST_TOOLS_TIMEOUT_SECONDS`` looked
243+
(live-found investigating a ~60s Assist-pipeline hang: a real tool doing
244+
real work never got anywhere near its own asyncio budget).
245+
246+
``verify=False`` is not a security relaxation: ``url`` is always
247+
``http://127.0.0.1:<port>...`` (see ``async_register_llm_api``) — a
248+
plain-HTTP loopback call that never negotiates TLS — so building a real
249+
SSL context would be pure waste. It also keeps this loop-safe the same
250+
way the shared client did: an SSL context built with ``verify=True``
251+
loads the system CA bundle SYNCHRONOUSLY (live-found — HA's
252+
blocking-call monitor flagged this exact line when the SDK built its own
253+
default client), and skipping verification skips that load entirely.
254+
The client is entered on the exit stack so it closes with the rest of
255+
the session.
244256
"""
245257
from mcp.client.session import ClientSession
246258

247-
try:
248-
from mcp.client.streamable_http import streamable_http_client
249-
250-
transport = (
251-
streamable_http_client(url=url, http_client=http_client)
252-
if http_client is not None
253-
else streamable_http_client(url=url)
254-
)
255-
except ImportError:
256-
# Pre-rename SDK (an older ha-mcp resolved by a pip-spec override
257-
# pins an older fastmcp/mcp): same call shape, deprecated name, but
258-
# no http_client kwarg — it builds its own client, so on those old
259-
# SDKs the blocking-SSL-setup warning is the accepted cost.
260-
from mcp.client.streamable_http import (
261-
streamablehttp_client,
262-
)
259+
async with AsyncExitStack() as stack:
260+
try:
261+
from mcp.client.streamable_http import streamable_http_client
262+
except ImportError:
263+
# Pre-rename SDK (an older ha-mcp resolved by a pip-spec override
264+
# pins an older fastmcp/mcp): same call shape, deprecated name,
265+
# and no http_client kwarg — it builds its own default client, so
266+
# on those old SDKs the blocking-SSL-setup cost is unavoidable.
267+
from mcp.client.streamable_http import streamablehttp_client
268+
269+
transport = streamablehttp_client(url=url)
270+
else:
271+
import httpx
263272

264-
transport = streamablehttp_client(url=url)
273+
http_client = await stack.enter_async_context(
274+
httpx.AsyncClient(
275+
verify=False,
276+
timeout=httpx.Timeout(_CALL_TOOL_TIMEOUT_SECONDS),
277+
)
278+
)
279+
transport = streamable_http_client(url=url, http_client=http_client)
265280

266-
async with (
267-
transport as (read_stream, write_stream, _),
268-
ClientSession(read_stream, write_stream) as session,
269-
):
281+
read_stream, write_stream, _ = await stack.enter_async_context(transport)
282+
session = await stack.enter_async_context(
283+
ClientSession(read_stream, write_stream)
284+
)
270285
init_result = await session.initialize()
271286
yield session, init_result
272287

@@ -350,7 +365,7 @@ async def _forward_tool_call(
350365
try:
351366
async with (
352367
asyncio.timeout(_CALL_TOOL_TIMEOUT_SECONDS),
353-
_mcp_session(server_url, get_async_client(hass)) as (session, _init),
368+
_mcp_session(server_url) as (session, _init),
354369
):
355370
result = await session.call_tool(name, arguments)
356371
except _transport_errors() as err:
@@ -496,7 +511,7 @@ async def async_get_api_instance(
496511
try:
497512
async with (
498513
asyncio.timeout(_LIST_TOOLS_TIMEOUT_SECONDS),
499-
_mcp_session(self.server_url, get_async_client(self.hass)) as (
514+
_mcp_session(self.server_url) as (
500515
session,
501516
init_result,
502517
),

tests/src/unit/_embedded_stubs.py

Lines changed: 0 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -626,13 +626,6 @@ def setmod(name: str, **attrs: Any) -> ModuleType:
626626
"homeassistant.helpers.aiohttp_client",
627627
async_get_clientsession=MagicMock(name="async_get_clientsession"),
628628
)
629-
# Shared httpx client helper (#1745 llm_api): the SDK receives it as
630-
# http_client so session opens never build a client (and its blocking
631-
# SSL setup) inside the event loop.
632-
setmod(
633-
"homeassistant.helpers.httpx_client",
634-
get_async_client=MagicMock(name="get_async_client"),
635-
)
636629
setmod(
637630
"homeassistant.helpers.event",
638631
async_track_time_interval=MagicMock(

tests/src/unit/test_llm_api.py

Lines changed: 28 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -94,9 +94,8 @@ def _fake_session(
9494
init_result = SimpleNamespace(instructions=instructions)
9595

9696
@asynccontextmanager
97-
async def fake_mcp_session(url, http_client=None):
97+
async def fake_mcp_session(url):
9898
session.url = url
99-
session.http_client = http_client
10099
if raise_on_open is not None:
101100
raise raise_on_open
102101
if delay:
@@ -690,15 +689,25 @@ async def test_unknown_mode_degrades_to_tool_search(self, monkeypatch):
690689
assert [t.name for t in instance.tools] == ["ha_search_tools", "ha_call_tool"]
691690

692691

693-
class TestSharedHttpClientPassthrough:
694-
async def test_canonical_sdk_receives_hass_shared_client(self, monkeypatch):
695-
# The blocking-SSL-setup fix (live-found by HA's event-loop monitor):
696-
# sessions must hand HA's shared httpx client to the SDK so it never
697-
# constructs its own inside the loop. Faked at the sys.modules level
698-
# so _mcp_session's REAL wiring runs.
692+
class TestLoopbackHttpClientTimeout:
693+
async def test_sdk_receives_a_dedicated_client_with_generous_timeout(
694+
self, monkeypatch
695+
):
696+
# Regression test (kpop-timeout investigation): _mcp_session used to
697+
# hand the SDK Home Assistant's shared httpx client
698+
# (helpers.httpx_client.get_async_client). HA never configures that
699+
# client with an explicit timeout, so it silently carried httpx's own
700+
# hardcoded 5-second default — and the SDK applies no timeout of its
701+
# own when a caller-provided client is passed, so that 5s became the
702+
# REAL wire-level ceiling for every tool call regardless of how
703+
# generous _CALL_TOOL_TIMEOUT_SECONDS looked. _mcp_session must
704+
# instead build its own client with an explicit, generous timeout.
705+
# Faked at the sys.modules level so _mcp_session's REAL wiring runs.
699706
import sys
700707
from types import ModuleType
701708

709+
import httpx
710+
702711
opened: dict[str, Any] = {}
703712

704713
@asynccontextmanager
@@ -729,13 +738,18 @@ async def initialize(self):
729738
monkeypatch.setitem(sys.modules, "mcp.client.streamable_http", fake_transport)
730739
monkeypatch.setitem(sys.modules, "mcp.client.session", fake_session_mod)
731740

732-
shared_client = object()
733-
async with llm_api._mcp_session(
734-
"http://127.0.0.1:9584/private_x", shared_client
735-
):
736-
pass
741+
async with llm_api._mcp_session("http://127.0.0.1:9584/private_x"):
742+
used_client = opened["http_client"]
743+
assert isinstance(used_client, httpx.AsyncClient)
744+
assert used_client.timeout == httpx.Timeout(
745+
llm_api._CALL_TOOL_TIMEOUT_SECONDS
746+
)
747+
# Not httpx's hardcoded default — the exact bug being fixed.
748+
assert used_client.timeout != httpx.Timeout(5.0)
749+
assert not used_client.is_closed
737750

738-
assert opened["http_client"] is shared_client
751+
# Scoped to this one session: closed when the session exits.
752+
assert used_client.is_closed
739753

740754

741755
class TestPreRenameSdkFallback:

0 commit comments

Comments
 (0)