diff --git a/hermes_cli/banner.py b/hermes_cli/banner.py index eb5be5addc..3f38577138 100644 --- a/hermes_cli/banner.py +++ b/hermes_cli/banner.py @@ -4,6 +4,7 @@ import logging import os import shutil import subprocess +import sys import threading import time from pathlib import Path @@ -505,8 +506,36 @@ def _daemon(name: Optional[str], target) -> None: threading.Thread(target=lambda: _quiet(target), name=name, daemon=True).start() +def _skip_background_prefetch() -> bool: + """True when the banner's background prefetch threads must not start. + + Under pytest the prefetch daemon threads shell out to git + (``fetch``/``rev-parse``/``rev-list``) at an arbitrary point after import, + and any test that patches the process-wide ``subprocess`` singleton + (``patch("subprocess.run")`` / ``patch("subprocess.Popen")`` — note + ``subprocess.run`` calls ``subprocess.Popen`` internally, so a Popen patch + captures run() spawns too) can record that stray git spawn instead of — + or in addition to — the call it meant to pin. That cross-talk + manufactured CI flakes in tests/tui_gateway/test_subprocess_encoding.py + (``encoding=None`` from the update thread's un-encoded ``git fetch``) and + tests/tui_gateway/test_bot_relay_methods.py (``argv == ['rev-parse', + 'FETCH_HEAD']`` from the shallow-checkout path), both of which import + ``tui_gateway.server`` — which starts this prefetch at import time. + Nothing under pytest needs a live update check; tests that exercise the + prefetch itself monkeypatch this predicate to False. + """ + return "PYTEST_CURRENT_TEST" in os.environ or "pytest" in sys.modules + + def prefetch_update_check(): - """Kick off update check in a background daemon thread.""" + """Kick off update check in a background daemon thread. + + No-op under pytest — see ``_skip_background_prefetch``. + """ + if _skip_background_prefetch(): + _update_check_done.set() + return + def _run(): global _update_result _update_result = check_for_updates(passive=True) @@ -527,6 +556,13 @@ def prefetch_banner_data(): global _banner_data_prefetch_started if _banner_data_prefetch_started: return + if _skip_background_prefetch(): + # Same stray-git-spawn cross-talk class as prefetch_update_check: + # get_git_banner_state() shells out via the shared subprocess + # singleton from a daemon thread, poisoning process-wide subprocess + # mocks in unrelated tests. + _banner_data_prefetch_started = True + return _banner_data_prefetch_started = True _daemon("banner-data-prefetch", lambda: [_quiet(warm) for warm in ( get_git_banner_state, get_latest_release_tag, get_available_skills)]) diff --git a/tests/hermes_cli/test_update_check.py b/tests/hermes_cli/test_update_check.py index e872988dd7..46b4eee937 100644 --- a/tests/hermes_cli/test_update_check.py +++ b/tests/hermes_cli/test_update_check.py @@ -98,10 +98,12 @@ def test_cache_is_daily_but_invalidated_when_head_moves(git_repo, monkeypatch): tip.assert_called_once() -def test_prefetch_non_blocking(): +def test_prefetch_non_blocking(monkeypatch): """prefetch_update_check() should return immediately without blocking.""" + # Reset module state; force the real (non-pytest) thread path. banner._update_result = None banner._update_check_done = threading.Event() + monkeypatch.setattr(banner, "_skip_background_prefetch", lambda: False) with patch.object(banner, "check_for_updates", return_value=5): start = time.monotonic() @@ -111,6 +113,39 @@ def test_prefetch_non_blocking(): assert banner._update_result == 5 +def test_prefetch_update_check_is_noop_under_pytest(): + """Under pytest the prefetch must NOT start the git-spawning daemon + thread: a process-wide ``patch("subprocess.run")`` in an unrelated test + can capture the thread's ``git fetch``/``rev-parse`` spawns, flaking the + unrelated test's call_args assertions (seen in + tests/tui_gateway/test_subprocess_encoding.py and + test_bot_relay_methods.py on CI, 2026-08-28).""" + banner._update_result = None + banner._update_check_done = threading.Event() + + before = {t.ident for t in threading.enumerate()} + with patch.object(banner, "check_for_updates") as mock_check: + banner.prefetch_update_check() + # The done event is set synchronously so get_update_result() callers + # don't burn their timeout waiting on a check that will never run. + assert banner._update_check_done.is_set() + mock_check.assert_not_called() + after = {t.ident for t in threading.enumerate()} + assert after <= before, "prefetch_update_check spawned a thread under pytest" + + +def test_prefetch_banner_data_is_noop_under_pytest(monkeypatch): + """Same stray-git-spawn class: prefetch_banner_data must not start its + daemon thread under pytest.""" + monkeypatch.setattr(banner, "_banner_data_prefetch_started", False) + with patch.object(banner, "get_git_banner_state") as mock_state: + banner.prefetch_banner_data() + # Give a hypothetical stray thread a beat to run — nothing should. + time.sleep(0.05) + mock_state.assert_not_called() + assert banner._banner_data_prefetch_started is True + + def test_upstream_main_sha_ls_remote_fallback_disables_git_prompts(monkeypatch): """When the API is unreachable the HTTPS ls-remote fallback must never inherit the terminal.""" monkeypatch.setattr(banner, "_github_branch_tip", lambda slug, branch: None)