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.
This commit is contained in:
@@ -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(
|
||||
|
||||
@@ -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())
|
||||
) {
|
||||
|
||||
@@ -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,
|
||||
|
||||
@@ -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:
|
||||
|
||||
@@ -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=<name>``
|
||||
(profile alias ``<profile> 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,
|
||||
|
||||
82
tests/hermes_cli/test_isolated_serve_ledger_marker.py
Normal file
82
tests/hermes_cli/test_isolated_serve_ledger_marker.py
Normal file
@@ -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")
|
||||
@@ -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
|
||||
):
|
||||
|
||||
Reference in New Issue
Block a user