Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 5 additions & 2 deletions gateway/run.py
Original file line number Diff line number Diff line change
Expand Up @@ -2456,13 +2456,16 @@ async def start_gateway(config: Optional[GatewayConfig] = None) -> bool:
hermes_home = os.getenv("HERMES_HOME", "~/.hermes")
logger.error(
"Another gateway instance is already running (PID %d, HERMES_HOME=%s). "
"Use 'hermes gateway restart' to replace it, or 'hermes gateway stop' first.",
"Use 'hermes gateway run --replace' to force takeover, or 'hermes gateway stop' first.",
existing_pid, hermes_home,
)
print(
f"\n❌ Gateway already running (PID {existing_pid}).\n"
f" Use 'hermes gateway restart' to replace it,\n"
f" Use 'hermes gateway run --replace' to force takeover,\n"
f" or 'hermes gateway stop' to kill it first.\n"
f"\n"
f" For systemd/launchd services, reinstall with:\n"
f" hermes gateway install --force\n"
)
return False

Expand Down
54 changes: 53 additions & 1 deletion gateway/status.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,8 @@
"""

import os
import subprocess
import sys
from pathlib import Path
from typing import Optional

Expand All @@ -37,10 +39,53 @@ def remove_pid_file() -> None:
pass


def _is_hermes_gateway_process(pid: int) -> bool:
"""Check if a process with the given PID is actually a hermes gateway.

This prevents false positives when a PID is reused by an unrelated process
after the original gateway crashed without cleanup.
"""
# Patterns that indicate a hermes gateway process
gateway_patterns = (
"hermes_cli.main gateway",
"hermes gateway",
"gateway/run.py",
"gateway.run",
)

try:
# On Linux, check /proc/{pid}/cmdline directly (fast, no subprocess)
if sys.platform.startswith("linux"):
cmdline_path = Path(f"/proc/{pid}/cmdline")
if cmdline_path.exists():
cmdline = cmdline_path.read_bytes().replace(b"\x00", b" ").decode("utf-8", errors="replace")
return any(p in cmdline for p in gateway_patterns)

# macOS / fallback: use ps command
result = subprocess.run(
["ps", "-p", str(pid), "-o", "args="],
capture_output=True,
text=True,
timeout=5,
)
if result.returncode == 0:
cmdline = result.stdout.strip()
return any(p in cmdline for p in gateway_patterns)

except (subprocess.TimeoutExpired, FileNotFoundError, PermissionError, OSError):
pass

# If we can't determine, assume it's NOT a gateway (safer to allow startup)
return False


def get_running_pid() -> Optional[int]:
"""Return the PID of a running gateway instance, or ``None``.

Checks the PID file and verifies the process is actually alive.
Checks the PID file and verifies:
1. The process is actually alive
2. The process is actually a hermes gateway (not a PID collision)

Cleans up stale PID files automatically.
"""
pid_path = _get_pid_path()
Expand All @@ -49,6 +94,13 @@ def get_running_pid() -> Optional[int]:
try:
pid = int(pid_path.read_text().strip())
os.kill(pid, 0) # signal 0 = existence check, no actual signal sent

# Process exists — verify it's actually a hermes gateway
if not _is_hermes_gateway_process(pid):
# PID exists but it's not a hermes gateway — stale PID file
remove_pid_file()
return None

return pid
except (ValueError, ProcessLookupError, PermissionError):
# Stale PID file — process is gone
Expand Down
57 changes: 51 additions & 6 deletions hermes_cli/gateway.py
Original file line number Diff line number Diff line change
Expand Up @@ -156,19 +156,31 @@ def get_hermes_cli_path() -> str:
def generate_systemd_unit() -> str:
python_path = get_python_path()
working_dir = str(PROJECT_ROOT)
hermes_home = Path.home() / ".hermes"

return f"""[Unit]
Description={SERVICE_DESCRIPTION}
After=network.target
After=network-online.target
Wants=network-online.target

[Service]
Type=simple
ExecStart={python_path} -m hermes_cli.main gateway run
# Use --replace for idempotent startup: if a previous instance crashed
# and left a stale PID file, this ensures the new instance starts cleanly.
ExecStart={python_path} -m hermes_cli.main gateway run --replace
ExecStop={python_path} -m hermes_cli.main gateway stop
WorkingDirectory={working_dir}
# Only restart on actual failures, not clean exits.
# Use 'always' if you want the gateway to restart even after /update commands.
Restart=on-failure
RestartSec=10
RestartSec=15
# Limit restart bursts to prevent aggressive loops
StartLimitIntervalSec=300
StartLimitBurst=5
StandardOutput=journal
StandardError=journal
# Set HERMES_HOME explicitly in case user's environment isn't loaded
Environment=HERMES_HOME={hermes_home}

[Install]
WantedBy=default.target
Expand Down Expand Up @@ -271,6 +283,7 @@ def generate_launchd_plist() -> str:
working_dir = str(PROJECT_ROOT)
log_dir = Path.home() / ".hermes" / "logs"
log_dir.mkdir(parents=True, exist_ok=True)
hermes_home = Path.home() / ".hermes"

