From e5d9c447781fff4ff79e7ff59c6f2350e3462388 Mon Sep 17 00:00:00 2001 From: ethernet Date: Fri, 14 Aug 2026 07:09:03 -0400 Subject: [PATCH] fix(tests): dedupe toolcall regex, fix mocks returning wrong type there were two copies of the STALE_TOOL_CALL_MARKER_RE, and the reason was given that module A couldn't import B without causing some test failure. the tests that failed have been changed to mock get_hermes_home() correctly, as a Path, not a str, and the module import order has been reversed, so B now imports A to get the regex. --- agent/conversation_loop.py | 10 ++++------ hermes_state.py | 9 +++------ tests/tui_gateway/test_compaction_status.py | 3 ++- tests/tui_gateway/test_inline_rpc_gil_starvation.py | 3 ++- tests/tui_gateway/test_moa_reference_emit.py | 3 ++- tests/tui_gateway/test_protocol.py | 3 ++- tests/tui_gateway/test_review_summary_callback.py | 3 ++- tests/tui_gateway/test_slash_worker_profile_home.py | 3 --- tests/tui_gateway/test_subagent_child_mirror.py | 3 ++- tests/tui_gateway/test_subprocess_encoding.py | 3 ++- 10 files changed, 21 insertions(+), 22 deletions(-) diff --git a/agent/conversation_loop.py b/agent/conversation_loop.py index c05167d7685d0..6f50ac7c9e23a 100644 --- a/agent/conversation_loop.py +++ b/agent/conversation_loop.py @@ -58,11 +58,9 @@ from agent.message_sanitization import ( _strip_images_from_messages, _strip_non_ascii, ) -# Must mirror _STALE_TOOL_CALL_MARKER_RE in hermes_state.py — kept local -# to avoid importing hermes_state at module load time (its module-level -# DEFAULT_DB_PATH = get_hermes_home() / "state.db" breaks tests that -# monkeypatch get_hermes_home to return a str). -_STALE_MARKER_RE = re.compile(r"^\[[A-Za-z_][A-Za-z0-9_.-]*\]$") +# Matches a bare protocol/tool-name marker such as "[memory]" or "[skill_manage]". +STALE_TOOL_CALL_MARKER_RE = re.compile(r"^\[[A-Za-z_][A-Za-z0-9_.-]*\]$") + from agent.model_metadata import ( MINIMUM_CONTEXT_LENGTH, _estimate_tools_tokens_rough, @@ -6739,7 +6737,7 @@ def run_conversation( # post-tool fallback replay that token forever after compaction (#78148). if ( assistant_message.tool_calls - and _STALE_MARKER_RE.fullmatch(turn_content.strip()) + and STALE_TOOL_CALL_MARKER_RE.fullmatch(turn_content.strip()) ): logger.warning( "Discarding bare tool-call marker from assistant content: %s", diff --git a/hermes_state.py b/hermes_state.py index 1f44cfc184f5d..5142a48fb7227 100644 --- a/hermes_state.py +++ b/hermes_state.py @@ -41,6 +41,7 @@ from agent.skill_commands import ( SKILL_SCAFFOLD_SQL_LIKE, describe_skill_invocation, ) +from agent.conversation_loop import STALE_TOOL_CALL_MARKER_RE from hermes_constants import get_hermes_home from hermes_cli.sqlite_runtime import ( is_sqlite_wal_reset_vulnerable as _is_sqlite_wal_reset_vulnerable, @@ -663,10 +664,6 @@ def _strip_background_review_harness( return out -# Matches a bare protocol/tool-name marker such as "[memory]" or "[skill_manage]". -_STALE_TOOL_CALL_MARKER_RE = re.compile(r"^\[[A-Za-z_][A-Za-z0-9_.-]*\]$") - - def _is_stale_tool_call_marker_message(msg: Dict[str, Any]) -> bool: """True when ``msg`` is a persisted assistant turn whose content is a bare bracketed marker (e.g. ``[memory]``) left over from a tool-call turn. @@ -686,7 +683,7 @@ def _is_stale_tool_call_marker_message(msg: Dict[str, Any]) -> bool: content = msg.get("content") if not isinstance(content, str): return False - return bool(_STALE_TOOL_CALL_MARKER_RE.fullmatch(content.strip())) + return bool(STALE_TOOL_CALL_MARKER_RE.fullmatch(content.strip())) def _strip_stale_tool_call_markers( @@ -10651,7 +10648,7 @@ class SessionDB(SessionSearchMixin, SessionSchemaMixin, SessionPortabilityMixin) affected: List[int] = [] for row in cursor.fetchall(): content = row["content"] - if isinstance(content, str) and _STALE_TOOL_CALL_MARKER_RE.fullmatch(content.strip()): + if isinstance(content, str) and STALE_TOOL_CALL_MARKER_RE.fullmatch(content.strip()): affected.append(row["id"]) return affected diff --git a/tests/tui_gateway/test_compaction_status.py b/tests/tui_gateway/test_compaction_status.py index a676f70d4f04c..3c33974d444bc 100644 --- a/tests/tui_gateway/test_compaction_status.py +++ b/tests/tui_gateway/test_compaction_status.py @@ -7,6 +7,7 @@ silently reset mid-turn. """ from __future__ import annotations +from pathlib import Path import importlib @@ -23,7 +24,7 @@ def server(): "sys.modules", { "hermes_constants": MagicMock( - get_hermes_home=MagicMock(return_value="/tmp/hermes_test_compaction") + get_hermes_home=MagicMock(return_value=Path("/tmp/hermes_test_compaction")) ), "hermes_cli.env_loader": MagicMock(), "hermes_cli.banner": MagicMock(), diff --git a/tests/tui_gateway/test_inline_rpc_gil_starvation.py b/tests/tui_gateway/test_inline_rpc_gil_starvation.py index e6cfa141dfe3e..33be31ac89636 100644 --- a/tests/tui_gateway/test_inline_rpc_gil_starvation.py +++ b/tests/tui_gateway/test_inline_rpc_gil_starvation.py @@ -19,6 +19,7 @@ import json import sys import threading import time +from pathlib import Path from unittest.mock import MagicMock, patch import pytest @@ -38,7 +39,7 @@ def server(): # the whole test would poison modules first imported inside test bodies # (see tests/tui_gateway/test_protocol.py for the full rationale). with patch.dict("sys.modules", { - "hermes_constants": MagicMock(get_hermes_home=MagicMock(return_value="/tmp/hermes_test")), + "hermes_constants": MagicMock(get_hermes_home=MagicMock(return_value=Path("/tmp/hermes_test"))), "hermes_cli.env_loader": MagicMock(), "hermes_cli.banner": MagicMock(), "hermes_state": MagicMock(), diff --git a/tests/tui_gateway/test_moa_reference_emit.py b/tests/tui_gateway/test_moa_reference_emit.py index ee19b3f3b149f..ecf7b35a24732 100644 --- a/tests/tui_gateway/test_moa_reference_emit.py +++ b/tests/tui_gateway/test_moa_reference_emit.py @@ -8,6 +8,7 @@ thinking block tagged with its source model. """ from __future__ import annotations +from pathlib import Path from unittest.mock import MagicMock, patch @@ -22,7 +23,7 @@ def server(): "sys.modules", { "hermes_constants": MagicMock( - get_hermes_home=MagicMock(return_value="/tmp/hermes_test_moa_emit") + get_hermes_home=MagicMock(return_value=Path("/tmp/hermes_test_moa_emit")) ), "hermes_cli.env_loader": MagicMock(), "hermes_cli.banner": MagicMock(), diff --git a/tests/tui_gateway/test_protocol.py b/tests/tui_gateway/test_protocol.py index 08934c7201ad4..7e6a7f5b5ab86 100644 --- a/tests/tui_gateway/test_protocol.py +++ b/tests/tui_gateway/test_protocol.py @@ -6,6 +6,7 @@ import sys import threading import time import types +from pathlib import Path from unittest.mock import MagicMock, patch import pytest @@ -28,7 +29,7 @@ def server(): # (a fixed shared path) forever, leaking active-session registry entries # across every later test in the process. Scope the patch to the import. with patch.dict("sys.modules", { - "hermes_constants": MagicMock(get_hermes_home=MagicMock(return_value="/tmp/hermes_test")), + "hermes_constants": MagicMock(get_hermes_home=MagicMock(return_value=Path("/tmp/hermes_test"))), "hermes_cli.env_loader": MagicMock(), "hermes_cli.banner": MagicMock(), "hermes_state": MagicMock(), diff --git a/tests/tui_gateway/test_review_summary_callback.py b/tests/tui_gateway/test_review_summary_callback.py index 0f4130a604654..1e7083ec2c65f 100644 --- a/tests/tui_gateway/test_review_summary_callback.py +++ b/tests/tui_gateway/test_review_summary_callback.py @@ -10,6 +10,7 @@ transcript line. """ from __future__ import annotations +from pathlib import Path from unittest.mock import MagicMock, patch @@ -24,7 +25,7 @@ def server(): "sys.modules", { "hermes_constants": MagicMock( - get_hermes_home=MagicMock(return_value="/tmp/hermes_test_review_summary") + get_hermes_home=MagicMock(return_value=Path("/tmp/hermes_test_review_summary")) ), "hermes_cli.env_loader": MagicMock(), "hermes_cli.banner": MagicMock(), diff --git a/tests/tui_gateway/test_slash_worker_profile_home.py b/tests/tui_gateway/test_slash_worker_profile_home.py index 22dd576fe1877..b498a32404edd 100644 --- a/tests/tui_gateway/test_slash_worker_profile_home.py +++ b/tests/tui_gateway/test_slash_worker_profile_home.py @@ -12,9 +12,6 @@ import pytest def test_slash_worker_accepts_profile_home(): """_SlashWorker.__init__ accepts profile_home parameter.""" with patch.dict("sys.modules", { - # get_hermes_home() returns a Path in production; hermes_state.py's - # module-level DEFAULT_DB_PATH = get_hermes_home() / "state.db" does path - # division, so a str mock makes the import raise TypeError (str / str). "hermes_constants": MagicMock(get_hermes_home=MagicMock(return_value=Path("/tmp/hermes_test"))), }): with patch("subprocess.Popen") as mock_popen: diff --git a/tests/tui_gateway/test_subagent_child_mirror.py b/tests/tui_gateway/test_subagent_child_mirror.py index 5a94dc8ed0b18..79ecc439a5dfa 100644 --- a/tests/tui_gateway/test_subagent_child_mirror.py +++ b/tests/tui_gateway/test_subagent_child_mirror.py @@ -9,6 +9,7 @@ shows a real midstream turn instead of sitting silent until persistence. """ from __future__ import annotations +from pathlib import Path from unittest.mock import MagicMock, patch @@ -23,7 +24,7 @@ def server(): "sys.modules", { "hermes_constants": MagicMock( - get_hermes_home=MagicMock(return_value="/tmp/hermes_test_child_mirror") + get_hermes_home=MagicMock(return_value=Path("/tmp/hermes_test_child_mirror")) ), "hermes_cli.env_loader": MagicMock(), "hermes_cli.banner": MagicMock(), diff --git a/tests/tui_gateway/test_subprocess_encoding.py b/tests/tui_gateway/test_subprocess_encoding.py index 91cabb187d628..8ab03adb3c8da 100644 --- a/tests/tui_gateway/test_subprocess_encoding.py +++ b/tests/tui_gateway/test_subprocess_encoding.py @@ -13,6 +13,7 @@ the crash class cannot silently regress. """ from __future__ import annotations +from pathlib import Path import subprocess from unittest.mock import MagicMock, patch @@ -42,7 +43,7 @@ def test_slash_worker_popen_uses_utf8_replace(): """ with patch.dict("sys.modules", { "hermes_constants": MagicMock( - get_hermes_home=MagicMock(return_value="/tmp/hermes_test") + get_hermes_home=MagicMock(return_value=Path("/tmp/hermes_test")) ), }): with patch("subprocess.Popen") as mock_popen: