Merge branch 'main' of github.com:NousResearch/hermes-agent into feat/ink-refactor

tests/agent/test_credential_pool.py

@@ -1091,6 +1091,7 @@ def test_load_pool_seeds_copilot_via_gh_auth_token(tmp_path, monkeypatch):
     assert len(entries) == 1
     assert entries[0].source == "gh_cli"
     assert entries[0].access_token == "gho_fake_token_abc123"
+    assert entries[0].base_url == "https://api.githubcopilot.com"
 
 
 def test_load_pool_does_not_seed_copilot_when_no_token(tmp_path, monkeypatch):

tests/gateway/test_background_process_notifications.py

@@ -383,15 +383,27 @@ def test_parse_session_key_valid():
 
 
 def test_parse_session_key_with_extra_parts():
-    """Thread ID (6th part) is extracted; further parts are ignored."""
+    """6th part in a group key may be a user_id, not a thread_id — omit it."""
     result = _parse_session_key("agent:main:discord:group:chan123:thread456")
-    assert result == {"platform": "discord", "chat_type": "group", "chat_id": "chan123", "thread_id": "thread456"}
+    assert result == {"platform": "discord", "chat_type": "group", "chat_id": "chan123"}
 
 
 def test_parse_session_key_with_user_id_part():
-    """7th part (user_id) is ignored — only up to thread_id is extracted."""
-    result = _parse_session_key("agent:main:telegram:group:chat1:thread42:user99")
-    assert result == {"platform": "telegram", "chat_type": "group", "chat_id": "chat1", "thread_id": "thread42"}
+    """Group keys with per-user isolation have user_id as 6th part — don't return as thread_id."""
+    result = _parse_session_key("agent:main:telegram:group:chat1:user99")
+    assert result == {"platform": "telegram", "chat_type": "group", "chat_id": "chat1"}
+
+
+def test_parse_session_key_dm_with_thread():
+    """DM keys use parts[5] as thread_id unambiguously."""
+    result = _parse_session_key("agent:main:telegram:dm:chat1:topic42")
+    assert result == {"platform": "telegram", "chat_type": "dm", "chat_id": "chat1", "thread_id": "topic42"}
+
+
+def test_parse_session_key_thread_chat_type():
+    """Thread-typed keys use parts[5] as thread_id unambiguously."""
+    result = _parse_session_key("agent:main:discord:thread:chan1:thread99")
+    assert result == {"platform": "discord", "chat_type": "thread", "chat_id": "chan1", "thread_id": "thread99"}
 
 
 def test_parse_session_key_too_short():

tests/gateway/test_telegram_approval_buttons.py

@@ -263,7 +263,7 @@ class TestTelegramApprovalCallback:
         mock_resolve.assert_not_called()
 
     @pytest.mark.asyncio
-    async def test_update_prompt_callback_not_affected(self):
+    async def test_update_prompt_callback_not_affected(self, tmp_path):
         """Ensure update prompt callbacks still work."""
         adapter = _make_adapter()
 
@@ -281,11 +281,63 @@ class TestTelegramApprovalCallback:
         context = MagicMock()
 
         with patch("tools.approval.resolve_gateway_approval") as mock_resolve:
-            with patch("hermes_constants.get_hermes_home", return_value=Path("/tmp/test")):
-                try:
+            with patch("hermes_constants.get_hermes_home", return_value=tmp_path):
+                with patch.dict(os.environ, {"TELEGRAM_ALLOWED_USERS": ""}):
                     await adapter._handle_callback_query(update, context)
-                except Exception:
-                    pass  # May fail on file write, that's fine
 
         # Should NOT have triggered approval resolution
         mock_resolve.assert_not_called()
+        assert (tmp_path / ".update_response").read_text() == "y"
+
+    @pytest.mark.asyncio
+    async def test_update_prompt_callback_rejects_unauthorized_user(self, tmp_path):
+        """Update prompt buttons should honor TELEGRAM_ALLOWED_USERS."""
+        adapter = _make_adapter()
+
+        query = AsyncMock()
+        query.data = "update_prompt:y"
+        query.message = MagicMock()
+        query.message.chat_id = 12345
+        query.from_user = MagicMock()
+        query.from_user.id = 222
+        query.answer = AsyncMock()
+        query.edit_message_text = AsyncMock()
+
+        update = MagicMock()
+        update.callback_query = query
+        context = MagicMock()
+
+        with patch("hermes_constants.get_hermes_home", return_value=tmp_path):
+            with patch.dict(os.environ, {"TELEGRAM_ALLOWED_USERS": "111"}):
+                await adapter._handle_callback_query(update, context)
+
+        query.answer.assert_called_once()
+        assert "not authorized" in query.answer.call_args[1]["text"].lower()
+        query.edit_message_text.assert_not_called()
+        assert not (tmp_path / ".update_response").exists()
+
+    @pytest.mark.asyncio
+    async def test_update_prompt_callback_allows_authorized_user(self, tmp_path):
+        """Allowed Telegram users can still answer update prompt buttons."""
+        adapter = _make_adapter()
+
+        query = AsyncMock()
+        query.data = "update_prompt:n"
+        query.message = MagicMock()
+        query.message.chat_id = 12345
+        query.from_user = MagicMock()
+        query.from_user.id = 111
+        query.answer = AsyncMock()
+        query.edit_message_text = AsyncMock()
+
+        update = MagicMock()
+        update.callback_query = query
+        context = MagicMock()
+
+        with patch("hermes_constants.get_hermes_home", return_value=tmp_path):
+            with patch.dict(os.environ, {"TELEGRAM_ALLOWED_USERS": "111"}):
+                await adapter._handle_callback_query(update, context)
+
+        query.answer.assert_called_once()
+        query.edit_message_text.assert_called_once()
+        assert (tmp_path / ".update_response").read_text() == "n"

