fix(update): a killed custom-branch merge is restored too, from every console script
On a custom branch the updater runs `git merge --no-edit origin/<branch>` inside the marker window. Its files are the merge of both sides, a blob that is neither pre nor target, so the restore took them for user edits: it put the upstream-only files back to pre, kept the merged ones and spent the marker, leaving a mixed tree (a real random-kill of that merge: 104 of 500 trials broken). - _early_recovery: when pre and target diverge, `git merge-tree --write-tree pre target` gives the tree the merge was writing; its blobs (and prefixes of them, for a file cut short) count as git's like the target's. Conflicted paths, and on git < 2.38 every path both sides changed, count as git's whatever their content. Paths with a newline are hashed one by one (`--stdin-paths` is newline-delimited). The docstring lists the by-design limits. - run_agent (`hermes-agent`) and acp_adapter.entry (`hermes-acp`) never import hermes_cli.main, so they now run the same restore right after hermes_bootstrap (run_agent only when hermes_cli.main is not loaded, since it is also a library module). - Tests: the second test kills inside a clean custom-branch merge (merged file, upstream-only file, a cut-short file, a user edit); the first pins that each console script's entry module imports no other checkout module before the restore runs. Both red on the previous head. - evals/update_pipeline/interrupted_pull_ab.sh gains scenario E: a kill inside the custom-branch merge, then the `hermes-agent` import.
This commit is contained in:
teknium1
@@ -20,6 +20,13 @@ else:
|
||||
# Stop a ``utils/``/``proxy/``/``ui/`` package in the launch cwd from shadowing Hermes modules.
|
||||
hermes_bootstrap.harden_import_path()
|
||||
|
||||
# `hermes-acp` runs without hermes_cli.main: repair a `hermes update` killed mid-pull here, before
|
||||
# importing anything else from the checkout (a no-op under `hermes acp`, which already did).
|
||||
from hermes_cli import _early_recovery
|
||||
|
||||
if _early_recovery.restore_interrupted_pull():
|
||||
_early_recovery.relaunch_after_restore()
|
||||
|
||||
import argparse
|
||||
import asyncio
|
||||
import logging
|
||||
|
||||
@@ -4,14 +4,15 @@
|
||||
# evals/update_pipeline/interrupted_pull_ab.sh <repo> <installed-sha> <label> [python]
|
||||
#
|
||||
# Builds a disposable origin + install at <installed-sha>; origin/main gets one commit that changes
|
||||
# utils.py, makes hermes_cli/config.py import a new utils name, changes run_agent.py and adds a
|
||||
# package. Runs the REAL autostash + _pull_updates from the install's own tree and the REAL entry point
|
||||
# utils.py, makes hermes_cli/config.py and run_agent.py import a new utils name, and adds a package. Runs the REAL autostash + _pull_updates from the install's own tree and the REAL entry point
|
||||
# (`python -m hermes_cli.main config path`) under a disposable HOME/HERMES_HOME:
|
||||
# A updater SIGKILLed before git wrote anything; user edits upstream-changed files, fetches, runs hermes
|
||||
# B custom branch whose commit conflicts upstream: update exits 1; user merges by hand, runs hermes
|
||||
# C git wrote config.py + the new package, SIGKILL; user edits an upstream-changed file git never wrote
|
||||
# D C in a linked-worktree install (`.git` is a file)
|
||||
# Fixed: A/B untouched, C/D restored with the user edit kept and the re-update lands; one VERDICT line.
|
||||
# E custom branch whose commit merges cleanly: the kill lands inside the `git merge`, after it wrote
|
||||
# the merged run_agent.py; the user edits a file git never wrote, then runs `hermes-agent`'s import
|
||||
# Fixed: A/B untouched, C/D/E restored with the user edit kept and the re-update lands; one VERDICT line.
|
||||
# [python] defaults to <repo>/venv/bin/python (needs the Hermes deps).
|
||||
set -u
|
||||
REPO=$1; REF=$2; LABEL=$3
|
||||
@@ -48,7 +49,7 @@ setup() { # $1 = install kind: clone | worktree
|
||||
printf '\n\ndef _torn_probe():\n return 1\n' >> utils.py
|
||||
sed -i 's/^from utils import atomic_replace, fast_safe_load, file_signature$/from utils import atomic_replace, fast_safe_load, file_signature, _torn_probe # noqa: F401/' hermes_cli/config.py
|
||||
grep -q _torn_probe hermes_cli/config.py || { echo "SETUP: config.py import line not found"; exit 1; }
|
||||
echo "# upstream change" >> run_agent.py
|
||||
echo "from utils import _torn_probe # noqa: E402,F401" >> run_agent.py
|
||||
mkdir -p torn_newpkg && echo "from utils import _torn_probe" > torn_newpkg/__init__.py
|
||||
git add -A && git commit -qm "upstream B" )
|
||||
if [ "$1" = worktree ]; then
|
||||
@@ -141,7 +142,36 @@ EOF
|
||||
echo " after re-update: HEAD==origin/main? $([ "$(git rev-parse HEAD)" = "$(git rev-parse origin/main)" ] && echo yes || echo no) user edit kept=$(grep -c USER_EDIT run_agent.py)"
|
||||
[ "$(git rev-parse HEAD)" = "$(git rev-parse origin/main)" ] && [ "$(grep -c USER_EDIT run_agent.py)" = 1 ] && CD_OK=$((CD_OK + 1))
|
||||
done
|
||||
# ---- E: the custom-branch `git merge` wrote its merged run_agent.py, SIGKILL; `hermes-agent` launches ----
|
||||
echo; echo "--- E: kill inside the custom-branch merge, then the hermes-agent entry point (run_agent)"
|
||||
setup clone >/dev/null
|
||||
git checkout -q -b mywork && sed -i "1a LOCAL_WORK = 1" run_agent.py && git commit -qam "local work that merges cleanly"
|
||||
cat > "$U/fakebin/git" <<'EOF'
|
||||
#!/bin/bash
|
||||
# The merge writes run_agent.py as the merge of both sides (upstream's half imports a utils name git has
|
||||
# not written yet), holds index.lock, and is SIGKILLed.
|
||||
for a in "$@"; do
|
||||
if [ "$a" = "--no-edit" ]; then
|
||||
/usr/bin/git show "$(/usr/bin/git merge-tree --write-tree HEAD origin/main):run_agent.py" > run_agent.py
|
||||
touch "$(/usr/bin/git rev-parse --git-dir)/index.lock"
|
||||
sleep 30
|
||||
fi
|
||||
done
|
||||
exec /usr/bin/git "$@"
|
||||
EOF
|
||||
chmod +x "$U/fakebin/git"
|
||||
PID=$(pull_bg); for i in $(seq 60); do grep -q _torn_probe run_agent.py && break; sleep 0.5; done; sleep 1
|
||||
kill -KILL -- -$PID; wait $PID 2>/dev/null
|
||||
echo " torn: HEAD=$(git rev-parse --short HEAD) [$(git status --porcelain | tr '\n' ' ')] marker=[$(marker | cut -c1-60)...]"
|
||||
sed -i '2a MY_UNCOMMITTED_WORK = 42' utils.py # upstream changes this file too; git never wrote it
|
||||
PYTHONPATH=$U/install timeout 120 "$PY" -c "import run_agent; print('run_agent imported')" 2>&1 | grep -v '^$' | tail -4 | sed 's/^/ hermes-agent> /'
|
||||
echo " RESULT E: [$(git status --porcelain | tr '\n' ' ')] user edit kept=$(grep -c MY_UNCOMMITTED_WORK utils.py) marker=[$(marker)]"
|
||||
PATH=/usr/bin:$PATH $PY $U/pull.py $U/install 2>&1 | grep PULL | sed 's/^/ re-update> /'
|
||||
E_MERGED=$(git merge-base --is-ancestor origin/main HEAD && grep -q LOCAL_WORK run_agent.py && echo yes || echo no)
|
||||
echo " after re-update: origin/main merged with the local commit? $E_MERGED user edit kept=$(grep -c MY_UNCOMMITTED_WORK utils.py)"
|
||||
E_OK=$([ "$E_MERGED" = yes ] && [ "$(grep -c MY_UNCOMMITTED_WORK utils.py)" = 1 ] && echo 1 || echo 0)
|
||||
|
||||
echo
|
||||
if [ "$A_OK$B_OK$CD_OK" = 112 ]; then echo "VERDICT: FIXED ($LABEL) — user work untouched in A/B, torn tree restored with user edits kept in C/D"
|
||||
else echo "VERDICT: FIRES ($LABEL) — A_ok=$A_OK B_ok=$B_OK CD_ok=$CD_OK/2"; fi
|
||||
if [ "$A_OK$B_OK$CD_OK$E_OK" = 1121 ]; then echo "VERDICT: FIXED ($LABEL) — user work untouched in A/B, torn tree restored with user edits kept in C/D/E"
|
||||
else echo "VERDICT: FIRES ($LABEL) — A_ok=$A_OK B_ok=$B_OK CD_ok=$CD_OK/2 E_ok=$E_OK"; fi
|
||||
rm -rf "$U"
|
||||
|
||||
@@ -237,47 +237,81 @@ def interrupted_pull_marker(root: Path) -> Path:
|
||||
return _git_dir(root) / INTERRUPTED_PULL_MARKER
|
||||
|
||||
|
||||
def _trees_git_could_write(git, pre: str, target: str) -> tuple[list[str], set[str]]:
|
||||
"""The trees the killed git was moving the checkout to, and the paths whose new content is unknowable.
|
||||
|
||||
A fast-forward or ``reset --hard`` writes ``target``. On a custom branch the updater runs
|
||||
``git merge``, whose files are the merge of both sides: ``merge-tree`` computes the same tree. A
|
||||
conflicted path (its markers carry other labels) and, on git < 2.38 (no ``--write-tree``), every
|
||||
path both sides changed count as git's whatever their content.
|
||||
"""
|
||||
base = git("merge-base", pre, target).stdout.strip()
|
||||
if not base or base == pre: # fast-forward, or unrelated histories (only a reset can land those)
|
||||
return [target], set()
|
||||
merged = git("merge-tree", "--write-tree", "-z", "--name-only", "--no-messages", pre, target)
|
||||
if merged.returncode in (0, 1): # 1: conflicts
|
||||
tree, *conflicted = merged.stdout.split("\0")
|
||||
return [target, tree], set(filter(None, conflicted))
|
||||
changed = [set(filter(None, git("diff", "--name-only", "-z", "--no-renames", base, side).stdout.split("\0")))
|
||||
for side in (pre, target)]
|
||||
return [target], changed[0] & changed[1]
|
||||
|
||||
|
||||
def _hash_worktree(git, paths: list[str]) -> dict[str, str]:
|
||||
"""Blob ids of the checkout's files, through the repo's clean filters, like ``git add`` would store."""
|
||||
listed = [p for p in paths if "\n" not in p] # --stdin-paths is newline-delimited
|
||||
blobs = {}
|
||||
if listed:
|
||||
hashed = git("hash-object", "--stdin-paths", stdin="\n".join(listed) + "\n")
|
||||
if hashed.returncode != 0:
|
||||
raise subprocess.SubprocessError(hashed.stderr.strip())
|
||||
blobs.update(zip(listed, hashed.stdout.split()))
|
||||
for path in set(paths) - set(listed):
|
||||
blobs[path] = git("hash-object", "--", path).stdout.strip()
|
||||
return blobs
|
||||
|
||||
|
||||
def _paths_git_wrote(git, root: Path, pre: str, target: str) -> tuple[list[str], list[str]] | None:
|
||||
"""Paths the killed git already touched on the way to ``target``: (restore from HEAD, delete as added).
|
||||
|
||||
Git rewrites a file as unlink, create, write, so a kill leaves it missing, empty or cut short:
|
||||
all of those count as git's, like the full ``target`` blob. Content that matches neither side and
|
||||
is not the start of the target is the user's own edit (e.g. a re-applied stash) and is left
|
||||
alone. ``None``: git no longer knows ``target``.
|
||||
all of those count as git's, like the full new blob. Content that matches neither side and is not
|
||||
the start of a new blob is the user's own edit (e.g. a re-applied stash) and is left alone.
|
||||
``None``: git no longer knows ``target``.
|
||||
"""
|
||||
diff = git("diff", "--raw", "-z", "--no-renames", "--no-abbrev", pre, target)
|
||||
if diff.returncode != 0:
|
||||
if git("rev-parse", "-q", "--verify", f"{target}^{{commit}}").returncode != 0:
|
||||
return None
|
||||
parts = diff.stdout.split("\0")
|
||||
entries = [] # (status, path, pre mode, pre blob, target blob)
|
||||
for meta, path in zip(parts[::2], parts[1::2]):
|
||||
old_mode, new_mode, old_blob, new_blob, status = meta.lstrip(":").split()
|
||||
if status == "D" and old_mode in _REGULAR_FILE_MODES:
|
||||
entries.append((status, path, old_mode, old_blob, None))
|
||||
elif status != "D" and new_mode in _REGULAR_FILE_MODES:
|
||||
entries.append((status, path, old_mode, old_blob, new_blob))
|
||||
present = [path for _s, path, _m, _o, blob in entries if blob and (root / path).is_file()]
|
||||
hashed = git("hash-object", "--stdin-paths", stdin="\n".join(present) + "\n") if present else None
|
||||
if hashed is not None and hashed.returncode != 0:
|
||||
raise subprocess.SubprocessError(hashed.stderr.strip())
|
||||
worktree_blob = dict(zip(present, hashed.stdout.split() if hashed else ()))
|
||||
trees, unknown = _trees_git_could_write(git, pre, target)
|
||||
entries = {} # path -> (pre mode, pre blob or None when git adds it, [(new mode, new blob or None)])
|
||||
for tree in trees:
|
||||
diff = git("diff", "--raw", "-z", "--no-renames", "--no-abbrev", pre, tree)
|
||||
if diff.returncode != 0:
|
||||
raise subprocess.SubprocessError(diff.stderr.strip())
|
||||
parts = diff.stdout.split("\0")
|
||||
for meta, path in zip(parts[::2], parts[1::2]):
|
||||
old_mode, new_mode, old_blob, new_blob, status = meta.lstrip(":").split()
|
||||
if (old_mode if status == "D" else new_mode) not in _REGULAR_FILE_MODES:
|
||||
continue
|
||||
entry = entries.setdefault(path, (old_mode, None if status == "A" else old_blob, []))
|
||||
entry[2].append((new_mode, None if status == "D" else new_blob))
|
||||
worktree_blob = _hash_worktree(git, [path for path in entries if (root / path).is_file()])
|
||||
restore, added = [], []
|
||||
for status, path, old_mode, old_blob, blob in entries:
|
||||
file = root / path
|
||||
if blob is None:
|
||||
written = not file.exists()
|
||||
elif path not in worktree_blob:
|
||||
written = status != "A" # unlinked, not yet recreated
|
||||
elif worktree_blob[path] not in (old_blob, blob): # git's own file, cut short, starts the target
|
||||
smudged = subprocess.run(["git", "-C", str(root), "cat-file", "--filters", f"--path={path}", blob],
|
||||
capture_output=True, check=True, timeout=120, stdin=subprocess.DEVNULL)
|
||||
written = smudged.stdout.startswith(file.read_bytes())
|
||||
elif old_blob == blob: # mode-only: only the exec bit tells whether git got here
|
||||
written = sys.platform != "win32" and bool(file.stat().st_mode & 0o100) != (old_mode == "100755")
|
||||
else:
|
||||
written = worktree_blob[path] == blob
|
||||
for path, (old_mode, old_blob, new) in entries.items():
|
||||
file, blobs = root / path, {blob for _mode, blob in new if blob}
|
||||
if path not in worktree_blob:
|
||||
written = old_blob is not None # unlinked (or deleted), not yet recreated
|
||||
elif worktree_blob[path] == old_blob: # only a mode change tells whether git got here
|
||||
written = (sys.platform != "win32" and any(b == old_blob and m != old_mode for m, b in new)
|
||||
and bool(file.stat().st_mode & 0o100) != (old_mode == "100755"))
|
||||
elif worktree_blob[path] in blobs or path in unknown:
|
||||
written = True
|
||||
else: # git's own file cut short starts one of the new blobs
|
||||
content = file.read_bytes()
|
||||
written = any(subprocess.run(["git", "-C", str(root), "cat-file", "--filters", f"--path={path}", blob],
|
||||
capture_output=True, check=True, timeout=120,
|
||||
stdin=subprocess.DEVNULL).stdout.startswith(content) for blob in blobs)
|
||||
if written:
|
||||
(added if status == "A" else restore).append(path)
|
||||
(added if old_blob is None else restore).append(path)
|
||||
return restore, added
|
||||
|
||||
|
||||
@@ -292,6 +326,12 @@ def restore_interrupted_pull(project_root: Path | None = None) -> bool:
|
||||
(the target's content, or torn on the way there) returns to HEAD (the commit the venv was built
|
||||
for), so the install is whole again and ``hermes update`` redoes the update from the start. Local
|
||||
edits are never touched; the updater's autostash (if any) stays in ``git stash list``.
|
||||
|
||||
Limits, by design: a torn ``hermes_cli/__init__.py`` or ``hermes_bootstrap.py`` fails before this
|
||||
runs (``git -C <root> reset --hard <pre>`` from the marker repairs it). 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.
|
||||
"""
|
||||
try:
|
||||
root = _project_root() if project_root is None else project_root
|
||||
|
||||
11
run_agent.py
11
run_agent.py
@@ -12,12 +12,21 @@ try:
|
||||
except ModuleNotFoundError:
|
||||
pass # partial `hermes update` — only skips the Windows UTF-8 stdio setup
|
||||
|
||||
import sys
|
||||
|
||||
# `hermes-agent` runs this module without hermes_cli.main, which repairs a `hermes update` killed
|
||||
# while git wrote the new tree; do it here, before importing anything else from the checkout.
|
||||
if "hermes_cli.main" not in sys.modules:
|
||||
from hermes_cli import _early_recovery
|
||||
|
||||
if _early_recovery.restore_interrupted_pull():
|
||||
_early_recovery.relaunch_after_restore()
|
||||
|
||||
import json
|
||||
import logging
|
||||
logger = logging.getLogger(__name__)
|
||||
import os
|
||||
import re
|
||||
import sys
|
||||
import time
|
||||
import threading
|
||||
import uuid
|
||||
|
||||
@@ -24,6 +24,29 @@ def _git(root: Path, *args: str) -> str:
|
||||
text=True, encoding="utf-8").stdout.strip()
|
||||
|
||||
|
||||
_MULTI = "top = 1\nx = 0\ny = 0\nz = 0\nend = 1\n"
|
||||
|
||||
|
||||
# Runs an entry module with the repair replaced by a probe that lists the checkout modules imported so
|
||||
# far (the entry module, its packages and what hermes_bootstrap needs excluded), then stops.
|
||||
_ENTRY_SPY = """
|
||||
import importlib, json, os, sys
|
||||
import hermes_bootstrap
|
||||
from hermes_cli import _early_recovery as er
|
||||
|
||||
before, venv, entry = set(sys.modules), os.path.realpath(sys.prefix), sys.argv[1]
|
||||
|
||||
def probe():
|
||||
loaded = (n for n in set(sys.modules) - before if not f"{entry}.".startswith(n + "."))
|
||||
files = {n: os.path.realpath(str(getattr(sys.modules[n], "__file__", None))) for n in loaded}
|
||||
print(json.dumps(sorted(n for n, f in files.items() if f.startswith(os.getcwd()) and not f.startswith(venv))))
|
||||
raise SystemExit(0)
|
||||
|
||||
er.restore_interrupted_pull = probe
|
||||
importlib.import_module(entry)
|
||||
"""
|
||||
|
||||
|
||||
@pytest.fixture
|
||||
def checkout(tmp_path, monkeypatch):
|
||||
"""An install at commit A whose fetched ``origin/main`` is B (modifies, deletes, adds, flips a mode)."""
|
||||
@@ -33,12 +56,12 @@ def checkout(tmp_path, monkeypatch):
|
||||
_git(origin, "config", "user.email", "t@example.invalid")
|
||||
_git(origin, "config", "user.name", "t")
|
||||
files = {"utils.py": "OLD = 1\n", "other.py": "a = 1\n", "gone.py": "x = 1\n", "cut.py": "c = 1\n",
|
||||
"blank.py": "b = 1\n", "half.py": "h = 1\n", "tool.sh": "echo\n"}
|
||||
"blank.py": "b = 1\n", "half.py": "h = 1\n", "tool.sh": "echo\n", "multi.py": _MULTI}
|
||||
for name, body in files.items():
|
||||
(origin / name).write_text(body, encoding="utf-8", newline="")
|
||||
_git(origin, "add", "-A")
|
||||
_git(origin, "commit", "-qm", "A")
|
||||
for name, body in {"utils.py": "NEW = 1\n", "other.py": "a = 2\n", "cut.py": "c = 2\n",
|
||||
for name, body in {"utils.py": "NEW = 1\n", "other.py": "a = 2\n", "cut.py": "c = 2\n", "multi.py": "top = 2\n" + _MULTI[8:],
|
||||
"blank.py": "b = 2\n", "half.py": "h = 2 # long enough to span pages\n"}.items():
|
||||
(origin / name).write_text(body, encoding="utf-8", newline="")
|
||||
(origin / "gone.py").unlink()
|
||||
@@ -111,6 +134,14 @@ def test_killed_pull_is_restored_on_next_launch_and_update_reruns(checkout, monk
|
||||
_pull(root) # `hermes update` again: a normal fast-forward
|
||||
assert _git(root, "rev-parse", "HEAD") == b and not marker.exists()
|
||||
|
||||
# Every console script (`hermes`, `hermes-agent`, `hermes-acp`) repairs before its entry module imports
|
||||
# any other checkout module past hermes_bootstrap: any of them may be a half-written file.
|
||||
repo = os.path.realpath(Path(er.__file__).parent.parent)
|
||||
for entry in ("hermes_cli.main", "run_agent", "acp_adapter.entry"):
|
||||
run = subprocess.run([sys.executable, "-c", _ENTRY_SPY, entry], cwd=repo, capture_output=True, text=True,
|
||||
encoding="utf-8", env={**os.environ, "PYTHONPATH": repo}, timeout=120)
|
||||
assert run.stdout.strip().splitlines()[-1:] == ["[]"], (entry, run.stdout[-500:], run.stderr[-2000:])
|
||||
|
||||
|
||||
def test_restore_never_touches_user_work_when_git_wrote_nothing(checkout):
|
||||
"""sys.exit on a merge conflict is not a kill, and a marker git never acted on restores nothing."""
|
||||
@@ -145,3 +176,21 @@ def test_restore_never_touches_user_work_when_git_wrote_nothing(checkout):
|
||||
# A target git no longer knows (gc, re-clone) can never be compared against: drop the marker.
|
||||
marker.write_text(stale.replace(b, "0" * 40), encoding="utf-8", newline="")
|
||||
assert er.restore_interrupted_pull(root) is False and not marker.exists()
|
||||
|
||||
# Killed inside the custom-branch `git merge`: its files are the merge of both sides, not origin's
|
||||
# blob, and still git's (torn ones too), while the user's own edit survives.
|
||||
_git(root, "reset", "-q", "--hard", a)
|
||||
(root / "multi.py").write_text(_MULTI.replace("end = 1", "end = 'mine'"), encoding="utf-8", newline="")
|
||||
_git(root, "commit", "-qam", "local work that merges cleanly")
|
||||
pre = _git(root, "rev-parse", "HEAD")
|
||||
merged = _git(root, "merge-tree", "--write-tree", pre, b)
|
||||
merged_multi = _git(root, "show", f"{merged}:multi.py") + "\n"
|
||||
assert merged_multi == "top = 2\nx = 0\ny = 0\nz = 0\nend = 'mine'\n" # neither side's blob
|
||||
(root / "multi.py").write_text(merged_multi, encoding="utf-8", newline="")
|
||||
(root / "utils.py").write_text("NEW = 1\n", encoding="utf-8", newline="")
|
||||
(root / "half.py").write_bytes(b"h = 2 # long")
|
||||
(root / "other.py").write_text("a = 1 # my edit\n", encoding="utf-8", newline="")
|
||||
marker.write_text(f"pid=0\npre={pre}\ntarget={b}\nstash=\n", encoding="utf-8", newline="")
|
||||
assert er.restore_interrupted_pull(root) is True
|
||||
assert _git(root, "rev-parse", "HEAD") == pre and not marker.exists()
|
||||
assert _git(root, "status", "--porcelain", "--untracked-files=all") == "M other.py"
|
||||
|
||||
Reference in New Issue
Block a user