diff --git a/hermes_cli/_early_recovery.py b/hermes_cli/_early_recovery.py index 6a2c317e53..3587d7d196 100644 --- a/hermes_cli/_early_recovery.py +++ b/hermes_cli/_early_recovery.py @@ -245,6 +245,19 @@ def interrupted_pull_marker(root: Path) -> Path: return _git_dir(root) / INTERRUPTED_PULL_MARKER +# The repair's own code is in the tree a killed move tears (``hermes_bootstrap``, this module, the +# package initializer, the lock/custody modules). ``update_cmd_commit.arm_tree_move`` publishes this +# closure, as committed at the marker's ``pre``, beside the marker before git writes; a minted +# launcher whose checkout import fails runs the repair from there (``_launchers._launcher_script``). +# The modules import only the stdlib and each other; the package initializer is published empty. +RECOVERY_CLOSURE_DIR = "hermes-update-recovery" +RECOVERY_CLOSURE = ("hermes_cli/_early_recovery.py", "hermes_cli/update_lock.py", "hermes_cli/update_custody.py") + + +def recovery_closure_dir(root: Path, pre: str) -> Path: + return _git_dir(root) / RECOVERY_CLOSURE_DIR / pre + + def _git_executable(recorded_by_updater: str = "") -> str: """The git ``hermes update`` runs (``_subprocess_compat.expose_pm_git``), without installing it. @@ -741,8 +754,11 @@ def restore_interrupted_pull(project_root: Path | None = None, *, after_failure: edits are never touched; the updater's autostash (if any) stays in ``git stash list``. Concurrent launches take turns (``_restore_claim``); a launch that waited out another's restore relaunches. - Limits, by design: a torn ``hermes_cli/__init__.py``, ``hermes_bootstrap.py``, ``agent/__init__.py`` - or ``agent/jiter_preload.py`` (imported before the ``hermes-agent`` hook) fails before this runs. A file git also changes that the user deleted, emptied or cut to a prefix of git's version + A torn ``hermes_bootstrap.py``, ``hermes_cli/__init__.py`` or recovery/lock/custody module fails + before (or inside) the checkout's copy of this code: a minted launcher then runs the copy the + updater published beside the marker (``RECOVERY_CLOSURE``). Limits, by design: other entry points + (``python -m``, the ``hermes-agent`` hook's ``agent/__init__.py``) have no such fallback, and a + launcher minted before that fallback existed has none either. A file git also changes that the user deleted, emptied or cut to a prefix of git's version looks exactly like git's own half-written file and is restored too, as is a user edit to a conflicted path or, on git < 2.38, to a path both sides of a custom-branch merge changed. """ diff --git a/hermes_cli/_launchers.py b/hermes_cli/_launchers.py index eed03d79f0..1153f88169 100644 --- a/hermes_cli/_launchers.py +++ b/hermes_cli/_launchers.py @@ -317,19 +317,55 @@ def _launcher_script(name: str, repo_root: Path, dependencies: Path | None) -> s # ``hermes_bootstrap`` right after the repair (``_PIN_DEFAULT_HOME``), and here again for a # bootstrap that predates that hook. root = str(repo_root.resolve()) + from hermes_cli._early_recovery import INTERRUPTED_PULL_MARKER, RECOVERY_CLOSURE_DIR + + # A killed move can tear the repair's own code (``hermes_bootstrap``, ``hermes_cli``): with the + # marker still there, run the copy the updater published beside it before git wrote (stdlib + + # that copy only; it still takes the checkout lock), then relaunch from the restored tree. + closure_repair = ( + "def _hermes_closure_repair():\n" + " from pathlib import Path\n" + f" root = Path({root!r})\n" + " git = root / '.git'\n" + " try:\n" + " if git.is_file():\n" + " text = git.read_text(encoding='utf-8-sig').strip()\n" + " git = root / text[7:].strip() if text.startswith('gitdir:') else git\n" + f" fields = dict(line.partition('=')[::2] for line in (git / {INTERRUPTED_PULL_MARKER!r})" + ".read_text(encoding='utf-8-sig').splitlines())\n" + " except OSError:\n" + " return\n" + " pre = fields.get('pre', '').strip()\n" + f" closure = git / {RECOVERY_CLOSURE_DIR!r} / pre\n" + " if not pre or not (closure / 'hermes_cli' / '_early_recovery.py').is_file():\n" + " return\n" + " for name in [n for n in sys.modules if n.split('.')[0] in ('hermes_cli', 'hermes_bootstrap', 'hermes_constants', 'pm')]:\n" + " del sys.modules[name]\n" + " tree = os.path.normcase(os.path.realpath(root))\n" + " sys.path[:] = [str(closure)] + [p for p in sys.path if p and os.path.normcase(os.path.realpath(p)) != tree\n" + " and not os.path.normcase(os.path.realpath(p)).startswith(tree + os.sep)]\n" + " print('hermes: the checkout cannot start after an interrupted `hermes update`; '\n" + " 'repairing it with the recovery code saved before the update.', file=sys.stderr)\n" + " sys.dont_write_bytecode = True\n" + " from hermes_cli import _early_recovery\n" + " if _early_recovery.restore_interrupted_pull(root):\n" + " _early_recovery.relaunch_after_restore()\n" + ) return ( "import os, re, sys\n" "os.environ.pop('PYTHONHOME', None)\n" "os.environ.pop('PYTHONPATH', None)\n" f"sys.path.insert(0, {root!r})\n" + + closure_repair + "if sys.argv[1:2] == ['--print-runtime-command']:\n" " sys.dont_write_bytecode = True\n" " from pathlib import Path\n" " try:\n" " from hermes_cli import _early_recovery\n" - " except ImportError:\n" - " _early_recovery = None\n" - f" if _early_recovery is not None and _early_recovery.restore_interrupted_pull(Path({root!r})):\n" + " except Exception:\n" + " _hermes_closure_repair()\n" + " raise\n" + f" if _early_recovery.restore_interrupted_pull(Path({root!r})):\n" " _early_recovery.relaunch_after_restore()\n" " from hermes_constants import get_default_hermes_root\n" " os.environ['HERMES_HOME'] = os.environ.get('HERMES_HOME') or str(get_default_hermes_root())\n" @@ -337,7 +373,11 @@ def _launcher_script(name: str, repo_root: Path, dependencies: Path | None) -> s f" print_runtime_command(Path({root!r}), sys.argv[2:])\n" " sys.exit(0)\n" f"sys.{PIN_DEFAULT_HOME_FLAG} = True\n" - "import hermes_bootstrap\n" + "try:\n" + " import hermes_bootstrap\n" + "except Exception:\n" + " _hermes_closure_repair()\n" + " raise\n" "if not os.environ.get('HERMES_HOME'):\n" " from hermes_constants import get_default_hermes_root\n" " os.environ['HERMES_HOME'] = str(get_default_hermes_root())\n" diff --git a/hermes_cli/update_cmd_commit.py b/hermes_cli/update_cmd_commit.py index ead24149cf..5abbd7b5d3 100644 --- a/hermes_cli/update_cmd_commit.py +++ b/hermes_cli/update_cmd_commit.py @@ -18,10 +18,16 @@ from __future__ import annotations import os import subprocess import sys +from contextlib import suppress from pathlib import Path from typing import Optional -from hermes_cli._early_recovery import interrupted_pull_marker, restore_interrupted_pull +from hermes_cli._early_recovery import ( + RECOVERY_CLOSURE, + interrupted_pull_marker, + recovery_closure_dir, + restore_interrupted_pull, +) from hermes_cli.update_custody import run_git # What this run found before it armed anything: {path: bytes or None}. None = nothing armed yet. @@ -114,12 +120,51 @@ def arm_tree_move(git_cmd, root: Path, *, pre: str | None, target: str, stash: s redoes it first. """ marker = interrupted_pull_marker(root) + if pre: + with suppress(OSError, subprocess.SubprocessError): # no closure: the in-tree repair still runs + publish_recovery_closure(git_cmd, root, pre) marker.write_text(f"pid={os.getpid()}\npre={pre or ''}\ntarget={target}\nstash={stash or ''}\n" + (f"rollback={rollback}\n" if rollback else "") + (f"git={git}\n" if (git := _absolute_git(git_cmd)) else ""), encoding="utf-8") return marker +def publish_recovery_closure(git_cmd, root: Path, pre: str) -> Path: + """Copy the launch repair's modules, as committed at ``pre``, beside the marker (outside the tree). + + The bytes come from the commit the repair restores, never from the working tree: a rollback move + starts from a broken tree, and a second move in one run starts from code this process never ran. + Published by an atomic directory rename, so a present closure is a whole one; an older run's + closure (another ``pre``) is dropped. + """ + import shutil + + dest = recovery_closure_dir(Path(root), pre) + if all((dest / rel).is_file() for rel in RECOVERY_CLOSURE): + return dest + blobs = {} + for rel in RECOVERY_CLOSURE: + shown = run_git(git_cmd, ["cat-file", "blob", f"{pre}:{rel}"], cwd=str(root), capture_output=True, + stdin=subprocess.DEVNULL, timeout=120) + if shown.returncode != 0 or not isinstance(shown.stdout, bytes) or not shown.stdout: + raise OSError(f"{rel} is not in {pre[:10]}") + blobs[rel] = shown.stdout + staging = dest.with_name(f".{pre}.{os.getpid()}.staging") + shutil.rmtree(staging, ignore_errors=True) + try: + for rel, data in {**blobs, "hermes_cli/__init__.py": b""}.items(): + (staging / rel).parent.mkdir(parents=True, exist_ok=True) + (staging / rel).write_bytes(data) + shutil.rmtree(dest, ignore_errors=True) + os.replace(staging, dest) + finally: + shutil.rmtree(staging, ignore_errors=True) + for stale in dest.parent.iterdir(): + if stale != dest: + shutil.rmtree(stale, ignore_errors=True) + return dest + + def _absolute_git(git_cmd) -> str: """The git this run moves the tree with, as an absolute path for the next launch's repair. diff --git a/tests/hermes_cli/test_update_recovery_closure.py b/tests/hermes_cli/test_update_recovery_closure.py new file mode 100644 index 0000000000..ca3d14005f --- /dev/null +++ b/tests/hermes_cli/test_update_recovery_closure.py @@ -0,0 +1,140 @@ +"""A killed tree move that tears the launch repair's own code is still repaired at the next launch. + +The repair (``hermes_bootstrap`` -> ``_early_recovery`` under the checkout lock) lives in the tree git +rewrites. ``arm_tree_move`` publishes that closure beside the marker before git writes; the minted +launcher runs it when the checkout's copy cannot even import. Real git, really killed mid-write (a +smudge-filter barrier), the production marker writer in a child that exits, the production launcher; +the application entry is an inert receipt. +""" + +from __future__ import annotations + +import contextlib +import os +import shlex +import shutil +import signal +import subprocess +import sys +import time +from pathlib import Path + +import pytest + +from hermes_cli import _launchers + +SOURCE = Path(__file__).resolve().parents[2] +# What a minted launcher imports before the application entry. +BOOT = ("hermes_bootstrap.py", "hermes_constants.py", "hermes_cli/__init__.py", "hermes_cli/_launchers.py", + "hermes_cli/_early_recovery.py", "hermes_cli/_parser.py", "hermes_cli/runtime_state.py", + "hermes_cli/venv_sync.py", "hermes_cli/steward.py", "hermes_cli/stderr_timestamp.py", + "hermes_cli/update_lock.py", "hermes_cli/update_custody.py", "pm/environments.py", + "pm/filesystem.py", "pm/paths.py") +_HOLDER = ("import sys, time\nfrom pathlib import Path\nsys.path.insert(0, sys.argv[1])\n" + "from hermes_cli.update_lock import UpdateLock\n" + "lock = UpdateLock(path=Path(sys.argv[2]) / 'owner', install_root=Path(sys.argv[2]))\n" + "assert lock.acquire()\nprint('HELD', flush=True)\ntime.sleep(120)\n") + + +def _killed_mid_write(tmp_path: Path, rel: str): + """A checkout whose update git was SIGKILLed after unlinking ``rel``, before writing it.""" + root, home = tmp_path / "checkout", tmp_path / "home" + home.mkdir() + for f in BOOT: + if (SOURCE / f).is_file(): + (root / f).parent.mkdir(parents=True, exist_ok=True) + shutil.copy2(SOURCE / f, root / f) + (root / "hermes_cli/main.py").write_text("def main():\n print('APP_REACHED')\n return 0\n", encoding="utf-8") + env = {"HOME": str(home), "HERMES_HOME": str(home / ".hermes"), "PATH": os.environ.get("PATH", ""), + "LANG": "C.UTF-8", "GIT_CONFIG_NOSYSTEM": "1", "GIT_CONFIG_GLOBAL": os.devnull} + git_exe = shutil.which("git") or "git" + + def git(*args): + return subprocess.check_output([git_exe, "-C", str(root), *args], env=env, text=True, encoding="utf-8").strip() + + git("init", "-q", "-b", "main") + git("config", "user.name", "t") + git("config", "user.email", "t@example.invalid") + git("add", "-A") + git("commit", "-qm", "before") + pre = git("rev-parse", "HEAD") + original = (root / rel).read_bytes() + (root / rel).write_bytes(original + b"\nNEXT_VERSION = True\n") + git("commit", "-qam", "after") + target = git("rev-parse", "HEAD") + git("reset", "-q", "--hard", pre) + armed = subprocess.run( + [sys.executable, "-I", "-c", + "import sys; sys.path.insert(0, sys.argv[1]); from pathlib import Path; " + "from hermes_cli.update_cmd_commit import arm_tree_move; " + "arm_tree_move([sys.argv[2]], Path(sys.argv[3]), pre=sys.argv[4], target=sys.argv[5], stash=None)", + str(SOURCE), git_exe, str(root), pre, target], env=env, capture_output=True, text=True, encoding="utf-8", + errors="replace", timeout=60) + assert armed.returncode == 0, armed.stderr # the updater that armed the move is gone + + gate = tmp_path / "filter-ready" + (tmp_path / "hold.py").write_text(f"from pathlib import Path\nimport time\nPath({str(gate)!r}).touch()\n" + "time.sleep(120)\n", encoding="utf-8") + (root / ".git/info/attributes").write_text(f"{rel} filter=hold\n", encoding="utf-8") + git("config", "filter.hold.smudge", shlex.join([sys.executable, str(tmp_path / "hold.py")])) + git("config", "filter.hold.clean", "cat") + git("config", "filter.hold.required", "true") + merge = subprocess.Popen([git_exe, "-C", str(root), "merge", "--ff-only", target], env=env, + stdout=subprocess.DEVNULL, stderr=subprocess.PIPE, text=True, encoding="utf-8", start_new_session=True) + try: + deadline = time.monotonic() + 30 + while not gate.exists(): + assert merge.poll() is None and time.monotonic() < deadline, merge.communicate() + time.sleep(0.01) + finally: + with contextlib.suppress(ProcessLookupError): + os.killpg(merge.pid, signal.SIGKILL) # windows-footgun: ok — POSIX-only test (platforms marker) + merge.wait() + assert not (root / rel).exists() + git("config", "--unset", "filter.hold.smudge") + (root / ".git/info/attributes").unlink() + + (tmp_path / "bin").mkdir() + launcher = _launchers._mint_shell_launcher("hermes", tmp_path / "bin", Path(sys.executable), + _launchers._launcher_script("hermes", root, None)) + assert launcher is not None + return root, env, original, launcher + + +@pytest.mark.platforms("posix") +@pytest.mark.parametrize(("rel", "torn"), [ + ("hermes_bootstrap.py", False), + ("hermes_cli/_early_recovery.py", True), + ("hermes_cli/__init__.py", True), + ("hermes_cli/update_lock.py", True), +]) +def test_a_launch_repairs_a_move_killed_while_writing_the_repairs_own_code(tmp_path, rel, torn): + root, env, original, launcher = _killed_mid_write(tmp_path, rel) + if torn: # git's write cut short: a prefix of the new blob + (root / rel).write_bytes(original[:12]) + launch = subprocess.run([str(launcher)], cwd=tmp_path, env=env, capture_output=True, text=True, encoding="utf-8", + errors="replace", timeout=60) + assert launch.returncode == 0 and "APP_REACHED" in launch.stdout, launch.stderr + assert (root / rel).read_bytes() == original + assert not (root / ".git/hermes-update-pull").exists() + + +@pytest.mark.platforms("posix") +def test_the_published_repair_leaves_the_tree_to_a_live_writer_holding_the_checkout(tmp_path): + root, env, original, launcher = _killed_mid_write(tmp_path, "hermes_bootstrap.py") + holder = subprocess.Popen([sys.executable, "-I", "-c", _HOLDER, str(SOURCE), str(root)], env=env, + stdin=subprocess.DEVNULL, stdout=subprocess.PIPE, text=True, encoding="utf-8") + try: + assert holder.stdout is not None and holder.stdout.readline().strip() == "HELD" + launch = subprocess.run([str(launcher)], cwd=tmp_path, env=env, capture_output=True, text=True, encoding="utf-8", + errors="replace", timeout=60) + assert launch.returncode != 0 and "Not repairing the checkout now" in launch.stderr, launch.stderr + assert not (root / "hermes_bootstrap.py").exists(), "repaired under a live writer" + assert (root / ".git/hermes-update-pull").exists() + finally: + holder.kill() + holder.wait() + launch = subprocess.run([str(launcher)], cwd=tmp_path, env=env, capture_output=True, text=True, encoding="utf-8", + errors="replace", timeout=60) + assert launch.returncode == 0 and "APP_REACHED" in launch.stdout, launch.stderr + assert (root / "hermes_bootstrap.py").read_bytes() == original diff --git a/website/docs/developer-guide/source-update-completion.md b/website/docs/developer-guide/source-update-completion.md index 3bc61dab36..cb7c1cafd7 100644 --- a/website/docs/developer-guide/source-update-completion.md +++ b/website/docs/developer-guide/source-update-completion.md @@ -116,10 +116,21 @@ runner to be importable. If either module is torn, launch stops with `Cannot safely repair` and retains the interrupted-update marker instead of writing without exclusion. An updater that already imported healthy code can still hold and write the checkout even when its on-disk modules are damaged. -Wait for any running update to finish; persistent damage requires checkout repair -before launching again. This safety refusal is not automatic recovery of the -recovery modules themselves: the launcher still depends on checkout bootstrap -code, which a killed tree move can also leave unavailable. +Wait for any running update to finish. + +The repair code lives in the tree a killed move tears (`hermes_bootstrap.py`, +`hermes_cli/__init__.py`, `_early_recovery.py`, `update_lock.py`, +`update_custody.py`). Before git writes, every tree move publishes those +recovery/lock/custody modules, as committed at the move's starting commit, to +`/hermes-update-recovery/
/`, outside the working tree and keyed
+to the marker's `pre`. When the checkout's own import fails while the marker
+exists, the minted launcher (`.hermes/bin/hermes`) runs that published copy:
+stdlib plus the copy only, the same restore claim and checkout lock (a live
+writer still gets `Not repairing the checkout now`), then relaunches from the
+restored tree. Limits: other entry points (`python -m hermes_cli.main`, the
+`hermes-agent` hook) have no such fallback, a launcher minted before this
+fallback existed has none, and the first update performed by such an old
+launcher's code publishes no closure.
 
 ## Historical surface