mirror of
https://github.com/NCBM/plyngent.git
synced 2026-11-09 00:47:54 +08:00
runtime/mcp: spawn servers in their own process group
A terminal Ctrl+C is delivered to every process in the foreground process group, so a server spawned in our group died on any Ctrl+C during a turn - killing in-flight tool calls and leaving the connection dead until /mcp reconnect. start_new_session (POSIX) / CREATE_NEW_PROCESS_GROUP (Windows) puts each server in its own session/group instead. Tests assert the spawned server's process group and session differ from the CLI's (they used to be the same group).
This commit is contained in:
@@ -11,11 +11,15 @@ import asyncio
|
|||||||
import importlib.metadata
|
import importlib.metadata
|
||||||
import json
|
import json
|
||||||
import os
|
import os
|
||||||
|
import subprocess
|
||||||
|
import sys
|
||||||
from collections import deque
|
from collections import deque
|
||||||
from dataclasses import dataclass
|
from dataclasses import dataclass
|
||||||
from typing import TYPE_CHECKING, cast
|
from typing import TYPE_CHECKING, cast
|
||||||
|
|
||||||
if TYPE_CHECKING:
|
if TYPE_CHECKING:
|
||||||
|
from typing import Any
|
||||||
|
|
||||||
from plyngent.config.models import McpConfig, McpServerConfig
|
from plyngent.config.models import McpConfig, McpServerConfig
|
||||||
|
|
||||||
MCP_PROTOCOL_VERSION = "2025-06-18"
|
MCP_PROTOCOL_VERSION = "2025-06-18"
|
||||||
@@ -45,6 +49,19 @@ def _client_version() -> str:
|
|||||||
return "0.0.0"
|
return "0.0.0"
|
||||||
|
|
||||||
|
|
||||||
|
def _isolated_spawn_kwargs() -> dict[str, Any]:
|
||||||
|
"""Spawn kwargs that keep a server out of our process/console group.
|
||||||
|
|
||||||
|
Ctrl+C in the terminal is delivered to every process in the foreground
|
||||||
|
process group (POSIX) / console process group (Windows), so without this a
|
||||||
|
single Ctrl+C during a turn also killed every MCP server — killing in-flight
|
||||||
|
tool calls and leaving the connection dead until ``/mcp reconnect``.
|
||||||
|
"""
|
||||||
|
if sys.platform == "win32":
|
||||||
|
return {"creationflags": subprocess.CREATE_NEW_PROCESS_GROUP}
|
||||||
|
return {"start_new_session": True}
|
||||||
|
|
||||||
|
|
||||||
class McpConnectionError(RuntimeError):
|
class McpConnectionError(RuntimeError):
|
||||||
"""Server process failed to spawn, handshake, or answer in time."""
|
"""Server process failed to spawn, handshake, or answer in time."""
|
||||||
|
|
||||||
@@ -143,6 +160,7 @@ class McpServerConnection:
|
|||||||
stderr=asyncio.subprocess.PIPE,
|
stderr=asyncio.subprocess.PIPE,
|
||||||
env=os.environ | self._config.env,
|
env=os.environ | self._config.env,
|
||||||
cwd=self._config.cwd or None,
|
cwd=self._config.cwd or None,
|
||||||
|
**_isolated_spawn_kwargs(),
|
||||||
)
|
)
|
||||||
except (OSError, ValueError) as exc:
|
except (OSError, ValueError) as exc:
|
||||||
self.error = f"spawn {self._config.command!r}: {exc}"
|
self.error = f"spawn {self._config.command!r}: {exc}"
|
||||||
|
|||||||
@@ -2,6 +2,7 @@
|
|||||||
|
|
||||||
from __future__ import annotations
|
from __future__ import annotations
|
||||||
|
|
||||||
|
import os
|
||||||
import sys
|
import sys
|
||||||
from pathlib import Path
|
from pathlib import Path
|
||||||
|
|
||||||
@@ -97,6 +98,26 @@ async def test_close_terminates_subprocess() -> None:
|
|||||||
assert proc.returncode is not None
|
assert proc.returncode is not None
|
||||||
|
|
||||||
|
|
||||||
|
@pytest.mark.skipif(sys.platform == "win32", reason="POSIX process groups only")
|
||||||
|
async def test_server_spawned_outside_our_process_group() -> None:
|
||||||
|
"""Regression: a terminal Ctrl+C must not reach the MCP server.
|
||||||
|
|
||||||
|
The tty delivers SIGINT to every process in the foreground process group, so
|
||||||
|
a server spawned in *our* group died on any Ctrl+C during a turn (killing
|
||||||
|
in-flight tool calls and leaving the connection dead). ``start_new_session``
|
||||||
|
moves the server into its own session/group.
|
||||||
|
"""
|
||||||
|
connection = McpServerConnection("fake", _server_config())
|
||||||
|
await connection.start()
|
||||||
|
try:
|
||||||
|
proc = connection._proc
|
||||||
|
assert proc is not None
|
||||||
|
assert os.getpgid(proc.pid) != os.getpgid(0)
|
||||||
|
assert os.getsid(proc.pid) != os.getsid(0)
|
||||||
|
finally:
|
||||||
|
await connection.aclose()
|
||||||
|
|
||||||
|
|
||||||
async def test_manager_start_status_and_call() -> None:
|
async def test_manager_start_status_and_call() -> None:
|
||||||
manager = McpManager(McpConfig(servers={"fake": _server_config()}, disable=[]))
|
manager = McpManager(McpConfig(servers={"fake": _server_config()}, disable=[]))
|
||||||
await manager.ensure_started()
|
await manager.ensure_started()
|
||||||
|
|||||||
Reference in New Issue
Block a user