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/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: /* diff --git a/api/models.py b/api/models.py index 0a6000fe..62afc9ed 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/bootstrap.py b/bootstrap.py index 8981c4c7..ccdbee9a 100644 --- a/bootstrap.py +++ b/bootstrap.py @@ -208,9 +208,83 @@ 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) +# +# 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", + "NOTIFY_SOCKET", + "XPC_SERVICE_NAME", + "SUPERVISOR_ENABLED", +) + + +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. + + 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: + value = os.environ.get(name, "") + if _is_real_supervisor_value(name, value): + return name + return None + + def main() -> int: args = parse_args() ensure_supported_platform() @@ -229,21 +303,59 @@ 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 + # 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]) + # 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..4ec433b0 --- /dev/null +++ b/docs/supervisor.md @@ -0,0 +1,237 @@ +# 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 — 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`` | + +### 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 + +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/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/static/i18n.js b/static/i18n.js index eb4225eb..d2c64725 100644 --- a/static/i18n.js +++ b/static/i18n.js @@ -7529,6 +7529,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_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")] diff --git a/tests/test_bootstrap_foreground.py b/tests/test_bootstrap_foreground.py new file mode 100644 index 00000000..7b7ed8c6 --- /dev/null +++ b/tests/test_bootstrap_foreground.py @@ -0,0 +1,455 @@ +""" +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. ``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 +8. ``main()`` calls ``os.execv`` (NOT ``subprocess.Popen``) when a supervisor + env var is set even without the explicit flag +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) +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 +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 +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 + 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) + + +@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" + + +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 ------------------------------------------------ + + +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 + + +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 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():