fix: resolve 7 failing CI tests (#3936)

1. matrix voice: _on_room_message_media unconditionally overwrote
   media_urls with the image cache path (always None for non-images),
   wiping the locally-cached voice path. Now only overrides when
   cached_path is truthy.

2. cli_tools_command: /tools disable no longer prompts for confirmation
   (input() removed in earlier commit to fix TUI hang), but tests still
   expected the old Y/N prompt flow. Updated tests to match current
   behavior (direct apply + session reset).

3. slack app_mention: connect() was refactored for multi-workspace
   (creates AsyncWebClient per token), but test only mocked the old
   self._app.client path. Added AsyncWebClient and acquire_scoped_lock
   mocks.

4. website_policy: module-level _cached_policy from earlier tests caused
   fast-path return of None. Added invalidate_cache() before assertion.

5. codex 401 refresh: already passing on current main (fixed by
   intervening commit).
This commit is contained in:
Teknium
2026-03-30 08:10:14 -07:00
committed by GitHub
parent 0b0c1b326c
commit 1e896b0251
4 changed files with 45 additions and 18 deletions

View File

@@ -904,8 +904,9 @@ class MatrixAdapter(BasePlatformAdapter):
thread_id=thread_id, thread_id=thread_id,
) )
# Use cached local path for images, HTTP URL for other media types # Use cached local path for images (voice messages already handled above).
media_urls = [cached_path] if cached_path else ([http_url] if http_url else None) if cached_path:
media_urls = [cached_path]
media_types = [media_type] if media_urls else None media_types = [media_type] if media_urls else None
msg_event = MessageEvent( msg_event = MessageEvent(

View File

@@ -126,9 +126,20 @@ class TestAppMentionHandler:
"user": "testbot", "user": "testbot",
}) })
# Mock AsyncWebClient so multi-workspace auth_test is awaitable
mock_web_client = AsyncMock()
mock_web_client.auth_test = AsyncMock(return_value={
"user_id": "U_BOT",
"user": "testbot",
"team_id": "T_FAKE",
"team": "FakeTeam",
})
with patch.object(_slack_mod, "AsyncApp", return_value=mock_app), \ with patch.object(_slack_mod, "AsyncApp", return_value=mock_app), \
patch.object(_slack_mod, "AsyncWebClient", return_value=mock_web_client), \
patch.object(_slack_mod, "AsyncSocketModeHandler", return_value=MagicMock()), \ patch.object(_slack_mod, "AsyncSocketModeHandler", return_value=MagicMock()), \
patch.dict(os.environ, {"SLACK_APP_TOKEN": "xapp-fake"}), \ patch.dict(os.environ, {"SLACK_APP_TOKEN": "xapp-fake"}), \
patch("gateway.status.acquire_scoped_lock", return_value=(True, None)), \
patch("asyncio.create_task"): patch("asyncio.create_task"):
asyncio.run(adapter.connect()) asyncio.run(adapter.connect())

View File

