fix(update): make the in-progress marker a real cross-process lock

Three surfaces start updates against one checkout: a terminal
"hermes update", the dashboard's Update button (which spawns that same
command detached), and the desktop's, which hands off to the Tauri
updater. Only the Tauri updater published the in-progress marker, and
only Electron read it -- to gate backend startup, not to stop a second
updater. So a dashboard-spawned update and an installer-driven git
checkout could mutate the same tree concurrently, rewriting source under
a live interpreter.

Claim the same marker from cmd_update rather than adding a second
mechanism: same path, same pid+started_at payload the Rust and Electron
readers already parse. A marker only counts as live when its pid is alive
and it is inside the shared age ceiling, so a crashed updater self-heals
instead of wedging every future update. Release only removes a marker we
still own, leaving a handoff partner's claim intact.

Refusing exits 2, matching the existing concurrent-instance contract the
Tauri updater already recognizes.

hermes_cli/main.py

@@ -8787,9 +8787,28 @@ def cmd_update(args):
     # writes to a closed stdout.  No-op in gateway mode.  See
     # _install_hangup_protection for rationale.
     _update_io_state = _install_hangup_protection(gateway_mode=gateway_mode)
+    # Cross-process mutual exclusion. The dashboard's Update button spawns
+    # this same command detached, and the desktop hands off to the Tauri
+    # updater / install-mode bootstrap — all three mutate one checkout. Two of
+    # them running together rewrite source under a live interpreter and strand
+    # the tree half-updated. Share the marker the Tauri updater and Electron
+    # already use rather than inventing a second lock.
+    from hermes_cli.update_lock import (
+        UPDATE_EXIT_CONCURRENT,
+        UpdateLock,
+        describe_holder,
+    )
+
+    _update_lock = UpdateLock()
+    if not _update_lock.acquire():
+        print(describe_holder(_update_lock.holder))
+        _finalize_update_output(_update_io_state)
+        sys.exit(UPDATE_EXIT_CONCURRENT)
+
     try:
         _cmd_update_impl(args, gateway_mode=gateway_mode)
     finally:
+        _update_lock.release()
         _finalize_update_output(_update_io_state)
 
 

hermes_cli/update_lock.py

