fix(update): don't ZIP-fallback on dependency failures or dirty trees

Surgical reapply of PR #87878 (@kshitijk4poor's salvage of #87327 by
@liruixinch) onto current main — the receipt-boundary and summary
changes from this session made the original commits conflict.

- ZIP fallback now keys on git ACTUALLY having failed
  (_should_zip_fallback_on_update_error): a dependency-install failure
  after a successful pull can't be fixed by re-downloading source and
  would clobber the tree (#87331 cascade trigger, #87304).
- _abort_zip_update_if_dirty_tree: refuse to overlay a dirty checkout
  (-uall so user gitconfig can't blind the guard) + pre-swap TOCTOU
  re-check with our own staging artifacts filtered (#91962, #87304).
- Failure-stage naming (_format_update_failure_stage) + stderr tail so
  'Git update failed' stops mislabeling pip/uv failures.
- Receipt finalize preserved on the no-fallback failure path.

Co-authored-by: kshitijk4poor <82637225+kshitijk4poor@users.noreply.github.com>
Co-authored-by: liruixinch <liruixinch@outlook.com>
This commit is contained in:
kshitijk4poor
2026-08-21 19:36:18 -07:00
committed by Teknium
parent fc7523ca31
commit eac3f645ef
3 changed files with 450 additions and 3 deletions

View File

@@ -1158,6 +1158,166 @@ def _print_update_completion(message: str) -> None:
print(f"=== hermes-update completed {action_id} ===")
def _called_process_error_cmd_parts(exc: subprocess.CalledProcessError) -> list[str]:
"""Normalize ``CalledProcessError.cmd`` into argv-style tokens."""
cmd = exc.cmd
if cmd is None:
return []
if isinstance(cmd, (str, bytes)):
text = cmd.decode("utf-8", "replace") if isinstance(cmd, bytes) else cmd
try:
return shlex.split(text, posix=os.name != "nt")
except ValueError:
return text.split()
return [str(part) for part in cmd]
def _called_process_error_is_git(exc: subprocess.CalledProcessError) -> bool:
"""True when the failed subprocess was git itself."""
parts = _called_process_error_cmd_parts(exc)
if not parts:
return False
# Windows argv may use backslashes; basename() on POSIX would otherwise
# keep the whole path. Normalize separators before taking the name.
name = os.path.basename(parts[0].replace("\\", "/")).lower()
return name in {"git", "git.exe"}
def _called_process_error_is_python_dep_install(
exc: subprocess.CalledProcessError,
) -> bool:
"""True when the failed subprocess was a uv/pip (or ensurepip) install."""
parts = [part.lower() for part in _called_process_error_cmd_parts(exc)]
if not parts:
return False
exe = os.path.basename(parts[0].replace("\\", "/"))
if "ensurepip" in parts:
return True
if "install" in parts and (
"pip" in parts or exe in {"pip", "pip.exe", "pip3", "pip3.exe", "uv", "uv.exe"}
):
return True
return False
def _format_update_failure_stage(exc: subprocess.CalledProcessError) -> str:
"""Name the update stage that actually failed.
The git pull and the Python-dependency install share one ``try`` in
``_cmd_update_impl``. Calling every ``CalledProcessError`` a git failure
(the historical Windows message) sent users hunting in the wrong place
and, worse, keyed the ZIP overlay on exception *type* rather than on git
actually having failed (#87304, #85840).
"""
if _called_process_error_is_python_dep_install(exc):
return "Python dependency install failed"
if _called_process_error_is_git(exc):
return "Git update failed"
return "Update step failed"
def _should_zip_fallback_on_update_error(exc: BaseException) -> bool:
"""ZIP fallback is for Windows git file-I/O breakage, not later stages.
A dependency-install failure (locked ``hermes.exe`` / ``uv pip install``
exit 2) is not a git failure. The pull has already succeeded by then, so
re-downloading the source ZIP cannot fix the install and would replace
every top-level entry except ``venv`` / ``node_modules`` / ``.git`` /
``.env`` — permanently deleting uncommitted edits and untracked files.
"""
return (
isinstance(exc, subprocess.CalledProcessError)
and _m()._is_windows()
and _called_process_error_is_git(exc)
)
def _print_called_process_error_tail(
exc: subprocess.CalledProcessError, *, limit: int = 12
) -> None:
"""Print a captured stderr/stdout tail when the failing call recorded one."""
blob = exc.stderr or exc.stdout or ""
if isinstance(blob, bytes):
blob = blob.decode("utf-8", "replace")
lines = [line for line in str(blob).splitlines() if line.strip()]
if not lines:
return
print(" Last output:")
for line in lines[-limit:]:
print(f" {line}")
def _zip_overlay_block_reason(
root: Path, *, ignore_staging_artifacts: bool = False
) -> Optional[str]:
"""Why overlaying a ZIP onto ``root`` would destroy work, or None if safe.
The ZIP path swaps every top-level entry (except a tiny preserve set) and
then deletes the backups, so uncommitted edits and untracked files under
a replaced directory are gone. Fail closed when git status cannot run:
unknown dirtiness is not a license to clobber the tree (#87304).
``ignore_staging_artifacts`` is for the pre-swap re-check: phase 1 of the
two-phase replace creates ``*.hermes-update-staging`` siblings inside the
checkout, which git reports as untracked. Those are our own artifacts,
not user work — without the filter the re-check would always refuse.
"""
if not (root / ".git").exists():
return None
git_cmd = ["git"]
if sys.platform == "win32":
git_cmd = ["git", "-c", "windows.appendAtomically=false"]
result = subprocess.run(
# -uall: a user-level ``status.showUntrackedFiles = no`` git config
# would otherwise hide untracked files and silently blind this guard.
git_cmd + ["status", "--porcelain", "--untracked-files=all"],
cwd=root,
capture_output=True,
text=True,
encoding="utf-8",
errors="replace",
)
if result.returncode != 0:
detail = (result.stderr or result.stdout or "").strip().splitlines()
suffix = f" ({detail[0]})" if detail else ""
return f"could not check the working tree{suffix}"
lines = [line for line in (result.stdout or "").splitlines() if line.strip()]
if ignore_staging_artifacts:
lines = [
line for line in lines if not _is_zip_staging_artifact_status_line(line)
]
if lines:
return "the working tree has uncommitted changes or untracked files"
return None
_ZIP_STAGING_ARTIFACT_SUFFIXES = (".hermes-update-staging", ".hermes-update-old")
def _is_zip_staging_artifact_status_line(line: str) -> bool:
"""True when a porcelain status line is our own two-phase-swap artifact."""
payload = line[3:] if len(line) >= 3 else line
top_level = (
payload.strip().strip('"').replace("\\", "/").rstrip("/").split("/", 1)[0]
)
return top_level.endswith(_ZIP_STAGING_ARTIFACT_SUFFIXES)
def _abort_zip_update_if_dirty_tree() -> None:
"""Refuse to overlay a ZIP onto a dirty git checkout (#87304)."""
reason = _zip_overlay_block_reason(_m().PROJECT_ROOT)
if reason is None:
return
print(f"✗ ZIP fallback refused: {reason}.")
print(
" Overlaying the ZIP would overwrite uncommitted edits and permanently "
"delete untracked files."
)
print(" Stash or commit your changes, then rerun `hermes update`.")
print(" To inspect: git status --porcelain")
_m().sys.exit(1)
def _read_project_version() -> str | None:
"""Read the ``version`` field from the checkout's pyproject.toml.
@@ -1268,6 +1428,7 @@ def _update_via_zip(args, *, had_desktop_app_before_update: bool = False) -> boo
f"--branch {branch}`, or update against main with `hermes update`."
)
_m().sys.exit(1)
_abort_zip_update_if_dirty_tree()
zip_url = (
f"https://github.com/NousResearch/hermes-agent/archive/refs/heads/{branch}.zip"
)
@@ -1368,6 +1529,23 @@ def _update_via_zip(args, *, had_desktop_app_before_update: bool = False) -> boo
raise
try:
# Re-check the tree right before the swap (#87304 TOCTOU): the
# download + extract + staging window above can take minutes, and
# work created in it would be destroyed by the commit below. Our
# own phase-1 staging siblings are filtered out — they are the
# expected artifacts of getting here, not user work.
recheck_reason = _zip_overlay_block_reason(
_m().PROJECT_ROOT, ignore_staging_artifacts=True
)
if recheck_reason is not None:
_discard_staged(staged)
print(f"✗ ZIP fallback aborted before the swap: {recheck_reason}.")
print(
" Files appeared in the checkout while the update was "
"downloading; committing the swap would delete them."
)
print(" Stash or commit your changes, then rerun `hermes update`.")
_m().sys.exit(1)
_commit_staged_replacements(staged)
except Exception:
# The rollback already restored every swapped entry, but staging
@@ -7786,8 +7964,9 @@ def _cmd_update_impl(args, gateway_mode: bool):
sys.exit(1)
except subprocess.CalledProcessError as e:
if _m()._is_windows():
print(f"⚠ Git update failed: {e}")
stage = _format_update_failure_stage(e)
if _should_zip_fallback_on_update_error(e):
print(f"⚠ {stage}: {e}")
print("→ Falling back to ZIP download...")
print()
desktop_build_ok = _update_via_zip(
@@ -7797,7 +7976,20 @@ def _cmd_update_impl(args, gateway_mode: bool):
if gateway_mode:
_write_gateway_update_exit_code(desktop_build_ok)
else:
print(f"✗ Update failed: {e}")
print(f"✗ {stage}: {e}")
_print_called_process_error_tail(e)
if _called_process_error_is_python_dep_install(e):
print(
" The git update already finished. Re-downloading the source "
"ZIP cannot fix a dependency install error and would overwrite "
"local files."
)
if _m()._is_windows():
print(" Retry through the venv interpreter:")
print(
' venv\\Scripts\\python.exe -c '
'"from hermes_cli.main import main; main()" update --yes'
)
try:
from hermes_cli.update_receipt import finalize_update_receipt

View File

@@ -0,0 +1,249 @@
"""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 subprocess
from types import SimpleNamespace
from unittest.mock import patch
import pytest
from hermes_cli import main as hermes_main
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_windows_dep_failure_does_not_zip_fallback(monkeypatch):
monkeypatch.setattr(hermes_main, "_is_windows", lambda: True)
exc = _cpe([r"C:\venv\Scripts\uv.exe", "pip", "install", "-e", "."])
assert update_cmd._should_zip_fallback_on_update_error(exc) is False
def test_windows_git_failure_still_zips(monkeypatch):
monkeypatch.setattr(hermes_main, "_is_windows", lambda: True)
exc = _cpe(["git", "pull"], returncode=1)
assert update_cmd._should_zip_fallback_on_update_error(exc) is True
def test_posix_git_failure_does_not_zip(monkeypatch):
monkeypatch.setattr(hermes_main, "_is_windows", lambda: False)
exc = _cpe(["git", "pull"], returncode=1)
assert update_cmd._should_zip_fallback_on_update_error(exc) is False
def test_error_tail_prints_last_lines(capsys):
stderr = "\n".join(f"line-{i}" for i in range(20))
exc = _cpe(["uv", "pip", "install"], stderr=stderr)
update_cmd._print_called_process_error_tail(exc)
out = capsys.readouterr().out
assert "Last output:" in out
assert "line-19" in out
assert "line-0" not in out
assert "line-7" not in out
assert "line-8" in out
# ---------------------------------------------------------------------------
# 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="")
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:
hermes_main._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"
out = capsys.readouterr().out
assert "ZIP fallback refused" in out
assert "Downloading latest version" not in out
# ---------------------------------------------------------------------------
# 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():
is_artifact = update_cmd._is_zip_staging_artifact_status_line
assert is_artifact("?? agent.hermes-update-staging/")
assert is_artifact("?? cli.py.hermes-update-staging")
assert 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-old\n"),
)
assert (
update_cmd._zip_overlay_block_reason(tmp_path, ignore_staging_artifacts=True)
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, ignore_staging_artifacts=True
)
assert reason is not None

View File

@@ -41,8 +41,14 @@ def test_update_via_zip_rejects_symlink_member(tmp_path, monkeypatch):
target="/etc/passwd",
)
fake_root = tmp_path / "install_dir"
fake_root.mkdir()
from hermes_cli import main as hermes_main
from hermes_cli.main import _update_via_zip
monkeypatch.setattr(hermes_main, "PROJECT_ROOT", fake_root)
args = type("Args", (), {})()
# Patch urlretrieve to "download" our pre-built malicious ZIP into the