@@ -60,34 +60,43 @@ class TestToolsSlashList:
class TestToolsSlashDisableWithReset: class TestToolsSlashDisableWithReset:
def test_disable_confirms_then_resets_session(self): def test_disable_applies_directly_and_resets_session(self):
"""Disable applies immediately (no confirmation prompt) and resets session."""
cli_obj = _make_cli(["web", "memory"]) cli_obj = _make_cli(["web", "memory"])
with patch("hermes_cli.tools_config.load_config", with patch("hermes_cli.tools_config.load_config",
return_value={"platform_toolsets": {"cli": ["web", "memory"]}}), \ return_value={"platform_toolsets": {"cli": ["web", "memory"]}}), \
patch("hermes_cli.tools_config.save_config"), \ patch("hermes_cli.tools_config.save_config"), \
patch("hermes_cli.tools_config._get_platform_tools", return_value={"memory"}), \ patch("hermes_cli.tools_config._get_platform_tools", return_value={"memory"}), \
patch("hermes_cli.config.load_config", return_value={}), \ patch("hermes_cli.config.load_config", return_value={}), \
patch.object(cli_obj, "new_session") as mock_reset, \ patch.object(cli_obj, "new_session") as mock_reset:
patch("builtins.input", return_value="y"):
cli_obj._handle_tools_command("/tools disable web") cli_obj._handle_tools_command("/tools disable web")
mock_reset.assert_called_once() mock_reset.assert_called_once()
assert "web" not in cli_obj.enabled_toolsets assert "web" not in cli_obj.enabled_toolsets
def test_disable_cancelled_does_not_reset(self): def test_disable_does_not_prompt_for_confirmation(self):
"""Disable no longer uses input() — it applies directly."""
cli_obj = _make_cli(["web", "memory"]) cli_obj = _make_cli(["web", "memory"])
with patch.object(cli_obj, "new_session") as mock_reset, \ with patch("hermes_cli.tools_config.load_config",
patch("builtins.input", return_value="n"): return_value={"platform_toolsets": {"cli": ["web", "memory"]}}), \
patch("hermes_cli.tools_config.save_config"), \
patch("hermes_cli.tools_config._get_platform_tools", return_value={"memory"}), \
patch("hermes_cli.config.load_config", return_value={}), \
patch.object(cli_obj, "new_session"), \
patch("builtins.input") as mock_input:
cli_obj._handle_tools_command("/tools disable web") cli_obj._handle_tools_command("/tools disable web")
mock_reset.assert_not_called() mock_input.assert_not_called()
# Toolsets unchanged
assert cli_obj.enabled_toolsets == {"web", "memory"}
def test_disable_eof_cancels(self): def test_disable_always_resets_session(self):
"""Even without a confirmation prompt, disable always resets the session."""
cli_obj = _make_cli(["web", "memory"]) cli_obj = _make_cli(["web", "memory"])
with patch.object(cli_obj, "new_session") as mock_reset, \ with patch("hermes_cli.tools_config.load_config",
patch("builtins.input", side_effect=EOFError): return_value={"platform_toolsets": {"cli": ["web", "memory"]}}), \
patch("hermes_cli.tools_config.save_config"), \
patch("hermes_cli.tools_config._get_platform_tools", return_value={"memory"}), \
patch("hermes_cli.config.load_config", return_value={}), \
patch.object(cli_obj, "new_session") as mock_reset:
cli_obj._handle_tools_command("/tools disable web") cli_obj._handle_tools_command("/tools disable web")
mock_reset.assert_not_called() mock_reset.assert_called_once()
def test_disable_missing_name_prints_usage(self, capsys): def test_disable_missing_name_prints_usage(self, capsys):
cli_obj = _make_cli() cli_obj = _make_cli()
@@ -101,15 +110,15 @@ class TestToolsSlashDisableWithReset:
class TestToolsSlashEnableWithReset: class TestToolsSlashEnableWithReset:
def test_enable_confirms_then_resets_session(self): def test_enable_applies_directly_and_resets_session(self):
"""Enable applies immediately (no confirmation prompt) and resets session."""
cli_obj = _make_cli(["memory"]) cli_obj = _make_cli(["memory"])
with patch("hermes_cli.tools_config.load_config", with patch("hermes_cli.tools_config.load_config",
return_value={"platform_toolsets": {"cli": ["memory"]}}), \ return_value={"platform_toolsets": {"cli": ["memory"]}}), \
patch("hermes_cli.tools_config.save_config"), \ patch("hermes_cli.tools_config.save_config"), \
patch("hermes_cli.tools_config._get_platform_tools", return_value={"memory", "web"}), \ patch("hermes_cli.tools_config._get_platform_tools", return_value={"memory", "web"}), \
patch("hermes_cli.config.load_config", return_value={}), \ patch("hermes_cli.config.load_config", return_value={}), \
patch.object(cli_obj, "new_session") as mock_reset, \ patch.object(cli_obj, "new_session") as mock_reset:
patch("builtins.input", return_value="y"):
cli_obj._handle_tools_command("/tools enable web") cli_obj._handle_tools_command("/tools enable web")
mock_reset.assert_called_once() mock_reset.assert_called_once()
assert "web" in cli_obj.enabled_toolsets assert "web" in cli_obj.enabled_toolsets

View File

@@ -259,6 +259,12 @@ def test_check_website_access_uses_dynamic_hermes_home(monkeypatch, tmp_path):
monkeypatch.setenv("HERMES_HOME", str(hermes_home)) monkeypatch.setenv("HERMES_HOME", str(hermes_home))
# Invalidate the module-level cache so the new HERMES_HOME is picked up.
# A prior test may have cached a default policy (enabled=False) under the
# old HERMES_HOME set by the autouse _isolate_hermes_home fixture.
from tools.website_policy import invalidate_cache
invalidate_cache()
blocked = check_website_access("https://dynamic.example/path") blocked = check_website_access("https://dynamic.example/path")
assert blocked is not None assert blocked is not None