From aedb8ac83b8c6ecb89b9d370909fa46ff4733da2 Mon Sep 17 00:00:00 2001 From: Teknium <127238744+teknium1@users.noreply.github.com> Date: Tue, 19 May 2026 03:01:02 -0700 Subject: [PATCH] feat(update): syntax-validate critical files post-pull, auto-rollback on failure (#28669) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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) --- hermes_cli/main.py | 109 +++++++++++++ .../test_update_post_pull_syntax_guard.py | 153 ++++++++++++++++++ 2 files changed, 262 insertions(+) create mode 100644 tests/hermes_cli/test_update_post_pull_syntax_guard.py diff --git a/hermes_cli/main.py b/hermes_cli/main.py index aad45eb2e37..4ae845d2e53 100644 --- a/hermes_cli/main.py +++ b/hermes_cli/main.py @@ -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 ") + sys.exit(1) + update_succeeded = True finally: if auto_stash_ref is not None: diff --git a/tests/hermes_cli/test_update_post_pull_syntax_guard.py b/tests/hermes_cli/test_update_post_pull_syntax_guard.py new file mode 100644 index 00000000000..805ac1c0f02 --- /dev/null +++ b/tests/hermes_cli/test_update_post_pull_syntax_guard.py @@ -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}" + )