mirror of
https://github.com/NousResearch/hermes-agent.git
synced 2026-05-04 02:21:47 +00:00
fix(cli): tighten stale-dashboard match to explicit patterns
Replace the Linux/macOS pgrep regex ("hermes.*dashboard") with a ps
scan + the same explicit patterns list already used on the Windows
branch and in hermes_cli.gateway._scan_gateway_pids:
hermes dashboard
hermes_cli.main dashboard
hermes_cli/main.py dashboard
The old greedy regex would match any cmdline containing both words —
e.g. a chat session whose argv mentions "dashboard" or an unrelated
grafana/dashboard-server process. Added regression tests for both.
Follow-up tightening on #16881.
This commit is contained in:
parent
66b1142384
commit
9048fd020f
2 changed files with 109 additions and 25 deletions
|
|
@ -16,24 +16,36 @@ import pytest
|
|||
from hermes_cli.main import _warn_stale_dashboard_processes
|
||||
|
||||
|
||||
def _ps_line(pid: int, cmd: str) -> str:
|
||||
"""Format a line as it would appear in ``ps -A -o pid=,command=`` output."""
|
||||
return f"{pid:>7} {cmd}"
|
||||
|
||||
|
||||
class TestWarnStaleDashboardProcesses:
|
||||
"""Unit tests for the stale dashboard process warning."""
|
||||
|
||||
def test_no_warning_when_no_dashboard_running(self, capsys):
|
||||
"""pgrep finds nothing — no warning should be printed."""
|
||||
"""ps returns no matching processes — no warning should be printed."""
|
||||
with patch("subprocess.run") as mock_run:
|
||||
mock_run.return_value = MagicMock(
|
||||
returncode=1, stdout="", stderr=""
|
||||
returncode=0,
|
||||
stdout=_ps_line(111, "/usr/bin/python3 -m some.other.module")
|
||||
+ "\n"
|
||||
+ _ps_line(222, "/usr/bin/bash")
|
||||
+ "\n",
|
||||
stderr="",
|
||||
)
|
||||
_warn_stale_dashboard_processes()
|
||||
output = capsys.readouterr().out
|
||||
assert "dashboard process" not in output
|
||||
|
||||
def test_warning_printed_for_running_dashboard(self, capsys):
|
||||
"""pgrep finds a dashboard PID — warning with PID should appear."""
|
||||
"""ps finds a dashboard PID — warning with PID should appear."""
|
||||
with patch("subprocess.run") as mock_run:
|
||||
mock_run.return_value = MagicMock(
|
||||
returncode=0, stdout="12345\n", stderr=""
|
||||
returncode=0,
|
||||
stdout=_ps_line(12345, "python3 -m hermes_cli.main dashboard --port 9119") + "\n",
|
||||
stderr="",
|
||||
)
|
||||
_warn_stale_dashboard_processes()
|
||||
output = capsys.readouterr().out
|
||||
|
|
@ -45,7 +57,13 @@ class TestWarnStaleDashboardProcesses:
|
|||
"""Multiple dashboard processes — all PIDs listed."""
|
||||
with patch("subprocess.run") as mock_run:
|
||||
mock_run.return_value = MagicMock(
|
||||
returncode=0, stdout="12345\n12346\n12347\n", stderr=""
|
||||
returncode=0,
|
||||
stdout="\n".join([
|
||||
_ps_line(12345, "python3 -m hermes_cli.main dashboard --port 9119"),
|
||||
_ps_line(12346, "hermes dashboard --port 9120 --no-open"),
|
||||
_ps_line(12347, "python /home/x/hermes_cli/main.py dashboard"),
|
||||
]) + "\n",
|
||||
stderr="",
|
||||
)
|
||||
_warn_stale_dashboard_processes()
|
||||
output = capsys.readouterr().out
|
||||
|
|
@ -57,35 +75,39 @@ class TestWarnStaleDashboardProcesses:
|
|||
def test_self_pid_excluded(self, capsys):
|
||||
"""The current process PID should not be reported."""
|
||||
with patch("subprocess.run") as mock_run:
|
||||
# Return the current process PID
|
||||
mock_run.return_value = MagicMock(
|
||||
returncode=0,
|
||||
stdout=f"{os.getpid()}\n12345\n",
|
||||
stdout="\n".join([
|
||||
_ps_line(os.getpid(), "python3 -m hermes_cli.main dashboard"),
|
||||
_ps_line(12345, "hermes dashboard --port 9119"),
|
||||
]) + "\n",
|
||||
stderr="",
|
||||
)
|
||||
_warn_stale_dashboard_processes()
|
||||
output = capsys.readouterr().out
|
||||
assert str(os.getpid()) not in output
|
||||
# The self PID may still appear inside an unrelated context, so anchor
|
||||
# the check to "PID <self>" which is how the warning prints.
|
||||
assert f"PID {os.getpid()}" not in output
|
||||
assert "PID 12345" in output
|
||||
|
||||
def test_pgrep_not_found_silently_ignored(self, capsys):
|
||||
"""If pgrep is missing (FileNotFoundError), no crash, no warning."""
|
||||
def test_ps_not_found_silently_ignored(self, capsys):
|
||||
"""If ps is missing (FileNotFoundError), no crash, no warning."""
|
||||
with patch("subprocess.run", side_effect=FileNotFoundError):
|
||||
_warn_stale_dashboard_processes()
|
||||
output = capsys.readouterr().out
|
||||
assert output == ""
|
||||
|
||||
def test_pgrep_timeout_silently_ignored(self, capsys):
|
||||
"""If pgrep times out, no crash, no warning."""
|
||||
def test_ps_timeout_silently_ignored(self, capsys):
|
||||
"""If ps times out, no crash, no warning."""
|
||||
import subprocess as sp
|
||||
|
||||
with patch("subprocess.run", side_effect=sp.TimeoutExpired("pgrep", 5)):
|
||||
with patch("subprocess.run", side_effect=sp.TimeoutExpired("ps", 10)):
|
||||
_warn_stale_dashboard_processes()
|
||||
output = capsys.readouterr().out
|
||||
assert output == ""
|
||||
|
||||
def test_empty_pgrep_output_no_warning(self, capsys):
|
||||
"""pgrep returns 0 but empty stdout — no warning."""
|
||||
def test_empty_ps_output_no_warning(self, capsys):
|
||||
"""ps returns 0 but empty stdout — no warning."""
|
||||
with patch("subprocess.run") as mock_run:
|
||||
mock_run.return_value = MagicMock(
|
||||
returncode=0, stdout="\n", stderr=""
|
||||
|
|
@ -95,12 +117,61 @@ class TestWarnStaleDashboardProcesses:
|
|||
assert "dashboard process" not in output
|
||||
|
||||
def test_invalid_pid_lines_skipped(self, capsys):
|
||||
"""Non-numeric lines from pgrep should be skipped gracefully."""
|
||||
"""Malformed ps lines should be skipped gracefully."""
|
||||
with patch("subprocess.run") as mock_run:
|
||||
mock_run.return_value = MagicMock(
|
||||
returncode=0, stdout="notapid\n12345\nalso_bad\n", stderr=""
|
||||
returncode=0,
|
||||
stdout="\n".join([
|
||||
"notapid hermes dashboard --bad",
|
||||
_ps_line(12345, "hermes dashboard --port 9119"),
|
||||
" ",
|
||||
]) + "\n",
|
||||
stderr="",
|
||||
)
|
||||
_warn_stale_dashboard_processes()
|
||||
output = capsys.readouterr().out
|
||||
assert "PID 12345" in output
|
||||
assert "1 dashboard process" in output
|
||||
|
||||
def test_unrelated_process_containing_word_dashboard_not_matched(self, capsys):
|
||||
"""A process whose cmdline contains 'dashboard' but isn't a hermes
|
||||
dashboard process must NOT be flagged. This guards against the old
|
||||
``pgrep -f "hermes.*dashboard"`` greedy regex that matched e.g. a
|
||||
chat session argv containing both words.
|
||||
"""
|
||||
with patch("subprocess.run") as mock_run:
|
||||
mock_run.return_value = MagicMock(
|
||||
returncode=0,
|
||||
stdout="\n".join([
|
||||
# Legitimate dashboard — should match.
|
||||
_ps_line(12345, "python3 -m hermes_cli.main dashboard --port 9119"),
|
||||
# hermes running something else, with "dashboard" as a
|
||||
# substring of an unrelated arg — should NOT match.
|
||||
_ps_line(22222, "python3 -m hermes_cli.main chat -q 'rewrite my dashboard'"),
|
||||
# Completely unrelated process mentioning dashboard.
|
||||
_ps_line(33333, "node /opt/grafana/dashboard-server.js"),
|
||||
]) + "\n",
|
||||
stderr="",
|
||||
)
|
||||
_warn_stale_dashboard_processes()
|
||||
output = capsys.readouterr().out
|
||||
assert "1 dashboard process" in output
|
||||
assert "PID 12345" in output
|
||||
assert "PID 22222" not in output
|
||||
assert "PID 33333" not in output
|
||||
|
||||
def test_grep_lines_ignored(self, capsys):
|
||||
"""Lines containing 'grep' (from a pipe in ps output) are ignored."""
|
||||
with patch("subprocess.run") as mock_run:
|
||||
mock_run.return_value = MagicMock(
|
||||
returncode=0,
|
||||
stdout="\n".join([
|
||||
_ps_line(99999, "grep hermes dashboard"),
|
||||
_ps_line(12345, "hermes dashboard --port 9119"),
|
||||
]) + "\n",
|
||||
stderr="",
|
||||
)
|
||||
_warn_stale_dashboard_processes()
|
||||
output = capsys.readouterr().out
|
||||
assert "PID 99999" not in output
|
||||
assert "PID 12345" in output
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue