mirror of
https://github.com/NousResearch/hermes-agent.git
synced 2026-07-31 19:16:29 +00:00
fix(dashboard): isolate Desktop-inherited env from standalone launch (#69891)
Supersedes #52948 and #67402. Closes #52945. Standalone hermes dashboard/serve was trusting HERMES_WEB_DIST and HERMES_SERVE_HEADLESS inherited from a Desktop Electron parent, which could serve the packaged desktop renderer ("Desktop IPC bridge is unavailable") or disable the SPA. Drop only Electron-packaged WEB_DIST paths (app.asar*) when HERMES_DESKTOP!=1, and clear inherited headless for non-serve launches, while preserving caller-managed custom dist overrides and the desktop-spawned backend path. Co-authored-by: Bartok9 <danielrpike9@gmail.com> Co-authored-by: Commander <commander@tianji.local>
This commit is contained in:
parent
0721d2ea80
commit
a8e6c0f853
2 changed files with 197 additions and 0 deletions
|
|
@ -12922,6 +12922,21 @@ def _read_ssh_session_token_file(path: str) -> str:
|
|||
os.close(root_fd)
|
||||
|
||||
|
||||
def _is_electron_packaged_web_dist(path: str) -> bool:
|
||||
"""True when *path* looks like an Electron-packaged renderer dist.
|
||||
|
||||
Packaged Desktop sets ``HERMES_WEB_DIST`` to ``.../app.asar/dist`` or
|
||||
``.../app.asar.unpacked/dist``. A standalone ``hermes dashboard`` that
|
||||
inherits that value serves the desktop frontend in the browser
|
||||
(issue #52945 — "Desktop IPC bridge is unavailable").
|
||||
"""
|
||||
if not path:
|
||||
return False
|
||||
# Both app.asar and app.asar.unpacked contain this marker; normalize
|
||||
# separators so Windows paths match too.
|
||||
return "app.asar" in path.replace("\\", "/")
|
||||
|
||||
|
||||
def cmd_dashboard(args):
|
||||
"""Start the web UI server, or (with --stop/--status) manage running ones."""
|
||||
_token_file = getattr(args, "ssh_session_token_file", None)
|
||||
|
|
@ -12959,6 +12974,25 @@ def cmd_dashboard(args):
|
|||
if _token_file and not _headless_backend:
|
||||
raise SystemExit("--ssh-session-token-file is only valid with hermes serve")
|
||||
|
||||
# ── Sanitize Desktop-inherited env that hijacks a standalone launch ─
|
||||
# Desktop Electron spawns its backend with HERMES_DESKTOP=1 plus
|
||||
# HERMES_WEB_DIST=<packaged app.asar[/unpacked]/dist> (and often
|
||||
# HERMES_SERVE_HEADLESS=1 on the serve path). A shell that inherits
|
||||
# those vars then runs `hermes dashboard` would otherwise:
|
||||
# - serve the desktop renderer → "Desktop IPC bridge is unavailable"
|
||||
# (issue #52945), or
|
||||
# - disable the SPA via inherited HERMES_SERVE_HEADLESS.
|
||||
# Only strip Electron-packaged WEB_DIST contamination — caller-managed
|
||||
# HERMES_WEB_DIST overrides (dev / custom builds) must still work.
|
||||
# The desktop-spawned backend itself (HERMES_DESKTOP=1) keeps its dist.
|
||||
# Intentionally headless `serve` re-sets HERMES_SERVE_HEADLESS below.
|
||||
if os.environ.get("HERMES_DESKTOP") != "1":
|
||||
_inherited_web_dist = os.environ.get("HERMES_WEB_DIST", "")
|
||||
if _is_electron_packaged_web_dist(_inherited_web_dist):
|
||||
os.environ.pop("HERMES_WEB_DIST", None)
|
||||
if not _headless_backend:
|
||||
os.environ.pop("HERMES_SERVE_HEADLESS", None)
|
||||
|
||||
# ── Unified profile launch routing ────────────────────────────────
|
||||
# The dashboard is a MACHINE management surface: it can read/write any
|
||||
# profile via the per-request ?profile= scoping. Running one dashboard
|
||||
|
|
|
|||
|
|
@ -246,3 +246,166 @@ def test_skip_build_custom_env_dist_missing_does_not_attempt_recovery(
|
|||
assert started == []
|
||||
out = capsys.readouterr().out
|
||||
assert "--skip-build was passed but no web dist found" in out
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# Desktop-inherited env isolation (issue #52945 / supersedes #52948, #67402)
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
|
||||
@pytest.mark.parametrize(
|
||||
"path,expected",
|
||||
[
|
||||
("/Applications/Hermes.app/Contents/Resources/app.asar/dist", True),
|
||||
("/Applications/Hermes.app/Contents/Resources/app.asar.unpacked/dist", True),
|
||||
(r"C:\Users\u\AppData\Local\Programs\Hermes\resources\app.asar\dist", True),
|
||||
("/home/u/custom-dashboard-dist", False),
|
||||
("", False),
|
||||
],
|
||||
)
|
||||
def test_is_electron_packaged_web_dist(main_mod, path, expected):
|
||||
assert main_mod._is_electron_packaged_web_dist(path) is expected
|
||||
|
||||
|
||||
def test_standalone_dashboard_drops_electron_packaged_web_dist(
|
||||
main_mod, monkeypatch
|
||||
):
|
||||
"""Inherited app.asar WEB_DIST must be stripped so the bundled web UI
|
||||
is built/served instead of the desktop renderer."""
|
||||
_wire_common(main_mod, monkeypatch)
|
||||
monkeypatch.delenv("HERMES_DESKTOP", raising=False)
|
||||
packaged = "/Applications/Hermes.app/Contents/Resources/app.asar/dist"
|
||||
monkeypatch.setenv("HERMES_WEB_DIST", packaged)
|
||||
|
||||
started = []
|
||||
monkeypatch.setitem(
|
||||
sys.modules,
|
||||
"hermes_cli.web_server",
|
||||
types.SimpleNamespace(start_server=lambda **k: started.append(k)),
|
||||
)
|
||||
builds = []
|
||||
monkeypatch.setattr(
|
||||
main_mod, "_build_web_ui", lambda *a, **k: builds.append(a) or True
|
||||
)
|
||||
|
||||
main_mod.cmd_dashboard(_args())
|
||||
|
||||
import os
|
||||
|
||||
assert "HERMES_WEB_DIST" not in os.environ
|
||||
assert len(builds) == 1
|
||||
assert len(started) == 1
|
||||
|
||||
|
||||
def test_standalone_dashboard_keeps_caller_managed_web_dist(
|
||||
main_mod, monkeypatch, tmp_path
|
||||
):
|
||||
"""A non-Electron custom HERMES_WEB_DIST override must survive."""
|
||||
_wire_common(main_mod, monkeypatch)
|
||||
monkeypatch.delenv("HERMES_DESKTOP", raising=False)
|
||||
dist = tmp_path / "my-custom-dist"
|
||||
dist.mkdir()
|
||||
(dist / "index.html").write_text("<html></html>", encoding="utf-8")
|
||||
monkeypatch.setenv("HERMES_WEB_DIST", str(dist))
|
||||
|
||||
started = []
|
||||
monkeypatch.setitem(
|
||||
sys.modules,
|
||||
"hermes_cli.web_server",
|
||||
types.SimpleNamespace(start_server=lambda **k: started.append(k)),
|
||||
)
|
||||
builds = []
|
||||
monkeypatch.setattr(
|
||||
main_mod, "_build_web_ui", lambda *a, **k: builds.append(a) or True
|
||||
)
|
||||
|
||||
main_mod.cmd_dashboard(_args())
|
||||
|
||||
import os
|
||||
|
||||
assert os.environ["HERMES_WEB_DIST"] == str(dist)
|
||||
assert builds == []
|
||||
assert len(started) == 1
|
||||
|
||||
|
||||
def test_desktop_spawned_backend_keeps_electron_web_dist(
|
||||
main_mod, monkeypatch, tmp_path
|
||||
):
|
||||
"""HERMES_DESKTOP=1 legitimately points at the packaged dist — do not strip."""
|
||||
_wire_common(main_mod, monkeypatch)
|
||||
packaged_root = tmp_path / "app.asar" / "dist"
|
||||
packaged_root.mkdir(parents=True)
|
||||
(packaged_root / "index.html").write_text("<html></html>", encoding="utf-8")
|
||||
monkeypatch.setenv("HERMES_DESKTOP", "1")
|
||||
monkeypatch.setenv("HERMES_WEB_DIST", str(packaged_root))
|
||||
|
||||
started = []
|
||||
monkeypatch.setitem(
|
||||
sys.modules,
|
||||
"hermes_cli.web_server",
|
||||
types.SimpleNamespace(start_server=lambda **k: started.append(k)),
|
||||
)
|
||||
builds = []
|
||||
monkeypatch.setattr(
|
||||
main_mod, "_build_web_ui", lambda *a, **k: builds.append(a) or True
|
||||
)
|
||||
|
||||
main_mod.cmd_dashboard(_args())
|
||||
|
||||
import os
|
||||
|
||||
assert os.environ["HERMES_WEB_DIST"] == str(packaged_root)
|
||||
assert builds == []
|
||||
assert len(started) == 1
|
||||
|
||||
|
||||
def test_standalone_dashboard_clears_inherited_serve_headless(
|
||||
main_mod, monkeypatch
|
||||
):
|
||||
"""Inherited HERMES_SERVE_HEADLESS must not disable the SPA for dashboard."""
|
||||
_wire_common(main_mod, monkeypatch)
|
||||
monkeypatch.delenv("HERMES_DESKTOP", raising=False)
|
||||
monkeypatch.delenv("HERMES_WEB_DIST", raising=False)
|
||||
monkeypatch.setenv("HERMES_SERVE_HEADLESS", "1")
|
||||
|
||||
started = []
|
||||
monkeypatch.setitem(
|
||||
sys.modules,
|
||||
"hermes_cli.web_server",
|
||||
types.SimpleNamespace(start_server=lambda **k: started.append(k)),
|
||||
)
|
||||
monkeypatch.setattr(main_mod, "_build_web_ui", lambda *a, **k: True)
|
||||
|
||||
main_mod.cmd_dashboard(_args())
|
||||
|
||||
import os
|
||||
|
||||
assert os.environ.get("HERMES_SERVE_HEADLESS") != "1"
|
||||
assert len(started) == 1
|
||||
|
||||
|
||||
def test_headless_serve_reasserts_serve_headless(main_mod, monkeypatch):
|
||||
"""`hermes serve` must still set HERMES_SERVE_HEADLESS after the clear."""
|
||||
_wire_common(main_mod, monkeypatch)
|
||||
monkeypatch.delenv("HERMES_DESKTOP", raising=False)
|
||||
monkeypatch.delenv("HERMES_WEB_DIST", raising=False)
|
||||
monkeypatch.delenv("HERMES_SERVE_HEADLESS", raising=False)
|
||||
|
||||
started = []
|
||||
monkeypatch.setitem(
|
||||
sys.modules,
|
||||
"hermes_cli.web_server",
|
||||
types.SimpleNamespace(start_server=lambda **k: started.append(k)),
|
||||
)
|
||||
builds = []
|
||||
monkeypatch.setattr(
|
||||
main_mod, "_build_web_ui", lambda *a, **k: builds.append(a) or True
|
||||
)
|
||||
|
||||
main_mod.cmd_dashboard(_args(headless_backend=True))
|
||||
|
||||
import os
|
||||
|
||||
assert os.environ.get("HERMES_SERVE_HEADLESS") == "1"
|
||||
assert builds == []
|
||||
assert len(started) == 1
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue