Merge origin/main, resolve conflicts (self._base_url_lower)

tests/hermes_cli/test_banner.py

@@ -0,0 +1,70 @@
+"""Tests for banner toolset name normalization and skin color usage."""
+
+from unittest.mock import patch
+
+from rich.console import Console
+
+import hermes_cli.banner as banner
+import model_tools
+import tools.mcp_tool
+
+
+def test_display_toolset_name_strips_legacy_suffix():
+    assert banner._display_toolset_name("homeassistant_tools") == "homeassistant"
+    assert banner._display_toolset_name("honcho_tools") == "honcho"
+    assert banner._display_toolset_name("web_tools") == "web"
+
+
+def test_display_toolset_name_preserves_clean_names():
+    assert banner._display_toolset_name("browser") == "browser"
+    assert banner._display_toolset_name("file") == "file"
+    assert banner._display_toolset_name("terminal") == "terminal"
+
+
+def test_display_toolset_name_handles_empty():
+    assert banner._display_toolset_name("") == "unknown"
+    assert banner._display_toolset_name(None) == "unknown"
+
+
+def test_build_welcome_banner_uses_normalized_toolset_names():
+    """Unavailable toolsets should not have '_tools' appended in banner output."""
+    with (
+        patch.object(
+            model_tools,
+            "check_tool_availability",
+            return_value=(
+                ["web"],
+                [
+                    {"name": "homeassistant", "tools": ["ha_call_service"]},
+                    {"name": "honcho", "tools": ["honcho_conclude"]},
+                ],
+            ),
+        ),
+        patch.object(banner, "get_available_skills", return_value={}),
+        patch.object(banner, "get_update_result", return_value=None),
+        patch.object(tools.mcp_tool, "get_mcp_status", return_value=[]),
+    ):
+        console = Console(
+            record=True, force_terminal=False, color_system=None, width=160
+        )
+        banner.build_welcome_banner(
+            console=console,
+            model="anthropic/test-model",
+            cwd="/tmp/project",
+            tools=[
+                {"function": {"name": "web_search"}},
+                {"function": {"name": "read_file"}},
+            ],
+            get_toolset_for_tool=lambda name: {
+                "web_search": "web_tools",
+                "read_file": "file",
+            }.get(name),
+        )
+
+    output = console.export_text()
+    assert "homeassistant:" in output
+    assert "honcho:" in output
+    assert "web:" in output
+    assert "homeassistant_tools:" not in output
+    assert "honcho_tools:" not in output
+    assert "web_tools:" not in output

tests/hermes_cli/test_banner_skills.py

@@ -0,0 +1,68 @@
+"""Tests for banner get_available_skills() — disabled and platform filtering."""
+
+from unittest.mock import patch
+
+import pytest
+
+
+_MOCK_SKILLS = [
+    {"name": "skill-a", "description": "A skill", "category": "tools"},
+    {"name": "skill-b", "description": "B skill", "category": "tools"},
+    {"name": "skill-c", "description": "C skill", "category": "creative"},
+]
+
+
+def test_get_available_skills_delegates_to_find_all_skills():
+    """get_available_skills should call _find_all_skills (which handles filtering)."""
+    with patch("tools.skills_tool._find_all_skills", return_value=list(_MOCK_SKILLS)):
+        from hermes_cli.banner import get_available_skills
+        result = get_available_skills()
+
+    assert "tools" in result
+    assert "creative" in result
+    assert sorted(result["tools"]) == ["skill-a", "skill-b"]
+    assert result["creative"] == ["skill-c"]
+
+
+def test_get_available_skills_excludes_disabled():
+    """Disabled skills should not appear in the banner count."""
+    # _find_all_skills already filters disabled skills, so if we give it
+    # a filtered list, get_available_skills should reflect that.
+    filtered = [s for s in _MOCK_SKILLS if s["name"] != "skill-b"]
+    with patch("tools.skills_tool._find_all_skills", return_value=filtered):
+        from hermes_cli.banner import get_available_skills
+        result = get_available_skills()
+
+    all_names = [n for names in result.values() for n in names]
+    assert "skill-b" not in all_names
+    assert "skill-a" in all_names
+    assert len(all_names) == 2
+
+
+def test_get_available_skills_empty_when_no_skills():
+    """No skills installed returns empty dict."""
+    with patch("tools.skills_tool._find_all_skills", return_value=[]):
+        from hermes_cli.banner import get_available_skills
+        result = get_available_skills()
+
+    assert result == {}
+
+
+def test_get_available_skills_handles_import_failure():
+    """If _find_all_skills import fails, return empty dict gracefully."""
+    with patch("tools.skills_tool._find_all_skills", side_effect=ImportError("boom")):
+        from hermes_cli.banner import get_available_skills
+        result = get_available_skills()
+
+    assert result == {}
+
+
+def test_get_available_skills_null_category_becomes_general():
+    """Skills with None category should be grouped under 'general'."""
+    skills = [{"name": "orphan-skill", "description": "No cat", "category": None}]
+    with patch("tools.skills_tool._find_all_skills", return_value=skills):
+        from hermes_cli.banner import get_available_skills
+        result = get_available_skills()
+
+    assert "general" in result
+    assert result["general"] == ["orphan-skill"]

