common/hermes
2
0
Fork 0
Code Issues Pull Requests Actions 1 Packages Projects Releases Wiki Activity

fix(ci): stabilize main test suite regressions (#17660)

Browse Source 
* fix: stabilize main test suite regressions

* test(agent): update MiniMax normalization expectation

* test: stabilize remaining CI assertions

* test: harden config helper monkeypatching

* test: harden CI-only assertions

* fix(agent): propagate fast streaming interrupts
This commit is contained in:
Stephen Schoettler
2026-04-29 23:18:55 -07:00
committed by GitHub
parent e7beaaf184
commit f73364b1c4
37 changed files with 450 additions and 127 deletions
Download Patch File Download Diff File Expand all files Collapse all files

17
tests/tools/test_approval_heartbeat.py
View File

@@ -131,15 +131,15 @@ class TestApprovalHeartbeat:
"""Polling slices don't delay responsiveness — resolve is near-instant."""
from tools.approval import (
check_all_command_guards,
has_blocking_approval,
register_gateway_notify,
resolve_gateway_approval,
)
register_gateway_notify(self.SESSION_KEY, lambda _payload: None)
start_time = time.monotonic()
result_holder: dict = {}
register_gateway_notify(self.SESSION_KEY, lambda _payload: None)
def _run_check():
result_holder["result"] = check_all_command_guards(
"rm -rf /tmp/nonexistent-fast-target", "local"
@@ -148,9 +148,18 @@ class TestApprovalHeartbeat:
thread = threading.Thread(target=_run_check, daemon=True)
thread.start()
# Wait until the worker has actually enqueued the approval. Resolving
# before registration is a test race, not a responsiveness signal.
deadline = time.monotonic() + 5.0
while time.monotonic() < deadline:
if has_blocking_approval(self.SESSION_KEY):
break
time.sleep(0.01)
assert has_blocking_approval(self.SESSION_KEY)
# Resolve almost immediately — the wait loop should return within
# its current 1s poll slice.
time.sleep(0.1)
start_time = time.monotonic()
resolve_gateway_approval(self.SESSION_KEY, "once")
thread.join(timeout=5)
elapsed = time.monotonic() - start_time

1
tests/tools/test_browser_orphan_reaper.py
View File

@@ -354,6 +354,7 @@ class TestOwnerPidCrossProcess:
monkeypatch.setattr(
bt, "_requires_real_termux_browser_install", lambda *a: False
)
monkeypatch.setattr(bt, "_chromium_installed", lambda: True)
monkeypatch.setattr(
bt, "_get_session_info",
lambda task_id: {"session_name": session_name},

20
tests/tools/test_clipboard.py
View File

@@ -205,24 +205,28 @@ class TestMacosOsascript:
class TestIsWsl:
def setup_method(self):
# _is_wsl is now hermes_constants.is_wsl — reset its cache
# _is_wsl is hermes_constants.is_wsl; reset the function's own module
# globals so this stays stable even if hermes_constants was imported
# through a different module object earlier in a large xdist run.
import hermes_constants
hermes_constants._wsl_detected = None
_is_wsl.__globals__["_wsl_detected"] = None
def teardown_method(self):
# Reset again after the test so we don't leak a cached value
# (True/False) into whichever test the xdist worker runs next.
import hermes_constants
hermes_constants._wsl_detected = None
_is_wsl.__globals__["_wsl_detected"] = None
def test_wsl2_detected(self):
content = "Linux version 5.15.0 (microsoft-standard-WSL2)"
with patch("builtins.open", mock_open(read_data=content)):
with patch.dict(_is_wsl.__globals__, {"open": mock_open(read_data=content)}):
assert _is_wsl() is True
def test_wsl1_detected(self):
content = "Linux version 4.4.0-microsoft-standard"
with patch("builtins.open", mock_open(read_data=content)):
with patch.dict(_is_wsl.__globals__, {"open": mock_open(read_data=content)}):
assert _is_wsl() is True
def test_regular_linux(self):
@@ -234,20 +238,20 @@ class TestIsWsl:
# short-circuits on the cache. setup_method resets, so we just
# need to be sure the patched `open` is actually reached.
content = "Linux version 6.14.0-37-generic (buildd@lcy02-amd64-049)"
with patch("hermes_constants.open", mock_open(read_data=content), create=True):
with patch.dict(_is_wsl.__globals__, {"open": mock_open(read_data=content)}):
assert _is_wsl() is False
def test_proc_version_missing(self):
with patch("hermes_constants.open", side_effect=FileNotFoundError, create=True):
with patch.dict(_is_wsl.__globals__, {"open": MagicMock(side_effect=FileNotFoundError)}):
assert _is_wsl() is False
def test_result_is_cached(self):
import hermes_constants
content = "Linux version 5.15.0 (microsoft-standard-WSL2)"
with patch("hermes_constants.open", mock_open(read_data=content), create=True) as m:
opener = mock_open(read_data=content)
with patch.dict(_is_wsl.__globals__, {"open": opener}):
assert _is_wsl() is True
assert _is_wsl() is True
m.assert_called_once() # only read once
opener.assert_called_once() # only read once
# ── WSL (powershell.exe) ────────────────────────────────────────────────

67
tests/tools/test_local_interrupt_cleanup.py
View File

@@ -16,6 +16,7 @@ import signal
import subprocess
import threading
import time
from types import SimpleNamespace
import pytest
@@ -37,6 +38,58 @@ def _pgid_still_alive(pgid: int) -> bool:
return False
def _process_group_snapshot(pgid: int) -> str:
"""Return a process-table snapshot for diagnostics."""
return subprocess.run(
["ps", "-o", "pid,ppid,pgid,stat,cmd", "-g", str(pgid)],
capture_output=True,
text=True,
check=False,
).stdout.strip()
def _wait_for_pgid_exit(pgid: int, timeout: float = 10.0) -> bool:
"""Wait for a process group to disappear under loaded xdist hosts."""
deadline = time.monotonic() + timeout
while time.monotonic() < deadline:
if not _pgid_still_alive(pgid):
return True
time.sleep(0.1)
return not _pgid_still_alive(pgid)
def test_kill_process_uses_cached_pgid_if_wrapper_already_exited(monkeypatch):
"""If the shell wrapper exits before cleanup, still kill its process group.
Without the cached pgid fallback, ``os.getpgid(proc.pid)`` raises for the
dead wrapper and cleanup falls back to ``proc.kill()``, which cannot reach
orphaned grandchildren still running in the original process group.
"""
env = object.__new__(LocalEnvironment)
proc = SimpleNamespace(
pid=12345,
_hermes_pgid=67890,
poll=lambda: 0,
kill=lambda: None,
)
killpg_calls = []
def fake_getpgid(_pid):
raise ProcessLookupError
def fake_killpg(pgid, sig):
killpg_calls.append((pgid, sig))
if sig == 0:
raise ProcessLookupError
monkeypatch.setattr(os, "getpgid", fake_getpgid)
monkeypatch.setattr(os, "killpg", fake_killpg)
env._kill_process(proc)
assert killpg_calls == [(67890, signal.SIGTERM), (67890, 0)]
def test_wait_for_process_kills_subprocess_on_keyboardinterrupt():
"""When KeyboardInterrupt arrives mid-poll, the subprocess group must be
killed before the exception is re-raised."""
@@ -118,19 +171,15 @@ def test_wait_for_process_kills_subprocess_on_keyboardinterrupt():
assert not t.is_alive(), "worker didn't exit within 5 s of the interrupt"
# The critical assertion: the subprocess GROUP must be dead. Not
# just the bash wrapper — the 'sleep 30' child too.
# Give the SIGTERM+1s wait+SIGKILL escalation a moment to complete.
deadline = time.monotonic() + 3.0
while time.monotonic() < deadline:
if not _pgid_still_alive(pgid):
break
time.sleep(0.1)
assert not _pgid_still_alive(pgid), (
# just the bash wrapper — the 'sleep 30' child too. Under xdist load,
# process-group disappearance can lag briefly after the worker exits,
# especially if the process is already dying or waiting to be reaped.
assert _wait_for_pgid_exit(pgid), (
f"subprocess group {pgid} is STILL ALIVE after worker received "
f"KeyboardInterrupt — orphan bug regressed. This is the "
f"sleep-300-survives-SIGTERM scenario from Physikal's Apr 2026 "
f"report. See tools/environments/base.py _wait_for_process "
f"except-block."
f"except-block.\n{_process_group_snapshot(pgid)}"
)
# And the worker should have observed the KeyboardInterrupt (i.e.
# it re-raised cleanly, not silently swallowed).

9
tests/tools/test_tirith_security.py
View File

@@ -997,10 +997,13 @@ class TestHermesHomeIsolation:
assert "hermes_test" in hermes_home, "Should point to test temp dir"
def test_get_hermes_home_fallback(self):
"""Without HERMES_HOME set, falls back to ~/.hermes."""
"""Without HERMES_HOME set, falls back to the active OS home."""
from tools.tirith_security import _get_hermes_home
with patch.dict(os.environ, {}, clear=True):
# Remove HERMES_HOME entirely
# Remove HERMES_HOME entirely. With HOME also absent, expanduser
# falls back to the account database; compute expected under the
# same environment instead of after patch.dict restores HOME.
os.environ.pop("HERMES_HOME", None)
expected = os.path.join(os.path.expanduser("~"), ".hermes")
result = _get_hermes_home()
assert result == os.path.join(os.path.expanduser("~"), ".hermes")
assert result == expected

6
tests/tools/test_transcription.py
View File

@@ -36,14 +36,16 @@ class TestGetProvider:
monkeypatch.setenv("VOICE_TOOLS_OPENAI_KEY", "sk-test")
monkeypatch.delenv("GROQ_API_KEY", raising=False)
with patch("tools.transcription_tools._HAS_FASTER_WHISPER", False), \
patch("tools.transcription_tools._HAS_OPENAI", True):
patch("tools.transcription_tools._HAS_OPENAI", True), \
patch("tools.transcription_tools._has_local_command", return_value=False):
from tools.transcription_tools import _get_provider
assert _get_provider({"provider": "local"}) == "none"
def test_local_nothing_available(self, monkeypatch):
monkeypatch.delenv("VOICE_TOOLS_OPENAI_KEY", raising=False)
with patch("tools.transcription_tools._HAS_FASTER_WHISPER", False), \
patch("tools.transcription_tools._HAS_OPENAI", False):
patch("tools.transcription_tools._HAS_OPENAI", False), \
patch("tools.transcription_tools._has_local_command", return_value=False):
from tools.transcription_tools import _get_provider
assert _get_provider({"provider": "local"}) == "none"

22
tests/tools/test_transcription_dotenv_fallback.py
View File

@@ -36,6 +36,28 @@ class TestProviderSelectionGate:
configure ``{"enabled": True, "provider": ...}`` for explicit tests.
"""
def test_import_after_config_env_patch_uses_restored_dotenv_loader(self):
"""Importing STT while hermes_cli.config.get_env_value is patched must
not freeze that temporary helper into this module forever.
"""
import importlib
import hermes_cli.config as config_mod
from tools import transcription_tools as tt
with pytest.MonkeyPatch.context() as mp:
mp.setattr(config_mod, "get_env_value", lambda name, default=None: "")
tt = importlib.reload(tt)
try:
with patch.object(tt, "_HAS_FASTER_WHISPER", False), \
patch.object(tt, "_HAS_OPENAI", True), \
patch.object(tt, "_has_local_command", return_value=False), \
patch("hermes_cli.config.load_env",
return_value={"GROQ_API_KEY": "dotenv-secret"}):
assert tt._get_provider({"enabled": True, "provider": "groq"}) == "groq"
finally:
importlib.reload(tt)
def test_explicit_groq_sees_dotenv(self):
from tools import transcription_tools as tt

15
tests/tools/test_transcription_tools.py
View File

@@ -758,19 +758,12 @@ class TestValidateAudioFileEdgeCases:
f = tmp_path / "test.ogg"
f.write_bytes(b"data")
from tools.transcription_tools import _validate_audio_file
real_stat = f.stat()
call_count = 0
def stat_side_effect(*args, **kwargs):
nonlocal call_count
call_count += 1
# First calls are from exists() and is_file(), let them pass
if call_count <= 2:
return real_stat
raise OSError("disk error")
with patch("pathlib.Path.stat", side_effect=stat_side_effect):
with patch("pathlib.Path.exists", return_value=True), \
patch("pathlib.Path.is_file", return_value=True), \
patch("pathlib.Path.stat", side_effect=OSError("disk error")):
result = _validate_audio_file(str(f))
assert result is not None
assert "Failed to access" in result["error"]

39
tests/tools/test_tts_dotenv_fallback.py
View File

@@ -174,6 +174,45 @@ class TestRegressionGuard:
key while ``os.environ`` does not.
"""
def test_import_after_config_env_patch_uses_restored_dotenv_loader(self, tmp_path, monkeypatch):
"""Importing TTS while hermes_cli.config.get_env_value is patched must
not freeze that temporary helper into this module forever.
"""
import importlib
import hermes_cli.config as config_mod
from tools import tts_tool
monkeypatch.delenv("MINIMAX_API_KEY", raising=False)
with pytest.MonkeyPatch.context() as mp:
mp.setattr(config_mod, "get_env_value", lambda name: "")
tts_tool = importlib.reload(tts_tool)
try:
captured: dict = {}
def fake_post(url, **kwargs):
captured["headers"] = kwargs.get("headers", {})
response = MagicMock()
response.json.return_value = {
"data": {"audio": b"\x00".hex()},
"base_resp": {"status_code": 0},
}
response.raise_for_status = MagicMock()
return response
with patch(
"hermes_cli.config.load_env",
return_value={"MINIMAX_API_KEY": "dotenv-secret"},
), patch("requests.post", side_effect=fake_post):
tts_tool._generate_minimax_tts(
"hi", str(tmp_path / "out.mp3"), {}
)
assert captured["headers"]["Authorization"] == "Bearer dotenv-secret"
finally:
importlib.reload(tts_tool)
def test_minimax_missing_when_only_in_dotenv_before_fix(self, tmp_path, monkeypatch):
from tools import tts_tool