@@ -0,0 +1,216 @@
+"""Cross-process mutual exclusion for in-flight Hermes updates.
+
+Three different surfaces can start an update of the same install tree:
+
+* ``hermes update`` from a terminal,
+* the dashboard's Update button (``POST /api/hermes/update`` →
+  ``_spawn_hermes_action(["update"])``, detached),
+* the desktop's Update button, which hands off to the Tauri
+  ``hermes-setup --update`` and, on its failure screen, to install-mode
+  bootstrap (``install.ps1`` / ``install.sh``).
+
+Until now only the Tauri updater published an "update in progress" marker
+(``UpdateMarkerGuard`` in ``apps/bootstrap-installer/src-tauri/src/update.rs``),
+and only the Electron desktop consumed it (``electron/update-marker.ts``, to
+gate local backend startup). Nothing stopped two *updaters* from running at
+once — so a dashboard-spawned ``hermes update`` and an installer-driven
+``git checkout`` could mutate the same checkout concurrently, rewriting source
+under a live interpreter and leaving the tree half-updated.
+
+This module makes that same marker the single lock for **all** update
+entrypoints instead of adding a fourth mechanism. Format and location are
+unchanged and remain byte-compatible with the Rust and Electron readers:
+
+    <HERMES_HOME>/.hermes-update-in-progress   body: "<pid>\\n<started_at_unix>"
+
+A marker only counts as a live update when its pid is alive AND it is younger
+than :data:`UPDATE_MARKER_MAX_AGE_MS` — mirroring ``readLiveUpdateMarker`` so a
+crashed updater self-heals instead of wedging every future update. A stale
+marker is removed on read by whoever notices it first.
+"""
+
+from __future__ import annotations
+
+import logging
+import os
+import time
+from dataclasses import dataclass
+from pathlib import Path
+
+logger = logging.getLogger(__name__)
+
+# Keep in sync with UPDATE_MARKER_MAX_AGE_MS in
+# apps/desktop/electron/update-marker.ts — the same marker is read by both, and
+# a shorter ceiling here would let Python steal a lock Electron still considers
+# live. A full update (git pull + uv sync + desktop rebuild) is minutes.
+UPDATE_MARKER_MAX_AGE_SECONDS = 20 * 60
+
+MARKER_NAME = ".hermes-update-in-progress"
+
+# Exit code meaning "another updater/instance owns this install right now".
+# Already the de-facto contract: the Windows shim + venv-holder guards in
+# _cmd_update_impl exit 2, and the Tauri updater matches on it
+# (UPDATE_EXIT_CONCURRENT in apps/bootstrap-installer/src-tauri/src/update.rs)
+# to show "Hermes is still running" instead of a generic failure. Naming it
+# here keeps the concurrent-update refusal on that same understood contract.
+UPDATE_EXIT_CONCURRENT = 2
+
+
+def update_marker_path() -> Path:
+    """Path of the shared update marker.
+
+    Uses the *process* Hermes home (never the context-local profile override):
+    the Rust updater resolves ``$HERMES_HOME`` or the platform default, and the
+    desktop pins that same value into the updater's env. A profile-scoped path
+    here would put the lock somewhere the other two owners never look.
+    """
+    from hermes_constants import get_process_hermes_home
+
+    return get_process_hermes_home() / MARKER_NAME
+
+
+def _pid_alive(pid: int) -> bool:
+    """True when a process with ``pid`` currently exists.
+
+    ``os.kill(pid, 0)`` is POSIX-only in spirit but CPython implements the
+    signal-0 existence probe on Windows too. ``PermissionError`` means the pid
+    exists but is owned by another user — still alive for our purposes.
+
+    A value too large for the platform's ``pid_t`` raises ``OverflowError``
+    (not ``OSError``), so a corrupt marker would otherwise crash every update
+    that reads it. Treat any unusable pid as dead: the marker is garbage and
+    must not wedge the lock.
+    """
+    if pid <= 0:
+        return False
+    try:
+        os.kill(pid, 0)
+    except ProcessLookupError:
+        return False
+    except PermissionError:
+        return True
+    except (OverflowError, ValueError):
+        return False
+    except OSError:
+        # Unexpected errno: assume alive rather than stomping a live updater.
+        return True
+    return True
+
+
+@dataclass(frozen=True)
+class UpdateHolder:
+    """A confirmed-live update currently holding the lock."""
+
+    pid: int
+    age_seconds: float
+
+
+def read_live_update(*, path: Path | None = None) -> UpdateHolder | None:
+    """Return the live update holding the lock, or ``None``.
+
+    Mirrors ``readLiveUpdateMarker`` in ``electron/update-marker.ts``: absent,
+    unreadable, malformed, dead-pid, and past-the-ceiling all mean "no live
+    update", and a stale marker file is deleted so it can't strand future runs.
+    Never raises.
+    """
+    marker = path or update_marker_path()
+    try:
+        raw = marker.read_text(encoding="utf-8")
+    except OSError:
+        return None  # absent or unreadable => no live update
+
+    lines = raw.splitlines()
+    try:
+        pid = int(lines[0].strip())
+    except (IndexError, ValueError):
+        pid = -1
+    try:
+        started_at = float(lines[1].strip())
+    except (IndexError, ValueError):
+        started_at = float("-inf")
+
+    age = time.time() - started_at
+    if not _pid_alive(pid) or age > UPDATE_MARKER_MAX_AGE_SECONDS:
+        try:
+            marker.unlink()
+        except OSError:
+            pass
+        return None
+
+    return UpdateHolder(pid=pid, age_seconds=age)
+
+
+def describe_holder(holder: UpdateHolder) -> str:
+    """One-line, user-facing explanation of who holds the update lock."""
+    minutes, seconds = divmod(int(max(holder.age_seconds, 0)), 60)
+    elapsed = f"{minutes}m {seconds}s" if minutes else f"{seconds}s"
+    return (
+        f"✗ Another Hermes update is already running (PID {holder.pid}, "
+        f"started {elapsed} ago).\n"
+        "\n"
+        "  Two updates mutating the same checkout corrupt it: one rewrites\n"
+        "  source while the other is mid-install. Wait for it to finish, or\n"
+        "  close the window/dashboard tab that started it, then retry."
+    )
+
+
+class UpdateLock:
+    """Context manager owning the shared update marker for this process.
+
+    ``acquired`` is False when another live update already holds it — callers
+    decide whether that's a hard refusal (CLI/dashboard) or a wait. Releasing
+    only removes the marker when *we* still own it, so a marker rewritten by a
+    handoff partner (the Tauri updater overwrites it with its own pid) is never
+    deleted out from under its new owner.
+    """
+
+    def __init__(self, *, path: Path | None = None) -> None:
+        self.path = path or update_marker_path()
+        self.acquired = False
+        self.holder: UpdateHolder | None = None
+
+    def acquire(self) -> bool:
+        """Claim the lock. Returns False (and sets ``holder``) if it's taken."""
+        existing = read_live_update(path=self.path)
+        if existing is not None:
+            self.holder = existing
+            return False
+        try:
+            self.path.parent.mkdir(parents=True, exist_ok=True)
+            self.path.write_text(
+                f"{os.getpid()}\n{int(time.time())}\n", encoding="utf-8"
+            )
+        except OSError as exc:
+            # Best-effort, exactly like the Rust guard: an unwritable marker
+            # must not block the update itself (that would be a worse failure
+            # than the race it prevents). Degrade to the pre-lock behavior.
+            logger.debug("Could not write update marker %s: %s", self.path, exc)
+            return True
+        self.acquired = True
+        return True
+
+    def release(self) -> None:
+        """Drop the marker if this process still owns it. Never raises."""
+        if not self.acquired:
+            return
+        self.acquired = False
+        try:
+            raw = self.path.read_text(encoding="utf-8")
+            owner = int(raw.splitlines()[0].strip())
+        except (OSError, IndexError, ValueError):
+            return
+        if owner != os.getpid():
+            # A handoff partner took ownership (e.g. the Tauri updater wrote
+            # its own pid). Leave it alone — it's still a live update.
+            return
+        try:
+            self.path.unlink()
+        except OSError:
+            pass
+
+    def __enter__(self) -> "UpdateLock":
+        self.acquire()
+        return self
+
+    def __exit__(self, *_exc) -> None:
+        self.release()