tests/hermes_cli/test_commands.py

@@ -99,7 +99,7 @@ class TestResolveCommand:
     def test_alias_resolves_to_canonical(self):
         assert resolve_command("bg").name == "background"
         assert resolve_command("reset").name == "new"
-        assert resolve_command("q").name == "quit"
+        assert resolve_command("q").name == "queue"
         assert resolve_command("exit").name == "quit"
         assert resolve_command("gateway").name == "platforms"
         assert resolve_command("set-home").name == "sethome"

tests/hermes_cli/test_doctor.py

@@ -343,3 +343,57 @@ def test_run_doctor_kimi_cn_env_is_detected_and_probe_is_null_safe(monkeypatch,
     assert "Kimi / Moonshot (China)" in out
     assert "str expected, not NoneType" not in out
     assert any(url == "https://api.moonshot.cn/v1/models" for url, _, _ in calls)
+
+
+@pytest.mark.parametrize("base_url", [None, "https://opencode.ai/zen/go/v1"])
+def test_run_doctor_opencode_go_skips_invalid_models_probe(monkeypatch, tmp_path, base_url):
+    home = tmp_path / ".hermes"
+    home.mkdir(parents=True, exist_ok=True)
+    (home / "config.yaml").write_text("memory: {}\n", encoding="utf-8")
+    (home / ".env").write_text("OPENCODE_GO_API_KEY=***\n", encoding="utf-8")
+    project = tmp_path / "project"
+    project.mkdir(exist_ok=True)
+
+    monkeypatch.setattr(doctor_mod, "HERMES_HOME", home)
+    monkeypatch.setattr(doctor_mod, "PROJECT_ROOT", project)
+    monkeypatch.setattr(doctor_mod, "_DHH", str(home))
+    monkeypatch.setenv("OPENCODE_GO_API_KEY", "sk-test")
+    if base_url:
+        monkeypatch.setenv("OPENCODE_GO_BASE_URL", base_url)
+    else:
+        monkeypatch.delenv("OPENCODE_GO_BASE_URL", raising=False)
+
+    fake_model_tools = types.SimpleNamespace(
+        check_tool_availability=lambda *a, **kw: ([], []),
+        TOOLSET_REQUIREMENTS={},
+    )
+    monkeypatch.setitem(sys.modules, "model_tools", fake_model_tools)
+
+    try:
+        from hermes_cli import auth as _auth_mod
+        monkeypatch.setattr(_auth_mod, "get_nous_auth_status", lambda: {})
+        monkeypatch.setattr(_auth_mod, "get_codex_auth_status", lambda: {})
+    except ImportError:
+        pass
+
+    calls = []
+
+    def fake_get(url, headers=None, timeout=None):
+        calls.append((url, headers, timeout))
+        return types.SimpleNamespace(status_code=200)
+
+    import httpx
+    monkeypatch.setattr(httpx, "get", fake_get)
+
+    import io, contextlib
+    buf = io.StringIO()
+    with contextlib.redirect_stdout(buf):
+        doctor_mod.run_doctor(Namespace(fix=False))
+    out = buf.getvalue()
+
+    assert any(
+        "OpenCode Go" in line and "(key configured)" in line
+        for line in out.splitlines()
+    )
+    assert not any(url == "https://opencode.ai/zen/go/v1/models" for url, _, _ in calls)
+    assert not any("opencode" in url.lower() and "models" in url.lower() for url, _, _ in calls)

tests/run_agent/test_run_agent_codex_responses.py

@@ -259,6 +259,23 @@ def test_copilot_acp_stays_on_chat_completions_for_gpt_5_models(monkeypatch):
     assert agent.api_mode == "chat_completions"
 
 
+def test_copilot_gpt_5_mini_stays_on_chat_completions(monkeypatch):
+    _patch_agent_bootstrap(monkeypatch)
+    agent = run_agent.AIAgent(
+        model="gpt-5-mini",
+        base_url="https://api.githubcopilot.com",
+        provider="copilot",
+        api_key="gh-token",
+        api_mode="chat_completions",
+        quiet_mode=True,
+        max_iterations=1,
+        skip_context_files=True,
+        skip_memory=True,
+    )
+    assert agent.provider == "copilot"
+    assert agent.api_mode == "chat_completions"
+
+
 def test_build_api_kwargs_codex(monkeypatch):
     agent = _build_agent(monkeypatch)
     kwargs = agent._build_api_kwargs(

tests/run_agent/test_unicode_ascii_codec.py

@@ -294,3 +294,79 @@ class TestApiKeyClientSync:
 
         assert agent.api_key == "sk-proj-"
         assert agent.client is None  # should not have been touched
+
+
+class TestApiMessagesAndApiKwargsSanitized:
+    """Regression tests for #6843 follow-up: api_messages and api_kwargs must
+    be sanitized alongside messages during ASCII-codec recovery.
+
+    The original fix only sanitized the canonical `messages` list.
+    api_messages is a separate API-copy built before the retry loop; it may
+    carry extra fields (reasoning_content, extra_body) with non-ASCII chars
+    that are not present in `messages`.  Without sanitizing api_messages and
+    api_kwargs, the retry still raises UnicodeEncodeError even after the
+    'System encoding is ASCII — stripped...' log line appears.
+    """
+
+    def test_api_messages_with_reasoning_content_is_sanitized(self):
+        """api_messages may contain reasoning_content not in messages."""
+        api_messages = [
+            {"role": "system", "content": "You are helpful."},
+            {"role": "user", "content": "hi"},
+            {
+                "role": "assistant",
+                "content": "Sure!",
+                # reasoning_content is injected by the API-copy builder and
+                # is NOT present in the canonical messages list
+                "reasoning_content": "Let me think \xab step by step \xbb",
+            },
+        ]
+        found = _sanitize_messages_non_ascii(api_messages)
+        assert found is True
+        assert "\xab" not in api_messages[2]["reasoning_content"]
+        assert "\xbb" not in api_messages[2]["reasoning_content"]
+
+    def test_api_kwargs_with_non_ascii_extra_body_is_sanitized(self):
+        """api_kwargs may contain non-ASCII in extra_body or other fields."""
+        api_kwargs = {
+            "model": "glm-5.1",
+            "messages": [{"role": "user", "content": "ok"}],
+            "extra_body": {
+                "system": "Think carefully \u2192 answer",
+            },
+        }
+        found = _sanitize_structure_non_ascii(api_kwargs)
+        assert found is True
+        assert "\u2192" not in api_kwargs["extra_body"]["system"]
+
+    def test_messages_clean_but_api_messages_dirty_both_get_sanitized(self):
+        """Even when canonical messages are clean, api_messages may be dirty."""
+        messages = [{"role": "user", "content": "hello"}]
+        api_messages = [
+            {"role": "user", "content": "hello"},
+            {
+                "role": "assistant",
+                "content": "ok",
+                "reasoning_content": "step \xab done",
+            },
+        ]
+        # messages sanitize returns False (nothing to clean)
+        assert _sanitize_messages_non_ascii(messages) is False
+        # api_messages sanitize must catch the dirty reasoning_content
+        assert _sanitize_messages_non_ascii(api_messages) is True
+        assert "\xab" not in api_messages[1]["reasoning_content"]
+
+    def test_reasoning_field_in_canonical_messages_is_sanitized(self):
+        """The canonical messages list stores reasoning as 'reasoning', not
+        'reasoning_content'.  The extra-fields loop must catch it."""
+        messages = [
+            {"role": "user", "content": "hello"},
+            {
+                "role": "assistant",
+                "content": "ok",
+                "reasoning": "Let me think \xab carefully \xbb",
+            },
+        ]
+        assert _sanitize_messages_non_ascii(messages) is True
+        assert "\xab" not in messages[1]["reasoning"]
+        assert "\xbb" not in messages[1]["reasoning"]

tests/tools/test_terminal_tool.py

@@ -88,3 +88,18 @@ def test_cached_sudo_password_is_used_when_env_is_unset(monkeypatch):
 
     assert transformed == "echo ok && sudo -S -p '' whoami"
     assert sudo_stdin == "cached-pass\n"
+
+
+def test_validate_workdir_allows_windows_drive_paths():
+    assert terminal_tool._validate_workdir(r"C:\Users\Alice\project") is None
+    assert terminal_tool._validate_workdir("C:/Users/Alice/project") is None
+
+
+def test_validate_workdir_allows_windows_unc_paths():
+    assert terminal_tool._validate_workdir(r"\\server\share\project") is None
+
+
+def test_validate_workdir_blocks_shell_metacharacters_in_windows_paths():
+    assert terminal_tool._validate_workdir(r"C:\Users\Alice\project; rm -rf /")
+    assert terminal_tool._validate_workdir(r"C:\Users\Alice\project$(whoami)")
+    assert terminal_tool._validate_workdir("C:\\Users\\Alice\\project\nwhoami")