hermes_cli: add nssm_truth helper for service-state-vs-reality checks by bbasketballer75 · Pull Request #2 · bbasketballer75/hermes-agent

bbasketballer75 · 2026-07-05T22:59:04Z

Problem

The NSSM false-positive pattern is real and recurring: when a service gets into the PAUSED state on Windows NSSM 2.24, the actual python process is often still alive and bound to its port. NSSM's state-lag means the service reports PAUSED while:

The PID file (gateway.lock / gateway.pid) still points to a live python
The port (e.g. 9720) is still bound and serving HTTP 200

Three scripts in this codebase have hit this pattern and copy-pasted the same PID-file + alive-check logic to handle it:

scripts/nssm-self-heal.py
scripts/fix-gateway-paused-state.py
scripts/audit-tools.py

Fix

This PR consolidates the duplicated logic into a single shared helper module at hermes_cli/nssm_truth.py:

from hermes_cli.nssm_truth import is_hermes_service_actually_running

ok, detail = is_hermes_service_actually_running("HermesGateway")
if ok:
    # Live gateway per PID file — treat NSSM PAUSED as benign
    ...
else:
    # Confirmed dead — force recovery
    ...

The helper checks (in order):

gateway.lock / gateway.pid for a live python with hermes_cli/main + gateway in argv (source of truth)
nssm status <service> as fallback (may report PAUSED false-positive)

Returns (is_running: bool, detail: str) tuple.

Why a separate PR from #1

PR #1 (memory_tool: auto-consolidate on overflow) is feature work. This PR is a refactor with 20 tests. Keeping them separate makes review easier.

Tests

20 cases in tests/hermes_cli/test_nssm_truth.py cover:

PID alive/dead detection (Windows: OpenProcess + GetExitCodeProcess, exit code 259 = STILL_ACTIVE)
Lock file parsing (gateway.lock preferred over gateway.pid)
Malformed JSON handling (returns None, no crash)
Missing file handling (returns False)
PID-file takes precedence over nssm state (NSSM PAUSED does NOT trigger if PID file has live gateway)
nssm not in PATH graceful fallback
Service name override
NSSM state transitions (RUNNING/PAUSED/STOPPED)

Net effect

~5088 chars of duplicated inline logic eliminated
7528 chars of single-source-of-truth helper added
469 insertions, 0 deletions (refactor is additive)
20 new tests

Risk

Low. Pure refactor with no behavior change beyond consolidation. The 3 call sites need to be updated separately (not in this PR) to use the new helper, which I'll do as a follow-up to keep this PR focused.

gemini-code-assist

Code Review

This pull request introduces several security, robustness, and platform compatibility enhancements, including user-defined command deny rules, an opt-in egress lockdown toggle (docker_network) for Docker environments, graceful character-budget truncation for file reads, and a unified service-running check (nssm_truth.py). The code review feedback highlights critical cross-platform compatibility issues in the new nssm_truth.py module—such as Windows-only imports and APIs being called on POSIX systems—along with a hardcoded personal directory path and potential crashes on malformed JSON. Additionally, the reviewer noted a type-handling issue with the new deny rules configuration and false-positive warnings in the file truncation logic when a line matches the maximum character limit exactly.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

gemini-code-assist · 2026-07-05T23:01:12Z

+import json
+import os
+import subprocess
+from ctypes import wintypes


On non-Windows platforms (Linux/macOS), from ctypes import wintypes raises an ImportError because wintypes is only available on Windows. Since this module is imported or scanned during startup/testing, it will crash the application on non-Windows systems.

Please guard this import with a try/except block to ensure cross-platform compatibility.

Suggested change

from ctypes import wintypes

try:

from ctypes import wintypes

except ImportError:

wintypes = None

gemini-code-assist · 2026-07-05T23:01:13Z

+def is_pid_alive(pid: int) -> bool:
+    """Windows: check if PID is still in the process table.
+
+    Uses OpenProcess + GetExitCodeProcess. Exit code 259 (STILL_ACTIVE)
+    means the process is alive.
+    """
+    if pid <= 0:
+        return False
+    try:
+        kernel32 = ctypes.WinDLL("kernel32", use_last_error=True)
+    except OSError:
+        return False
+    h = kernel32.OpenProcess(PROCESS_QUERY_LIMITED_INFORMATION, False, pid)
+    if not h:
+        return False
+    try:
+        code = wintypes.DWORD()
+        if kernel32.GetExitCodeProcess(h, ctypes.byref(code)):
+            return code.value == STILL_ACTIVE
+    finally:
+        kernel32.CloseHandle(h)
+    return False


