diff --git a/.gitignore b/.gitignore index a72536f00779..cd05306af00b 100644 --- a/.gitignore +++ b/.gitignore @@ -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). diff --git a/hermes_cli/main.py b/hermes_cli/main.py index aff5ec07d817..2ccfe808d4fe 100644 --- a/hermes_cli/main.py +++ b/hermes_cli/main.py @@ -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 diff --git a/hermes_cli/plugins_cmd.py b/hermes_cli/plugins_cmd.py index 07f2f3198a9b..8bc1edd9d05e 100644 --- a/hermes_cli/plugins_cmd.py +++ b/hermes_cli/plugins_cmd.py @@ -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: diff --git a/tests/hermes_cli/test_bytecode_sweep.py b/tests/hermes_cli/test_bytecode_sweep.py new file mode 100644 index 000000000000..59bc03d955cb --- /dev/null +++ b/tests/hermes_cli/test_bytecode_sweep.py @@ -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/ +# --------------------------------------------------------------------------- + +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