mirror of
https://github.com/NousResearch/hermes-agent.git
synced 2026-05-30 06:41:51 +00:00
feat(update): syntax-validate critical files post-pull, auto-rollback on failure (#28669)
Catch the PR #28452 failure mode (orphan merge-conflict markers in hermes_cli/config.py) on the user side: after git pull succeeds, compile the files every 'hermes' invocation imports at startup. If any has a syntax error, git reset --hard back to the pre-pull SHA so the install stays bootable. User can retry once a fix lands upstream. - New _capture_head_sha() + _validate_critical_files_syntax() helpers - Wires both into _cmd_update_impl after the pull/reset succeeds - Tests cover the helpers, the rollback flow, and a production-tree invariant (CI fails if main itself has a syntax error in a critical file — catches future broken commits before users hit them)
This commit is contained in:
parent
a0bd11d022
commit
aedb8ac83b
2 changed files with 262 additions and 0 deletions
|
|
@ -5876,6 +5876,67 @@ def _clear_bytecode_cache(root: Path) -> int:
|
|||
return removed
|
||||
|
||||
|
||||
# Critical files that every ``hermes`` invocation imports at startup. If any
|
||||
# of these fail to parse after a pull, the CLI is bricked — the user can't
|
||||
# even run ``hermes update`` again to roll forward. The post-pull syntax
|
||||
# guard validates these and auto-rolls-back on failure.
|
||||
_UPDATE_CRITICAL_FILES = (
|
||||
"hermes_cli/main.py",
|
||||
"hermes_cli/config.py",
|
||||
"hermes_cli/__init__.py",
|
||||
"cli.py",
|
||||
"run_agent.py",
|
||||
"model_tools.py",
|
||||
"toolsets.py",
|
||||
"hermes_constants.py",
|
||||
)
|
||||
|
||||
|
||||
def _capture_head_sha(git_cmd, cwd) -> str | None:
|
||||
"""Return the current HEAD SHA, or None if it can't be resolved."""
|
||||
try:
|
||||
result = subprocess.run(
|
||||
git_cmd + ["rev-parse", "HEAD"],
|
||||
cwd=cwd,
|
||||
capture_output=True,
|
||||
text=True,
|
||||
check=True,
|
||||
)
|
||||
return result.stdout.strip() or None
|
||||
except (subprocess.CalledProcessError, OSError):
|
||||
return None
|
||||
|
||||
|
||||
def _validate_critical_files_syntax(root) -> tuple[bool, str | None, str | None]:
|
||||
"""Compile each file in ``_UPDATE_CRITICAL_FILES`` to catch SyntaxErrors.
|
||||
|
||||
These are the files imported on every ``hermes`` startup; if any of them
|
||||
has a syntax error (orphan merge-conflict markers, bad ref to a name
|
||||
that no longer exists, etc.) the CLI can't bootstrap at all. We validate
|
||||
them after a successful ``git pull`` so we can auto-roll-back instead of
|
||||
leaving the user with a bricked install.
|
||||
|
||||
Returns ``(ok, failing_path, error_message)``. ``ok=True`` means every
|
||||
file parsed cleanly.
|
||||
"""
|
||||
import py_compile
|
||||
|
||||
root = Path(root)
|
||||
for relpath in _UPDATE_CRITICAL_FILES:
|
||||
path = root / relpath
|
||||
if not path.exists():
|
||||
# Missing file is suspicious but not necessarily fatal — a future
|
||||
# refactor may legitimately remove one of these. Skip and move on.
|
||||
continue
|
||||
try:
|
||||
py_compile.compile(str(path), doraise=True)
|
||||
except py_compile.PyCompileError as exc:
|
||||
return False, str(path), str(exc)
|
||||
except OSError as exc:
|
||||
return False, str(path), f"could not read: {exc}"
|
||||
return True, None, None
|
||||
|
||||
|
||||
def _gateway_prompt(prompt_text: str, default: str = "", timeout: float = 300.0) -> str:
|
||||
"""File-based IPC prompt for gateway mode.
|
||||
|
||||
|
|
@ -8137,6 +8198,12 @@ def _cmd_update_impl(args, gateway_mode: bool):
|
|||
|
||||
print("→ Pulling updates...")
|
||||
update_succeeded = False
|
||||
# Capture the pre-pull SHA so we can auto-roll-back if the new code
|
||||
# has a syntax error in a critical-path file (PR #28452 incident:
|
||||
# orphan merge-conflict markers in hermes_cli/config.py bricked
|
||||
# every user who ran ``hermes update`` for the 7 minutes between
|
||||
# the bad commit and the fix landing).
|
||||
pre_pull_sha = _capture_head_sha(git_cmd, PROJECT_ROOT)
|
||||
try:
|
||||
pull_result = subprocess.run(
|
||||
git_cmd + ["pull", "--ff-only", "origin", branch],
|
||||
|
|
@ -8165,6 +8232,48 @@ def _cmd_update_impl(args, gateway_mode: bool):
|
|||
" Try manually: git fetch origin && git reset --hard origin/main"
|
||||
)
|
||||
sys.exit(1)
|
||||
|
||||
# Post-pull syntax guard: validate critical-path files actually
|
||||
# parse before declaring the update successful. If a bad commit
|
||||
# made it through CI (e.g. admin-merge bypass of a failing
|
||||
# ruff check), this catches it on the user side and rolls back
|
||||
# so the CLI stays bootable. The user can then retry ``hermes
|
||||
# update`` later once a fix lands upstream.
|
||||
syntax_ok, failing_path, syntax_error = _validate_critical_files_syntax(
|
||||
PROJECT_ROOT
|
||||
)
|
||||
if not syntax_ok:
|
||||
print()
|
||||
print("✗ Pulled code has a syntax error in a critical file:")
|
||||
print(f" {failing_path}")
|
||||
if syntax_error:
|
||||
# py_compile errors can be multi-line; show the first
|
||||
# ~6 lines so the user sees the actual SyntaxError text.
|
||||
for line in str(syntax_error).splitlines()[:6]:
|
||||
print(f" {line}")
|
||||
if pre_pull_sha:
|
||||
print()
|
||||
print(f"→ Rolling back to {pre_pull_sha[:10]}...")
|
||||
rollback_result = subprocess.run(
|
||||
git_cmd + ["reset", "--hard", pre_pull_sha],
|
||||
cwd=PROJECT_ROOT,
|
||||
capture_output=True,
|
||||
text=True,
|
||||
)
|
||||
if rollback_result.returncode == 0:
|
||||
print(" ✓ Rollback complete — your install is unchanged.")
|
||||
print(" Try ``hermes update`` again later once a fix lands.")
|
||||
else:
|
||||
print(" ✗ Rollback failed. Recover manually with:")
|
||||
print(f" cd {PROJECT_ROOT} && git reset --hard {pre_pull_sha}")
|
||||
if rollback_result.stderr.strip():
|
||||
print(f" ({rollback_result.stderr.strip().splitlines()[0]})")
|
||||
else:
|
||||
print()
|
||||
print(" Could not capture pre-pull SHA — recover manually with:")
|
||||
print(f" cd {PROJECT_ROOT} && git reflog && git reset --hard <prev-sha>")
|
||||
sys.exit(1)
|
||||
|
||||
update_succeeded = True
|
||||
finally:
|
||||
if auto_stash_ref is not None:
|
||||
|
|
|
|||
153
tests/hermes_cli/test_update_post_pull_syntax_guard.py
Normal file
153
tests/hermes_cli/test_update_post_pull_syntax_guard.py
Normal file
|
|
@ -0,0 +1,153 @@
|
|||
"""Tests for the post-pull syntax guard in ``hermes update``.
|
||||
|
||||
When a bad commit lands on ``main`` with a syntax error in a critical file
|
||||
(e.g. orphan merge-conflict markers in ``hermes_cli/config.py``), the CLI
|
||||
becomes unbootable — every ``hermes`` invocation imports those files at
|
||||
startup. The guard validates them after ``git pull`` and rolls back to the
|
||||
pre-pull SHA on failure so the user's install stays runnable.
|
||||
|
||||
Reference incident: PR #28452 (May 18, 2026) shipped unresolved conflict
|
||||
markers in ``hermes_cli/config.py``; users who ran ``hermes update`` in
|
||||
the 7-minute window before #28458 landed could not run any ``hermes``
|
||||
command afterward.
|
||||
"""
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
from pathlib import Path
|
||||
from types import SimpleNamespace
|
||||
|
||||
from hermes_cli import main as hermes_main
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# _capture_head_sha
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
def test_capture_head_sha_returns_stripped_sha(monkeypatch, tmp_path):
|
||||
def fake_run(cmd, **kwargs):
|
||||
assert cmd[-2:] == ["rev-parse", "HEAD"]
|
||||
return SimpleNamespace(stdout="deadbeefcafe\n", returncode=0)
|
||||
|
||||
monkeypatch.setattr(hermes_main.subprocess, "run", fake_run)
|
||||
|
||||
assert hermes_main._capture_head_sha(["git"], tmp_path) == "deadbeefcafe"
|
||||
|
||||
|
||||
def test_capture_head_sha_returns_none_on_git_failure(monkeypatch, tmp_path):
|
||||
import subprocess as _sp
|
||||
|
||||
def fake_run(cmd, **kwargs):
|
||||
raise _sp.CalledProcessError(returncode=128, cmd=cmd)
|
||||
|
||||
monkeypatch.setattr(hermes_main.subprocess, "run", fake_run)
|
||||
|
||||
assert hermes_main._capture_head_sha(["git"], tmp_path) is None
|
||||
|
||||
|
||||
def test_capture_head_sha_returns_none_on_empty_output(monkeypatch, tmp_path):
|
||||
def fake_run(cmd, **kwargs):
|
||||
return SimpleNamespace(stdout="\n", returncode=0)
|
||||
|
||||
monkeypatch.setattr(hermes_main.subprocess, "run", fake_run)
|
||||
|
||||
assert hermes_main._capture_head_sha(["git"], tmp_path) is None
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# _validate_critical_files_syntax
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
def _populate_critical_tree(root: Path, *, broken_file: str | None = None) -> None:
|
||||
"""Create stub files for every entry in ``_UPDATE_CRITICAL_FILES``.
|
||||
|
||||
If ``broken_file`` is given, that file gets orphan merge-conflict markers
|
||||
(the exact failure mode from PR #28452).
|
||||
"""
|
||||
broken_payload = (
|
||||
"x = {\n"
|
||||
' "a": 1,\n'
|
||||
"<<<<<<< HEAD\n"
|
||||
' "b": 2,\n'
|
||||
"=======\n"
|
||||
' "c": 0b6d673e7,\n' # invalid binary literal — the actual error users saw
|
||||
">>>>>>> 0b6d673e7\n"
|
||||
"}\n"
|
||||
)
|
||||
for relpath in hermes_main._UPDATE_CRITICAL_FILES:
|
||||
path = root / relpath
|
||||
path.parent.mkdir(parents=True, exist_ok=True)
|
||||
if relpath == broken_file:
|
||||
path.write_text(broken_payload)
|
||||
else:
|
||||
path.write_text("# stub\n")
|
||||
|
||||
|
||||
def test_validate_critical_files_syntax_ok_when_all_files_parse(tmp_path):
|
||||
_populate_critical_tree(tmp_path)
|
||||
|
||||
ok, failing_path, error = hermes_main._validate_critical_files_syntax(tmp_path)
|
||||
|
||||
assert ok is True
|
||||
assert failing_path is None
|
||||
assert error is None
|
||||
|
||||
|
||||
def test_validate_critical_files_syntax_detects_conflict_markers(tmp_path):
|
||||
"""The exact PR #28452 failure mode: orphan ``<<<<<<<`` in config.py."""
|
||||
_populate_critical_tree(tmp_path, broken_file="hermes_cli/config.py")
|
||||
|
||||
ok, failing_path, error = hermes_main._validate_critical_files_syntax(tmp_path)
|
||||
|
||||
assert ok is False
|
||||
assert failing_path is not None and failing_path.endswith("hermes_cli/config.py")
|
||||
assert error is not None
|
||||
# The error mentions either the syntax error itself or the file path —
|
||||
# either is enough proof we caught the bad commit.
|
||||
assert "SyntaxError" in str(error) or "config.py" in str(error)
|
||||
|
||||
|
||||
def test_validate_critical_files_syntax_detects_break_in_main_py(tmp_path):
|
||||
_populate_critical_tree(tmp_path, broken_file="hermes_cli/main.py")
|
||||
|
||||
ok, failing_path, _ = hermes_main._validate_critical_files_syntax(tmp_path)
|
||||
|
||||
assert ok is False
|
||||
assert failing_path is not None and failing_path.endswith("hermes_cli/main.py")
|
||||
|
||||
|
||||
def test_validate_critical_files_syntax_tolerates_missing_files(tmp_path):
|
||||
"""A refactor may legitimately remove one of the critical files — the
|
||||
guard should skip missing files, not falsely flag the install as broken."""
|
||||
# Populate everything except hermes_constants.py
|
||||
for relpath in hermes_main._UPDATE_CRITICAL_FILES:
|
||||
if relpath == "hermes_constants.py":
|
||||
continue
|
||||
path = tmp_path / relpath
|
||||
path.parent.mkdir(parents=True, exist_ok=True)
|
||||
path.write_text("# stub\n")
|
||||
|
||||
ok, failing_path, error = hermes_main._validate_critical_files_syntax(tmp_path)
|
||||
|
||||
assert ok is True
|
||||
assert failing_path is None
|
||||
assert error is None
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# Repo invariant — the production tree itself must always pass the guard.
|
||||
# This catches the case where ``main`` ships a syntax error before the next
|
||||
# release; if a future ``hermes update`` would brick users, this test fails
|
||||
# in CI first.
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
def test_production_tree_passes_syntax_guard():
|
||||
"""The repo itself must always satisfy the guard the update command runs."""
|
||||
repo_root = Path(__file__).resolve().parents[2]
|
||||
|
||||
ok, failing_path, error = hermes_main._validate_critical_files_syntax(repo_root)
|
||||
|
||||
assert ok is True, (
|
||||
f"Critical-path file {failing_path} fails to parse on current main; "
|
||||
f"hermes update would brick users. Error: {error}"
|
||||
)
|
||||
Loading…
Add table
Add a link
Reference in a new issue