There are two issues here:

POSIX Support: On non-Windows platforms, we should provide a fallback implementation using os.kill(pid, 0) so that the process check works cross-platform.

ctypes Pointer Truncation: On 64-bit Windows, calling OpenProcess, GetExitCodeProcess, and CloseHandle without explicitly defining their argtypes and restype can lead to pointer truncation (since ctypes defaults to 32-bit integers for return values and arguments). This can cause crashes or invalid handle errors.

Let's define the argtypes and restype for these functions, and add a POSIX fallback.

def is_pid_alive(pid: int) -> bool: """Check if PID is still in the process table. On Windows, uses OpenProcess + GetExitCodeProcess. On POSIX, uses os.kill(pid, 0). """ if pid <= 0: return False if os.name != "nt": try: os.kill(pid, 0) return True except OSError: return False if wintypes is None: return False try: kernel32 = ctypes.WinDLL("kernel32", use_last_error=True) kernel32.OpenProcess.argtypes = [wintypes.DWORD, wintypes.BOOL, wintypes.DWORD] kernel32.OpenProcess.restype = wintypes.HANDLE kernel32.GetExitCodeProcess.argtypes = [wintypes.HANDLE, ctypes.POINTER(wintypes.DWORD)] kernel32.GetExitCodeProcess.restype = wintypes.BOOL kernel32.CloseHandle.argtypes = [wintypes.HANDLE] kernel32.CloseHandle.restype = wintypes.BOOL except OSError: return False h = kernel32.OpenProcess(PROCESS_QUERY_LIMITED_INFORMATION, False, pid) if not h: return False try: code = wintypes.DWORD() if kernel32.GetExitCodeProcess(h, ctypes.byref(code)): return code.value == STILL_ACTIVE finally: kernel32.CloseHandle(h) return False

gemini-code-assist · 2026-07-05T23:01:13Z

+    candidates = [
+        Path(r"C:\Users\bbask\AppData\Local\hermes"),
+        Path.home() / "AppData/Local/hermes",
+    ]


The path C:\Users\bbask\AppData\Local\hermes contains a hardcoded personal username (bbask). This will fail for any other user on Windows.

We should remove this hardcoded path and instead dynamically resolve the local AppData directory using the LOCALAPPDATA environment variable.

local_appdata = os.environ.get("LOCALAPPDATA") candidates = [] if local_appdata: candidates.append(Path(local_appdata) / "hermes") candidates.append(Path.home() / "AppData/Local/hermes")

gemini-code-assist · 2026-07-05T23:01:13Z

+        try:
+            return json.loads(p.read_text(encoding="utf-8"))
+        except (json.JSONDecodeError, OSError, UnicodeDecodeError):
+            continue


If the lock file contains a valid JSON value that is not an object (e.g., [] or "string"), json.loads will return a list or string. This will cause _lock_file_indicates_live_gateway to crash with an AttributeError when it tries to call .get("pid").

We should defensively verify that the parsed JSON is a dictionary before returning it.

Suggested change

try:

return json.loads(p.read_text(encoding="utf-8"))

except (json.JSONDecodeError, OSError, UnicodeDecodeError):

continue

try:

data = json.loads(p.read_text(encoding="utf-8"))

if isinstance(data, dict):

return data

except (json.JSONDecodeError, OSError, UnicodeDecodeError):

continue

gemini-code-assist · 2026-07-05T23:01:13Z

+    globs = [p.strip() for p in deny_patterns
+             if isinstance(p, str) and p.strip()]


If deny in config.yaml is misconfigured as a string instead of a list (e.g., deny: "git push"), deny_patterns will be a string. Iterating over a string yields individual characters, which means globs will contain single-character patterns like ['g', 'i', 't', ' ', 'p', 'u', 's', 'h']. This can lead to unexpected blocking of commands.

We should explicitly check if deny_patterns is a string and wrap it in a list, or ignore it if it's not an iterable collection.

if isinstance(deny_patterns, str): deny_patterns = [deny_patterns] elif not isinstance(deny_patterns, (list, tuple, set)): return None globs = [p.strip() for p in deny_patterns if isinstance(p, str) and p.strip()]