return f"""<?xml version="1.0" encoding="UTF-8"?>
<!DOCTYPE plist PUBLIC "-//Apple//DTD PLIST 1.0//EN" "http://www.apple.com/DTDs/PropertyList-1.0.dtd">
Expand All @@ -286,11 +299,18 @@ def generate_launchd_plist() -> str:
<string>hermes_cli.main</string>
<string>gateway</string>
<string>run</string>
<string>--replace</string>
</array>

<key>WorkingDirectory</key>
<string>{working_dir}</string>

<key>EnvironmentVariables</key>
<dict>
<key>HERMES_HOME</key>
<string>{hermes_home}</string>
</dict>

<key>RunAtLoad</key>
<true/>

Expand All @@ -300,6 +320,9 @@ def generate_launchd_plist() -> str:
<false/>
</dict>

<key>ThrottleInterval</key>
<integer>15</integer>

<key>StandardOutPath</key>
<string>{log_dir}/gateway.log</string>

Expand Down Expand Up @@ -377,10 +400,31 @@ def launchd_status(deep: bool = False):
# Gateway Runner
# =============================================================================

def run_gateway(verbose: bool = False):
"""Run the gateway in foreground."""
def run_gateway(verbose: bool = False, replace: bool = False):
"""Run the gateway in foreground.

Args:
verbose: Enable verbose logging.
replace: If True, forcibly remove any existing PID lock before starting.
Useful for systemd units to achieve idempotent startup.
"""
sys.path.insert(0, str(PROJECT_ROOT))

# Handle --replace: clean up stale PID lock before starting
if replace:
from gateway.status import get_running_pid, remove_pid_file
existing_pid = get_running_pid()
if existing_pid is not None and existing_pid != os.getpid():
print(f"⚠ Replacing existing gateway (PID {existing_pid})...")
try:
os.kill(existing_pid, signal.SIGTERM)
# Give it a moment to exit gracefully
import time
time.sleep(2)
except (ProcessLookupError, PermissionError):
pass
remove_pid_file()

from gateway.run import start_gateway

print("┌─────────────────────────────────────────────────────────┐")
Expand Down Expand Up @@ -765,7 +809,8 @@ def gateway_command(args):
# Default to run if no subcommand
if subcmd is None or subcmd == "run":
verbose = getattr(args, 'verbose', False)
run_gateway(verbose)
replace = getattr(args, 'replace', False)
run_gateway(verbose=verbose, replace=replace)
return

if subcmd == "setup":
Expand Down
4 changes: 4 additions & 0 deletions hermes_cli/main.py
Original file line number Diff line number Diff line change
Expand Up @@ -1315,6 +1315,10 @@ def main():
# gateway run (default)
gateway_run = gateway_subparsers.add_parser("run", help="Run gateway in foreground")
gateway_run.add_argument("-v", "--verbose", action="store_true")
gateway_run.add_argument(
"--replace", action="store_true",
help="Replace any existing gateway (clean PID lock before starting)"
)

# gateway start
gateway_start = gateway_subparsers.add_parser("start", help="Start gateway service")
Expand Down
78 changes: 78 additions & 0 deletions tests/gateway/test_status.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,78 @@
"""Tests for gateway/status.py PID file handling."""

import os
from pathlib import Path
from unittest import mock

import pytest


def test_is_hermes_gateway_process_returns_false_for_nonexistent_pid():
"""Verify _is_hermes_gateway_process returns False for PIDs that don't exist."""
from gateway.status import _is_hermes_gateway_process
# Use a very high PID unlikely to exist
result = _is_hermes_gateway_process(9999999)
assert result is False


def test_is_hermes_gateway_process_returns_false_for_non_gateway():
"""Verify _is_hermes_gateway_process returns False for non-hermes processes."""
from gateway.status import _is_hermes_gateway_process
# Current process is running pytest, not hermes gateway
result = _is_hermes_gateway_process(os.getpid())
assert result is False


def test_get_running_pid_returns_none_when_no_pidfile(tmp_path: Path):
"""Verify get_running_pid returns None when PID file doesn't exist."""
from gateway.status import get_running_pid
with mock.patch.dict(os.environ, {"HERMES_HOME": str(tmp_path)}):
result = get_running_pid()
assert result is None


def test_get_running_pid_cleans_stale_pidfile(tmp_path: Path):
"""Verify stale PID files (nonexistent process) are cleaned up."""
from gateway.status import get_running_pid
pid_file = tmp_path / "gateway.pid"
pid_file.write_text("9999999") # Very high PID, unlikely to exist

with mock.patch.dict(os.environ, {"HERMES_HOME": str(tmp_path)}):
result = get_running_pid()

assert result is None
assert not pid_file.exists(), "Stale PID file should be removed"


def test_get_running_pid_cleans_reused_pid(tmp_path: Path):
"""Verify PID files pointing to non-gateway processes are cleaned up.

This tests the core fix for issue #576: when a PID is reused by a
different process after a gateway crash, the startup should not fail.
"""
from gateway.status import get_running_pid

# Write current process PID (pytest, not gateway)
pid_file = tmp_path / "gateway.pid"
pid_file.write_text(str(os.getpid()))

with mock.patch.dict(os.environ, {"HERMES_HOME": str(tmp_path)}):
result = get_running_pid()

# Current process is pytest, not a gateway, so should return None
assert result is None
assert not pid_file.exists(), "PID file pointing to non-gateway should be removed"


def test_write_and_remove_pid_file(tmp_path: Path):
"""Test basic write and remove operations."""
from gateway.status import write_pid_file, remove_pid_file, _get_pid_path

with mock.patch.dict(os.environ, {"HERMES_HOME": str(tmp_path)}):
write_pid_file()
pid_path = _get_pid_path()
assert pid_path.exists()
assert pid_path.read_text() == str(os.getpid())

remove_pid_file()
assert not pid_path.exists()