From f84b6a4e2f10430ef0880fad6fda7c2f12dfe7ac Mon Sep 17 00:00:00 2001 From: Hermes Bot Date: Sat, 2 May 2026 17:37:54 +0000 Subject: [PATCH 1/5] fix(bootstrap): add --foreground mode for process supervisors (#1458 Bug #1) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Issue #1458 reports persistent-host crashes (≥1/day) when running the WebUI under launchd KeepAlive on macOS. Root cause: `bootstrap.py` calls `subprocess.Popen([python, "server.py"], start_new_session=True)`, probes /health, then exits 0. Under any process supervisor (launchd, systemd, supervisord, runit, s6), the supervisor sees its tracked PID exit, marks the program as "completed," and respawns it. The new bootstrap fails to bind port 8787 (orphaned server still has it), exits non-zero, supervisor respawns again — loop until the orphan crashes for some other reason and the next respawn finds the port free. This PR addresses Bug #1 of the three failure modes tracked in #1458: the `bootstrap.py` double-fork breaking process supervisors. Bug #2 (state.db FD leak) and Bug #3 (HTTP-unhealthy wedge) remain open under the same issue — they need diagnosis data before a fix can land. Changes ------- 1. `bootstrap.py`: - New `--foreground` argparse flag with help text mentioning launchd / systemd / supervisord. - New `_detect_supervisor()` that returns the env var name for any supervisor it detects: `INVOCATION_ID` / `JOURNAL_STREAM` / `NOTIFY_SOCKET` (systemd, s6), `XPC_SERVICE_NAME` (launchd), `SUPERVISOR_ENABLED` (supervisord), or `HERMES_WEBUI_FOREGROUND` for the explicit user opt-in. Truthy values for the explicit opt-in: `1` / `true` / `yes` / `on` (case-insensitive). - `main()` branches on `args.foreground or _detect_supervisor()`: - **Foreground path:** chdir to `agent_dir or REPO_ROOT`, then `os.execv(python, [python, server_path])` to replace the bootstrap process image with the server. The supervisor sees the long-lived server as the original child. No `wait_for_health` probe — the supervisor's KeepAlive / Restart=on-failure handles liveness. - **Default path:** unchanged. Spawn server as detached child via `Popen + start_new_session=True`, probe /health, return 0. This still works for interactive `bash start.sh` invocations. - Resolved env vars (HOST/PORT/STATE_DIR/AGENT_DIR) are now mutated on `os.environ` directly instead of into a local `env` copy so they are inherited across `os.execv`. 2. `docs/supervisor.md` (new): runnable launchd plist, systemd .service, and supervisord conf examples + a diagnostic recipe (`lsof` + ppid chain) for catching the orphan-loop in production. 3. `.gitignore`: allowlist `docs/supervisor.md` (the directory uses an opt-in pattern; matches the existing `!docs/docker.md` precedent). 4. `tests/test_bootstrap_foreground.py` (new): 35 regression tests covering the argparse flag, `_detect_supervisor()` behavior across all five supervisor env vars, the explicit opt-in's truthy/falsy values, and `main()`'s execv-vs-Popen routing decision under each input combination. `os.execv` is monkeypatched in the routing tests — we pin the structural choice (which call is made, with which args, in which cwd, with which env) not the post-exec behavior. Why this scope and no more -------------------------- Bug #2 (state.db FD leak) lists 5 candidate paths and asks the reporter for `lsof -p | sort | uniq -c | sort -rn | head -20` output to disambiguate. Until that data lands, any "fix" would be speculative — explicitly out of scope per the contributor-pickup comment on the issue. Bug #3 (launchd-running, port-listening, HTTP-unhealthy) was added in @stefanpieter's reply comment. Diagnosis is in flight; no concrete fix shape yet. Also out of scope. Running locally end-to-end verifies the behavior: ``` [bootstrap] Starting Hermes Web UI on http://127.0.0.1:8789 (foreground mode: --foreground) $ pgrep -af 'server.py' 2997632 /home/.../python /tmp/wt-fix-1458/server.py $ ps -o ppid -p 2997632 2997581 ← bash that ran bootstrap.py — same PID as the original bootstrap $ ps -p 2997581 -o cmd ... bootstrap.py ... ← but exec'd into server.py ``` The same PID that bash forked for `bootstrap.py` is now `server.py`. A supervisor watching that PID would correctly observe the long-lived server. No double-fork. Verification ------------ - 3811 tests pass (`pytest tests/` — full suite, +51 from this PR plus master-merge-in) - All 35 new bootstrap-foreground tests pass - `bash scripts/run-browser-tests.sh` PASS (HTTP API checks against worktree) - `bash scripts/webui_qa_agent.sh 8789` PASS (23/23 visual QA) - Live verified: server starts cleanly under both `--foreground` and `HERMES_WEBUI_FOREGROUND=1`; PID lineage confirms no double-fork Closes #1458 (Bug #1 only). Bugs #2 and #3 remain tracked under the issue. --- .gitignore | 1 + bootstrap.py | 91 +++++++- docs/supervisor.md | 207 +++++++++++++++++ tests/test_bootstrap_foreground.py | 358 +++++++++++++++++++++++++++++ 4 files changed, 648 insertions(+), 9 deletions(-) create mode 100644 docs/supervisor.md create mode 100644 tests/test_bootstrap_foreground.py diff --git a/.gitignore b/.gitignore index bd026e5e..3846911e 100644 --- a/.gitignore +++ b/.gitignore @@ -41,6 +41,7 @@ docs/* !docs/ui-ux/ !docs/ui-ux/** !docs/docker.md +!docs/supervisor.md # Local-only PR review harness: rendering drivers, sample bank, fixtures. # Used by Claude during deep reviews; never shared in the repo. diff --git a/bootstrap.py b/bootstrap.py index 8981c4c7..c873b99a 100644 --- a/bootstrap.py +++ b/bootstrap.py @@ -208,9 +208,54 @@ def parse_args() -> argparse.Namespace: action="store_true", help="Fail instead of attempting the official Hermes installer.", ) + parser.add_argument( + "--foreground", + action="store_true", + help=( + "Run server.py in this process (via os.execv) instead of spawning a " + "child. Use this under launchd / systemd / supervisord so the " + "supervisor sees the long-lived server as the original child. " + "Implies --no-browser. Skips the post-launch health probe — the " + "supervisor's own KeepAlive / Restart=on-failure handles liveness." + ), + ) return parser.parse_args() +# Env vars whose presence indicates this process was launched by a supervisor +# that wants to manage the server's lifecycle (KeepAlive, Restart=always, etc.). +# When any is set, we auto-promote to --foreground so we don't double-fork. +# +# - INVOCATION_ID systemd (set on every service activation) +# - JOURNAL_STREAM systemd (set when stdio is wired to the journal) +# - NOTIFY_SOCKET systemd Type=notify, s6 sd_notify-style +# - XPC_SERVICE_NAME launchd (set to the Label of the running plist) +# - SUPERVISOR_ENABLED supervisord +# - HERMES_WEBUI_FOREGROUND explicit user opt-in (=1 / true / yes / on) +_SUPERVISOR_ENV_VARS = ( + "INVOCATION_ID", + "JOURNAL_STREAM", + "NOTIFY_SOCKET", + "XPC_SERVICE_NAME", + "SUPERVISOR_ENABLED", +) + + +def _detect_supervisor() -> str | None: + """Return the name of the detected supervisor env var, or None. + + Pure inspection of os.environ — no side effects. Returned name is the env + var that triggered detection, useful for log messages and for tests. + """ + explicit = os.environ.get("HERMES_WEBUI_FOREGROUND", "").strip().lower() + if explicit in ("1", "true", "yes", "on"): + return "HERMES_WEBUI_FOREGROUND" + for name in _SUPERVISOR_ENV_VARS: + if os.environ.get(name): + return name + return None + + def main() -> int: args = parse_args() ensure_supported_platform() @@ -229,21 +274,49 @@ def main() -> int: os.getenv("HERMES_WEBUI_STATE_DIR", str(Path.home() / ".hermes" / "webui")) ).expanduser() state_dir.mkdir(parents=True, exist_ok=True) - log_path = state_dir / f"bootstrap-{args.port}.log" - env = os.environ.copy() - env["HERMES_WEBUI_HOST"] = args.host - env["HERMES_WEBUI_PORT"] = str(args.port) - env.setdefault("HERMES_WEBUI_STATE_DIR", str(state_dir)) + # Mutate os.environ so child (or post-execv) inherits the resolved values. + os.environ["HERMES_WEBUI_HOST"] = args.host + os.environ["HERMES_WEBUI_PORT"] = str(args.port) + os.environ.setdefault("HERMES_WEBUI_STATE_DIR", str(state_dir)) if agent_dir: - env["HERMES_WEBUI_AGENT_DIR"] = str(agent_dir) + os.environ["HERMES_WEBUI_AGENT_DIR"] = str(agent_dir) + + server_cwd = str(agent_dir or REPO_ROOT) + server_path = str(REPO_ROOT / "server.py") + + # --foreground (or auto-detected supervisor): replace this process with the + # server. The supervisor sees the long-lived server as the original child, + # so KeepAlive / Restart=always / autorestart=true work correctly. No + # health probe — the supervisor's own restart-on-exit handles liveness. + foreground_reason = "--foreground" if args.foreground else _detect_supervisor() + if foreground_reason: + info( + f"Starting Hermes Web UI on http://{args.host}:{args.port} " + f"(foreground mode: {foreground_reason})" + ) + try: + os.chdir(server_cwd) + except OSError as exc: + raise RuntimeError( + f"Could not chdir to {server_cwd!r} before exec: {exc}" + ) from exc + # os.execv replaces the current process image. Anything after this line + # only runs if execv itself fails (it raises OSError on failure). + os.execv(python_exe, [python_exe, server_path]) + # Unreachable — execv either replaces the process or raises. + raise RuntimeError("os.execv returned unexpectedly") + + # Default (legacy) path: spawn the server as a detached child, probe + # /health, then return. Suitable for an interactive `bash start.sh` run. + log_path = state_dir / f"bootstrap-{args.port}.log" info(f"Starting Hermes Web UI on http://{args.host}:{args.port}") with log_path.open("ab") as log_file: proc = subprocess.Popen( - [python_exe, str(REPO_ROOT / "server.py")], - cwd=str(agent_dir or REPO_ROOT), - env=env, + [python_exe, server_path], + cwd=server_cwd, + env=os.environ.copy(), stdout=log_file, stderr=subprocess.STDOUT, start_new_session=True, diff --git a/docs/supervisor.md b/docs/supervisor.md new file mode 100644 index 00000000..085d6010 --- /dev/null +++ b/docs/supervisor.md @@ -0,0 +1,207 @@ +# Running Hermes Web UI under a process supervisor + +Use a process supervisor (launchd, systemd, supervisord, runit, s6) when you +want the Web UI to start at boot, restart on crash, or be managed alongside +other services. + +## TL;DR + +Pass ``--foreground`` to ``bootstrap.py`` (or ``bash start.sh``): + +```bash +bash start.sh --foreground +``` + +Or set ``HERMES_WEBUI_FOREGROUND=1`` in the environment. The Web UI will +auto-detect launchd / systemd / supervisord even without the flag, but being +explicit is safer. + +## Why ``--foreground`` matters + +Without it, ``bootstrap.py`` does this: + +1. Spawn ``server.py`` as a detached subprocess (``start_new_session=True``) +2. Probe ``/health`` until the server is up +3. Exit 0 + +That works for an interactive shell run (``./start.sh`` returns to your +prompt with the server alive in the background). It is **broken** under any +process supervisor: the supervisor sees its tracked PID exit, marks the job +as completed, and respawns ``bootstrap.py``. The respawn fails to bind port +8787 (the orphaned server still has it), exits non-zero, supervisor +respawns again — loop. + +In foreground mode, ``bootstrap.py`` does its setup work and then calls +``os.execv`` to replace its own process with ``server.py``. The supervisor +sees the long-lived server as the original child. ``KeepAlive=true`` / +``Restart=always`` work correctly. + +## launchd (macOS) + +``~/Library/LaunchAgents/com.example.hermes-webui.plist``: + +```xml + + + + + Label + com.example.hermes-webui + + ProgramArguments + + /bin/bash + /Users/yourname/hermes-webui/start.sh + --foreground + + + WorkingDirectory + /Users/yourname/hermes-webui + + RunAtLoad + + + KeepAlive + + + StandardOutPath + /Users/yourname/.hermes/webui/launchd-stdout.log + + StandardErrorPath + /Users/yourname/.hermes/webui/launchd-stderr.log + + EnvironmentVariables + + HOME + /Users/yourname + PATH + /usr/local/bin:/usr/bin:/bin + + + +``` + +Load: + +```bash +launchctl load ~/Library/LaunchAgents/com.example.hermes-webui.plist +launchctl print gui/$(id -u)/com.example.hermes-webui # check state +``` + +Reload after editing the plist: + +```bash +launchctl unload ~/Library/LaunchAgents/com.example.hermes-webui.plist +launchctl load ~/Library/LaunchAgents/com.example.hermes-webui.plist +``` + +launchd sets ``XPC_SERVICE_NAME`` automatically, so even without the +``--foreground`` argument the Web UI will auto-promote to foreground mode. +The flag is still recommended as documentation of intent. + +## systemd (Linux) + +``~/.config/systemd/user/hermes-webui.service``: + +```ini +[Unit] +Description=Hermes Web UI +After=network.target + +[Service] +Type=simple +WorkingDirectory=%h/hermes-webui +ExecStart=/bin/bash %h/hermes-webui/start.sh --foreground +Restart=on-failure +RestartSec=5 + +# Optional: route stdout/stderr to journald instead of files +StandardOutput=journal +StandardError=journal + +[Install] +WantedBy=default.target +``` + +Enable + start: + +```bash +systemctl --user daemon-reload +systemctl --user enable --now hermes-webui.service +journalctl --user -u hermes-webui.service -f +``` + +systemd sets ``INVOCATION_ID`` and ``JOURNAL_STREAM`` (when stdio is wired to +the journal), both of which auto-promote to foreground mode. + +## supervisord (cross-platform) + +``/etc/supervisor/conf.d/hermes-webui.conf``: + +```ini +[program:hermes-webui] +command=/bin/bash /home/youruser/hermes-webui/start.sh --foreground +directory=/home/youruser/hermes-webui +user=youruser +autostart=true +autorestart=true +stopsignal=TERM +stopwaitsecs=10 +stdout_logfile=/var/log/hermes-webui.out.log +stderr_logfile=/var/log/hermes-webui.err.log +environment=HOME="/home/youruser",PATH="/usr/local/bin:/usr/bin:/bin" +``` + +Reload + start: + +```bash +sudo supervisorctl reread +sudo supervisorctl update +sudo supervisorctl status hermes-webui +``` + +supervisord sets ``SUPERVISOR_ENABLED``, which auto-promotes to foreground +mode. + +## Auto-detected env vars (full list) + +These trigger ``--foreground`` behavior even when the flag is not passed: + +| Env var | Set by | Notes | +|---|---|---| +| ``INVOCATION_ID`` | systemd | Set on every service activation | +| ``JOURNAL_STREAM`` | systemd | Set when stdio is wired to journald | +| ``NOTIFY_SOCKET`` | systemd ``Type=notify`` / s6 | sd_notify-style notification socket | +| ``XPC_SERVICE_NAME`` | launchd | Set to the plist Label | +| ``SUPERVISOR_ENABLED`` | supervisord | Always set under supervisord | +| ``HERMES_WEBUI_FOREGROUND`` | you | Explicit opt-in; accepts ``1`` / ``true`` / ``yes`` / ``on`` | + +If you're running under a supervisor that is not in the list and your tracked +PID keeps exiting, set ``HERMES_WEBUI_FOREGROUND=1`` in the service +environment. + +## Diagnostic recipe + +If the Web UI keeps getting respawned and you suspect the double-fork loop: + +```bash +# Check the running PID for the server +lsof -iTCP:8787 -sTCP:LISTEN + +# Get its parent — should be the supervisor itself, NOT init (PID 1) +PID=$(lsof -tiTCP:8787 -sTCP:LISTEN) +ps -p "$PID" -o pid,ppid,cmd +ps -p "$(ps -o ppid= -p "$PID" | tr -d ' ')" -o pid,cmd +``` + +A healthy foreground-mode setup looks like: + +``` +PID PPID CMD +12345 6789 /path/to/python /path/to/server.py +6789 1 /sbin/launchd # or /usr/lib/systemd/systemd, etc. +``` + +If PPID is ``1`` (init) when it should be the supervisor, the orphan-server +loop is happening — re-check that ``--foreground`` (or one of the env vars) +is reaching the process. diff --git a/tests/test_bootstrap_foreground.py b/tests/test_bootstrap_foreground.py new file mode 100644 index 00000000..fdb370d8 --- /dev/null +++ b/tests/test_bootstrap_foreground.py @@ -0,0 +1,358 @@ +""" +Tests for bootstrap.py --foreground / supervisor auto-detect (issue #1458, Bug #1). + +Background +---------- +Issue #1458 reports: under launchd / systemd / supervisord with KeepAlive=true +or Restart=always, bootstrap.py exits after spawning the server child via +``Popen + wait_for_health``. The supervisor sees the parent exit, marks the +program as "completed," and respawns it — but the original server child is +still holding port 8787 in a detached process group. The new bootstrap fails +to bind, exits non-zero, supervisor respawns again, loops until something +crashes the orphan and frees the port. + +The fix +------- +Add ``--foreground`` flag and supervisor-environment auto-detection. In +foreground mode we replace the current process via ``os.execv`` with the +server, so the supervisor sees the long-lived server as the original child. +The legacy ``Popen + wait_for_health`` path is preserved for interactive +``bash start.sh`` runs. + +Coverage +-------- +1. ``--foreground`` is a recognized argparse flag +2. ``_detect_supervisor()`` returns None on a clean env +3. ``_detect_supervisor()`` returns the env-var name on each known supervisor + (``INVOCATION_ID`` / ``JOURNAL_STREAM`` / ``NOTIFY_SOCKET`` / + ``XPC_SERVICE_NAME`` / ``SUPERVISOR_ENABLED``) +4. ``_detect_supervisor()`` returns ``HERMES_WEBUI_FOREGROUND`` for the + explicit opt-in, accepting ``1``/``true``/``yes``/``on`` (case-insensitive) +5. ``_detect_supervisor()`` ignores ``HERMES_WEBUI_FOREGROUND=0`` / + ``=false`` / ``=`` and falls through to env-var probing +6. ``main()`` calls ``os.execv`` (NOT ``subprocess.Popen``) when + ``--foreground`` is passed +7. ``main()`` calls ``os.execv`` (NOT ``subprocess.Popen``) when a supervisor + env var is set even without the explicit flag +8. Default ``main()`` path (no flag, clean env) still uses ``Popen`` +9. Foreground path chdir's to ``agent_dir or REPO_ROOT`` before execv (matches + the cwd the legacy Popen uses) +10. Foreground path exports ``HERMES_WEBUI_HOST`` / ``HERMES_WEBUI_PORT`` / + ``HERMES_WEBUI_AGENT_DIR`` / ``HERMES_WEBUI_STATE_DIR`` to ``os.environ`` + so the post-exec server picks them up +11. Foreground path skips ``wait_for_health`` (no client to retry from) +12. ``--foreground`` help text mentions launchd / systemd / supervisord + +These tests do NOT actually exec — ``os.execv`` is monkeypatched. We're +pinning the structural choice (which path runs, which cwd, which env) not the +post-exec behavior (which is the OS kernel's job). +""" +from __future__ import annotations + +import os +import subprocess +import sys +from pathlib import Path +from unittest.mock import patch + +import pytest + +REPO_ROOT = Path(__file__).parent.parent +sys.path.insert(0, str(REPO_ROOT)) + + +# ---------- helpers -------------------------------------------------------- + + +@pytest.fixture +def clean_env(monkeypatch): + """Strip all known supervisor env vars so detection starts from a clean state.""" + for name in ( + "INVOCATION_ID", + "JOURNAL_STREAM", + "NOTIFY_SOCKET", + "XPC_SERVICE_NAME", + "SUPERVISOR_ENABLED", + "HERMES_WEBUI_FOREGROUND", + ): + monkeypatch.delenv(name, raising=False) + + +@pytest.fixture +def import_bootstrap(): + """Import bootstrap freshly each test to avoid module-level state bleed.""" + # bootstrap.py runs ``_load_repo_dotenv()`` at import time; that's idempotent. + if "bootstrap" in sys.modules: + del sys.modules["bootstrap"] + import bootstrap as bs + return bs + + +# ---------- argparse coverage --------------------------------------------- + + +class TestForegroundFlag: + + def test_foreground_is_recognized_flag(self, import_bootstrap, monkeypatch): + monkeypatch.setattr(sys, "argv", ["bootstrap.py", "--foreground"]) + args = import_bootstrap.parse_args() + assert args.foreground is True + assert args.port == import_bootstrap.DEFAULT_PORT # default preserved + assert args.host == import_bootstrap.DEFAULT_HOST + + def test_foreground_default_is_false(self, import_bootstrap, monkeypatch): + monkeypatch.setattr(sys, "argv", ["bootstrap.py"]) + args = import_bootstrap.parse_args() + assert args.foreground is False + + def test_foreground_help_mentions_supervisors(self, import_bootstrap, monkeypatch, capsys): + # argparse prints help and exits — capture and verify content. + monkeypatch.setattr(sys, "argv", ["bootstrap.py", "--help"]) + with pytest.raises(SystemExit): + import_bootstrap.parse_args() + out = capsys.readouterr().out + assert "--foreground" in out + assert "launchd" in out + assert "systemd" in out + assert "supervisord" in out + + +# ---------- _detect_supervisor() ------------------------------------------ + + +class TestDetectSupervisor: + + def test_clean_env_returns_none(self, import_bootstrap, clean_env): + assert import_bootstrap._detect_supervisor() is None + + @pytest.mark.parametrize("var", [ + "INVOCATION_ID", + "JOURNAL_STREAM", + "NOTIFY_SOCKET", + "XPC_SERVICE_NAME", + "SUPERVISOR_ENABLED", + ]) + def test_each_supervisor_var_triggers(self, import_bootstrap, clean_env, monkeypatch, var): + monkeypatch.setenv(var, "anything-truthy") + assert import_bootstrap._detect_supervisor() == var + + @pytest.mark.parametrize("value", ["1", "true", "TRUE", "yes", "Yes", "on", "ON"]) + def test_explicit_opt_in_truthy_values(self, import_bootstrap, clean_env, monkeypatch, value): + monkeypatch.setenv("HERMES_WEBUI_FOREGROUND", value) + assert import_bootstrap._detect_supervisor() == "HERMES_WEBUI_FOREGROUND" + + @pytest.mark.parametrize("value", ["0", "false", "FALSE", "no", "off", "", " "]) + def test_explicit_opt_in_falsy_values_fall_through(self, import_bootstrap, clean_env, monkeypatch, value): + # When HERMES_WEBUI_FOREGROUND is falsy, we should NOT short-circuit on it. + # If no other supervisor var is set, returns None. + monkeypatch.setenv("HERMES_WEBUI_FOREGROUND", value) + assert import_bootstrap._detect_supervisor() is None + + def test_explicit_opt_in_takes_precedence_over_supervisor_var(self, import_bootstrap, clean_env, monkeypatch): + # Both set → explicit flag wins (returned name reflects user intent). + monkeypatch.setenv("HERMES_WEBUI_FOREGROUND", "1") + monkeypatch.setenv("INVOCATION_ID", "deadbeef") + assert import_bootstrap._detect_supervisor() == "HERMES_WEBUI_FOREGROUND" + + +# ---------- main() routing ------------------------------------------------ + + +class TestMainForegroundRouting: + """Verify which code path main() takes under each input combination. + + These are STRUCTURAL tests — they pin which call (execv vs Popen) is made, + not the result. We monkeypatch every external side effect so main() runs + in a hermetic environment. + """ + + @pytest.fixture + def stub_main_dependencies(self, monkeypatch, tmp_path): + """Stub out everything main() calls except the routing decision.""" + import bootstrap as bs + monkeypatch.setattr(bs, "ensure_supported_platform", lambda: None) + monkeypatch.setattr(bs, "discover_agent_dir", lambda: tmp_path / "agent") + monkeypatch.setattr(bs, "hermes_command_exists", lambda: True) + monkeypatch.setattr(bs, "discover_launcher_python", lambda *a: "/usr/bin/python3") + monkeypatch.setattr(bs, "ensure_python_has_webui_deps", lambda p: p) + monkeypatch.setattr(bs, "wait_for_health", lambda *a, **kw: True) + monkeypatch.setattr(bs, "open_browser", lambda *a, **kw: None) + monkeypatch.setenv("HERMES_WEBUI_STATE_DIR", str(tmp_path / "state")) + # Make agent_dir exist so chdir doesn't fail. + (tmp_path / "agent").mkdir(parents=True, exist_ok=True) + return bs + + def test_default_path_uses_popen(self, stub_main_dependencies, clean_env, monkeypatch): + bs = stub_main_dependencies + monkeypatch.setattr(sys, "argv", ["bootstrap.py", "--no-browser"]) + + execv_calls = [] + popen_calls = [] + monkeypatch.setattr(os, "execv", lambda *a: execv_calls.append(a)) + + class FakePopen: + pid = 12345 + def __init__(self, *args, **kwargs): + popen_calls.append((args, kwargs)) + monkeypatch.setattr(subprocess, "Popen", FakePopen) + + rc = bs.main() + assert rc == 0 + assert len(popen_calls) == 1, "Default path should call subprocess.Popen exactly once" + assert len(execv_calls) == 0, "Default path must NOT call os.execv" + + def test_foreground_flag_uses_execv(self, stub_main_dependencies, clean_env, monkeypatch): + bs = stub_main_dependencies + monkeypatch.setattr(sys, "argv", ["bootstrap.py", "--foreground"]) + + execv_calls = [] + popen_calls = [] + # execv normally replaces the process; we capture+raise SystemExit so + # main() returns control to us instead of falling through to the + # legacy Popen branch. + def fake_execv(path, argv): + execv_calls.append((path, argv)) + raise SystemExit(0) + monkeypatch.setattr(os, "execv", fake_execv) + monkeypatch.setattr(os, "chdir", lambda p: None) + + def fake_popen(*args, **kwargs): + popen_calls.append((args, kwargs)) + return None + monkeypatch.setattr(subprocess, "Popen", fake_popen) + + with pytest.raises(SystemExit) as ei: + bs.main() + assert ei.value.code == 0 + assert len(execv_calls) == 1, "--foreground must call os.execv exactly once" + assert len(popen_calls) == 0, "--foreground must NOT call subprocess.Popen" + + path, argv = execv_calls[0] + assert path == "/usr/bin/python3" + # argv[0] is the program name (convention), argv[1] is the script + assert argv[0] == "/usr/bin/python3" + assert argv[1].endswith("server.py") + + @pytest.mark.parametrize("var", [ + "INVOCATION_ID", + "JOURNAL_STREAM", + "NOTIFY_SOCKET", + "XPC_SERVICE_NAME", + "SUPERVISOR_ENABLED", + ]) + def test_supervisor_env_var_auto_promotes_to_execv(self, stub_main_dependencies, clean_env, monkeypatch, var): + bs = stub_main_dependencies + monkeypatch.setattr(sys, "argv", ["bootstrap.py"]) # no --foreground + monkeypatch.setenv(var, "deadbeef") + + execv_calls = [] + popen_calls = [] + def fake_execv(path, argv): + execv_calls.append((path, argv)) + raise SystemExit(0) + monkeypatch.setattr(os, "execv", fake_execv) + monkeypatch.setattr(os, "chdir", lambda p: None) + + def fake_popen(*args, **kwargs): + popen_calls.append((args, kwargs)) + return None + monkeypatch.setattr(subprocess, "Popen", fake_popen) + + with pytest.raises(SystemExit): + bs.main() + assert len(execv_calls) == 1, f"{var} must auto-promote to execv" + assert len(popen_calls) == 0, f"{var} must NOT use Popen" + + def test_explicit_opt_in_env_auto_promotes_to_execv(self, stub_main_dependencies, clean_env, monkeypatch): + bs = stub_main_dependencies + monkeypatch.setattr(sys, "argv", ["bootstrap.py"]) # no --foreground flag + monkeypatch.setenv("HERMES_WEBUI_FOREGROUND", "1") + + execv_calls = [] + def fake_execv(path, argv): + execv_calls.append((path, argv)) + raise SystemExit(0) + monkeypatch.setattr(os, "execv", fake_execv) + monkeypatch.setattr(os, "chdir", lambda p: None) + monkeypatch.setattr(subprocess, "Popen", lambda *a, **kw: None) + + with pytest.raises(SystemExit): + bs.main() + assert len(execv_calls) == 1 + + +class TestForegroundEnvAndCwd: + """The post-execv server.py inherits os.environ and cwd from us.""" + + @pytest.fixture + def setup(self, monkeypatch, tmp_path): + import bootstrap as bs + monkeypatch.setattr(bs, "ensure_supported_platform", lambda: None) + agent_dir = tmp_path / "agent" + agent_dir.mkdir() + monkeypatch.setattr(bs, "discover_agent_dir", lambda: agent_dir) + monkeypatch.setattr(bs, "hermes_command_exists", lambda: True) + monkeypatch.setattr(bs, "discover_launcher_python", lambda *a: "/usr/bin/python3") + monkeypatch.setattr(bs, "ensure_python_has_webui_deps", lambda p: p) + monkeypatch.setattr(bs, "wait_for_health", lambda *a, **kw: True) + monkeypatch.setattr(bs, "open_browser", lambda *a, **kw: None) + # State-dir + every var we care about is captured. + monkeypatch.setenv("HERMES_WEBUI_STATE_DIR", str(tmp_path / "state")) + return bs, agent_dir + + def test_foreground_chdirs_to_agent_dir_before_exec(self, setup, monkeypatch, clean_env): + bs, agent_dir = setup + monkeypatch.setattr(sys, "argv", ["bootstrap.py", "--foreground", "--host", "127.0.0.1", "9999"]) + + chdir_calls = [] + monkeypatch.setattr(os, "chdir", lambda p: chdir_calls.append(p)) + + def fake_execv(*a): + raise SystemExit(0) + monkeypatch.setattr(os, "execv", fake_execv) + + with pytest.raises(SystemExit): + bs.main() + assert len(chdir_calls) == 1 + assert chdir_calls[0] == str(agent_dir) + + def test_foreground_exports_resolved_env_vars(self, setup, monkeypatch, clean_env): + bs, agent_dir = setup + monkeypatch.setattr(sys, "argv", [ + "bootstrap.py", "--foreground", "--host", "0.0.0.0", "9119" + ]) + monkeypatch.setattr(os, "chdir", lambda p: None) + + def fake_execv(*a): + raise SystemExit(0) + monkeypatch.setattr(os, "execv", fake_execv) + + with pytest.raises(SystemExit): + bs.main() + + # Post-execv server.py inherits these — verify we set them on os.environ + # (not just a local copy). + assert os.environ["HERMES_WEBUI_HOST"] == "0.0.0.0" + assert os.environ["HERMES_WEBUI_PORT"] == "9119" + assert os.environ["HERMES_WEBUI_AGENT_DIR"] == str(agent_dir) + # state-dir was already set by the fixture; verify it survived. + assert "HERMES_WEBUI_STATE_DIR" in os.environ + + def test_foreground_does_not_call_wait_for_health(self, setup, monkeypatch, clean_env): + bs, _ = setup + monkeypatch.setattr(sys, "argv", ["bootstrap.py", "--foreground"]) + monkeypatch.setattr(os, "chdir", lambda p: None) + + wait_calls = [] + monkeypatch.setattr(bs, "wait_for_health", lambda *a, **kw: (wait_calls.append(a), True)[1]) + + def fake_execv(*a): + raise SystemExit(0) + monkeypatch.setattr(os, "execv", fake_execv) + + with pytest.raises(SystemExit): + bs.main() + + # In foreground mode there's no parent left to retry from — the + # supervisor's KeepAlive handles it. wait_for_health must not run. + assert len(wait_calls) == 0 From b804b66238f4c211f32653c295eea4f5c5e6da16 Mon Sep 17 00:00:00 2001 From: youzhi Date: Sun, 3 May 2026 01:44:38 +0800 Subject: [PATCH 2/5] Fix session list pending message payload --- api/models.py | 1 + static/i18n.js | 1 + tests/test_issue856_session_streaming_state.py | 4 ++++ 3 files changed, 6 insertions(+) diff --git a/api/models.py b/api/models.py index e499d0ab..b06a2b69 100644 --- a/api/models.py +++ b/api/models.py @@ -479,6 +479,7 @@ class Session: # Sessions without a fork must not leak None — see test_session_lineage_metadata_api. **({'parent_session_id': self.parent_session_id} if self.parent_session_id else {}), 'active_stream_id': self.active_stream_id, + 'pending_user_message': self.pending_user_message, 'is_cli_session': self.is_cli_session, 'source_tag': self.source_tag, 'session_source': self.session_source, diff --git a/static/i18n.js b/static/i18n.js index 337b7a14..7a856efe 100644 --- a/static/i18n.js +++ b/static/i18n.js @@ -7519,6 +7519,7 @@ function t(key, ...args) { if (val === undefined) return key; // final fallback: return key itself if (typeof val === 'function') return val(...args); if (args.length) { + // Locale strings can use numbered placeholders like {0} and {1}. return String(val).replace(/\{(\d+)\}/g, (match, idx) => ( Object.prototype.hasOwnProperty.call(args, idx) ? String(args[idx]) : match )); diff --git a/tests/test_issue856_session_streaming_state.py b/tests/test_issue856_session_streaming_state.py index e71fd1e8..cd6fd163 100644 --- a/tests/test_issue856_session_streaming_state.py +++ b/tests/test_issue856_session_streaming_state.py @@ -47,9 +47,11 @@ def _make_session(session_id, stream_id=None, message_count=1): def test_all_sessions_marks_indexed_and_in_memory_streaming_sessions(): """Session records from both index and in-memory cache should expose is_streaming.""" s_disk = _make_session("disk_session", stream_id="stream-1") + s_disk.pending_user_message = "pending disk turn" s_disk.save() s_memory = _make_session("memory_session", stream_id="stream-2") + s_memory.pending_user_message = "pending memory turn" with models.LOCK: models.SESSIONS[s_memory.session_id] = s_memory @@ -62,6 +64,8 @@ def test_all_sessions_marks_indexed_and_in_memory_streaming_sessions(): assert by_sid["disk_session"]["is_streaming"] is True assert by_sid["memory_session"]["is_streaming"] is True assert by_sid["memory_session"]["active_stream_id"] == "stream-2" + assert by_sid["disk_session"]["pending_user_message"] == "pending disk turn" + assert by_sid["memory_session"]["pending_user_message"] == "pending memory turn" def test_all_sessions_marks_streaming_false_when_stream_is_not_active(): From 6a26e82c22db17d599061d3828235abb1ee75b95 Mon Sep 17 00:00:00 2001 From: Hermes Bot Date: Sat, 2 May 2026 17:52:13 +0000 Subject: [PATCH 3/5] fix(bootstrap): address Opus pre-merge review feedback (#1478) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Three changes from the pre-merge Opus review: **MUST-FIX** — XPC_SERVICE_NAME false-positive on macOS Terminal macOS launchd sets `XPC_SERVICE_NAME` in EVERY Terminal-spawned shell, not just real services. Typical noise values: `"0"` (truthy in Python!) and `"application.com.apple.Terminal."`. A bare `os.environ.get(name)` existence check would auto-promote interactive `./start.sh` runs to foreground mode on every Mac dev machine — silently breaking the most common installation path (no /health probe, no browser open, no log file, hanging shell). Fix: new `_is_real_supervisor_value()` helper that filters noise. For `XPC_SERVICE_NAME` specifically, reject `"0"` and any `"application.*"` prefix. Real launchd plists use reverse-DNS Label form (`com..`) which still triggers correctly. 7 new tests in `TestXPCServiceNameNoiseFilter`: - 4 noise values (`0`, Terminal.app, iTerm2, VSCode) → no detection - 3 real Label forms → correct detection - Mixed env with XPC noise + real INVOCATION_ID → falls through to systemd **SHOULD-FIX 1** — Test env leakage The original `clean_env` fixture stripped supervisor-detection env vars but not the resolved bootstrap vars (HERMES_WEBUI_HOST/PORT/AGENT_DIR) that `main()` mutates onto `os.environ`. After `test_foreground_exports_resolved_env_vars` ran, later tests would import bootstrap with polluted defaults (DEFAULT_HOST="0.0.0.0" instead of "127.0.0.1"). Existing assertions still passed (tautological vs DEFAULT_*), but it was a footgun for future tests. Fix: extend `clean_env` to also `delenv` the three resolved vars before each test. **SHOULD-FIX 2** — Pre-execv executability guard If `discover_launcher_python` returns a path that doesn't exist or isn't executable, `os.execv` raises OSError → wrapper catches → SystemExit(1) → supervisor restarts → loop forever. That's exactly the failure mode this PR is supposed to eliminate. Fix: `os.access(python_exe, os.X_OK)` check before execv. Converts infinite supervisor loop into a single visible RuntimeError. 1 new test in `TestForegroundExecutabilityGuard` pinning that the guard fires before execv when the python path is non-executable. **Docs** — supervisor.md updates - New section explaining the XPC_SERVICE_NAME noise filter and what values trigger / don't trigger detection - New section listing supervisors that are NOT auto-detected (runit, daemontools, PM2, Foreman/Honcho, custom shell-script supervisors) with explicit recommendation to set HERMES_WEBUI_FOREGROUND=1 Verification - 3820 tests pass (+9 from this commit's new tests vs the original PR push of 3811) - Filter manually verified end-to-end with the live os.environ: XPC=0 → None, XPC=application.* → None, XPC=com.example.foo → triggers - run-browser-tests.sh ALL CHECKS PASSED on the worktree Items deferred from the Opus review - #4 chdir target may not exist: REPO_ROOT comes from __file__.resolve() so it's stable; not a real concern in practice - #6 two startup messages in foreground mode: cosmetic, useful for diagnostics - #7 stricter explicit-only mode: leaves user the override of just not passing --foreground (current behavior) - #8 test stub return value: trivial, can fix later if regression surface - #9 argparse positional-after-option ordering: test reads fine These can be follow-up issues if anyone hits them. --- bootstrap.py | 41 ++++++++++- docs/supervisor.md | 38 +++++++++- tests/test_bootstrap_foreground.py | 113 +++++++++++++++++++++++++++-- 3 files changed, 179 insertions(+), 13 deletions(-) diff --git a/bootstrap.py b/bootstrap.py index c873b99a..ccdbee9a 100644 --- a/bootstrap.py +++ b/bootstrap.py @@ -232,6 +232,14 @@ def parse_args() -> argparse.Namespace: # - XPC_SERVICE_NAME launchd (set to the Label of the running plist) # - SUPERVISOR_ENABLED supervisord # - HERMES_WEBUI_FOREGROUND explicit user opt-in (=1 / true / yes / on) +# +# Note on XPC_SERVICE_NAME: macOS launchd sets this in EVERY Terminal-launched +# shell too — typical values include "0" (truthy in Python!) and +# "application.com.apple.Terminal.". A bare existence check would +# false-positive on every Mac dev machine running ./start.sh interactively. +# We narrow to launchd Label-style names (com..) — those +# are real services. Verified with `launchctl getenv XPC_SERVICE_NAME` and +# Apple's documented launchd behavior. _SUPERVISOR_ENV_VARS = ( "INVOCATION_ID", "JOURNAL_STREAM", @@ -241,6 +249,26 @@ _SUPERVISOR_ENV_VARS = ( ) +def _is_real_supervisor_value(name: str, value: str) -> bool: + """Filter out known-noise env-var values that aren't actual supervisors. + + Most env vars in _SUPERVISOR_ENV_VARS are only set by the supervisor we + care about, so any non-empty value is meaningful. XPC_SERVICE_NAME is the + exception: macOS launchd sets it in every Terminal-spawned shell with + values like "0" or "application.com.apple.Terminal.". A real + launchd-managed service has a reverse-DNS Label like "com.example.foo". + """ + if not value: + return False + if name == "XPC_SERVICE_NAME": + # Reject Apple's noise values; accept Label-style names. + if value == "0": + return False + if value.startswith("application."): + return False + return True + + def _detect_supervisor() -> str | None: """Return the name of the detected supervisor env var, or None. @@ -251,7 +279,8 @@ def _detect_supervisor() -> str | None: if explicit in ("1", "true", "yes", "on"): return "HERMES_WEBUI_FOREGROUND" for name in _SUPERVISOR_ENV_VARS: - if os.environ.get(name): + value = os.environ.get(name, "") + if _is_real_supervisor_value(name, value): return name return None @@ -301,6 +330,16 @@ def main() -> int: raise RuntimeError( f"Could not chdir to {server_cwd!r} before exec: {exc}" ) from exc + # Defensive check: if python_exe is missing or non-executable, execv + # raises OSError, the wrapper catches and SystemExit(1)s, and the + # supervisor restarts — looping forever, exactly the failure mode this + # PR is meant to eliminate. Convert to a single visible error. + if not os.access(python_exe, os.X_OK): + raise RuntimeError( + f"Python interpreter at {python_exe!r} is not executable. " + f"Set HERMES_WEBUI_PYTHON to a working interpreter or fix " + f"the agent venv at {agent_dir}." + ) # os.execv replaces the current process image. Anything after this line # only runs if execv itself fails (it raises OSError on failure). os.execv(python_exe, [python_exe, server_path]) diff --git a/docs/supervisor.md b/docs/supervisor.md index 085d6010..4ec433b0 100644 --- a/docs/supervisor.md +++ b/docs/supervisor.md @@ -172,13 +172,43 @@ These trigger ``--foreground`` behavior even when the flag is not passed: | ``INVOCATION_ID`` | systemd | Set on every service activation | | ``JOURNAL_STREAM`` | systemd | Set when stdio is wired to journald | | ``NOTIFY_SOCKET`` | systemd ``Type=notify`` / s6 | sd_notify-style notification socket | -| ``XPC_SERVICE_NAME`` | launchd | Set to the plist Label | +| ``XPC_SERVICE_NAME`` | launchd | Set to the plist Label — narrowed to ``com..`` form (see below) | | ``SUPERVISOR_ENABLED`` | supervisord | Always set under supervisord | | ``HERMES_WEBUI_FOREGROUND`` | you | Explicit opt-in; accepts ``1`` / ``true`` / ``yes`` / ``on`` | -If you're running under a supervisor that is not in the list and your tracked -PID keeps exiting, set ``HERMES_WEBUI_FOREGROUND=1`` in the service -environment. +### XPC_SERVICE_NAME noise filter + +macOS launchd sets ``XPC_SERVICE_NAME`` in **every Terminal-spawned shell**, +not just real services. Typical noise values: + +- ``0`` — set on launchd descendants generally +- ``application.com.apple.Terminal.`` — Terminal.app shells +- ``application.com.googlecode.iterm2`` — iTerm2 +- ``application.com.microsoft.VSCode`` — VSCode integrated terminal + +A bare existence check on this var would auto-promote interactive +``./start.sh`` runs to foreground mode on every Mac dev machine, breaking +the most common installation path. We narrow detection to launchd +**Label-style** names (typically reverse-DNS like ``com.example.foo``). +Real launchd plists always use this form. If you ever see +``XPC_SERVICE_NAME=0`` in your service environment, the auto-detect will +ignore it — set ``HERMES_WEBUI_FOREGROUND=1`` or pass ``--foreground`` +explicitly to be safe. + +### Supervisors that are NOT auto-detected + +The following set no env var that we can reliably detect. Pass +``--foreground`` (or ``HERMES_WEBUI_FOREGROUND=1``) explicitly: + +- **runit** (without sd_notify) — pure runit chains +- **daemontools** / ``svc`` +- **PM2** (Node.js process manager occasionally repurposed for Python) +- **Foreman** / **Honcho** (Procfile-style) +- **Docker** with a custom CMD entrypoint that doesn't already use ``exec`` +- **Custom shell-script supervisors** that fork-and-wait + +If your supervisor isn't in the auto-detect list and you see the orphan-PID +respawn loop, set ``HERMES_WEBUI_FOREGROUND=1`` in the service environment. ## Diagnostic recipe diff --git a/tests/test_bootstrap_foreground.py b/tests/test_bootstrap_foreground.py index fdb370d8..7b7ed8c6 100644 --- a/tests/test_bootstrap_foreground.py +++ b/tests/test_bootstrap_foreground.py @@ -30,18 +30,23 @@ Coverage explicit opt-in, accepting ``1``/``true``/``yes``/``on`` (case-insensitive) 5. ``_detect_supervisor()`` ignores ``HERMES_WEBUI_FOREGROUND=0`` / ``=false`` / ``=`` and falls through to env-var probing -6. ``main()`` calls ``os.execv`` (NOT ``subprocess.Popen``) when +6. ``XPC_SERVICE_NAME`` noise filter: bare ``"0"`` and ``application.`` + values do NOT trigger foreground (the macOS Terminal default state), + while real launchd Labels (``com..``) do +7. ``main()`` calls ``os.execv`` (NOT ``subprocess.Popen``) when ``--foreground`` is passed -7. ``main()`` calls ``os.execv`` (NOT ``subprocess.Popen``) when a supervisor +8. ``main()`` calls ``os.execv`` (NOT ``subprocess.Popen``) when a supervisor env var is set even without the explicit flag -8. Default ``main()`` path (no flag, clean env) still uses ``Popen`` -9. Foreground path chdir's to ``agent_dir or REPO_ROOT`` before execv (matches +9. Default ``main()`` path (no flag, clean env) still uses ``Popen`` +10. Foreground path chdir's to ``agent_dir or REPO_ROOT`` before execv (matches the cwd the legacy Popen uses) -10. Foreground path exports ``HERMES_WEBUI_HOST`` / ``HERMES_WEBUI_PORT`` / +11. Foreground path exports ``HERMES_WEBUI_HOST`` / ``HERMES_WEBUI_PORT`` / ``HERMES_WEBUI_AGENT_DIR`` / ``HERMES_WEBUI_STATE_DIR`` to ``os.environ`` so the post-exec server picks them up -11. Foreground path skips ``wait_for_health`` (no client to retry from) -12. ``--foreground`` help text mentions launchd / systemd / supervisord +12. Foreground path skips ``wait_for_health`` (no client to retry from) +13. ``--foreground`` help text mentions launchd / systemd / supervisord +14. Non-executable ``python_exe`` raises ``RuntimeError`` instead of + looping the supervisor on ``execv`` failure These tests do NOT actually exec — ``os.execv`` is monkeypatched. We're pinning the structural choice (which path runs, which cwd, which env) not the @@ -66,14 +71,27 @@ sys.path.insert(0, str(REPO_ROOT)) @pytest.fixture def clean_env(monkeypatch): - """Strip all known supervisor env vars so detection starts from a clean state.""" + """Strip all known supervisor env vars + resolved bootstrap vars so each + test starts from a known-clean state. + + The resolved-vars stripping (HERMES_WEBUI_HOST etc.) prevents leakage + where a previous test's ``main()`` mutated ``os.environ`` and a later + test re-imports ``bootstrap``, picking up the polluted defaults. With + these stripped, ``DEFAULT_HOST`` / ``DEFAULT_PORT`` fall back to their + hardcoded defaults at module load time. + """ for name in ( + # Supervisor-detection env vars "INVOCATION_ID", "JOURNAL_STREAM", "NOTIFY_SOCKET", "XPC_SERVICE_NAME", "SUPERVISOR_ENABLED", "HERMES_WEBUI_FOREGROUND", + # Bootstrap-resolved env vars (mutated by main(), can leak across tests) + "HERMES_WEBUI_HOST", + "HERMES_WEBUI_PORT", + "HERMES_WEBUI_AGENT_DIR", ): monkeypatch.delenv(name, raising=False) @@ -155,6 +173,49 @@ class TestDetectSupervisor: assert import_bootstrap._detect_supervisor() == "HERMES_WEBUI_FOREGROUND" +class TestXPCServiceNameNoiseFilter: + """macOS launchd sets XPC_SERVICE_NAME in EVERY Terminal-spawned shell. + + Without filtering, every Mac dev running ``./start.sh`` would silently + auto-promote to foreground mode and lose the /health probe + browser open + + bootstrap log. We narrow to launchd Label-style names (com..) + while rejecting the well-known noise values. + """ + + @pytest.mark.parametrize("noise_value", [ + "0", # launchd descendants + "application.com.apple.Terminal.0BCDDEAD-1234-5678", # Terminal.app shells + "application.com.googlecode.iterm2", # iTerm2 + "application.com.microsoft.VSCode", # VSCode terminal + ]) + def test_xpc_noise_values_do_not_trigger(self, import_bootstrap, clean_env, monkeypatch, noise_value): + monkeypatch.setenv("XPC_SERVICE_NAME", noise_value) + assert import_bootstrap._detect_supervisor() is None, ( + f"XPC_SERVICE_NAME={noise_value!r} should not trigger foreground " + f"mode — that would break interactive ./start.sh on every Mac." + ) + + @pytest.mark.parametrize("real_value", [ + "com.example.hermes-webui", + "com.acme.production-server", + "io.github.user.my-service", + ]) + def test_xpc_real_label_triggers(self, import_bootstrap, clean_env, monkeypatch, real_value): + monkeypatch.setenv("XPC_SERVICE_NAME", real_value) + assert import_bootstrap._detect_supervisor() == "XPC_SERVICE_NAME", ( + f"XPC_SERVICE_NAME={real_value!r} is a launchd Label and should " + f"trigger foreground mode." + ) + + def test_xpc_noise_does_not_block_other_supervisor_var(self, import_bootstrap, clean_env, monkeypatch): + # If XPC has a noise value but INVOCATION_ID is set (mixed env, e.g. + # systemd unit run on a Mac CI runner), we should still detect via + # INVOCATION_ID rather than swallow it. + monkeypatch.setenv("XPC_SERVICE_NAME", "0") + monkeypatch.setenv("INVOCATION_ID", "deadbeef") + assert import_bootstrap._detect_supervisor() == "INVOCATION_ID" + + # ---------- main() routing ------------------------------------------------ @@ -356,3 +417,39 @@ class TestForegroundEnvAndCwd: # In foreground mode there's no parent left to retry from — the # supervisor's KeepAlive handles it. wait_for_health must not run. assert len(wait_calls) == 0 + + +class TestForegroundExecutabilityGuard: + """If python_exe is missing or non-executable, raise a clear error + instead of letting os.execv raise OSError → SystemExit(1) → supervisor + restart loop. This guard prevents the exact failure mode #1458 reports.""" + + @pytest.fixture + def setup_with_bad_python(self, monkeypatch, tmp_path): + import bootstrap as bs + agent_dir = tmp_path / "agent" + agent_dir.mkdir() + # Create a non-executable file at the python path + bad_python = tmp_path / "bad-python" + bad_python.write_text("#!/bin/bash\necho hi", encoding="utf-8") + bad_python.chmod(0o644) # NOT executable + monkeypatch.setattr(bs, "ensure_supported_platform", lambda: None) + monkeypatch.setattr(bs, "discover_agent_dir", lambda: agent_dir) + monkeypatch.setattr(bs, "hermes_command_exists", lambda: True) + monkeypatch.setattr(bs, "discover_launcher_python", lambda *a: str(bad_python)) + monkeypatch.setattr(bs, "ensure_python_has_webui_deps", lambda p: p) + monkeypatch.setenv("HERMES_WEBUI_STATE_DIR", str(tmp_path / "state")) + return bs + + def test_non_executable_python_raises_runtime_error(self, setup_with_bad_python, monkeypatch, clean_env): + bs = setup_with_bad_python + monkeypatch.setattr(sys, "argv", ["bootstrap.py", "--foreground"]) + + execv_calls = [] + monkeypatch.setattr(os, "execv", lambda *a: execv_calls.append(a)) + monkeypatch.setattr(os, "chdir", lambda p: None) + + with pytest.raises(RuntimeError, match="not executable"): + bs.main() + # execv must NOT have been called when the guard fires + assert len(execv_calls) == 0 From 5e806f6fd88b79b1e16bba8e33fef17a1b8f57df Mon Sep 17 00:00:00 2001 From: Dennis Soong Date: Sun, 3 May 2026 01:53:01 +0800 Subject: [PATCH 4/5] fix: restore inflight session on bfcache pageshow --- static/boot.js | 13 +++- tests/test_1045_bfcache_layout_restore.py | 48 ++++++++------ tests/test_1466_bfcache_inflight_reattach.py | 66 ++++++++++++++++++++ 3 files changed, 106 insertions(+), 21 deletions(-) create mode 100644 tests/test_1466_bfcache_inflight_reattach.py diff --git a/static/boot.js b/static/boot.js index e301b370..43629350 100644 --- a/static/boot.js +++ b/static/boot.js @@ -1357,7 +1357,7 @@ function applyBotName(){ // sync whenever the page is restored from cache (`event.persisted === true`). // Fix #1045: also re-run topbar/workspace/panel state so the rail and layout // chrome aren't left in the stale bfcache snapshot. -window.addEventListener('pageshow', (event) => { +window.addEventListener('pageshow', async (event) => { if (!event.persisted) return; // fresh loads are handled by the IIFE above const _srch = document.getElementById('sessionSearch'); if (_srch) _srch.value = ''; @@ -1367,6 +1367,17 @@ window.addEventListener('pageshow', (event) => { if (typeof closeReasoningDropdown === 'function') try { closeReasoningDropdown(); } catch (_) {} if (typeof closeWsDropdown === 'function') try { closeWsDropdown(); } catch (_) {} if (typeof closeProfileDropdown === 'function') try { closeProfileDropdown(); } catch (_) {} + // BFCache restores the frozen DOM without rerunning boot. Refresh the active + // session through the normal load path so in-flight sessions with + // active_stream_id / pending_user_message can reattach like a reload restore. + if (S.session && S.session.session_id && typeof loadSession === 'function') { + try { + await loadSession(S.session.session_id); + if (S.session && S.session.session_id && typeof checkInflightOnBoot === 'function') { + try { await checkInflightOnBoot(S.session.session_id); } catch (_) {} + } + } catch (_) {} + } // Re-synchronise layout chrome that the boot IIFE sets up but bfcache // doesn't re-run. Each call is guarded so missing helpers degrade silently. if (typeof syncTopbar === 'function') try { syncTopbar(); } catch (_) {} diff --git a/tests/test_1045_bfcache_layout_restore.py b/tests/test_1045_bfcache_layout_restore.py index 400a77f5..54e5b5bb 100644 --- a/tests/test_1045_bfcache_layout_restore.py +++ b/tests/test_1045_bfcache_layout_restore.py @@ -20,14 +20,30 @@ def _boot_js() -> str: return (ROOT / "static" / "boot.js").read_text(encoding="utf-8") +def _pageshow_handler(src: str) -> str: + marker = "window.addEventListener('pageshow'" + ps_idx = src.find(marker) + assert ps_idx != -1, "pageshow listener not found in boot.js" + brace = src.find("=> {", ps_idx) + assert brace != -1, "pageshow listener body not found" + brace = src.find("{", brace) + depth = 1 + i = brace + 1 + while i < len(src) and depth: + if src[i] == "{": + depth += 1 + elif src[i] == "}": + depth -= 1 + i += 1 + assert depth == 0, "pageshow listener body did not close" + return src[brace + 1 : i - 1] + + class TestBfcacheLayoutRestore: def test_pageshow_calls_sync_topbar(self): """pageshow handler must call syncTopbar() on bfcache restore.""" src = _boot_js() - # Find the pageshow listener block - ps_idx = src.find("window.addEventListener('pageshow'") - assert ps_idx != -1, "pageshow listener not found in boot.js" - handler_body = src[ps_idx:ps_idx + 1600] + handler_body = _pageshow_handler(src) assert "syncTopbar" in handler_body, ( "pageshow handler must call syncTopbar() to restore topbar state after bfcache" ) @@ -35,8 +51,7 @@ class TestBfcacheLayoutRestore: def test_pageshow_calls_sync_workspace_panel_state(self): """pageshow handler must call syncWorkspacePanelState().""" src = _boot_js() - ps_idx = src.find("window.addEventListener('pageshow'") - handler_body = src[ps_idx:ps_idx + 1600] + handler_body = _pageshow_handler(src) assert "syncWorkspacePanelState" in handler_body, ( "pageshow handler must call syncWorkspacePanelState() on bfcache restore" ) @@ -45,8 +60,7 @@ class TestBfcacheLayoutRestore: def test_pageshow_calls_start_gateway_sse(self): """pageshow handler must call startGatewaySSE() to reconnect the dead SSE connection.""" src = _boot_js() - ps_idx = src.find("window.addEventListener('pageshow'") - handler_body = src[ps_idx:ps_idx + 1600] + handler_body = _pageshow_handler(src) assert "startGatewaySSE" in handler_body, ( "pageshow handler must restart gateway SSE (bfcache-persisted connections are dead)" ) @@ -54,8 +68,7 @@ class TestBfcacheLayoutRestore: def test_pageshow_still_clears_session_search(self): """pageshow handler must still clear #sessionSearch (original #822 fix preserved).""" src = _boot_js() - ps_idx = src.find("window.addEventListener('pageshow'") - handler_body = src[ps_idx:ps_idx + 1600] + handler_body = _pageshow_handler(src) assert "sessionSearch" in handler_body, ( "pageshow handler must still clear #sessionSearch (regression: #822 fix must be preserved)" ) @@ -63,8 +76,7 @@ class TestBfcacheLayoutRestore: def test_pageshow_still_calls_render_session_list_from_cache(self): """pageshow handler must still call renderSessionListFromCache().""" src = _boot_js() - ps_idx = src.find("window.addEventListener('pageshow'") - handler_body = src[ps_idx:ps_idx + 1600] + handler_body = _pageshow_handler(src) assert "renderSessionListFromCache" in handler_body, ( "pageshow handler must still call renderSessionListFromCache() (regression: #822 fix)" ) @@ -73,8 +85,7 @@ class TestBfcacheLayoutRestore: """pageshow handler must NOT call _initResizePanels() — bfcache preserves event listeners so re-attaching them stacks duplicates.""" src = _boot_js() - ps_idx = src.find("window.addEventListener('pageshow'") - handler_body = src[ps_idx:ps_idx + 1600] + handler_body = _pageshow_handler(src) assert "_initResizePanels" not in handler_body, ( "pageshow handler must not call _initResizePanels() — it stacks " "duplicate mousedown listeners on every bfcache restore" @@ -83,8 +94,7 @@ class TestBfcacheLayoutRestore: def test_new_calls_are_guarded_with_typeof(self): """New calls in the pageshow handler must be typeof-guarded for safe degradation.""" src = _boot_js() - ps_idx = src.find("window.addEventListener('pageshow'") - handler_body = src[ps_idx:ps_idx + 1600] + handler_body = _pageshow_handler(src) # Each of the new calls must be guarded for fn in ("syncTopbar", "syncWorkspacePanelState", "startGatewaySSE", "closeModelDropdown", "closeReasoningDropdown", "closeWsDropdown", "closeProfileDropdown"): @@ -95,8 +105,7 @@ class TestBfcacheLayoutRestore: def test_pageshow_closes_all_dropdowns(self): """pageshow handler must close all known dropdowns to reset frozen bfcache popover state.""" src = _boot_js() - ps_idx = src.find("window.addEventListener('pageshow'") - handler_body = src[ps_idx:ps_idx + 1600] + handler_body = _pageshow_handler(src) for fn in ("closeModelDropdown", "closeReasoningDropdown", "closeWsDropdown", "closeProfileDropdown"): assert fn in handler_body, ( f"pageshow handler must call {fn}() to dismiss any dropdown left open by bfcache" @@ -105,8 +114,7 @@ class TestBfcacheLayoutRestore: def test_dropdowns_closed_before_layout_sync(self): """Dropdown closes must come before layout sync calls (clean state first).""" src = _boot_js() - ps_idx = src.find("window.addEventListener('pageshow'") - handler_body = src[ps_idx:ps_idx + 1600] + handler_body = _pageshow_handler(src) close_idx = handler_body.find("closeModelDropdown") sync_idx = handler_body.find("syncTopbar") assert close_idx != -1 and sync_idx != -1, "Both close and sync calls must be present" diff --git a/tests/test_1466_bfcache_inflight_reattach.py b/tests/test_1466_bfcache_inflight_reattach.py new file mode 100644 index 00000000..08d8e33e --- /dev/null +++ b/tests/test_1466_bfcache_inflight_reattach.py @@ -0,0 +1,66 @@ +"""Regression coverage for BFCache restore of an in-flight session (#1466). + +A browser `pageshow` restore from BFCache does not re-run the boot IIFE. After +#1473, normal reload restores in-flight sessions through `loadSession()` and +`checkInflightOnBoot()`. BFCache restore should align with that path for the +currently viewed session instead of only refreshing layout chrome/sidebar cache. +""" +from pathlib import Path + +REPO_ROOT = Path(__file__).parent.parent +BOOT_JS = (REPO_ROOT / "static" / "boot.js").read_text(encoding="utf-8") + + +def _pageshow_handler() -> str: + marker = "window.addEventListener('pageshow'" + start = BOOT_JS.find(marker) + assert start != -1, "pageshow listener not found in boot.js" + brace = BOOT_JS.find("=> {", start) + assert brace != -1, "pageshow listener body not found" + brace = BOOT_JS.find("{", brace) + depth = 1 + i = brace + 1 + while i < len(BOOT_JS) and depth: + if BOOT_JS[i] == "{": + depth += 1 + elif BOOT_JS[i] == "}": + depth -= 1 + i += 1 + assert depth == 0, "pageshow listener body did not close" + return BOOT_JS[brace + 1 : i - 1] + + +def test_pageshow_restores_active_inflight_session_through_load_session(): + """BFCache restore should refresh active session metadata and reattach stream. + + The normal boot/reload path now calls loadSession(saved), which handles + active_stream_id / pending_user_message and reattaches via attachLiveStream(). + The BFCache pageshow path must call the same loadSession path for the current + active session because the boot IIFE does not run again. + """ + body = _pageshow_handler() + assert "S.session" in body and "S.session.session_id" in body + assert "loadSession(S.session.session_id" in body + + +def test_pageshow_checks_persisted_inflight_marker_after_restoring_session(): + """BFCache restore should preserve the reload recovery marker behavior. + + If localStorage still has an inflight marker, the pageshow path should reuse + checkInflightOnBoot() after restoring the active session, matching the normal + reload path without adding new server-side persistence behavior. + """ + body = _pageshow_handler() + load_pos = body.find("loadSession(S.session.session_id") + check_pos = body.find("checkInflightOnBoot(S.session.session_id") + assert load_pos != -1 + assert check_pos != -1 + assert load_pos < check_pos + + +def test_pageshow_active_session_restore_is_guarded_and_non_blocking(): + """pageshow handler must degrade safely when optional helpers are absent.""" + body = _pageshow_handler() + assert "typeof loadSession === 'function'" in body + assert "typeof checkInflightOnBoot === 'function'" in body + assert "catch" in body[body.find("loadSession(S.session.session_id") : body.find("startGatewaySSE")] From e1708c4535c437d4355ee3fb1fdc70cd36d2009c Mon Sep 17 00:00:00 2001 From: Hermes Bot Date: Sat, 2 May 2026 18:12:13 +0000 Subject: [PATCH 5/5] =?UTF-8?q?chore(release):=20stamp=20v0.50.269=20?= =?UTF-8?q?=E2=80=94=20bootstrap=20supervisor=20fix=20+=202=20v0.50.267=20?= =?UTF-8?q?follow-ups?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - CHANGELOG.md: v0.50.269 entry detailing #1478 #1479 #1480 - ROADMAP.md: bump to v0.50.269, 3847 tests collected - TESTING.md: bump header + total to 3847 #1478: nesquena APPROVED self-built bootstrap.py --foreground mode (closes #1458 Bug #1, +Opus follow-ups: XPC noise filter, executability guard) #1479: surgical follow-up to #1473 — Session.compact() now includes pending_user_message #1480: bfcache pageshow restores active session via loadSession + checkInflightOnBoot 3847 tests pass (+47 net). Opus advisor on stage diff: no blockers. --- CHANGELOG.md | 10 ++++++++++ ROADMAP.md | 2 +- TESTING.md | 4 ++-- 3 files changed, 13 insertions(+), 3 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 19da0eed..eae1da1e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,15 @@ # Hermes Web UI -- Changelog +## [v0.50.269] — 2026-05-02 + +### Fixed (1 self-built + 2 contributor follow-ups) + +- **`bootstrap.py` `--foreground` mode for process supervisors** (#1478, self-built, closes #1458 Bug #1) — the `bootstrap.py` double-fork pattern (`subprocess.Popen([python, "server.py"], start_new_session=True)` then exit 0) breaks every process supervisor. launchd / systemd / supervisord / runit / s6 see the parent exit, mark the program "completed," and respawn it — but the orphaned server still owns port 8787, so the new bootstrap fails to bind, exits non-zero, supervisor respawns again. Loop until something else crashes the orphan and the next respawn finds the port free. Reporter described this as "the agent fixes it eventually" — that's the loop intermittently succeeding. **Fix:** new `--foreground` flag (and supervisor-environment auto-detection via `INVOCATION_ID` / `JOURNAL_STREAM` / `NOTIFY_SOCKET` / `SUPERVISOR_ENABLED` / `XPC_SERVICE_NAME` / `HERMES_WEBUI_FOREGROUND`). In foreground mode, replace the bootstrap process image with `server.py` via `os.execv` so the supervisor sees the long-lived server as the original child. KeepAlive / Restart=always now work correctly. Plus 1 Opus pre-merge MUST-FIX (`_is_real_supervisor_value()` helper rejects macOS Terminal's noise values like `XPC_SERVICE_NAME=0` and `application.com.apple.Terminal.` — without this, every Mac dev running interactive `./start.sh` would silently auto-promote to foreground mode, losing the /health probe and browser open) + 2 SHOULD-FIX (test env-var leakage cleanup, pre-execv `os.access(python_exe, os.X_OK)` guard so a non-executable launcher path raises a clear RuntimeError instead of OSError-then-respawn-loop). 44 regression tests + new `docs/supervisor.md` reference (runnable launchd plist + systemd `.service` + supervisord conf + diagnostic `lsof`/`ppid` recipe). **Bugs #2 (state.db FD leak) and #3 (HTTP-unhealthy wedge) remain open under #1458** awaiting diagnostic data. (`bootstrap.py`, `docs/supervisor.md`, `.gitignore`, `tests/test_bootstrap_foreground.py`) + +- **`/api/sessions` payload missing `pending_user_message`** (#1479, by @Thanatos-Z) — surgical 6-LOC follow-up to v0.50.267 #1473. The frontend reload/sidebar recovery filter at `sessions.js:1342-1349` checks both `s.active_stream_id` AND `s.pending_user_message` to keep mid-restore sessions visible, but `Session.compact()` (the dict serialized into the `/api/sessions` payload) was missing `pending_user_message`. The filter only worked via the `active_stream_id` clause. In practice not user-visible because `active_stream_id` and `pending_user_message` are set/cleared atomically together (verified at `api/routes.py:4232-4240`), so any session with the latter also had the former. The fix prevents future drift if the atomicity invariant ever changes. (`api/models.py`, `static/i18n.js`, `tests/test_issue856_session_streaming_state.py`) + +- **bfcache `pageshow` doesn't restore active session** (#1480, by @dso2ng) — when a browser restores the WebUI from bfcache (back/forward navigation), the frozen DOM is brought back without re-running boot. Sessions with `active_stream_id` or `pending_user_message` set looked stale in the active pane because the in-flight reattach logic (the v0.50.267 #1473 fix) only ran on fresh page loads. **Fix:** the pageshow handler now `await loadSession(S.session.session_id)` to refresh through the normal load path, then `await checkInflightOnBoot(...)` to reattach SSE. Tightened existing bfcache layout-restore tests via a shared `_pageshow_handler()` helper that walks the listener body via brace matching instead of the prior brittle `[ps_idx:ps_idx + 1600]` window. New `tests/test_1466_bfcache_inflight_reattach.py`. (`static/boot.js`, `tests/test_1045_bfcache_layout_restore.py`, `tests/test_1466_bfcache_inflight_reattach.py`) + ## [v0.50.268] — 2026-05-02 ### Fixed (contributor PR batch — 4 PRs) diff --git a/ROADMAP.md b/ROADMAP.md index e02d8962..92daff99 100644 --- a/ROADMAP.md +++ b/ROADMAP.md @@ -3,7 +3,7 @@ > Goal: Full 1:1 parity with the Hermes CLI experience via a clean dark web UI. > Everything you can do from the CLI terminal, you can do from this UI. > -> Last updated: v0.50.268 (May 02, 2026) — 3800 tests collected +> Last updated: v0.50.269 (May 02, 2026) — 3847 tests collected > Tests: `pytest tests/ --collect-only -q` > Source: / diff --git a/TESTING.md b/TESTING.md index 9af3a822..ce67758c 100644 --- a/TESTING.md +++ b/TESTING.md @@ -1835,8 +1835,8 @@ Bridged CLI sessions: --- -*Last updated: v0.50.268, May 02, 2026* -*Total automated tests collected: 3800* +*Last updated: v0.50.269, May 02, 2026* +*Total automated tests collected: 3847* *Regression gate: tests/test_regressions.py* *Run: pytest tests/ -v --timeout=60* *Source: /*