From 2e0243b15839176e2196abfd1e41b5ffb31d40ab Mon Sep 17 00:00:00 2001 From: calvinnwq Date: Thu, 24 Sep 2026 21:32:16 +1000 Subject: [PATCH] fix(desktop): never attach to an isolated serve found in the spawn ledger hermes serve --isolated (the backend another machine's Desktop spawns over SSH) opts out of the host singleton on the CLI side, but its spawn ledger row carried no structured marker, so the local Desktop's attach-first discovery adopted it. Nothing on this host owns that process, so a Desktop-driven update left the local app on stale code. Record isolated=True in the ledger row and skip such rows in parseSpawnLedger. An ordinary serve is still attached even when an isolated record is newer. --- .../electron/backend-discovery.test.ts | 43 ++++++++++ apps/desktop/electron/backend-discovery.ts | 5 ++ hermes_cli/main.py | 1 + hermes_cli/process_identity.py | 7 +- hermes_cli/web_server.py | 7 +- .../test_isolated_serve_ledger_marker.py | 82 +++++++++++++++++++ .../test_serve_runtime_inventory.py | 10 +++ 7 files changed, 153 insertions(+), 2 deletions(-) create mode 100644 tests/hermes_cli/test_isolated_serve_ledger_marker.py diff --git a/apps/desktop/electron/backend-discovery.test.ts b/apps/desktop/electron/backend-discovery.test.ts index 090d48d2a7..fb3bf961dd 100644 --- a/apps/desktop/electron/backend-discovery.test.ts +++ b/apps/desktop/electron/backend-discovery.test.ts @@ -86,6 +86,49 @@ test('no backend record spawns exactly one backend', async () => { assert.equal(spawns, 1) }) +/** + * `serve --isolated` is another client's backend (Desktop SSH mode from a + * different machine). It is live, loopback and serves a valid token, but it is + * not this host's shared backend: attaching to it strands this app on + * whatever code that client started, across our own updates. + */ +test('an isolated serve record is never attached; startup spawns its own backend', async () => { + const isolatedOnly = JSON.stringify([{ ...JSON.parse(LEDGER)[0], isolated: true }]) + let spawns = 0 + + const setup = await runPrimaryBackendStartup({ + assertCurrentAttempt: () => {}, + attachHostBackend: () => + attachToHostBackend({ isolated: false, ledgerPath: '/ledger.json' }, attachDeps(isolatedOnly)), + connectRemote: async () => ({ mode: 'remote' }), + ensureLocalRuntime: async backend => backend, + prepareLocalBackend: () => { + spawns += 1 + + return { label: 'spawned' } + }, + resolveRemote: async () => null, + waitForDecision: async () => 'continue-local' as const, + waitForLocalStart: async () => undefined + }) + + assert.equal(setup.kind, 'local') + assert.equal(spawns, 1) +}) + +test('an ordinary serve is still attached when an isolated one is newer', async () => { + const [ordinary] = JSON.parse(LEDGER) + + const ledger = JSON.stringify([ + ordinary, + { ...ordinary, isolated: true, pid: 5150, port: 61_000, registered_at: ordinary.registered_at + 1 } + ]) + + const attached = await attachToHostBackend({ isolated: false, ledgerPath: '/ledger.json' }, attachDeps(ledger)) + + assert.equal(attached?.pid, 4711) +}) + /** A record that fails validation is not a backend: fall through to spawning. */ test('a record whose backend rejects the session token does not attach', async () => { const attached = await attachToHostBackend( diff --git a/apps/desktop/electron/backend-discovery.ts b/apps/desktop/electron/backend-discovery.ts index bf860c6abd..dcd62411af 100644 --- a/apps/desktop/electron/backend-discovery.ts +++ b/apps/desktop/electron/backend-discovery.ts @@ -50,6 +50,10 @@ function asInteger(value: unknown): number | null { * * A record without a bound port predates the structured detail (or belongs to * a purpose that never binds) and is skipped: a port is the whole point. + * A record marked `isolated` (`hermes serve --isolated`, e.g. the backend + * another machine's Desktop spawned here over SSH) opted out of the host + * singleton and belongs to that client, so it is skipped too; the CLI's + * `_attach_to_host_backend` honours the same flag. * Unreadable/corrupt JSON yields `[]` — discovery degrades to "spawn", never * to a wrong attach. */ @@ -85,6 +89,7 @@ export function parseSpawnLedger(contents: unknown): HostBackendRecord[] { port === null || port <= 0 || port > 65535 || + entry.isolated === true || !ATTACHABLE_PURPOSES.has(purpose) || !LOOPBACK_DIALABLE.has(host.toLowerCase()) ) { diff --git a/hermes_cli/main.py b/hermes_cli/main.py index d54d4fbe4f..69ae0ce567 100644 --- a/hermes_cli/main.py +++ b/hermes_cli/main.py @@ -2774,6 +2774,7 @@ def cmd_dashboard(args): allow_public=getattr(args, "insecure", False), initial_profile=getattr(args, "open_profile", "") or "", headless=_headless_backend, + isolated=getattr(args, "isolated", False), ssh_session_token=_ssh_session_token, ssh_owner_nonce=_ssh_owner_nonce, start_mcp_discovery_after_bind=_mcp_discovery_after_bind, diff --git a/hermes_cli/process_identity.py b/hermes_cli/process_identity.py index 25c9aa865a..62094c5c70 100644 --- a/hermes_cli/process_identity.py +++ b/hermes_cli/process_identity.py @@ -125,6 +125,9 @@ class LedgerEntry: port: Optional[int] = None profile: str = "" hermes_home: str = "" + # `serve --isolated`: opted out of the host singleton (Desktop's SSH backend for another + # machine). Attach-first readers must never adopt it; argv is truncated, so this is canonical. + isolated: bool = False def _ledger_path() -> Path: @@ -201,7 +204,8 @@ def register_self(purpose: str, *, project_root: Optional[Path] = None, detail: Called at the top of every long-lived entry point; dead ``(pid, create_time)`` entries are pruned on every write. ``detail`` may carry ``host``/``port``/``profile`` so the update - pipeline can relaunch a manually-started serve with its real bind address. + pipeline can relaunch a manually-started serve with its real bind address, and ``isolated`` + so attach-first discovery skips a backend that opted out of the host singleton. """ from hermes_constants import hermes_home_key @@ -214,6 +218,7 @@ def register_self(purpose: str, *, project_root: Optional[Path] = None, detail: entry.host = str(detail.get("host") or "") entry.port = int(detail["port"]) if detail.get("port") is not None else None entry.profile = str(detail.get("profile") or "") + entry.isolated = bool(detail.get("isolated")) except (TypeError, ValueError): pass try: diff --git a/hermes_cli/web_server.py b/hermes_cli/web_server.py index 0a43062d82..2600a1a792 100644 --- a/hermes_cli/web_server.py +++ b/hermes_cli/web_server.py @@ -1277,6 +1277,7 @@ def _on_server_started( host: str, port: int, headless: bool, + isolated: bool, open_browser: bool, initial_profile: str, start_mcp_discovery_after_bind: bool, @@ -1334,7 +1335,7 @@ def _on_server_started( register_self( "serve" if headless else "dashboard", - detail={"host": host, "port": actual_port, "profile": initial_profile or ""}, + detail={"host": host, "port": actual_port, "profile": initial_profile or "", "isolated": isolated}, ) attach_self_to_kill_on_close_job() @@ -1468,6 +1469,7 @@ def start_server( allow_public: bool = False, initial_profile: str = "", headless: bool = False, + isolated: bool = False, ssh_session_token: Optional[str] = None, ssh_owner_nonce: Optional[str] = None, start_mcp_discovery_after_bind: bool = False, @@ -1477,6 +1479,8 @@ def start_server( ``initial_profile`` is appended to the auto-opened URL as ``?profile=`` (profile alias `` dashboard``). ``headless`` is the ``serve`` path: JSON-RPC/WS backend, no UI build, no SPA mount (``HERMES_SERVE_HEADLESS``). + ``isolated`` (``--isolated``) is recorded in the spawn ledger so attach-first + discovery never adopts this process. ``ssh_session_token``/``ssh_owner_nonce`` are process-local Desktop SSH bootstrap state, never persisted or exported to children. ``start_mcp_discovery_after_bind`` (Desktop ``serve``) defers MCP discovery @@ -1562,6 +1566,7 @@ def start_server( host=host, port=port, headless=headless, + isolated=isolated, open_browser=open_browser, initial_profile=initial_profile, start_mcp_discovery_after_bind=start_mcp_discovery_after_bind, diff --git a/tests/hermes_cli/test_isolated_serve_ledger_marker.py b/tests/hermes_cli/test_isolated_serve_ledger_marker.py new file mode 100644 index 0000000000..7ce2381d45 --- /dev/null +++ b/tests/hermes_cli/test_isolated_serve_ledger_marker.py @@ -0,0 +1,82 @@ +"""An isolated serve says so in the spawn ledger; an ordinary serve does not. + +Desktop's attach-first discovery reads ``spawn-ledger.json`` and attaches to any live loopback +serve. ``hermes serve --isolated`` (the backend another machine's Desktop spawns over SSH) opts out +of the host singleton on the CLI side, so the ledger row must carry that fact as a structured field +for Desktop to honour it too. Real processes against a temp ``HERMES_HOME``; no mocks. +""" + +import json +import os +import socket +import subprocess +import sys +import time + +import psutil +import pytest + + +def _free_port(): + with socket.socket() as sock: + sock.bind(("127.0.0.1", 0)) + return sock.getsockname()[1] + + +def _await_ledger_row(ledger, proc, log, *, timeout=45): + deadline = time.monotonic() + timeout + while time.monotonic() < deadline: + if proc.poll() is not None: + break + pids = {proc.pid, *(c.pid for c in psutil.Process(proc.pid).children(recursive=True))} + try: + rows = json.loads(ledger.read_text(encoding="utf-8")) + except (OSError, ValueError): + rows = [] + for row in rows: + if row.get("purpose") == "serve" and row.get("pid") in pids and row.get("port"): + return row + time.sleep(0.1) + pytest.fail(f"serve never registered (exit={proc.poll()}): {log.read_text(encoding='utf-8', errors='replace')}") + + +def _stop(proc): + if proc.poll() is None: + parent = psutil.Process(proc.pid) + family = [*parent.children(recursive=True), parent] + for p in family: + p.terminate() + _, alive = psutil.wait_procs(family, timeout=5) + for p in alive: + p.kill() + proc.wait(timeout=10) + + +def test_isolated_serve_ledger_row_is_marked_and_ordinary_serve_is_not(tmp_path): + home = tmp_path / "home" + home.mkdir() + env = {**os.environ, "HERMES_HOME": str(home), "HERMES_GATEWAY_LOCK_DIR": str(tmp_path / "locks")} + for key in ("HERMES_DESKTOP", "HERMES_PARENT_PID", "HERMES_PARENT_START_MARKER", + "HERMES_DASHBOARD_SESSION_TOKEN", "HERMES_SPAWN"): + env.pop(key, None) + ledger = home / "spawn-ledger.json" + + # Ordinary first: started second it would attach to the isolated one and exit. + launches = [("ordinary", []), ("isolated", ["--isolated"])] + procs, rows = {}, {} + try: + for name, extra in launches: + log = tmp_path / f"{name}.log" + with log.open("w", encoding="utf-8") as out: + procs[name] = subprocess.Popen( + [sys.executable, "-m", "hermes_cli.main", "serve", *extra, + "--host", "127.0.0.1", "--port", str(_free_port())], + env=env, stdout=out, stderr=subprocess.STDOUT, + ) + rows[name] = _await_ledger_row(ledger, procs[name], log) + finally: + for proc in procs.values(): + _stop(proc) + + assert rows["isolated"].get("isolated") is True + assert not rows["ordinary"].get("isolated") diff --git a/tests/hermes_cli/test_serve_runtime_inventory.py b/tests/hermes_cli/test_serve_runtime_inventory.py index b2ea333e20..3359d9712f 100644 --- a/tests/hermes_cli/test_serve_runtime_inventory.py +++ b/tests/hermes_cli/test_serve_runtime_inventory.py @@ -55,6 +55,16 @@ def test_register_self_records_structured_detail(tmp_path, monkeypatch): assert e["port"] == 9119 assert e["profile"] == "work" + +def test_register_self_records_isolated_marker(tmp_path, monkeypatch): + from hermes_cli import process_identity as pi + + monkeypatch.setattr(pi, "_ledger_path", lambda: tmp_path / "ledger.json") + monkeypatch.setattr(pi, "install_id", lambda *a, **k: "inst") + assert pi.register_self("serve", detail={"host": "127.0.0.1", "port": 9119, "isolated": True}) + assert pi._read_ledger(tmp_path / "ledger.json")[-1]["isolated"] is True + + def test_register_self_without_detail_stays_backward_compatible( tmp_path, monkeypatch ):