mirror of
https://github.com/NousResearch/hermes-agent.git
synced 2026-07-29 18:46:59 +00:00
fix(update): close the stale-bytecode class with a launch-time checkout-fingerprint sweep
The stale-.pyc bug class (#6207, #60242, live WhatsApp report: gateway ImportError 'cannot import name parse_model_flags_detailed' after /update) has one shared shape: the checkout's .py files change while __pycache__ retains bytecode from the previous revision, and a later process trusts the stale .pyc. Update-time clears can never fully close this class: 'hermes update' always executes the PRE-pull updater code, so hardening added to it takes effect one update late — and a manual 'git pull' never runs the updater at all. Class fix: - Launch-time guard in main(): compare the checkout fingerprint (cheap file reads via _read_git_revision_fingerprint, no git subprocess) against a .bytecode-fingerprint stamp; sweep __pycache__ once when they diverge. Covers manual pulls, old updaters, ZIP restores — every entry point (CLI, gateway service, desktop backend) passes through main(). - Record the stamp at all three update-time clear sites (git path pre- install, git path post-install, ZIP path) so a normal update never triggers a redundant launch sweep. - Sibling site: 'hermes plugins update' + dashboard plugin update now clear __pycache__ under the plugin dir after git pull (plugin trees live outside the repo guard). E2E validated: reproduced the stale-pyc shadowing (same-size same-mtime source swap → old symbol wins in a fresh process), confirmed the launch sweep restores the new symbol, no-ops when the checkout is unchanged, and re-sweeps on the next pull. Sabotage-tested: reverting the sweep fails 4 of the new tests.
This commit is contained in:
parent
d76d0d61d8
commit
ea2499bcfe
4 changed files with 281 additions and 0 deletions
5
.gitignore
vendored
5
.gitignore
vendored
|
|
@ -152,6 +152,11 @@ docs/superpowers/*
|
|||
.update-incomplete
|
||||
.update-incomplete.lock
|
||||
|
||||
# Checkout fingerprint the __pycache__ tree was last validated against
|
||||
# (launch-time stale-bytecode sweep). Runtime state, never a code change.
|
||||
.bytecode-fingerprint
|
||||
.bytecode-fingerprint.tmp
|
||||
|
||||
# Installer-written method stamp in the managed checkout root (scripts/install.sh).
|
||||
# Runtime metadata only — never a code change. Ignore so `git status` stays clean
|
||||
# and `hermes update`'s untracked autostash does not treat it as a local edit (#66189 / #54855).
|
||||
|
|
|
|||
|
|
@ -4792,6 +4792,76 @@ def _reload_updated_runtime_modules() -> None:
|
|||
logger.debug("Could not refresh update runtime modules: %s", exc)
|
||||
|
||||
|
||||
# Stamp file recording the checkout fingerprint the bytecode cache was last
|
||||
# validated against. Lives next to the checkout (NOT in HERMES_HOME) because
|
||||
# __pycache__ is per-checkout state shared by every profile.
|
||||
_BYTECODE_FINGERPRINT_FILE = ".bytecode-fingerprint"
|
||||
|
||||
|
||||
def _record_bytecode_fingerprint() -> None:
|
||||
"""Persist the current checkout fingerprint after a bytecode sweep.
|
||||
|
||||
Never raises. A failed write just means the next launch re-sweeps —
|
||||
safe, merely redundant.
|
||||
"""
|
||||
try:
|
||||
fingerprint = _read_git_revision_fingerprint(PROJECT_ROOT)
|
||||
if not fingerprint:
|
||||
return
|
||||
stamp_path = PROJECT_ROOT / _BYTECODE_FINGERPRINT_FILE
|
||||
tmp_path = stamp_path.with_name(stamp_path.name + ".tmp")
|
||||
tmp_path.write_text(fingerprint, encoding="utf-8")
|
||||
tmp_path.replace(stamp_path)
|
||||
except OSError as exc:
|
||||
logger.debug("Could not record bytecode fingerprint: %s", exc)
|
||||
|
||||
|
||||
def _sweep_stale_bytecode_if_checkout_changed() -> None:
|
||||
"""Clear ``__pycache__`` at launch when the checkout changed underneath us.
|
||||
|
||||
The stale-bytecode bug class (issues #6207, #60242; Dhruv's WhatsApp
|
||||
``cannot import name 'parse_model_flags_detailed'`` report) has one
|
||||
shared shape: the checkout's ``.py`` files change (git pull inside
|
||||
``hermes update``, a manual ``git pull``, a ZIP update, a file-sync
|
||||
restore) while ``__pycache__`` retains bytecode from the previous
|
||||
revision, and a later process trusts the stale ``.pyc`` instead of the
|
||||
fresh source.
|
||||
|
||||
Update-time clears alone can never close this class: ``hermes update``
|
||||
always executes the PRE-pull updater code, so any hardening added to it
|
||||
only takes effect one update late, and manual ``git pull`` never runs
|
||||
the updater at all. This launch-time guard closes the loop: every
|
||||
``hermes`` entry point compares the checkout fingerprint (cheap file
|
||||
reads, no git subprocess) against the last-validated stamp and sweeps
|
||||
the bytecode cache once when they diverge.
|
||||
|
||||
Never raises — a failure here must not block launch.
|
||||
"""
|
||||
try:
|
||||
fingerprint = _read_git_revision_fingerprint(PROJECT_ROOT)
|
||||
if not fingerprint:
|
||||
return # non-git install — the ZIP update path clears explicitly
|
||||
stamp_path = PROJECT_ROOT / _BYTECODE_FINGERPRINT_FILE
|
||||
try:
|
||||
recorded = stamp_path.read_text(encoding="utf-8").strip()
|
||||
except OSError:
|
||||
recorded = ""
|
||||
if recorded == fingerprint:
|
||||
return
|
||||
removed = _clear_bytecode_cache(PROJECT_ROOT)
|
||||
if removed:
|
||||
logger.info(
|
||||
"Checkout changed since last launch (%s -> %s): cleared %d stale __pycache__ director%s",
|
||||
recorded or "unknown",
|
||||
fingerprint,
|
||||
removed,
|
||||
"y" if removed == 1 else "ies",
|
||||
)
|
||||
_record_bytecode_fingerprint()
|
||||
except Exception as exc:
|
||||
logger.debug("Stale-bytecode launch sweep failed: %s", exc)
|
||||
|
||||
|
||||
# Critical files that Hermes must be able to import immediately after an
|
||||
# update/install. Most are imported on every CLI startup; ``web_server.py``
|
||||
# is the desktop/dashboard backend path that a fresh Windows install launches
|
||||
|
|
@ -7810,6 +7880,7 @@ def _update_via_zip(args):
|
|||
print(
|
||||
f" ✓ Cleared {removed} stale __pycache__ director{'y' if removed == 1 else 'ies'}"
|
||||
)
|
||||
_record_bytecode_fingerprint()
|
||||
|
||||
# Reinstall Python dependencies. Prefer .[all], but if one optional extra
|
||||
# breaks on this machine, keep base deps and reinstall the remaining extras
|
||||
|
|
@ -12308,6 +12379,7 @@ def _cmd_update_impl(args, gateway_mode: bool):
|
|||
print(
|
||||
f" ✓ Cleared {removed} stale __pycache__ director{'y' if removed == 1 else 'ies'}"
|
||||
)
|
||||
_record_bytecode_fingerprint()
|
||||
|
||||
# Fork upstream sync logic (only for main branch on forks)
|
||||
if is_fork and branch == "main":
|
||||
|
|
@ -12396,6 +12468,7 @@ def _cmd_update_impl(args, gateway_mode: bool):
|
|||
print(
|
||||
f" ✓ Cleared {removed} stale __pycache__ director{'y' if removed == 1 else 'ies'}"
|
||||
)
|
||||
_record_bytecode_fingerprint()
|
||||
_reload_updated_runtime_modules()
|
||||
|
||||
# Upgrade pip before lazy refreshes — stale pip can fail source builds
|
||||
|
|
@ -15587,6 +15660,12 @@ def main():
|
|||
except Exception:
|
||||
pass
|
||||
|
||||
# If the checkout changed since the last launch (hermes update, manual
|
||||
# git pull, old-updater update that predates newer clears), sweep stale
|
||||
# __pycache__ once so no process — this one's lazy imports included —
|
||||
# resolves fresh source against old bytecode. Never raises.
|
||||
_sweep_stale_bytecode_if_checkout_changed()
|
||||
|
||||
# Self-heal a venv left half-built by an interrupted ``hermes update``
|
||||
# (Ctrl-C, terminal close, WSL OOM mid-install). Skip when the user is
|
||||
# *running* update — that flow writes and clears its own marker, and we
|
||||
|
|
|
|||
|
|
@ -660,6 +660,11 @@ def cmd_update(name: str) -> None:
|
|||
console.print(f"[red]Error:[/red] {output}")
|
||||
sys.exit(1)
|
||||
|
||||
# Same stale-bytecode class as the main checkout (#6207/#60242): the
|
||||
# pull just changed .py files under this plugin dir, so drop any
|
||||
# __pycache__ compiled from the previous revision.
|
||||
_clear_plugin_bytecode(target)
|
||||
|
||||
# Copy any new .example files
|
||||
_copy_example_files(target, console)
|
||||
|
||||
|
|
@ -1954,6 +1959,10 @@ def dashboard_update_user_plugin(name: str) -> dict[str, Any]:
|
|||
if not ok:
|
||||
return {"ok": False, "error": msg}
|
||||
|
||||
# Sibling of the CLI ``hermes plugins update`` path: drop bytecode
|
||||
# compiled from the pre-pull plugin revision.
|
||||
_clear_plugin_bytecode(target)
|
||||
|
||||
from rich.console import Console
|
||||
|
||||
_copy_example_files(target, Console())
|
||||
|
|
@ -1961,6 +1970,30 @@ def dashboard_update_user_plugin(name: str) -> dict[str, Any]:
|
|||
return {"ok": True, "name": name, "output": msg, "unchanged": unchanged}
|
||||
|
||||
|
||||
def _clear_plugin_bytecode(target: Path) -> int:
|
||||
"""Remove ``__pycache__`` dirs under a just-updated plugin checkout.
|
||||
|
||||
Plugin dirs live outside the main repo, so the launch-time checkout
|
||||
fingerprint sweep in ``hermes_cli.main`` never covers them. After a
|
||||
``git pull`` changes a plugin's ``.py`` files, stale bytecode here can
|
||||
produce the same ImportError class as #6207/#60242 in whichever
|
||||
process imports the plugin next. Never raises.
|
||||
"""
|
||||
removed = 0
|
||||
try:
|
||||
for cache_dir in target.rglob("__pycache__"):
|
||||
if not cache_dir.is_dir():
|
||||
continue
|
||||
try:
|
||||
shutil.rmtree(cache_dir)
|
||||
removed += 1
|
||||
except OSError:
|
||||
pass
|
||||
except OSError:
|
||||
pass
|
||||
return removed
|
||||
|
||||
|
||||
def _git_pull_plugin_dir(target: Path) -> tuple[bool, str]:
|
||||
git_exe = _resolve_git_executable()
|
||||
if not git_exe:
|
||||
|
|
|
|||
164
tests/hermes_cli/test_bytecode_sweep.py
Normal file
164
tests/hermes_cli/test_bytecode_sweep.py
Normal file
|
|
@ -0,0 +1,164 @@
|
|||
"""Tests for the launch-time stale-bytecode sweep (checkout fingerprint guard).
|
||||
|
||||
Bug class: the checkout's ``.py`` files change (``hermes update``, manual
|
||||
``git pull``, ZIP update) while ``__pycache__`` retains bytecode compiled
|
||||
from the previous revision; the next process to import trusts the stale
|
||||
``.pyc`` and dies with ``cannot import name ...`` (#6207, #60242).
|
||||
|
||||
The launch-time guard compares the current checkout fingerprint against the
|
||||
last-validated stamp and sweeps ``__pycache__`` once when they diverge —
|
||||
covering paths no update-time clear can reach (manual pulls, pre-hardening
|
||||
updaters).
|
||||
"""
|
||||
|
||||
from pathlib import Path
|
||||
|
||||
from hermes_cli import main as hermes_main
|
||||
|
||||
|
||||
def _make_repo(tmp_path: Path, sha: str = "a" * 40) -> Path:
|
||||
"""Minimal git checkout layout that _read_git_revision_fingerprint groks."""
|
||||
repo = tmp_path / "repo"
|
||||
git_dir = repo / ".git"
|
||||
(git_dir / "refs" / "heads").mkdir(parents=True)
|
||||
(git_dir / "HEAD").write_text("ref: refs/heads/main\n", encoding="utf-8")
|
||||
(git_dir / "refs" / "heads" / "main").write_text(sha + "\n", encoding="utf-8")
|
||||
return repo
|
||||
|
||||
|
||||
def _make_pycache(repo: Path, subdir: str = "hermes_cli") -> Path:
|
||||
cache = repo / subdir / "__pycache__"
|
||||
cache.mkdir(parents=True)
|
||||
(cache / "main.cpython-311.pyc").write_bytes(b"stale")
|
||||
return cache
|
||||
|
||||
|
||||
def test_sweep_clears_pycache_when_checkout_changed(monkeypatch, tmp_path):
|
||||
repo = _make_repo(tmp_path, sha="b" * 40)
|
||||
cache = _make_pycache(repo)
|
||||
monkeypatch.setattr(hermes_main, "PROJECT_ROOT", repo)
|
||||
# Stamp records a different (older) fingerprint.
|
||||
(repo / hermes_main._BYTECODE_FINGERPRINT_FILE).write_text(
|
||||
"git:refs/heads/main:" + "a" * 40, encoding="utf-8"
|
||||
)
|
||||
|
||||
hermes_main._sweep_stale_bytecode_if_checkout_changed()
|
||||
|
||||
assert not cache.exists()
|
||||
# Stamp updated to the current fingerprint.
|
||||
recorded = (repo / hermes_main._BYTECODE_FINGERPRINT_FILE).read_text(encoding="utf-8")
|
||||
assert recorded.strip().endswith("b" * 40)
|
||||
|
||||
|
||||
def test_sweep_noop_when_fingerprint_matches(monkeypatch, tmp_path):
|
||||
repo = _make_repo(tmp_path, sha="c" * 40)
|
||||
cache = _make_pycache(repo)
|
||||
monkeypatch.setattr(hermes_main, "PROJECT_ROOT", repo)
|
||||
fingerprint = hermes_main._read_git_revision_fingerprint(repo)
|
||||
assert fingerprint is not None
|
||||
(repo / hermes_main._BYTECODE_FINGERPRINT_FILE).write_text(fingerprint, encoding="utf-8")
|
||||
|
||||
hermes_main._sweep_stale_bytecode_if_checkout_changed()
|
||||
|
||||
assert cache.exists() # untouched — no needless recompiles on every launch
|
||||
|
||||
|
||||
def test_sweep_first_launch_clears_and_records(monkeypatch, tmp_path):
|
||||
"""No stamp yet (first launch with the guard) → sweep once, record."""
|
||||
repo = _make_repo(tmp_path, sha="d" * 40)
|
||||
cache = _make_pycache(repo)
|
||||
monkeypatch.setattr(hermes_main, "PROJECT_ROOT", repo)
|
||||
|
||||
hermes_main._sweep_stale_bytecode_if_checkout_changed()
|
||||
|
||||
assert not cache.exists()
|
||||
assert (repo / hermes_main._BYTECODE_FINGERPRINT_FILE).exists()
|
||||
|
||||
|
||||
def test_sweep_noop_on_non_git_install(monkeypatch, tmp_path):
|
||||
"""No .git → no fingerprint → guard must not touch anything or raise."""
|
||||
repo = tmp_path / "repo"
|
||||
repo.mkdir()
|
||||
cache = _make_pycache(repo)
|
||||
monkeypatch.setattr(hermes_main, "PROJECT_ROOT", repo)
|
||||
|
||||
hermes_main._sweep_stale_bytecode_if_checkout_changed()
|
||||
|
||||
assert cache.exists()
|
||||
assert not (repo / hermes_main._BYTECODE_FINGERPRINT_FILE).exists()
|
||||
|
||||
|
||||
def test_sweep_never_raises_on_unreadable_stamp(monkeypatch, tmp_path):
|
||||
repo = _make_repo(tmp_path, sha="e" * 40)
|
||||
cache = _make_pycache(repo)
|
||||
monkeypatch.setattr(hermes_main, "PROJECT_ROOT", repo)
|
||||
stamp = repo / hermes_main._BYTECODE_FINGERPRINT_FILE
|
||||
stamp.mkdir() # a directory where a file is expected → OSError on read
|
||||
|
||||
hermes_main._sweep_stale_bytecode_if_checkout_changed() # must not raise
|
||||
|
||||
# Unreadable stamp is treated as "changed": cache swept.
|
||||
assert not cache.exists()
|
||||
|
||||
|
||||
def test_record_bytecode_fingerprint_writes_atomically(monkeypatch, tmp_path):
|
||||
repo = _make_repo(tmp_path, sha="f" * 40)
|
||||
monkeypatch.setattr(hermes_main, "PROJECT_ROOT", repo)
|
||||
|
||||
hermes_main._record_bytecode_fingerprint()
|
||||
|
||||
stamp = repo / hermes_main._BYTECODE_FINGERPRINT_FILE
|
||||
assert stamp.read_text(encoding="utf-8").endswith("f" * 40)
|
||||
assert not stamp.with_name(stamp.name + ".tmp").exists()
|
||||
|
||||
|
||||
def test_record_bytecode_fingerprint_noop_without_git(monkeypatch, tmp_path):
|
||||
repo = tmp_path / "repo"
|
||||
repo.mkdir()
|
||||
monkeypatch.setattr(hermes_main, "PROJECT_ROOT", repo)
|
||||
|
||||
hermes_main._record_bytecode_fingerprint() # must not raise
|
||||
|
||||
assert not (repo / hermes_main._BYTECODE_FINGERPRINT_FILE).exists()
|
||||
|
||||
|
||||
def test_sweep_skips_venv_and_git_dirs(monkeypatch, tmp_path):
|
||||
"""The underlying clear must not touch venv/node_modules bytecode."""
|
||||
repo = _make_repo(tmp_path, sha="9" * 40)
|
||||
repo_cache = _make_pycache(repo, "hermes_cli")
|
||||
venv_cache = repo / "venv" / "lib" / "__pycache__"
|
||||
venv_cache.mkdir(parents=True)
|
||||
(venv_cache / "x.pyc").write_bytes(b"keep")
|
||||
monkeypatch.setattr(hermes_main, "PROJECT_ROOT", repo)
|
||||
|
||||
hermes_main._sweep_stale_bytecode_if_checkout_changed()
|
||||
|
||||
assert not repo_cache.exists()
|
||||
assert venv_cache.exists()
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# Plugin-update sibling site: __pycache__ under ~/.hermes/plugins/<name>
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
def test_clear_plugin_bytecode_removes_nested_caches(tmp_path):
|
||||
from hermes_cli import plugins_cmd
|
||||
|
||||
plugin = tmp_path / "myplugin"
|
||||
top = plugin / "__pycache__"
|
||||
nested = plugin / "sub" / "__pycache__"
|
||||
top.mkdir(parents=True)
|
||||
nested.mkdir(parents=True)
|
||||
(top / "a.pyc").write_bytes(b"stale")
|
||||
(nested / "b.pyc").write_bytes(b"stale")
|
||||
|
||||
removed = plugins_cmd._clear_plugin_bytecode(plugin)
|
||||
|
||||
assert removed == 2
|
||||
assert not top.exists()
|
||||
assert not nested.exists()
|
||||
|
||||
|
||||
def test_clear_plugin_bytecode_never_raises_on_missing_dir(tmp_path):
|
||||
from hermes_cli import plugins_cmd
|
||||
|
||||
assert plugins_cmd._clear_plugin_bytecode(tmp_path / "nope") == 0
|
||||
Loading…
Add table
Add a link
Reference in a new issue