fix(update): a launch repairs a move that tore the repair's own code (D1)
The launch-time repair (hermes_bootstrap -> _early_recovery under the checkout lock) lives in the tree a killed git rewrites. A merge SIGKILLed while writing hermes_bootstrap.py, _early_recovery.py, a prefix of hermes_cli/__init__.py or a custody module left every launch exiting 1 with the marker kept and the tree never restored. arm_tree_move now publishes the recovery/lock/custody modules, as committed at the move's pre commit (git cat-file, never the working tree), to <git dir>/hermes-update-recovery/<pre>/ by an atomic directory rename before it writes the marker. The minted launcher catches a failing checkout import; with the marker present and its pre's closure published it purges checkout modules, runs restore_interrupted_pull from that copy (same claim and checkout lock: a live writer still refuses) and relaunches from the restored tree. (cherry picked from commit 2bc12d9283d90a54f970a354a52f0a95d41bbfb0) (cherry picked from commit 924b2a9a2d4dde6bc2a9074ff3ee0cd8f87c8684)
This commit is contained in:
@@ -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.
|
||||
"""
|
||||
|
||||
@@ -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"
|
||||
|
||||
@@ -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.
|
||||
|
||||
|
||||
140
tests/hermes_cli/test_update_recovery_closure.py
Normal file
140
tests/hermes_cli/test_update_recovery_closure.py
Normal file
@@ -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
|
||||
@@ -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
|
||||
`<git dir>/hermes-update-recovery/<pre>/`, 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
|
||||
|
||||
|
||||
Reference in New Issue
Block a user