tests/hermes_cli/test_update_lock.py

@@ -0,0 +1,177 @@
+"""Cross-process update mutual exclusion (``hermes_cli.update_lock``).
+
+Three surfaces can start an update of one install tree: a terminal ``hermes
+update``, the dashboard's Update button (which spawns that same command
+detached), and the desktop's Update button (Tauri updater → install-mode
+bootstrap on its failure screen). Before the shared lock, two of them could run
+concurrently and rewrite source under a live interpreter — observed in the wild
+as an installer ``git checkout`` rewinding the checkout ~9k commits while a
+dashboard-spawned ``hermes update`` was mid-``npm install``, which then failed
+against the rewound tree's manifests.
+
+These exercise the real marker file against a temp home — no mocks — because
+the contract that matters is what the Rust updater and the Electron gate see on
+disk.
+"""
+
+from __future__ import annotations
+
+import os
+import time
+
+import pytest
+
+from hermes_cli.update_lock import (
+    UPDATE_MARKER_MAX_AGE_SECONDS,
+    UpdateLock,
+    describe_holder,
+    read_live_update,
+    update_marker_path,
+)
+
+# A pid no live process owns. os.kill(pid, 0) must report it dead so a crashed
+# updater can never wedge every future update. Deliberately larger than any
+# platform's pid_t so it also covers the corrupt-marker path (OverflowError).
+DEAD_PID = 4294967294
+
+
+@pytest.fixture
+def marker(tmp_path):
+    return tmp_path / ".hermes-update-in-progress"
+
+
+def test_marker_path_follows_process_hermes_home(tmp_path, monkeypatch):
+    """The lock must land where the Rust updater and Electron gate look.
+
+    All three resolve the *process* HERMES_HOME; a profile-scoped path would
+    put the lock somewhere the other two owners never read.
+    """
+    monkeypatch.setenv("HERMES_HOME", str(tmp_path))
+    assert update_marker_path() == tmp_path / ".hermes-update-in-progress"
+
+
+def test_acquire_writes_pid_and_start_time(marker):
+    lock = UpdateLock(path=marker)
+
+    assert lock.acquire() is True
+    assert lock.acquired is True
+
+    lines = marker.read_text(encoding="utf-8").splitlines()
+    assert int(lines[0]) == os.getpid(), "the Electron gate probes this pid for liveness"
+    assert int(lines[1]) == pytest.approx(time.time(), abs=5)
+    assert len(lines) == 2, "wire format is exactly pid + started_at"
+
+
+def test_second_acquire_is_refused_while_the_first_is_live(marker):
+    """The bug: two updaters mutating one checkout at the same time."""
+    first = UpdateLock(path=marker)
+    assert first.acquire() is True
+
+    second = UpdateLock(path=marker)
+    assert second.acquire() is False
+    assert second.holder is not None
+    assert second.holder.pid == os.getpid()
+    assert second.acquired is False
+
+
+def test_refused_lock_does_not_delete_the_live_owners_marker(marker):
+    first = UpdateLock(path=marker)
+    first.acquire()
+
+    second = UpdateLock(path=marker)
+    second.acquire()
+    second.release()
+
+    assert marker.exists(), "a refused claimant must never clear the live owner's lock"
+
+    first.release()
+    assert not marker.exists()
+
+
+def test_release_leaves_a_marker_a_handoff_partner_now_owns(marker):
+    """The desktop writes the marker, then the Tauri updater takes ownership.
+
+    Releasing must not delete a marker whose pid is no longer ours — that would
+    reopen the gate while the partner is still mid-update.
+    """
+    lock = UpdateLock(path=marker)
+    lock.acquire()
+
+    marker.write_text(f"{DEAD_PID}\n{int(time.time())}\n", encoding="utf-8")
+    lock.release()
+
+    assert marker.exists(), "the partner's marker is not ours to remove"
+
+
+def test_dead_owner_is_reclaimed_not_honored(marker):
+    marker.write_text(f"{DEAD_PID}\n{int(time.time())}\n", encoding="utf-8")
+
+    lock = UpdateLock(path=marker)
+    assert lock.acquire() is True
+    assert int(marker.read_text(encoding="utf-8").splitlines()[0]) == os.getpid()
+
+
+def test_owner_past_the_age_ceiling_is_reclaimed(marker):
+    """A live-but-wedged updater must not hold the lock forever."""
+    long_ago = int(time.time()) - UPDATE_MARKER_MAX_AGE_SECONDS - 60
+    marker.write_text(f"{os.getpid()}\n{long_ago}\n", encoding="utf-8")
+
+    lock = UpdateLock(path=marker)
+    assert lock.acquire() is True
+
+
+@pytest.mark.parametrize(
+    "body",
+    ["", "not-a-pid\n123\n", "\n\n", "12345"],
+    ids=["empty", "garbage-pid", "blank-lines", "no-start-time"],
+)
+def test_malformed_markers_never_block_an_update(marker, body):
+    marker.write_text(body, encoding="utf-8")
+
+    assert read_live_update(path=marker) is None
+    assert UpdateLock(path=marker).acquire() is True
+
+
+def test_stale_marker_is_removed_on_read(marker):
+    marker.write_text(f"{DEAD_PID}\n{int(time.time())}\n", encoding="utf-8")
+
+    assert read_live_update(path=marker) is None
+    assert not marker.exists(), "whoever notices a stale marker clears it"
+
+
+def test_absent_marker_reports_no_live_update(marker):
+    assert read_live_update(path=marker) is None
+
+
+def test_context_manager_releases_even_on_exception(marker):
+    with pytest.raises(RuntimeError):
+        with UpdateLock(path=marker) as lock:
+            assert lock.acquired is True
+            raise RuntimeError("update blew up mid-flight")
+
+    assert not marker.exists(), "a crashed update must not strand the lock"
+
+
+def test_describe_holder_names_the_pid_and_elapsed_time(marker):
+    lock = UpdateLock(path=marker)
+    lock.acquire()
+
+    holder = read_live_update(path=marker)
+    assert holder is not None
+    message = describe_holder(holder)
+
+    assert str(os.getpid()) in message, "the user needs the pid to find the other update"
+    assert "already running" in message
+
+
+def test_unwritable_marker_location_does_not_block_the_update(tmp_path):
+    """Degrade to pre-lock behavior rather than refusing to update at all.
+
+    An unwritable marker path is a worse reason to block an update than the
+    race the lock prevents.
+    """
+    lock = UpdateLock(path=tmp_path / "nonexistent-file" / "marker")
+    (tmp_path / "nonexistent-file").write_text("i am a file, not a dir", encoding="utf-8")
+
+    assert lock.acquire() is True
+    assert lock.acquired is False, "nothing was written, so there is nothing to release"