mirror of
https://github.com/langbot-app/LangBot.git
synced 2026-07-15 00:46:07 +00:00
refactor(mcp): make MCP test reuse the shared Box session (near-instant tests) (#2304)
* refactor(mcp): make MCP test reuse the shared Box session instead of a per-test session Testing an MCP server (config-page "test" button) previously spun up a fresh isolated mcp-test-<uuid> Box session every time: cold-start the container, run the dependency bootstrap, probe, then tear the whole session down. That is slow (tens of seconds) and, on an already-hosted server, wasteful — the server is already running in the shared session. Change the test to reuse the shared session / live process: - _build_box_session_id: transient tests now use mcp-shared, the same Box session as live servers, so a test reuses the running container (and, for an existing server, its live managed process) instead of a cold per-test session. - cleanup_session: a transient test no longer deletes the whole session (which under the shared model would kill every other MCP server in the container). It stops only its own process_id, exactly like a live server. Isolation is now at the process level (distinct process_id per server/test), not the session level. - test_mcp_server (persisted server): reuse the live connection with a real list_tools refresh/probe; only fall back to a full start() when there is no live connection to probe or the refresh fails, instead of an ERROR->start() rebuild. Trade-off: a failing test now shares the container with live servers rather than a throwaway session. Accepted deliberately in favour of near-instant tests; process-level isolation keeps a test from stopping another server's process. * chore(deps): pin langbot-plugin 0.4.9 for the nsjail RLIMIT_AS node/npx MCP fix --------- Co-authored-by: dadachann <185672915+dadachann@users.noreply.github.com>
This commit is contained in:
+1
-1
@@ -70,7 +70,7 @@ dependencies = [
|
|||||||
"chromadb>=1.0.0,<2.0.0",
|
"chromadb>=1.0.0,<2.0.0",
|
||||||
"qdrant-client (>=1.15.1,<2.0.0)",
|
"qdrant-client (>=1.15.1,<2.0.0)",
|
||||||
"pyseekdb==1.1.0.post3",
|
"pyseekdb==1.1.0.post3",
|
||||||
"langbot-plugin==0.4.8",
|
"langbot-plugin==0.4.9",
|
||||||
"asyncpg>=0.30.0",
|
"asyncpg>=0.30.0",
|
||||||
"line-bot-sdk>=3.19.0",
|
"line-bot-sdk>=3.19.0",
|
||||||
"matrix-nio>=0.25.2",
|
"matrix-nio>=0.25.2",
|
||||||
|
|||||||
@@ -188,10 +188,22 @@ class MCPService:
|
|||||||
persisted_session = runtime_mcp_session
|
persisted_session = runtime_mcp_session
|
||||||
|
|
||||||
async def _refresh_and_report() -> None:
|
async def _refresh_and_report() -> None:
|
||||||
if persisted_session.status == MCPSessionStatus.ERROR:
|
# Testing a persisted server should REUSE its live shared-session
|
||||||
|
# process, not rebuild it. Try a lightweight refresh (a real
|
||||||
|
# list_tools probe over the existing connection) first; only fall
|
||||||
|
# back to a full start() when the session has no live connection
|
||||||
|
# to probe (never connected, or the process is actually gone).
|
||||||
|
needs_start = persisted_session.status == MCPSessionStatus.ERROR or persisted_session.session is None
|
||||||
|
if needs_start:
|
||||||
await persisted_session.start()
|
await persisted_session.start()
|
||||||
else:
|
else:
|
||||||
await persisted_session.refresh()
|
try:
|
||||||
|
await persisted_session.refresh()
|
||||||
|
except Exception:
|
||||||
|
# The live connection was stale/dropped: reconnect once
|
||||||
|
# (reusing the live managed process where possible) and
|
||||||
|
# re-probe, instead of reporting a false failure.
|
||||||
|
await persisted_session.start()
|
||||||
# Surface the discovered tools so the config page can render them
|
# Surface the discovered tools so the config page can render them
|
||||||
# even for an already-hosted server.
|
# even for an already-hosted server.
|
||||||
ctx.metadata['runtime_info'] = persisted_session.get_runtime_info_dict()
|
ctx.metadata['runtime_info'] = persisted_session.get_runtime_info_dict()
|
||||||
|
|||||||
@@ -986,11 +986,14 @@ class RuntimeMCPSession:
|
|||||||
return self._box_stdio_runtime.uses_box_stdio()
|
return self._box_stdio_runtime.uses_box_stdio()
|
||||||
|
|
||||||
def _build_box_session_id(self) -> str:
|
def _build_box_session_id(self) -> str:
|
||||||
# Transient test sessions get their own isolated Box session so a
|
# Both live servers and transient config-page tests share ONE Box
|
||||||
# failing/short-lived test can never disturb the shared session that
|
# session ('mcp-shared'). A test therefore reuses the already-running
|
||||||
# hosts live, already-connected MCP servers.
|
# container (and, for an existing server, its live managed process)
|
||||||
if self.is_transient:
|
# instead of paying a full per-test session cold-start + dependency
|
||||||
return f'mcp-test-{self.server_uuid}'
|
# bootstrap. Isolation between a test and the live servers is provided
|
||||||
|
# at the *process* level: each server/test has its own process_id and a
|
||||||
|
# test only ever stops its own process_id (see cleanup_session), so it
|
||||||
|
# never disturbs another server's process or the shared session itself.
|
||||||
return 'mcp-shared'
|
return 'mcp-shared'
|
||||||
|
|
||||||
def _rewrite_path(self, path: str, host_path: str | None) -> str:
|
def _rewrite_path(self, path: str, host_path: str | None) -> str:
|
||||||
|
|||||||
@@ -370,16 +370,20 @@ class BoxStdioSessionRuntime:
|
|||||||
|
|
||||||
workspace = self._build_workspace(host_path=None)
|
workspace = self._build_workspace(host_path=None)
|
||||||
|
|
||||||
# Transient test sessions own their isolated Box session, so tear the
|
# Transient config-page tests now share the same 'mcp-shared' Box
|
||||||
# whole session down rather than leaking it. This cannot affect live
|
# session as live servers, so we must NOT tear the session down here —
|
||||||
# servers because they live in the separate shared session.
|
# that would kill every other MCP server in the container. A test is
|
||||||
|
# isolated at the process level: it ran under its own process_id, so we
|
||||||
|
# stop only that process, exactly like a live server does below. The
|
||||||
|
# shared session and all other servers' live processes are untouched.
|
||||||
|
# (Staged per-test workspace files are still cleaned up.)
|
||||||
if getattr(self.owner, 'is_transient', False):
|
if getattr(self.owner, 'is_transient', False):
|
||||||
try:
|
try:
|
||||||
await workspace.cleanup()
|
await workspace.stop_managed_process(self.process_id)
|
||||||
except Exception as exc:
|
except Exception as exc:
|
||||||
self.ap.logger.warning(
|
self.ap.logger.warning(
|
||||||
f'MCP server {self.server_name}: failed to delete transient test session '
|
f'MCP server {self.server_name}: failed to stop transient test process '
|
||||||
f'{self.owner._build_box_session_id()}: {type(exc).__name__}: {exc}'
|
f'process_id={self.process_id}: {type(exc).__name__}: {exc}'
|
||||||
)
|
)
|
||||||
await self._cleanup_staged_workspace()
|
await self._cleanup_staged_workspace()
|
||||||
return
|
return
|
||||||
|
|||||||
@@ -639,10 +639,13 @@ class TestGetRuntimeInfoDict:
|
|||||||
assert info['box_session_id'] == 'mcp-shared'
|
assert info['box_session_id'] == 'mcp-shared'
|
||||||
assert info['box_enabled'] is True
|
assert info['box_enabled'] is True
|
||||||
|
|
||||||
def test_transient_test_session_is_isolated_from_shared(self, mcp_module):
|
def test_transient_test_shares_session_but_isolated_by_process(self, mcp_module):
|
||||||
"""A transient test session (config-page "test", no persisted UUID)
|
"""A transient config-page "test" now shares the same 'mcp-shared' Box
|
||||||
must NOT share the live "mcp-shared" Box session. Regression: a failing
|
session as live servers (so a test reuses the running container / live
|
||||||
test churned the shared session and tore down healthy live servers."""
|
process instead of a cold per-test session bootstrap). Isolation is at
|
||||||
|
the PROCESS level: the test runs under its own process_id and only ever
|
||||||
|
stops that process_id, so it cannot disturb another server's live
|
||||||
|
process or the shared session itself."""
|
||||||
ap = _make_ap()
|
ap = _make_ap()
|
||||||
ap.box_service.available = True
|
ap.box_service.available = True
|
||||||
transient = _make_session(
|
transient = _make_session(
|
||||||
@@ -670,10 +673,12 @@ class TestGetRuntimeInfoDict:
|
|||||||
)
|
)
|
||||||
assert transient.is_transient is True
|
assert transient.is_transient is True
|
||||||
assert live.is_transient is False
|
assert live.is_transient is False
|
||||||
# Isolated session id for the test, shared for the live server.
|
# Both share ONE Box session ...
|
||||||
assert transient._build_box_session_id() == 'mcp-test-gen-uuid-123'
|
assert transient._build_box_session_id() == 'mcp-shared'
|
||||||
assert live._build_box_session_id() == 'mcp-shared'
|
assert live._build_box_session_id() == 'mcp-shared'
|
||||||
assert transient._build_box_session_id() != live._build_box_session_id()
|
assert transient._build_box_session_id() == live._build_box_session_id()
|
||||||
|
# ... but are isolated by distinct process_ids within that session.
|
||||||
|
assert transient._box_stdio_runtime.process_id != live._box_stdio_runtime.process_id
|
||||||
|
|
||||||
def test_stdio_session_refuses_when_box_unavailable(self, mcp_module):
|
def test_stdio_session_refuses_when_box_unavailable(self, mcp_module):
|
||||||
"""Policy: when Box is configured but unavailable (disabled in config
|
"""Policy: when Box is configured but unavailable (disabled in config
|
||||||
|
|||||||
@@ -2123,7 +2123,7 @@ requires-dist = [
|
|||||||
{ name = "ebooklib", specifier = ">=0.18" },
|
{ name = "ebooklib", specifier = ">=0.18" },
|
||||||
{ name = "gewechat-client", specifier = ">=0.1.5" },
|
{ name = "gewechat-client", specifier = ">=0.1.5" },
|
||||||
{ name = "html2text", specifier = ">=2024.2.26" },
|
{ name = "html2text", specifier = ">=2024.2.26" },
|
||||||
{ name = "langbot-plugin", specifier = "==0.4.8" },
|
{ name = "langbot-plugin", specifier = "==0.4.9" },
|
||||||
{ name = "langchain", specifier = ">=1.3.9" },
|
{ name = "langchain", specifier = ">=1.3.9" },
|
||||||
{ name = "langchain-core", specifier = ">=1.3.3" },
|
{ name = "langchain-core", specifier = ">=1.3.3" },
|
||||||
{ name = "langchain-text-splitters", specifier = ">=1.1.2" },
|
{ name = "langchain-text-splitters", specifier = ">=1.1.2" },
|
||||||
@@ -2187,7 +2187,7 @@ dev = [
|
|||||||
|
|
||||||
[[package]]
|
[[package]]
|
||||||
name = "langbot-plugin"
|
name = "langbot-plugin"
|
||||||
version = "0.4.8"
|
version = "0.4.9"
|
||||||
source = { registry = "https://pypi.org/simple" }
|
source = { registry = "https://pypi.org/simple" }
|
||||||
dependencies = [
|
dependencies = [
|
||||||
{ name = "aiofiles" },
|
{ name = "aiofiles" },
|
||||||
@@ -2208,9 +2208,9 @@ dependencies = [
|
|||||||
{ name = "watchdog" },
|
{ name = "watchdog" },
|
||||||
{ name = "websockets" },
|
{ name = "websockets" },
|
||||||
]
|
]
|
||||||
sdist = { url = "https://files.pythonhosted.org/packages/7f/f5/1b417a48d329f959a261eaf9a37b26197f18081d2b6f0ef3e4bbab687b81/langbot_plugin-0.4.8.tar.gz", hash = "sha256:e9614be248acf352acb9430d9e154e0abae981575de16812f68282c92a4e42d8", size = 334076, upload-time = "2026-07-03T04:15:28.233Z" }
|
sdist = { url = "https://files.pythonhosted.org/packages/54/52/f85939d3be929696dc5f9f3369f18bd6652981a8985c6e6f0dfe48e04f75/langbot_plugin-0.4.9.tar.gz", hash = "sha256:1cc2882f1be96cd52be6c392c3925e44cf785c97c10f82a321339636da318c6e", size = 334682, upload-time = "2026-07-03T04:57:42.813Z" }
|
||||||
wheels = [
|
wheels = [
|
||||||
{ url = "https://files.pythonhosted.org/packages/40/21/46bfa911ab27d6856a3b4363805c4bb8a657500cbc12e1e719d0b3b349dc/langbot_plugin-0.4.8-py3-none-any.whl", hash = "sha256:87c12d9d5579b0ab4dc43c9e559b4d15d7387db8c8b52d476cf27a3de74aedf5", size = 221244, upload-time = "2026-07-03T04:15:27.087Z" },
|
{ url = "https://files.pythonhosted.org/packages/47/12/1cf535377a81cf01607bbee5e3f4745a8c9f6dbb561887467001a182a533/langbot_plugin-0.4.9-py3-none-any.whl", hash = "sha256:693225a089ba38bc8d0b58d9e0fdf07b027b5be6b5e858cf7b4e56d095b68ba5", size = 221722, upload-time = "2026-07-03T04:57:41.578Z" },
|
||||||
]
|
]
|
||||||
|
|
||||||
[[package]]
|
[[package]]
|
||||||
|
|||||||
Reference in New Issue
Block a user