diff --git a/hermes_cli/main.py b/hermes_cli/main.py index 8cc01d1020..a9cd08e98f 100644 --- a/hermes_cli/main.py +++ b/hermes_cli/main.py @@ -5257,19 +5257,32 @@ def _warn_stale_dashboard_processes() -> None: except ValueError: pass else: - # Linux / macOS: scan /proc or fall back to ps. + # Linux / macOS: scan the process table via ps and match against + # the same explicit patterns list used on Windows. Using ps + # (rather than `pgrep -f "hermes.*dashboard"`) keeps us consistent + # with `hermes_cli.gateway._scan_gateway_pids` and avoids the + # greedy regex matching unrelated cmdlines that merely contain + # both words (e.g. a chat session discussing "dashboard"). result = subprocess.run( - ["pgrep", "-f", "hermes.*dashboard"], - capture_output=True, text=True, timeout=5, + ["ps", "-A", "-o", "pid=,command="], + capture_output=True, text=True, timeout=10, ) if result.returncode == 0: - for line in result.stdout.strip().splitlines(): + for line in result.stdout.split("\n"): + stripped = line.strip() + if not stripped or "grep" in stripped: + continue + parts = stripped.split(None, 1) + if len(parts) != 2: + continue try: - pid = int(line.strip()) - if pid != self_pid: - dashboard_pids.append(pid) + pid = int(parts[0]) except ValueError: - pass + continue + command = parts[1] + if (any(p in command for p in patterns) + and pid != self_pid): + dashboard_pids.append(pid) except (FileNotFoundError, subprocess.TimeoutExpired, OSError): return diff --git a/tests/hermes_cli/test_update_stale_dashboard.py b/tests/hermes_cli/test_update_stale_dashboard.py index 36a8c2e813..20c5eee98c 100644 --- a/tests/hermes_cli/test_update_stale_dashboard.py +++ b/tests/hermes_cli/test_update_stale_dashboard.py @@ -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 " 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