gemini-code-assist · 2026-07-05T23:01:13Z

+                    trimmed, lines_kept, _ = _truncate_to_char_budget(
+                        result_dict["content"], max_chars
+                    )
+                    next_offset = offset + lines_kept
+                    shown_end = offset + lines_kept - 1
+                    result_dict["content"] = trimmed
+                    result_dict["truncated"] = True
+                    result_dict["truncated_by"] = "bytes"
+                    result_dict["next_offset"] = next_offset
+                    result_dict["hint"] = (
+                        f"Output truncated at the {max_chars:,}-char read budget "
+                        f"after {lines_kept} line(s) (showing lines {offset}-"
+                        f"{shown_end} of {total_lines}). Use offset={next_offset} "
+                        "to continue."
+                    )
+                    if len(trimmed.split("\n", 1)[0]) >= max_chars:
+                        result_dict["hint"] += (
+                            " Note: the first line alone exceeded the budget and "
+                            "was clamped mid-line; its remainder is not "
+                            "retrievable via offset."
+                        )


The check len(trimmed.split("\n", 1)[0]) >= max_chars will trigger the "clamped mid-line" warning even when the first line is exactly max_chars long and was fully read without being clamped.

To avoid this false warning, we should check if the first line of the original content was strictly greater than max_chars before we overwrite result_dict["content"].

first_line_clamped = len(result_dict["content"].split("\n", 1)[0]) > max_chars trimmed, lines_kept, _ = _truncate_to_char_budget( result_dict["content"], max_chars ) next_offset = offset + lines_kept shown_end = offset + lines_kept - 1 result_dict["content"] = trimmed result_dict["truncated"] = True result_dict["truncated_by"] = "bytes" result_dict["next_offset"] = next_offset result_dict["hint"] = ( f"Output truncated at the {max_chars:,}-char read budget " f"after {lines_kept} line(s) (showing lines {offset}-" f"{shown_end} of {total_lines}). Use offset={next_offset} " "to continue." ) if first_line_clamped: result_dict["hint"] += ( " Note: the first line alone exceeded the budget and " "was clamped mid-line; its remainder is not " "retrievable via offset." )

gemini-code-assist · 2026-07-05T23:01:13Z

+            trimmed, lines_kept, _ = _truncate_to_char_budget(
+                result.content or "", max_chars
+            )
+            next_offset = offset + lines_kept
+            shown_end = offset + lines_kept - 1
+            result.content = trimmed
+            result_dict["content"] = trimmed
+            result_dict["truncated"] = True
+            result_dict["truncated_by"] = "bytes"
+            result_dict["next_offset"] = next_offset
+            result_dict["hint"] = (
+                f"Output truncated at the {max_chars:,}-char read budget after "
+                f"{lines_kept} line(s) (showing lines {offset}-{shown_end} of "
+                f"{total_lines}). Use offset={next_offset} to continue."
+            )
+            if len(trimmed.split("\n", 1)[0]) >= max_chars:
+                result_dict["hint"] += (
+                    " Note: the first line alone exceeded the budget and was "
+                    "clamped mid-line; its remainder is not retrievable via "
+                    "offset."
+                )


Similar to the previous finding, checking len(trimmed.split("\n", 1)[0]) >= max_chars triggers a false "clamped mid-line" warning when the first line is exactly max_chars long and was not clamped.

We should check if the first line of the original content was strictly greater than max_chars before overwriting result.content.

first_line_clamped = len((result.content or "").split("\n", 1)[0]) > max_chars trimmed, lines_kept, _ = _truncate_to_char_budget( result.content or "", max_chars ) next_offset = offset + lines_kept shown_end = offset + lines_kept - 1 result.content = trimmed result_dict["content"] = trimmed result_dict["truncated"] = True result_dict["truncated_by"] = "bytes" result_dict["next_offset"] = next_offset result_dict["hint"] = ( f"Output truncated at the {max_chars:,}-char read budget after " f"{lines_kept} line(s) (showing lines {offset}-{shown_end} of " f"{total_lines}). Use offset={next_offset} to continue." ) if first_line_clamped: result_dict["hint"] += ( " Note: the first line alone exceeded the budget and was " "clamped mid-line; its remainder is not retrievable via " "offset." )

FriendlyCityJohnstown