Compare commits

...

2 Commits

Author SHA1 Message Date
fddf78ca38 feat: MCP PID lock tests (#734)
Some checks failed
Contributor Attribution Check / check-attribution (pull_request) Failing after 39s
Docker Build and Publish / build-and-push (pull_request) Has been skipped
Supply Chain Audit / Scan PR for supply chain risks (pull_request) Successful in 46s
Tests / e2e (pull_request) Successful in 4m24s
Tests / test (pull_request) Failing after 56m25s
2026-04-15 02:50:49 +00:00
5e539b5eca feat: MCP PID file lock to prevent concurrent server instances (#734)
Preventive lock: check PID file before spawning, write after
spawn, release on shutdown. Stale PIDs auto-cleaned.

Closes #734
2026-04-15 02:50:43 +00:00
2 changed files with 147 additions and 0 deletions

View File

@@ -0,0 +1,60 @@
"""Tests for MCP PID file lock (#734)."""
import os
import tempfile
import pytest
from pathlib import Path
from unittest.mock import patch
# We test the functions by mocking _PID_DIR
import tools.mcp_pid_lock as pid_mod
class TestPidLock:
def setup_method(self):
self.tmp = tempfile.mkdtemp()
pid_mod._PID_DIR = Path(self.tmp)
def teardown_method(self):
import shutil
shutil.rmtree(self.tmp, ignore_errors=True)
def test_check_returns_none_when_no_file(self):
result = pid_mod.check_pid_lock("test-server")
assert result is None
def test_write_and_check_alive(self):
my_pid = os.getpid()
pid_mod.write_pid_lock("test-server", my_pid)
result = pid_mod.check_pid_lock("test-server")
assert result == my_pid
def test_stale_pid_cleaned(self):
# Write a PID that doesn't exist
pid_mod.write_pid_lock("test-server", 999999999)
result = pid_mod.check_pid_lock("test-server")
assert result is None
# PID file should be cleaned up
assert not pid_mod._pid_file("test-server").exists()
def test_corrupted_pid_cleaned(self):
pf = pid_mod._pid_file("test-server")
pf.write_text("not-a-number")
result = pid_mod.check_pid_lock("test-server")
assert result is None
assert not pf.exists()
def test_release_removes_file(self):
pid_mod.write_pid_lock("test-server", os.getpid())
assert pid_mod._pid_file("test-server").exists()
pid_mod.release_pid_lock("test-server")
assert not pid_mod._pid_file("test-server").exists()
def test_release_noop_when_no_file(self):
# Should not raise
pid_mod.release_pid_lock("nonexistent")
def test_multiple_servers_independent(self):
pid_mod.write_pid_lock("server-a", os.getpid())
assert pid_mod.check_pid_lock("server-a") == os.getpid()
assert pid_mod.check_pid_lock("server-b") is None

87
tools/mcp_pid_lock.py Normal file
View File

@@ -0,0 +1,87 @@
"""
PID file lock for MCP server instances — prevents concurrent spawning.
Before spawning an MCP server, check for a PID file. If the process is
alive, skip spawn. If stale, clean up. Write PID after spawn, remove
on shutdown.
Related: #714 (zombie cleanup), #734 (preventive lock)
"""
import os
import logging
from pathlib import Path
from typing import Optional
logger = logging.getLogger(__name__)
_PID_DIR = Path.home() / ".hermes" / "mcp"
def _pid_file(server_name: str) -> Path:
"""Return the PID file path for a named MCP server."""
_PID_DIR.mkdir(parents=True, exist_ok=True)
return _PID_DIR / f"{server_name}.pid"
def _is_process_alive(pid: int) -> bool:
"""Check if a process with the given PID is running."""
try:
os.kill(pid, 0) # Signal 0 = check existence without killing
return True
except (ProcessLookupError, PermissionError, OSError):
return False
def check_pid_lock(server_name: str) -> Optional[int]:
"""Check if an MCP server instance is already running.
Returns the running PID if locked, None if safe to spawn.
"""
pf = _pid_file(server_name)
if not pf.exists():
return None
try:
pid = int(pf.read_text().strip())
except (ValueError, OSError):
# Corrupted PID file — clean up
logger.warning("MCP PID file %s corrupted, removing", pf)
try:
pf.unlink()
except OSError:
pass
return None
if _is_process_alive(pid):
logger.info("MCP server '%s' already running (PID %d), skipping spawn", server_name, pid)
return pid
# Stale PID file — process is dead
logger.info("MCP server '%s' PID %d is stale, cleaning up", server_name, pid)
try:
pf.unlink()
except OSError:
pass
return None
def write_pid_lock(server_name: str, pid: int) -> None:
"""Write PID file after successful MCP server spawn."""
pf = _pid_file(server_name)
try:
pf.write_text(str(pid))
logger.debug("MCP server '%s' PID lock written: %d", server_name, pid)
except OSError as e:
logger.warning("Failed to write PID lock for '%s': %s", server_name, e)
def release_pid_lock(server_name: str) -> None:
"""Remove PID file on MCP server shutdown."""
pf = _pid_file(server_name)
try:
if pf.exists():
pf.unlink()
logger.debug("MCP server '%s' PID lock released", server_name)
except OSError as e:
logger.warning("Failed to release PID lock for '%s': %s", server_name, e)