tests/hermes_cli/test_gateway.py

@@ -1,6 +1,8 @@
 """Tests for hermes_cli.gateway."""
 
+import signal
 from types import SimpleNamespace
+from unittest.mock import patch, call
 
 import hermes_cli.gateway as gateway
 
@@ -169,3 +171,84 @@ def test_install_linux_gateway_from_setup_system_choice_as_root_installs(monkeyp
 
     assert (scope, did_install) == ("system", True)
     assert calls == [(True, True, "alice")]
+
+
+# ---------------------------------------------------------------------------
+# _wait_for_gateway_exit
+# ---------------------------------------------------------------------------
+
+
+class TestWaitForGatewayExit:
+    """PID-based wait with force-kill on timeout."""
+
+    def test_returns_immediately_when_no_pid(self, monkeypatch):
+        """If get_running_pid returns None, exit instantly."""
+        monkeypatch.setattr("gateway.status.get_running_pid", lambda: None)
+        # Should return without sleeping at all.
+        gateway._wait_for_gateway_exit(timeout=1.0, force_after=0.5)
+
+    def test_returns_when_process_exits_gracefully(self, monkeypatch):
+        """Process exits after a couple of polls — no SIGKILL needed."""
+        poll_count = 0
+
+        def mock_get_running_pid():
+            nonlocal poll_count
+            poll_count += 1
+            return 12345 if poll_count <= 2 else None
+
+        monkeypatch.setattr("gateway.status.get_running_pid", mock_get_running_pid)
+        monkeypatch.setattr("time.sleep", lambda _: None)
+
+        gateway._wait_for_gateway_exit(timeout=10.0, force_after=999.0)
+        # Should have polled until None was returned.
+        assert poll_count == 3
+
+    def test_force_kills_after_grace_period(self, monkeypatch):
+        """When the process doesn't exit, SIGKILL the saved PID."""
+        import time as _time
+
+        # Simulate monotonic time advancing past force_after
+        call_num = 0
+        def fake_monotonic():
+            nonlocal call_num
+            call_num += 1
+            # First two calls: initial deadline + force_deadline setup (time 0)
+            # Then each loop iteration advances time
+            return call_num * 2.0  # 2, 4, 6, 8, ...
+
+        kills = []
+        def mock_kill(pid, sig):
+            kills.append((pid, sig))
+
+        # get_running_pid returns the PID until kill is sent, then None
+        def mock_get_running_pid():
+            return None if kills else 42
+
+        monkeypatch.setattr("time.monotonic", fake_monotonic)
+        monkeypatch.setattr("time.sleep", lambda _: None)
+        monkeypatch.setattr("gateway.status.get_running_pid", mock_get_running_pid)
+        monkeypatch.setattr("os.kill", mock_kill)
+
+        gateway._wait_for_gateway_exit(timeout=10.0, force_after=5.0)
+        assert (42, signal.SIGKILL) in kills
+
+    def test_handles_process_already_gone_on_kill(self, monkeypatch):
+        """ProcessLookupError during SIGKILL is not fatal."""
+        import time as _time
+
+        call_num = 0
+        def fake_monotonic():
+            nonlocal call_num
+            call_num += 1
+            return call_num * 3.0  # Jump past force_after quickly
+
+        def mock_kill(pid, sig):
+            raise ProcessLookupError
+
+        monkeypatch.setattr("time.monotonic", fake_monotonic)
+        monkeypatch.setattr("time.sleep", lambda _: None)
+        monkeypatch.setattr("gateway.status.get_running_pid", lambda: 99)
+        monkeypatch.setattr("os.kill", mock_kill)
+
+        # Should not raise — ProcessLookupError means it's already gone.
+        gateway._wait_for_gateway_exit(timeout=10.0, force_after=2.0)