fix(update-windows): per-profile cold-start probes its own home and runs after the active spawn
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.
This commit is contained in:
@@ -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
|
||||
|
||||
|
||||
@@ -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)
|
||||
|
||||
@@ -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
|
||||
|
||||
Reference in New Issue
Block a user