diff --git a/.mailmap b/.mailmap index 0c385c5183..3f093fb5ab 100644 --- a/.mailmap +++ b/.mailmap @@ -105,3 +105,4 @@ tesseracttars-creator xinbenlv SaulJWu angelos +MestreY0d4-Uninter <241404605+MestreY0d4-Uninter@users.noreply.github.com> diff --git a/agent/credential_pool.py b/agent/credential_pool.py index 8a2fecf5d6..e1307e51f8 100644 --- a/agent/credential_pool.py +++ b/agent/credential_pool.py @@ -1162,6 +1162,7 @@ def _seed_from_singletons(provider: str, entries: List[PooledCredential]) -> Tup if token: source_name = "gh_cli" if "gh" in source.lower() else f"env:{source}" active_sources.add(source_name) + pconfig = PROVIDER_REGISTRY.get(provider) changed |= _upsert_entry( entries, provider, @@ -1170,6 +1171,7 @@ def _seed_from_singletons(provider: str, entries: List[PooledCredential]) -> Tup "source": source_name, "auth_type": AUTH_TYPE_API_KEY, "access_token": token, + "base_url": pconfig.inference_base_url if pconfig else "", "label": source, }, ) diff --git a/gateway/platforms/telegram.py b/gateway/platforms/telegram.py index 112b232d0a..0806362b36 100644 --- a/gateway/platforms/telegram.py +++ b/gateway/platforms/telegram.py @@ -163,6 +163,15 @@ class TelegramAdapter(BasePlatformAdapter): # Approval button state: message_id → session_key self._approval_state: Dict[int, str] = {} + @staticmethod + def _is_callback_user_authorized(user_id: str) -> bool: + """Return whether a Telegram inline-button caller may perform gated actions.""" + allowed_csv = os.getenv("TELEGRAM_ALLOWED_USERS", "").strip() + if not allowed_csv: + return True + allowed_ids = {uid.strip() for uid in allowed_csv.split(",") if uid.strip()} + return "*" in allowed_ids or user_id in allowed_ids + def _fallback_ips(self) -> list[str]: """Return validated fallback IPs from config (populated by _apply_env_overrides).""" configured = self.config.extra.get("fallback_ips", []) if getattr(self.config, "extra", None) else [] @@ -1440,12 +1449,9 @@ class TelegramAdapter(BasePlatformAdapter): # Only authorized users may click approval buttons. caller_id = str(getattr(query.from_user, "id", "")) - allowed_csv = os.getenv("TELEGRAM_ALLOWED_USERS", "").strip() - if allowed_csv: - allowed_ids = {uid.strip() for uid in allowed_csv.split(",") if uid.strip()} - if "*" not in allowed_ids and caller_id not in allowed_ids: - await query.answer(text="⛔ You are not authorized to approve commands.") - return + if not self._is_callback_user_authorized(caller_id): + await query.answer(text="⛔ You are not authorized to approve commands.") + return session_key = self._approval_state.pop(approval_id, None) if not session_key: @@ -1490,6 +1496,10 @@ class TelegramAdapter(BasePlatformAdapter): if not data.startswith("update_prompt:"): return answer = data.split(":", 1)[1] # "y" or "n" + caller_id = str(getattr(query.from_user, "id", "")) + if not self._is_callback_user_authorized(caller_id): + await query.answer(text="⛔ You are not authorized to answer update prompts.") + return await query.answer(text=f"Sent '{answer}' to the update process.") # Edit the message to show the choice and remove buttons label = "Yes" if answer == "y" else "No" diff --git a/gateway/run.py b/gateway/run.py index d9a2bfd2f9..71a02efd8d 100644 --- a/gateway/run.py +++ b/gateway/run.py @@ -486,9 +486,14 @@ def _parse_session_key(session_key: str) -> "dict | None": """Parse a session key into its component parts. Session keys follow the format - ``agent:main:{platform}:{chat_type}:{chat_id}[:{thread_id}[:{user_id}]]``. + ``agent:main:{platform}:{chat_type}:{chat_id}[:{extra}...]``. Returns a dict with ``platform``, ``chat_type``, ``chat_id``, and optionally ``thread_id`` keys, or None if the key doesn't match. + + The 6th element is only returned as ``thread_id`` for chat types where + it is unambiguous (``dm`` and ``thread``). For group/channel sessions + the suffix may be a user_id (per-user isolation) rather than a + thread_id, so we leave ``thread_id`` out to avoid mis-routing. """ parts = session_key.split(":") if len(parts) >= 5 and parts[0] == "agent" and parts[1] == "main": @@ -497,7 +502,7 @@ def _parse_session_key(session_key: str) -> "dict | None": "chat_type": parts[3], "chat_id": parts[4], } - if len(parts) > 5: + if len(parts) > 5 and parts[3] in ("dm", "thread"): result["thread_id"] = parts[5] return result return None diff --git a/gateway/stream_consumer.py b/gateway/stream_consumer.py index e6d96c802d..50321a3036 100644 --- a/gateway/stream_consumer.py +++ b/gateway/stream_consumer.py @@ -609,12 +609,15 @@ class GatewayStreamConsumer: content=text, metadata=self.metadata, ) - if result.success: - self._already_sent = True - return True + # Note: do NOT set _already_sent = True here. + # Commentary messages are interim status updates (e.g. "Using browser + # tool..."), not the final response. Setting already_sent would cause + # the final response to be incorrectly suppressed when there are + # multiple tool calls. See: https://github.com/NousResearch/hermes-agent/issues/10454 + return result.success except Exception as e: logger.error("Commentary send error: %s", e) - return False + return False async def _send_or_edit(self, text: str) -> bool: """Send or edit the streaming message. diff --git a/hermes_cli/auth.py b/hermes_cli/auth.py index 636416a974..1fd9a303c2 100644 --- a/hermes_cli/auth.py +++ b/hermes_cli/auth.py @@ -2384,7 +2384,7 @@ def get_api_key_provider_status(provider_id: str) -> Dict[str, Any]: if pconfig.base_url_env_var: env_url = os.getenv(pconfig.base_url_env_var, "").strip() - if provider_id == "kimi-coding": + if provider_id in ("kimi-coding", "kimi-coding-cn"): base_url = _resolve_kimi_base_url(api_key, pconfig.inference_base_url, env_url) elif env_url: base_url = env_url @@ -2470,7 +2470,7 @@ def resolve_api_key_provider_credentials(provider_id: str) -> Dict[str, Any]: if pconfig.base_url_env_var: env_url = os.getenv(pconfig.base_url_env_var, "").strip() - if provider_id == "kimi-coding": + if provider_id in ("kimi-coding", "kimi-coding-cn"): base_url = _resolve_kimi_base_url(api_key, pconfig.inference_base_url, env_url) elif provider_id == "zai": base_url = _resolve_zai_base_url(api_key, pconfig.inference_base_url, env_url) diff --git a/hermes_cli/commands.py b/hermes_cli/commands.py index 2d0107803b..a6cc66d6ee 100644 --- a/hermes_cli/commands.py +++ b/hermes_cli/commands.py @@ -168,7 +168,7 @@ COMMAND_REGISTRY: list[CommandDef] = [ # Exit CommandDef("quit", "Exit the CLI", "Exit", - cli_only=True, aliases=("exit", "q")), + cli_only=True, aliases=("exit",)), ] diff --git a/hermes_cli/doctor.py b/hermes_cli/doctor.py index b89a804091..69a24aff5f 100644 --- a/hermes_cli/doctor.py +++ b/hermes_cli/doctor.py @@ -814,7 +814,8 @@ def run_doctor(args): ("Vercel AI Gateway", ("AI_GATEWAY_API_KEY",), "https://ai-gateway.vercel.sh/v1/models", "AI_GATEWAY_BASE_URL", True), ("Kilo Code", ("KILOCODE_API_KEY",), "https://api.kilo.ai/api/gateway/models", "KILOCODE_BASE_URL", True), ("OpenCode Zen", ("OPENCODE_ZEN_API_KEY",), "https://opencode.ai/zen/v1/models", "OPENCODE_ZEN_BASE_URL", True), - ("OpenCode Go", ("OPENCODE_GO_API_KEY",), "https://opencode.ai/zen/go/v1/models", "OPENCODE_GO_BASE_URL", True), + # OpenCode Go has no shared /models endpoint; skip the health check. + ("OpenCode Go", ("OPENCODE_GO_API_KEY",), None, "OPENCODE_GO_BASE_URL", False), ] for _pname, _env_vars, _default_url, _base_env, _supports_health_check in _apikey_providers: _key = "" diff --git a/hermes_cli/models.py b/hermes_cli/models.py index fd8bff83ee..b4eb174d8c 100644 --- a/hermes_cli/models.py +++ b/hermes_cli/models.py @@ -526,7 +526,7 @@ CANONICAL_PROVIDERS: list[ProviderEntry] = [ ProviderEntry("deepseek", "DeepSeek", "DeepSeek (DeepSeek-V3, R1, coder — direct API)"), ProviderEntry("xai", "xAI", "xAI (Grok models — direct API)"), ProviderEntry("zai", "Z.AI / GLM", "Z.AI / GLM (Zhipu AI direct API)"), - ProviderEntry("kimi-coding", "Kimi / Moonshot", "Kimi / Moonshot (Moonshot AI direct API)"), + ProviderEntry("kimi-coding", "Kimi / Kimi Coding Plan", "Kimi Coding Plan (api.kimi.com) & Moonshot API"), ProviderEntry("kimi-coding-cn", "Kimi / Moonshot (China)", "Kimi / Moonshot China (Moonshot CN direct API)"), ProviderEntry("minimax", "MiniMax", "MiniMax (global direct API)"), ProviderEntry("minimax-cn", "MiniMax (China)", "MiniMax China (domestic direct API)"), diff --git a/run_agent.py b/run_agent.py index bde3a680ff..7e1668ef37 100644 --- a/run_agent.py +++ b/run_agent.py @@ -457,6 +457,15 @@ def _sanitize_messages_non_ascii(messages: list) -> bool: if sanitized != fn_args: fn["arguments"] = sanitized found = True + # Sanitize any additional top-level string fields (e.g. reasoning_content) + for key, value in msg.items(): + if key in {"content", "name", "tool_calls", "role"}: + continue + if isinstance(value, str): + sanitized = _strip_non_ascii(value) + if sanitized != value: + msg[key] = sanitized + found = True return found @@ -705,12 +714,13 @@ class AIAgent: except Exception: pass - # GPT-5.x models require the Responses API path — they are rejected - # on /v1/chat/completions by both OpenAI and OpenRouter. Also - # auto-upgrade for direct OpenAI URLs (api.openai.com) since all - # newer tool-calling models prefer Responses there. - # ACP runtimes are excluded: CopilotACPClient handles its own - # routing and does not implement the Responses API surface. + # GPT-5.x models usually require the Responses API path, but some + # providers have exceptions (for example Copilot's gpt-5-mini still + # uses chat completions). Also auto-upgrade for direct OpenAI URLs + # (api.openai.com) since all newer tool-calling models prefer + # Responses there. ACP runtimes are excluded: CopilotACPClient + # handles its own routing and does not implement the Responses API + # surface. if ( self.api_mode == "chat_completions" and self.provider != "copilot-acp" @@ -718,7 +728,10 @@ class AIAgent: and not str(self.base_url or "").lower().startswith("acp+tcp://") and ( self._is_direct_openai_url() - or self._model_requires_responses_api(self.model) + or self._provider_model_requires_responses_api( + self.model, + provider=self.provider, + ) ) ): self.api_mode = "codex_responses" @@ -1951,6 +1964,24 @@ class AIAgent: m = m.rsplit("/", 1)[-1] return m.startswith("gpt-5") + @staticmethod + def _provider_model_requires_responses_api( + model: str, + *, + provider: Optional[str] = None, + ) -> bool: + """Return True when this provider/model pair should use Responses API.""" + normalized_provider = (provider or "").strip().lower() + if normalized_provider == "copilot": + try: + from hermes_cli.models import _should_use_copilot_responses_api + return _should_use_copilot_responses_api(model) + except Exception: + # Fall back to the generic GPT-5 rule if Copilot-specific + # logic is unavailable for any reason. + pass + return AIAgent._model_requires_responses_api(model) + def _max_tokens_param(self, value: int) -> dict: """Return the correct max tokens kwarg for the current provider. @@ -5721,9 +5752,13 @@ class AIAgent: fb_api_mode = "anthropic_messages" elif self._is_direct_openai_url(fb_base_url): fb_api_mode = "codex_responses" - elif self._model_requires_responses_api(fb_model): - # GPT-5.x models need Responses API on every provider - # (OpenRouter, Copilot, direct OpenAI, etc.) + elif self._provider_model_requires_responses_api( + fb_model, + provider=fb_provider, + ): + # GPT-5.x models usually need Responses API, but keep + # provider-specific exceptions like Copilot gpt-5-mini on + # chat completions. fb_api_mode = "codex_responses" old_model = self.model @@ -9108,7 +9143,19 @@ class AIAgent: # ASCII codec: the system encoding can't handle # non-ASCII characters at all. Sanitize all # non-ASCII content from messages/tool schemas and retry. + # Sanitize both the canonical `messages` list and + # `api_messages` (the API-copy built before the retry + # loop, which may contain extra fields like + # reasoning_content that are not in `messages`). _messages_sanitized = _sanitize_messages_non_ascii(messages) + if isinstance(api_messages, list): + _sanitize_messages_non_ascii(api_messages) + # Also sanitize the last api_kwargs if already built, + # so a leftover non-ASCII value in a transformed field + # (e.g. extra_body, reasoning_content) doesn't survive + # into the next attempt via _build_api_kwargs cache paths. + if isinstance(api_kwargs, dict): + _sanitize_structure_non_ascii(api_kwargs) _prefill_sanitized = False if isinstance(getattr(self, "prefill_messages", None), list): _prefill_sanitized = _sanitize_messages_non_ascii(self.prefill_messages) @@ -9166,22 +9213,34 @@ class AIAgent: force=True, ) - if ( + # Always retry on ASCII codec detection — + # _force_ascii_payload guarantees the full + # api_kwargs payload is sanitized on the + # next iteration (line ~8475). Even when + # per-component checks above find nothing + # (e.g. non-ASCII only in api_messages' + # reasoning_content), the flag catches it. + # Bounded by _unicode_sanitization_passes < 2. + self._unicode_sanitization_passes += 1 + _any_sanitized = ( _messages_sanitized or _prefill_sanitized or _tools_sanitized or _system_sanitized or _headers_sanitized or _credential_sanitized - ): - self._unicode_sanitization_passes += 1 + ) + if _any_sanitized: self._vprint( f"{self.log_prefix}⚠️ System encoding is ASCII — stripped non-ASCII characters from request payload. Retrying...", force=True, ) - continue - # Nothing to sanitize in any payload component. - # Fall through to normal error path. + else: + self._vprint( + f"{self.log_prefix}⚠️ System encoding is ASCII — enabling full-payload sanitization for retry...", + force=True, + ) + continue status_code = getattr(api_error, "status_code", None) error_context = self._extract_api_error_context(api_error) diff --git a/scripts/release.py b/scripts/release.py index 73d663e55a..b40fd7c239 100755 --- a/scripts/release.py +++ b/scripts/release.py @@ -63,6 +63,7 @@ AUTHOR_MAP = { "70424851+insecurejezza@users.noreply.github.com": "insecurejezza", "259807879+Bartok9@users.noreply.github.com": "Bartok9", "268667990+Roy-oss1@users.noreply.github.com": "Roy-oss1", + "241404605+MestreY0d4-Uninter@users.noreply.github.com": "MestreY0d4-Uninter", # contributors (manual mapping from git names) "dmayhem93@gmail.com": "dmahan93", "samherring99@gmail.com": "samherring99", @@ -75,8 +76,11 @@ AUTHOR_MAP = { "abdullahfarukozden@gmail.com": "Farukest", "lovre.pesut@gmail.com": "rovle", "hakanerten02@hotmail.com": "teyrebaz33", + "ruzzgarcn@gmail.com": "Ruzzgar", "alireza78.crypto@gmail.com": "alireza78a", "brooklyn.bb.nicholson@gmail.com": "brooklynnicholson", + "4317663+helix4u@users.noreply.github.com": "helix4u", + "331214+counterposition@users.noreply.github.com": "counterposition", "gpickett00@gmail.com": "gpickett00", "mcosma@gmail.com": "wakamex", "clawdia.nash@proton.me": "clawdia-nash", @@ -191,7 +195,7 @@ AUTHOR_MAP = { "yangzhi.see@gmail.com": "SeeYangZhi", "yongtenglei@gmail.com": "yongtenglei", "young@YoungdeMacBook-Pro.local": "YoungYang963", - "ysfalweshcan@gmail.com": "Awsh1", + "ysfalweshcan@gmail.com": "Junass1", "ysfwaxlycan@gmail.com": "WAXLYY", "yusufalweshdemir@gmail.com": "Dusk1e", "zhouboli@gmail.com": "zhouboli", diff --git a/tests/agent/test_credential_pool.py b/tests/agent/test_credential_pool.py index ca232c12f9..c11782f690 100644 --- a/tests/agent/test_credential_pool.py +++ b/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): diff --git a/tests/gateway/test_background_process_notifications.py b/tests/gateway/test_background_process_notifications.py index eabf92be63..7351854a2c 100644 --- a/tests/gateway/test_background_process_notifications.py +++ b/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(): diff --git a/tests/gateway/test_telegram_approval_buttons.py b/tests/gateway/test_telegram_approval_buttons.py index 98d3cdc312..ec5bbd47ee 100644 --- a/tests/gateway/test_telegram_approval_buttons.py +++ b/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" diff --git a/tests/hermes_cli/test_commands.py b/tests/hermes_cli/test_commands.py index 09a830b040..c14e60224f 100644 --- a/tests/hermes_cli/test_commands.py +++ b/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" diff --git a/tests/hermes_cli/test_doctor.py b/tests/hermes_cli/test_doctor.py index dd15336f60..948cafaf71 100644 --- a/tests/hermes_cli/test_doctor.py +++ b/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) diff --git a/tests/run_agent/test_run_agent_codex_responses.py b/tests/run_agent/test_run_agent_codex_responses.py index 2b22955653..4ff00018d2 100644 --- a/tests/run_agent/test_run_agent_codex_responses.py +++ b/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( diff --git a/tests/run_agent/test_unicode_ascii_codec.py b/tests/run_agent/test_unicode_ascii_codec.py index a8a52c34ae..04b5e4043c 100644 --- a/tests/run_agent/test_unicode_ascii_codec.py +++ b/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"] diff --git a/tests/tools/test_terminal_tool.py b/tests/tools/test_terminal_tool.py index 42ed693a2e..dd2a674187 100644 --- a/tests/tools/test_terminal_tool.py +++ b/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") diff --git a/tools/terminal_tool.py b/tools/terminal_tool.py index 55f4c10a81..1aa2665222 100644 --- a/tools/terminal_tool.py +++ b/tools/terminal_tool.py @@ -148,9 +148,10 @@ def _check_all_guards(command: str, env_type: str) -> dict: # Allowlist: characters that can legitimately appear in directory paths. -# Covers alphanumeric, path separators, tilde, dot, hyphen, underscore, space, -# plus, at, equals, and comma. Everything else is rejected. -_WORKDIR_SAFE_RE = re.compile(r'^[A-Za-z0-9/_\-.~ +@=,]+$') +# Covers alphanumeric, path separators, Windows drive/UNC separators, tilde, +# dot, hyphen, underscore, space, plus, at, equals, and comma. Everything +# else is rejected. +_WORKDIR_SAFE_RE = re.compile(r'^[A-Za-z0-9/\\:_\-.~ +@=,]+$') def _validate_workdir(workdir: str) -> str | None: diff --git a/tools/tirith_security.py b/tools/tirith_security.py index b3055944e3..44710ee608 100644 --- a/tools/tirith_security.py +++ b/tools/tirith_security.py @@ -360,7 +360,21 @@ def _install_tirith(*, log_failures: bool = True) -> tuple[str | None, str]: src = os.path.join(tmpdir, "tirith") dest = os.path.join(_hermes_bin_dir(), "tirith") - shutil.move(src, dest) + try: + shutil.move(src, dest) + except OSError: + # Cross-device move (common in Docker, NFS): shutil.move() falls + # back to copy2 + unlink, but copy2's metadata step can raise + # PermissionError. Use plain copy + manual chmod instead. + try: + shutil.copy(src, dest) + except OSError: + # Clean up partial dest to prevent a non-executable retry loop + try: + os.unlink(dest) + except OSError: + pass + return None, "cross_device_copy_failed" os.chmod(dest, os.stat(dest).st_mode | stat.S_IXUSR | stat.S_IXGRP | stat.S_IXOTH) verification = "cosign + SHA-256" if cosign_verified else "SHA-256 only"