c57e829d0b
Pure move, behaviour-identical. The ZIP swap journal, owner lock and journal-driven recovery (~250 lines) are unrelated to the git-marker restore, yet had grown _early_recovery.py from 684 to 1455 lines. They now live in the topical sibling hermes_cli/_early_recovery_zip.py. restore_interrupted_pull imports it only when the journal exists, so the git-marker repair and its published closure (RECOVERY_CLOSURE) still need only _early_recovery.py; ZIP_SWAP_JOURNAL stays there for that fast-path stat. write_durable_text stays too (the marker and fleet writers use it). Importers and the one monkeypatch seam (_lock_fd, read by the lock) now point at the module that reads them. (review K132361 shape gate)
381 lines
16 KiB
Python
381 lines
16 KiB
Python
"""ZIP fallback must not fire on dependency failures or clobber a dirty tree.
|
|
|
|
Issue #87304: on Windows the update ``try`` spans git pull *and* ``uv pip
|
|
install``. A locked ``hermes.exe`` makes the install exit 2, the handler
|
|
prints ``Git update failed``, and ``_update_via_zip`` replaces every
|
|
top-level entry except ``venv`` / ``node_modules`` / ``.git`` / ``.env`` —
|
|
permanently deleting uncommitted edits and untracked files. The git pull
|
|
has already succeeded by then, so the ZIP cannot fix the actual failure.
|
|
"""
|
|
|
|
from __future__ import annotations
|
|
|
|
import shutil
|
|
import subprocess
|
|
from types import SimpleNamespace
|
|
from unittest.mock import patch
|
|
|
|
import pytest
|
|
|
|
from hermes_cli import main as hermes_main
|
|
import hermes_cli.main_install_repair as main_install_repair
|
|
from hermes_cli import update_cmd
|
|
|
|
|
|
def _cpe(cmd, returncode=2, stderr="", stdout="") -> subprocess.CalledProcessError:
|
|
exc = subprocess.CalledProcessError(returncode, cmd)
|
|
exc.stderr = stderr
|
|
exc.stdout = stdout
|
|
return exc
|
|
|
|
|
|
# ---------------------------------------------------------------------------
|
|
# Stage classification + ZIP gating
|
|
# ---------------------------------------------------------------------------
|
|
|
|
|
|
def test_uv_pip_install_is_a_dependency_failure_not_git():
|
|
exc = _cpe([r"C:\venv\Scripts\uv.exe", "pip", "install", "-e", "."])
|
|
assert update_cmd._called_process_error_is_git(exc) is False
|
|
assert update_cmd._called_process_error_is_python_dep_install(exc) is True
|
|
assert update_cmd._format_update_failure_stage(exc) == (
|
|
"Python dependency install failed"
|
|
)
|
|
|
|
|
|
def test_venv_pip_install_is_a_dependency_failure():
|
|
exc = _cpe([r"C:\venv\Scripts\python.exe", "-m", "pip", "install", "-e", "."])
|
|
assert update_cmd._called_process_error_is_python_dep_install(exc) is True
|
|
assert update_cmd._called_process_error_is_git(exc) is False
|
|
|
|
|
|
def test_ensurepip_is_a_dependency_failure():
|
|
exc = _cpe([r"C:\venv\Scripts\python.exe", "-m", "ensurepip", "--upgrade"])
|
|
assert update_cmd._called_process_error_is_python_dep_install(exc) is True
|
|
assert update_cmd._format_update_failure_stage(exc) == (
|
|
"Python dependency install failed"
|
|
)
|
|
|
|
|
|
def test_git_pull_is_classified_as_git():
|
|
exc = _cpe(["git", "-c", "windows.appendAtomically=false", "pull"], returncode=1)
|
|
assert update_cmd._called_process_error_is_git(exc) is True
|
|
assert update_cmd._called_process_error_is_python_dep_install(exc) is False
|
|
assert update_cmd._format_update_failure_stage(exc) == "Git update failed"
|
|
|
|
|
|
def test_git_exe_path_is_still_git():
|
|
exc = _cpe([r"C:\Program Files\Git\cmd\git.exe", "fetch", "origin", "main"])
|
|
assert update_cmd._called_process_error_is_git(exc) is True
|
|
|
|
|
|
def test_unknown_command_gets_generic_stage():
|
|
exc = _cpe(["npm", "install"], returncode=1)
|
|
assert update_cmd._format_update_failure_stage(exc) == "Update step failed"
|
|
|
|
|
|
|
|
|
|
|
|
|
|
def test_posix_git_failure_does_not_zip(monkeypatch):
|
|
monkeypatch.setattr(hermes_main, "_is_windows", lambda: False)
|
|
monkeypatch.setattr(main_install_repair, "_is_windows", lambda: False)
|
|
exc = _cpe(["git", "pull"], returncode=1)
|
|
assert update_cmd._should_zip_fallback_on_update_error(exc) is False
|
|
|
|
|
|
|
|
|
|
# ---------------------------------------------------------------------------
|
|
# Dirty-tree overlay guard
|
|
# ---------------------------------------------------------------------------
|
|
|
|
|
|
def _porcelain_run(stdout: str, returncode: int = 0):
|
|
def fake_run(cmd, **kwargs):
|
|
joined = " ".join(str(c) for c in cmd)
|
|
if "status" in joined and "--porcelain" in joined:
|
|
return subprocess.CompletedProcess(cmd, returncode, stdout=stdout, stderr="")
|
|
if "ls-tree" in joined: # tracked root entries = what the ZIP ships
|
|
return subprocess.CompletedProcess(cmd, 0, stdout="hermes_cli\nscratch\n", stderr="")
|
|
return subprocess.CompletedProcess(cmd, 0, stdout="", stderr="")
|
|
|
|
return fake_run
|
|
|
|
|
|
def test_zip_overlay_allowed_without_git(tmp_path):
|
|
assert update_cmd._zip_overlay_block_reason(tmp_path) is None
|
|
|
|
|
|
def test_zip_overlay_blocked_on_modified_file(tmp_path, monkeypatch):
|
|
(tmp_path / ".git").mkdir()
|
|
monkeypatch.setattr(
|
|
update_cmd.subprocess, "run", _porcelain_run(" M hermes_cli/update_cmd.py\n")
|
|
)
|
|
reason = update_cmd._zip_overlay_block_reason(tmp_path)
|
|
assert reason is not None
|
|
assert "uncommitted" in reason
|
|
|
|
|
|
def test_zip_overlay_blocked_on_untracked_file(tmp_path, monkeypatch):
|
|
(tmp_path / ".git").mkdir()
|
|
monkeypatch.setattr(update_cmd.subprocess, "run", _porcelain_run("?? notes.md\n"))
|
|
reason = update_cmd._zip_overlay_block_reason(tmp_path)
|
|
assert reason is not None
|
|
assert "untracked" in reason
|
|
|
|
|
|
def test_zip_overlay_blocked_when_git_status_fails(tmp_path, monkeypatch):
|
|
(tmp_path / ".git").mkdir()
|
|
monkeypatch.setattr(
|
|
update_cmd.subprocess,
|
|
"run",
|
|
_porcelain_run("", returncode=128),
|
|
)
|
|
reason = update_cmd._zip_overlay_block_reason(tmp_path)
|
|
assert reason is not None
|
|
assert "could not check" in reason
|
|
|
|
|
|
def test_zip_overlay_allowed_on_clean_git_checkout(tmp_path, monkeypatch):
|
|
(tmp_path / ".git").mkdir()
|
|
monkeypatch.setattr(update_cmd.subprocess, "run", _porcelain_run(""))
|
|
assert update_cmd._zip_overlay_block_reason(tmp_path) is None
|
|
|
|
|
|
def test_update_via_zip_aborts_before_download_when_dirty(
|
|
tmp_path, monkeypatch, capsys
|
|
):
|
|
"""The live tree must not be touched, and the ZIP must not be fetched."""
|
|
fake_root = tmp_path / "install"
|
|
fake_root.mkdir()
|
|
(fake_root / ".git").mkdir()
|
|
local = fake_root / "keep-me.txt"
|
|
local.write_text("local work\n", encoding="utf-8")
|
|
untracked_dir = fake_root / "agent" / "scratch"
|
|
untracked_dir.mkdir(parents=True)
|
|
(untracked_dir / "wip.py").write_text("print('wip')\n", encoding="utf-8")
|
|
|
|
monkeypatch.setattr(hermes_main, "PROJECT_ROOT", fake_root)
|
|
monkeypatch.setattr(
|
|
update_cmd.subprocess,
|
|
"run",
|
|
_porcelain_run(" M keep-me.txt\n?? agent/scratch/wip.py\n"),
|
|
)
|
|
|
|
with patch("urllib.request.urlretrieve") as download:
|
|
with pytest.raises(SystemExit) as exc_info:
|
|
update_cmd._update_via_zip(SimpleNamespace(branch=None))
|
|
|
|
assert exc_info.value.code == 1
|
|
download.assert_not_called()
|
|
assert local.read_text(encoding="utf-8") == "local work\n"
|
|
assert (untracked_dir / "wip.py").read_text(encoding="utf-8") == "print('wip')\n"
|
|
|
|
|
|
# ---------------------------------------------------------------------------
|
|
# Pre-swap TOCTOU re-check
|
|
# ---------------------------------------------------------------------------
|
|
|
|
|
|
def test_status_uses_untracked_files_all(tmp_path, monkeypatch):
|
|
"""A user git config hiding untracked files must not blind the guard."""
|
|
(tmp_path / ".git").mkdir()
|
|
seen = []
|
|
|
|
def fake_run(cmd, **kwargs):
|
|
seen.append(cmd)
|
|
return subprocess.CompletedProcess(cmd, 0, stdout="", stderr="")
|
|
|
|
monkeypatch.setattr(update_cmd.subprocess, "run", fake_run)
|
|
update_cmd._zip_overlay_block_reason(tmp_path)
|
|
assert seen and "--untracked-files=all" in seen[0]
|
|
|
|
|
|
def test_staging_artifact_lines_are_recognized():
|
|
def is_artifact(line):
|
|
return update_cmd._is_zip_staging_artifact_status_line(line, ["agent", "cli.py", "tools"])
|
|
|
|
assert is_artifact("?? agent.hermes-update-staging/")
|
|
assert is_artifact("?? cli.py.hermes-update-staging")
|
|
# Staging already removed every backup: one at re-check time is not this run's (review F15).
|
|
assert not is_artifact("?? tools.hermes-update-old/")
|
|
# Nested user files under a staging-lookalike directory don't match the
|
|
# top-level test only when the TOP level itself is not an artifact.
|
|
assert not is_artifact("?? agent/scratch/wip.py")
|
|
assert not is_artifact(" M hermes_cli/update_cmd.py")
|
|
assert not is_artifact("?? notes.hermes-update-staging.txt")
|
|
|
|
|
|
def test_recheck_ignores_own_staging_artifacts(tmp_path, monkeypatch):
|
|
(tmp_path / ".git").mkdir()
|
|
monkeypatch.setattr(
|
|
update_cmd.subprocess,
|
|
"run",
|
|
_porcelain_run("?? agent.hermes-update-staging/\n?? cli.py.hermes-update-staging\n"),
|
|
)
|
|
assert (
|
|
update_cmd._zip_overlay_block_reason(tmp_path, staged=["agent", "cli.py"])
|
|
is None
|
|
)
|
|
# Without the flag the same output still refuses (pre-download check).
|
|
assert update_cmd._zip_overlay_block_reason(tmp_path) is not None
|
|
|
|
|
|
def test_recheck_still_blocks_user_files_amid_staging_artifacts(tmp_path, monkeypatch):
|
|
(tmp_path / ".git").mkdir()
|
|
monkeypatch.setattr(
|
|
update_cmd.subprocess,
|
|
"run",
|
|
_porcelain_run("?? agent.hermes-update-staging/\n?? my-notes.md\n"),
|
|
)
|
|
reason = update_cmd._zip_overlay_block_reason(
|
|
tmp_path, staged=["agent"]
|
|
)
|
|
assert reason is not None
|
|
|
|
|
|
def test_zip_overlay_blocked_on_ignored_user_file(tmp_path, monkeypatch):
|
|
"""#87392 follow-up: a gitignored file outside the preserved entries is
|
|
still user data the overlay would delete — it must block."""
|
|
(tmp_path / ".git").mkdir()
|
|
monkeypatch.setattr(
|
|
update_cmd.subprocess, "run", _porcelain_run("!! scratch/notes.local\n")
|
|
)
|
|
reason = update_cmd._zip_overlay_block_reason(tmp_path)
|
|
assert reason is not None
|
|
assert "uncommitted" in reason or "untracked" in reason
|
|
|
|
|
|
def test_zip_overlay_flag_is_valid_against_real_git(tmp_path):
|
|
"""The ignored-mode flag must be one real git accepts — run REAL git.
|
|
|
|
Review of the first draft caught ``--ignored=all`` (not a valid mode:
|
|
git exits 128 'Invalid ignored mode'), which the mocked siblings could
|
|
not see; with an invalid flag every ZIP update would be refused as
|
|
'could not check the working tree'. This test creates a real repo with
|
|
a real .gitignore and asserts the guard both runs clean AND still sees
|
|
ignored user files.
|
|
"""
|
|
subprocess.run(["git", "init", "-q", str(tmp_path)], check=True)
|
|
(tmp_path / ".gitignore").write_text("*.local\nvenv/\n.venv/\n", encoding="utf-8")
|
|
subprocess.run(
|
|
["git", "-C", str(tmp_path), "add", ".gitignore"], check=True
|
|
)
|
|
subprocess.run(
|
|
[
|
|
"git", "-C", str(tmp_path),
|
|
"-c", "user.email=t@t", "-c", "user.name=t",
|
|
"commit", "-qm", "init",
|
|
],
|
|
check=True,
|
|
)
|
|
# Clean tree: guard must pass (flag valid, no false refusal).
|
|
assert update_cmd._zip_overlay_block_reason(tmp_path) is None
|
|
# Ignored user file under a shipped (tracked) dir: the swap would delete it, guard must block.
|
|
(tmp_path / "pkg").mkdir()
|
|
(tmp_path / "pkg" / "data.local").write_text("x", encoding="utf-8")
|
|
reason = update_cmd._zip_overlay_block_reason(tmp_path, shipped={"pkg"})
|
|
assert reason is not None
|
|
# Ignored preserved entry: still no refusal.
|
|
shutil.rmtree(tmp_path / "pkg")
|
|
(tmp_path / "venv").mkdir()
|
|
(tmp_path / "venv" / "lib.py").write_text("x", encoding="utf-8")
|
|
assert update_cmd._zip_overlay_block_reason(tmp_path) is None
|
|
# uv-default ``.venv`` is a supported layout (#112958): the ignored dir is the live runtime,
|
|
# not user data the overlay would destroy — refusing here made ZIP fallback impossible.
|
|
(tmp_path / ".venv").mkdir()
|
|
(tmp_path / ".venv" / "lib.py").write_text("x", encoding="utf-8")
|
|
status = subprocess.run(
|
|
["git", "-C", str(tmp_path), "status", "--porcelain", "--untracked-files=all", "--ignored=matching"],
|
|
capture_output=True, text=True,
|
|
).stdout
|
|
assert update_cmd._zip_overlay_block_reason(tmp_path) is None, status
|
|
|
|
|
|
|
|
def _git_install(root):
|
|
subprocess.run(["git", "init", "-q", str(root)], check=True)
|
|
(root / "alpha").mkdir()
|
|
(root / "alpha" / "x.py").write_text("x = 1\n", encoding="utf-8")
|
|
(root / "cli.py").write_text("print('cli')\n", encoding="utf-8")
|
|
subprocess.run(["git", "-C", str(root), "add", "-A"], check=True)
|
|
subprocess.run(["git", "-C", str(root), "-c", "user.email=t@t", "-c", "user.name=t", "commit", "-qm", "init"],
|
|
check=True)
|
|
|
|
|
|
@pytest.mark.parametrize("name", ["alpha.hermes-update-staging", "cli.py.hermes-update-old"])
|
|
def test_zip_gate_refuses_and_keeps_a_user_file_named_like_an_update_artifact(tmp_path, monkeypatch, name):
|
|
"""Staging deletes ``<entry>.hermes-update-staging``/``-old``: a user's own file by that name (no
|
|
journal claims it) is user work the gate must refuse on, leaving it untouched."""
|
|
_git_install(tmp_path)
|
|
(tmp_path / name).write_text("PRECIOUS USER BYTES", encoding="utf-8")
|
|
monkeypatch.setattr(hermes_main, "PROJECT_ROOT", tmp_path)
|
|
with pytest.raises(SystemExit) as exc_info:
|
|
update_cmd._abort_zip_update_if_dirty_tree()
|
|
assert exc_info.value.code == 1
|
|
assert (tmp_path / name).read_text(encoding="utf-8-sig") == "PRECIOUS USER BYTES"
|
|
|
|
|
|
def test_zip_gate_refuses_on_a_user_file_at_the_old_fixed_journal_temp_name(tmp_path, monkeypatch):
|
|
"""The gate exempted ``.hermes-update-zip-swap.tmp`` as Hermes' own before any journal existed, and
|
|
the journal writer then deleted it (review Z5). Only the writer's unpredictable temp is exempt now."""
|
|
_git_install(tmp_path)
|
|
user = tmp_path / ".hermes-update-zip-swap.tmp"
|
|
user.write_text("USER NOTES", encoding="utf-8")
|
|
monkeypatch.setattr(hermes_main, "PROJECT_ROOT", tmp_path)
|
|
with pytest.raises(SystemExit) as exc_info:
|
|
update_cmd._abort_zip_update_if_dirty_tree()
|
|
assert exc_info.value.code == 1 and user.read_text(encoding="utf-8-sig") == "USER NOTES"
|
|
assert update_cmd._is_zip_staging_artifact_status_line("?? .hermes-update-zip-swap.0123456789ab.tmp")
|
|
|
|
|
|
def test_zip_gate_admits_the_retry_after_an_interrupted_swap(tmp_path, monkeypatch):
|
|
"""The siblings an interrupted swap's journal owns never refuse the retry: they are settled first."""
|
|
from hermes_cli._early_recovery_zip import ZIP_SWAP_JOURNAL, write_zip_swap_journal, zip_entry_identity
|
|
|
|
_git_install(tmp_path)
|
|
leftover = tmp_path / "alpha.hermes-update-staging"
|
|
leftover.mkdir()
|
|
(leftover / "x.py").write_text("x = 2\n", encoding="utf-8")
|
|
write_zip_swap_journal(tmp_path, "staging", [["alpha", True, zip_entry_identity(leftover), ""],
|
|
["cli.py", True, "", ""]], "0123456789ab")
|
|
monkeypatch.setattr(hermes_main, "PROJECT_ROOT", tmp_path)
|
|
update_cmd._abort_zip_update_if_dirty_tree()
|
|
assert not leftover.exists() and not (tmp_path / ZIP_SWAP_JOURNAL).exists()
|
|
assert (tmp_path / "alpha" / "x.py").read_text(encoding="utf-8-sig") == "x = 1\n"
|
|
|
|
|
|
def test_preserved_filter_does_not_split_non_rename_lines():
|
|
"""A plain ignored FILE literally named 'venv -> node_modules' is ONE
|
|
path (porcelain v1 doesn't quote spaces) — splitting it would filter it
|
|
as two preserved tops and fail-open into the destructive swap."""
|
|
assert not update_cmd._is_zip_preserved_entry_status_line(
|
|
"!! venv -> node_modules"
|
|
)
|
|
assert not update_cmd._is_zip_preserved_entry_status_line(
|
|
"?? venv -> node_modules"
|
|
)
|
|
# Real rename crossing out of a preserved dir still blocks…
|
|
assert not update_cmd._is_zip_preserved_entry_status_line(
|
|
"R venv/x -> src/x"
|
|
)
|
|
# …and a rename fully inside preserved entries is still filtered.
|
|
assert update_cmd._is_zip_preserved_entry_status_line(
|
|
"R venv/a -> node_modules/b"
|
|
)
|
|
|
|
|
|
def test_zip_overlay_allows_ignored_preserved_entries(tmp_path, monkeypatch):
|
|
"""venv/node_modules are gitignored on every normal install and the swap
|
|
preserves them — the ignored probe must not turn them into a false
|
|
refusal."""
|
|
(tmp_path / ".git").mkdir()
|
|
monkeypatch.setattr(
|
|
update_cmd.subprocess,
|
|
"run",
|
|
_porcelain_run("!! venv/\n!! node_modules/\n!! .env\n"),
|
|
)
|
|
assert update_cmd._zip_overlay_block_reason(tmp_path) is None
|