From c728e6583e864eaf7b614091dcf50cfb3085a1cb Mon Sep 17 00:00:00 2001 From: kshitijk4poor <82637225+kshitijk4poor@users.noreply.github.com> Date: Mon, 14 Sep 2026 21:03:52 +0530 Subject: [PATCH] fix(update-windows): per-profile cold-start probes its own home and runs after the active spawn MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Review follow-ups on the per-profile obligation: - Order: the active profile's cold-start guard is fleet-wide (any live gateway ⇒ done), so a sibling spawned first would have left the active profile down. Per-profile spawns now run after it. - A token is built even when the active plan owes nothing (clean exit, autostart not installed), so a dead-attested sibling still rides on it. - Readiness for a per-profile spawn is probed in THAT home's identity files (`_live_gateway_pids(home=)` → `get_running_pid(home/"gateway.pid")`), not the fleet: a still-running sibling no longer vouches for a dead spawn. The wait/confirm helpers share one probe function. - The new PID is attested in the profile home (`_write_start_attestation(..., home=)`) so a death after the CLI exits reaches the next update, and an already-live profile is not spawned twice. --- hermes_cli/gateway_windows.py | 33 ++++++++++----- hermes_cli/update_cmd_windows.py | 27 ++++++++----- ...ws_gateway_cold_start_desktop_lifecycle.py | 40 +++++++++++++++++-- 3 files changed, 76 insertions(+), 24 deletions(-) diff --git a/hermes_cli/gateway_windows.py b/hermes_cli/gateway_windows.py index b54d87a085..d0087a28a7 100644 --- a/hermes_cli/gateway_windows.py +++ b/hermes_cli/gateway_windows.py @@ -812,7 +812,23 @@ def install( raise RuntimeError(f"Windows gateway install failed: {detail}") -def _confirm_gateway_stable(initial_pids: list[int], confirm_s: float, interval_s: float, all_profiles: bool = False) -> list[int]: +def _live_gateway_pids(all_profiles: bool = False, home: Path | None = None) -> list[int]: + """Live gateway PIDs for the readiness poll. ``home`` scopes the probe to ONE profile's identity + files (a still-running sibling must not vouch for a per-profile spawn, #110959); otherwise the + process-table discovery for the active profile or the whole fleet.""" + if home is not None: + from gateway.status import get_running_pid + + pid = get_running_pid(home / "gateway.pid", cleanup_stale=False) + return [pid] if pid else [] + from hermes_cli.gateway import find_gateway_pids + + return list(find_gateway_pids(all_profiles=all_profiles)) + + +def _confirm_gateway_stable( + initial_pids: list[int], confirm_s: float, interval_s: float, all_profiles: bool = False, home: Path | None = None, +) -> list[int]: """Re-check a freshly detected gateway for ``confirm_s`` seconds: one process-table hit proves the child was *created*, not that it survived startup (or a parent Job Object teardown). @@ -824,13 +840,11 @@ def _confirm_gateway_stable(initial_pids: list[int], confirm_s: float, interval_ """ if confirm_s <= 0: return initial_pids - from hermes_cli.gateway import find_gateway_pids - pids = initial_pids confirm_deadline = time.monotonic() + confirm_s while time.monotonic() < confirm_deadline: time.sleep(interval_s) - pids = list(find_gateway_pids(all_profiles=all_profiles)) + pids = _live_gateway_pids(all_profiles, home) if not pids: return [] return pids @@ -838,16 +852,15 @@ def _confirm_gateway_stable(initial_pids: list[int], confirm_s: float, interval_ def _wait_for_gateway_ready( timeout_s: float = 6.0, interval_s: float = 0.4, confirm_s: float = 2.0, all_profiles: bool = False, + home: Path | None = None, ) -> list[int]: """Poll for a live gateway for up to ``timeout_s``; a first hit is provisional until the gateway stays visible for ``confirm_s`` more seconds (a child that dies right after spawn earns no ✓).""" - from hermes_cli.gateway import find_gateway_pids - deadline = time.monotonic() + timeout_s while time.monotonic() < deadline: - pids = list(find_gateway_pids(all_profiles=all_profiles)) + pids = _live_gateway_pids(all_profiles, home) if pids: - confirmed = _confirm_gateway_stable(pids, confirm_s, interval_s, all_profiles=all_profiles) + confirmed = _confirm_gateway_stable(pids, confirm_s, interval_s, all_profiles=all_profiles, home=home) if confirmed: return confirmed continue # died during confirmation — keep polling until deadline @@ -872,14 +885,14 @@ def _start_attestation_path(home: Path | None = None) -> Path: return (home if home is not None else _hermes_home()).joinpath(*_START_ATTESTATION_RELATIVE) -def _write_start_attestation(pids: list[int], via: str) -> None: +def _write_start_attestation(pids: list[int], via: str, home: Path | None = None) -> None: """Persist the PIDs a ✓ vouched for. Best-effort, never raises. ``generation`` identifies this marker instance: the update resume token records the generation whose death authorized a cold-start, so execution consumes exactly that marker and never a newer one written by a concurrent ``hermes gateway start`` (#110020 review).""" try: - path = _start_attestation_path() + path = _start_attestation_path(home) path.parent.mkdir(parents=True, exist_ok=True) from hermes_cli.process_identity import _process_create_time diff --git a/hermes_cli/update_cmd_windows.py b/hermes_cli/update_cmd_windows.py index 4f6c0e216d..1f8a7d82fd 100644 --- a/hermes_cli/update_cmd_windows.py +++ b/hermes_cli/update_cmd_windows.py @@ -887,9 +887,11 @@ def _pause_windows_gateways_for_update() -> dict | None: profile_processes, service_gateways, service_gateway_pids, running_pids = _discover_windows_gateways() if not running_pids: token = _windows_cold_start_plan() - if token is not None: - _record_attested_cold_start_profiles(token, set()) - return token + # Other profiles may hold a dead attestation even when the active profile owes nothing + # (clean exit, autostart not installed): give them a token to ride on. + probe = token if token is not None else {"resume_needed": True, "profiles": {}, "unmapped_pids": [], "unmapped": []} + _record_attested_cold_start_profiles(probe, set()) + return probe if probe.get("cold_start_profiles") else token profiles, mapped_pids, socket_acks = _request_socket_pauses(running_pids, profile_processes, service_gateway_pids) # Resolve venv-side launchers BEFORE draining: a dead worker's parent cannot be recovered (NoSuchProcess). # The launcher keeps ``.pyd`` mapped and would trip the venv-holder guard; it is killed with the survivors. @@ -962,14 +964,17 @@ def _cold_start_attested_profiles(token: dict) -> None: for name, generation in sorted(pending.items()): home = Path(get_profile_dir(name)) with _best_effort(f"Could not cold-start Windows gateway profile {name} after update: %s"): - pid = gateway_windows._spawn_detached(home=home) - if not pid: - raise RuntimeError("cold-start did not return a process ID") - ready_pids = gateway_windows._wait_for_gateway_ready(all_profiles=True) + if not gateway_windows._live_gateway_pids(home=home): # a concurrent autostart must not be doubled + pid = gateway_windows._spawn_detached(home=home) + if not pid: + raise RuntimeError("cold-start did not return a process ID") + ready_pids = gateway_windows._wait_for_gateway_ready(home=home) if not ready_pids: - raise RuntimeError(f"PID {pid} did not become ready") + raise RuntimeError(f"gateway profile {name} did not become ready") gateway_windows._consume_start_attestation(generation, home=home) - print(f"\n✓ Gateway profile {name} started via cold-start after update (PID: {pid})") + # Keep the attestation chain: a death after this CLI exits must be visible to the next update. + gateway_windows._write_start_attestation(ready_pids, f"cold-start after update (profile {name})", home=home) + print(f"\n✓ Gateway profile {name} started via cold-start after update (PID: {ready_pids[0]})") token["cold_start_profiles"].pop(name, None) if not token["cold_start_profiles"]: token.pop("cold_start_profiles", None) @@ -1203,16 +1208,18 @@ def _resume_windows_gateways_after_update(token: dict | None) -> None: # autostart entry comes back on the current design at next login too. _m()._refresh_windows_gateway_launchers() _resume_windows_services(token) - _cold_start_attested_profiles(token) profiles = token.get("profiles") or {} unmapped = token.get("unmapped") or [] if not profiles and not any(u.get("argv") for u in unmapped): if token.get("cold_start_if_installed"): + # Before the per-profile spawns: this guard is fleet-wide (any live gateway ⇒ done). if not _m()._cold_start_windows_gateway_after_update(token): raise RuntimeError("Windows gateway cold-start was not verified") token["cold_start_if_installed"] = False + _cold_start_attested_profiles(token) token["resume_needed"] = False return + _cold_start_attested_profiles(token) relaunched, unmapped_relaunched = _relaunch_paused_gateways(token, profiles, unmapped) if relaunched or unmapped_relaunched: _verify_relaunched_gateways_alive(token, profiles, unmapped) diff --git a/tests/hermes_cli/test_windows_gateway_cold_start_desktop_lifecycle.py b/tests/hermes_cli/test_windows_gateway_cold_start_desktop_lifecycle.py index 55696c5d35..14312d1ef3 100644 --- a/tests/hermes_cli/test_windows_gateway_cold_start_desktop_lifecycle.py +++ b/tests/hermes_cli/test_windows_gateway_cold_start_desktop_lifecycle.py @@ -253,12 +253,14 @@ def _running_beta_pause_fixture(monkeypatch, tmp_path): monkeypatch.setattr(cli_main, "_venv_launcher_ancestors", lambda pids: []) monkeypatch.setattr(cli_main, "_wait_for_windows_update_gateway_exit", lambda pids, timeout: set()) monkeypatch.setattr(profiles_mod, "get_active_profile_name", lambda: "default") - monkeypatch.setattr(profiles_mod, "profiles_to_serve", lambda multiplex: list(homes.items())) + monkeypatch.setattr(profiles_mod, "profiles_to_serve", lambda multiplex: [(n, h) for n, h in homes.items() if n in ("default", "beta")]) monkeypatch.setattr(profiles_mod, "get_profile_dir", lambda name: homes[name]) # Resume side. monkeypatch.setattr(cli_main, "_refresh_windows_gateway_launchers", lambda: None) monkeypatch.setattr(hermes_gateway, "launch_detached_profile_gateway_restart", lambda p, o: True) - monkeypatch.setattr(gateway_windows, "_wait_for_gateway_ready", lambda *a, **k: [4242]) + ready_probes: list = [] + monkeypatch.setattr(gateway_windows, "_wait_for_gateway_ready", lambda *a, **k: ready_probes.append(k) or [4242]) + homes["_ready_probes"] = ready_probes return homes @@ -280,11 +282,15 @@ def test_dead_attested_default_is_cold_started_beside_running_beta(monkeypatch, spawned = [] monkeypatch.setattr(gateway_windows, "_spawn_detached", lambda **k: spawned.append(k) or 4242) - monkeypatch.setattr(gateway_windows, "_write_start_attestation", lambda *a, **k: None) update_cmd._resume_windows_gateways_after_update(token) assert spawned == [{"home": homes["default"]}] - assert not marker.exists() # consumed with the default profile's home + # Readiness is probed in the default profile's own home: live ``beta`` must not vouch for it. + assert {"home": homes["default"]} in homes["_ready_probes"] + # The authorizing generation is consumed and the NEW PID is attested in the same profile home, + # so a death after this CLI exits stays visible to the next update. + reattested = json.loads(marker.read_text(encoding="utf-8")) + assert (reattested["pids"], reattested["generation"] != generation) == ([4242], True) assert "cold_start_profiles" not in token assert token["relaunched_profiles"] == ["beta"] assert token["resume_needed"] is False @@ -305,3 +311,29 @@ def test_no_attested_profile_leaves_the_pause_token_unchanged(monkeypatch, tmp_p update_cmd._resume_windows_gateways_after_update(token) assert spawned == [] assert token["relaunched_profiles"] == ["beta"] + + +def test_every_dead_attested_profile_is_cold_started_when_nothing_runs(monkeypatch, tmp_path): + """Nothing running, active profile exited cleanly (plan → None), ``beta`` dead-attested: beta still + gets a token and a spawn. And when BOTH owe a spawn, the fleet-wide active cold-start runs FIRST — + a beta spawned earlier would satisfy its any-live-gateway guard and the active profile would stay down.""" + homes = _running_beta_pause_fixture(monkeypatch, tmp_path) + monkeypatch.setattr(update_cmd_windows, "_discover_windows_gateways", lambda: ({}, [], set(), [])) + monkeypatch.setattr(update_cmd_windows, "_windows_cold_start_plan", lambda: None) + gateway_windows._write_start_attestation([556], "direct spawn (PID 556)", home=homes["beta"]) + beta_marker = homes["beta"] / "state" / "gateway.start-attestation.json" + beta_generation = json.loads(beta_marker.read_text(encoding="utf-8"))["generation"] + + token = update_cmd._pause_windows_gateways_for_update() + assert token["cold_start_profiles"] == {"beta": beta_generation} + assert token["profiles"] == {} + + order = [] + monkeypatch.setattr(gateway_windows, "_spawn_detached", lambda **k: order.append(k.get("home")) or 4242) + monkeypatch.setattr( + cli_main, "_cold_start_windows_gateway_after_update", lambda token=None: order.append("active") or True) + token["cold_start_if_installed"] = True # both owe a spawn + update_cmd._resume_windows_gateways_after_update(token) + assert order == ["active", homes["beta"]] + assert json.loads(beta_marker.read_text(encoding="utf-8"))["generation"] != beta_generation + assert token["resume_needed"] is False