From e3275db9ae30cd188d3d89a3fe53bce4e2284f61 Mon Sep 17 00:00:00 2001 From: Teknium <127238744+teknium1@users.noreply.github.com> Date: Sun, 4 Oct 2026 23:07:29 -0700 Subject: [PATCH] fix(update): startup recovery resumes gateways only while holding the checkout lock (review F5-recovery-start-vs-checkout-mutation) What: _resume_claimed takes the checkout lock (bounded contention wait) around the tree gate and the verified gateway start; when an update already took the checkout after our claim, the claim is handed back unowned and untouched, so that update or the next launch owns it. Why: recovery judged the tree and launched gateways with no checkout custody, so a concurrent `hermes update` could mutate the checkout under freshly started gateways (the updater skips a live claim and may discover no running gateway to pause). Red: pre-fix update_pause_record.py -> "gateways restarted while an update owned the checkout: resume ['default']". Green: test_recovery_never_starts_gateways_under_an_update_that_took_the_checkout passes (set resumed by the next launch). --- hermes_cli/update_pause_record.py | 15 ++++++++++ tests/hermes_cli/test_update_pause_record.py | 30 ++++++++++++++++++++ 2 files changed, 45 insertions(+) diff --git a/hermes_cli/update_pause_record.py b/hermes_cli/update_pause_record.py index 03c6edf250..c5a22b68fb 100644 --- a/hermes_cli/update_pause_record.py +++ b/hermes_cli/update_pause_record.py @@ -647,6 +647,21 @@ def _has_work(token: dict) -> bool: def _resume_claimed(claim_path: Path, body: dict) -> None: + """Resume under the checkout lock, so no update mutates the tree between the gate and the + verified start; an update that took the checkout after our claim gets the set back untouched.""" + from hermes_cli.update_lock import UpdateLock + fence = UpdateLock() + if not fence.acquire_checkout(install_root()): + with suppress(OSError), _mutex(): + _atomic_write(claim_path, {**(read(claim_path) or body), "claimer": UNOWNED}) + return + try: + _resume_fenced(claim_path, body) + finally: + fence.release() + + +def _resume_fenced(claim_path: Path, body: dict) -> None: with _mutex(): # A live consumer may have checkpointed after claim() returned its snapshot. body = read(claim_path) or body diff --git a/tests/hermes_cli/test_update_pause_record.py b/tests/hermes_cli/test_update_pause_record.py index 05f2fd3b25..4df57760cb 100644 --- a/tests/hermes_cli/test_update_pause_record.py +++ b/tests/hermes_cli/test_update_pause_record.py @@ -183,6 +183,36 @@ def test_a_launch_killed_mid_recovery_leaves_the_set_to_the_next_launch(tmp_path assert out.strip() == "done", f"a resumed set was resumed again: {out}" +# An update takes the checkout right after this launch claimed the set (the race window). +_UPDATE_AFTER_CLAIM = """ + import subprocess, sys + from hermes_cli import update_pause_record as r + hold = ("import time; from hermes_cli import update_lock as l; from hermes_cli.update_pause_record import install_root;" + "print(l.UpdateLock().acquire_checkout(install_root()), flush=True); time.sleep(120)") + real_claim, updates = r.claim, [] + def claim(src): + won = real_claim(src) + updates.append(subprocess.Popen([sys.executable, "-c", hold], stdout=subprocess.PIPE, text=True)) + assert updates[-1].stdout.readline().strip() == "True" + return won + r.claim = claim +""" + _RECOVER + """ + for update in updates: + update.kill() + update.wait() +""" + + +@pytest.mark.live_system_guard_bypass +def test_recovery_never_starts_gateways_under_an_update_that_took_the_checkout(tmp_path): + _orphan(tmp_path, {"default": 4242}) + env = {"HERMES_HOME": str(tmp_path)} + out, _ = _child(_UPDATE_AFTER_CLAIM, "ok", env=env).communicate(timeout=60) + assert out.strip() == "done", f"gateways restarted while an update owned the checkout: {out}" + out, _ = _child(_RECOVER, "ok", env=env).communicate(timeout=60) + assert out.splitlines()[:1] == ["resume ['default']"], f"the handed-back set was stranded: {out}" + + _HOLDER = """ import subprocess, sys, time from hermes_cli.update_lock import UpdateLock