Merge remote-tracking branch 'origin/main' into codex/coding-tooling-optimization

This commit is contained in:
Xubin Ren
2026-05-21 14:44:56 +08:00
8 changed files with 836 additions and 84 deletions
+526
View File
@@ -1,6 +1,7 @@
import asyncio
import json
import tempfile
import time
from pathlib import Path
from types import SimpleNamespace
from unittest.mock import AsyncMock
@@ -374,6 +375,7 @@ async def test_send_uses_typing_start_and_cancel_when_ticket_available() -> None
channel._client = object()
channel._token = "token"
channel._context_tokens["wx-user"] = "ctx-typing"
channel._context_token_at["wx-user"] = time.time()
channel._send_text = AsyncMock()
channel._api_post = AsyncMock(
side_effect=[
@@ -402,6 +404,7 @@ async def test_send_still_sends_text_when_typing_ticket_missing() -> None:
channel._client = object()
channel._token = "token"
channel._context_tokens["wx-user"] = "ctx-no-ticket"
channel._context_token_at["wx-user"] = time.time()
channel._send_text = AsyncMock()
channel._api_post = AsyncMock(return_value={"ret": 1, "errmsg": "no config"})
@@ -1254,3 +1257,526 @@ async def test_send_text_succeeds_on_zero_errcode() -> None:
await channel._send_text("wx-user", "hello", "ctx-ok")
channel._api_post.assert_awaited_once()
@pytest.mark.asyncio
async def test_send_text_raises_on_nonzero_ret_even_when_errcode_zero() -> None:
"""_send_text must raise when the API returns ret != 0, even if errcode is 0.
The iLink API signals failure through either field. Checking only errcode
caused silent message drops (responses generated but never delivered).
"""
channel, _bus = _make_channel()
channel._client = object()
channel._token = "token"
channel._api_post = AsyncMock(
return_value={"ret": -100, "errcode": 0, "errmsg": "internal error"}
)
with pytest.raises(RuntimeError, match="WeChat send text error.*ret=-100.*errcode=0"):
await channel._send_text("wx-user", "hello", "ctx-ok")
channel._api_post.assert_awaited_once()
# ---------------------------------------------------------------------------
# Tests for _poll_once not silently dropping messages on processing errors
# ---------------------------------------------------------------------------
@pytest.mark.asyncio
async def test_poll_once_logs_exception_on_process_message_failure(monkeypatch) -> None:
"""When _process_message raises, _poll_once must log the error and continue
processing remaining messages instead of silently swallowing the exception."""
channel, _bus = _make_channel()
channel._client = SimpleNamespace(timeout=None)
channel._token = "token"
channel._get_updates_buf = "old-buf"
calls = []
logged_messages: list[str] = []
async def _failing_process(msg: dict) -> None:
calls.append(msg.get("message_id"))
if msg.get("message_id") == "msg-1":
raise RuntimeError("processing failed")
channel._process_message = _failing_process # type: ignore[method-assign]
monkeypatch.setattr(
channel.logger,
"exception",
lambda message, *args, **kwargs: logged_messages.append(str(message)),
)
channel._api_post = AsyncMock( # type: ignore[method-assign]
return_value={
"ret": 0,
"errcode": 0,
"get_updates_buf": "new-buf",
"msgs": [
{"message_id": "msg-1", "message_type": 1},
{"message_id": "msg-2", "message_type": 1},
],
}
)
await channel._poll_once()
# Both messages should have been attempted
assert calls == ["msg-1", "msg-2"]
# Buffer should still advance (already updated before processing)
assert channel._get_updates_buf == "new-buf"
# Error should be logged
assert any("Failed to process WeChat message" in m for m in logged_messages)
@pytest.mark.asyncio
async def test_poll_loop_logs_exception_and_continues_on_poll_failure(monkeypatch) -> None:
"""When _poll_once raises a non-timeout exception, the start() loop must log
the error and continue polling instead of exiting silently."""
channel, _bus = _make_channel()
channel._client = object()
channel._token = "token"
channel.config.token = "token" # skip QR login in start()
channel._running = True
call_count = 0
logged_messages: list[str] = []
async def _failing_poll() -> None:
nonlocal call_count
call_count += 1
if call_count == 1:
raise RuntimeError("poll exploded")
channel._running = False # Stop after second call
channel._poll_once = _failing_poll # type: ignore[method-assign]
monkeypatch.setattr(
channel.logger,
"exception",
lambda message, *args, **kwargs: logged_messages.append(str(message)),
)
# Use a tiny retry delay so the test finishes quickly
original_retry = weixin_mod.RETRY_DELAY_S
weixin_mod.RETRY_DELAY_S = 0.01
try:
await channel.start()
finally:
weixin_mod.RETRY_DELAY_S = original_retry
assert call_count == 2
assert any("WeChat poll loop error" in m for m in logged_messages)
# ---------------------------------------------------------------------------
# Tool-hint buffering
# ---------------------------------------------------------------------------
@pytest.mark.asyncio
async def test_buffer_single_tool_hint_not_sent_immediately() -> None:
channel, _bus = _make_channel()
channel._client = object()
channel._token = "token"
channel.send_tool_hints = True
channel._context_tokens["wx-user"] = "ctx-1"
channel._context_token_at["wx-user"] = time.time()
channel._send_text = AsyncMock()
await channel.send(
type(
"Msg",
(),
{
"chat_id": "wx-user",
"content": "Using tool",
"media": [],
"metadata": {"_progress": True, "_tool_hint": True},
},
)()
)
channel._send_text.assert_not_awaited()
assert channel._pending_tool_hints["wx-user"] == ["Using tool"]
@pytest.mark.asyncio
async def test_buffer_multiple_tool_hints_flushed_on_final_answer() -> None:
channel, _bus = _make_channel()
channel._client = object()
channel._token = "token"
channel.send_tool_hints = True
channel._context_tokens["wx-user"] = "ctx-1"
channel._context_token_at["wx-user"] = time.time()
channel._send_text = AsyncMock()
for hint in ["tool1", "tool2"]:
await channel.send(
type(
"Msg",
(),
{
"chat_id": "wx-user",
"content": hint,
"media": [],
"metadata": {"_progress": True, "_tool_hint": True},
},
)()
)
await channel.send(
type(
"Msg",
(),
{
"chat_id": "wx-user",
"content": "Done",
"media": [],
"metadata": {},
},
)()
)
assert channel._send_text.await_count == 2
channel._send_text.assert_any_await("wx-user", "tool1\n\ntool2", "ctx-1")
channel._send_text.assert_any_await("wx-user", "Done", "ctx-1")
assert "wx-user" not in channel._pending_tool_hints
@pytest.mark.asyncio
async def test_thought_progress_flushes_tool_hints() -> None:
"""Thoughts are visible progress messages and must act as separators,
flushing buffered tool hints before they are sent."""
channel, _bus = _make_channel()
channel._client = object()
channel._token = "token"
channel.send_tool_hints = True
channel._context_tokens["wx-user"] = "ctx-1"
channel._context_token_at["wx-user"] = time.time()
channel._send_text = AsyncMock()
# Buffer a tool hint
await channel.send(
type(
"Msg",
(),
{
"chat_id": "wx-user",
"content": "search 'foo'",
"media": [],
"metadata": {"_progress": True, "_tool_hint": True},
},
)()
)
# Send a thought — progress but not a tool_hint.
# It must act as a separator and flush the buffered hint.
await channel.send(
type(
"Msg",
(),
{
"chat_id": "wx-user",
"content": "Let me think...",
"media": [],
"metadata": {"_progress": True},
},
)()
)
# The buffered hint was flushed before the thought was sent.
channel._send_text.assert_any_await("wx-user", "search 'foo'", "ctx-1")
channel._send_text.assert_any_await("wx-user", "Let me think...", "ctx-1")
assert "wx-user" not in channel._pending_tool_hints
# Final answer arrives with nothing left to flush.
await channel.send(
type(
"Msg",
(),
{
"chat_id": "wx-user",
"content": "Done",
"media": [],
"metadata": {},
},
)()
)
assert channel._send_text.await_count == 3
channel._send_text.assert_any_await("wx-user", "Done", "ctx-1")
@pytest.mark.asyncio
async def test_reasoning_delta_does_not_flush_tool_hints() -> None:
"""Reasoning deltas are invisible in WeChat and must NOT flush buffered
tool hints — otherwise hints separated only by hidden reasoning would
fail to coalesce."""
channel, _bus = _make_channel()
channel._client = object()
channel._token = "token"
channel.send_tool_hints = True
channel._context_tokens["wx-user"] = "ctx-1"
channel._context_token_at["wx-user"] = time.time()
channel._send_text = AsyncMock()
# Buffer a tool hint
await channel.send(
type(
"Msg",
(),
{
"chat_id": "wx-user",
"content": "search 'foo'",
"media": [],
"metadata": {"_progress": True, "_tool_hint": True},
},
)()
)
# Send a reasoning delta — invisible in WeChat, must NOT flush
await channel.send(
type(
"Msg",
(),
{
"chat_id": "wx-user",
"content": "Thinking step 1...",
"media": [],
"metadata": {"_progress": True, "_reasoning_delta": True},
},
)()
)
# Reasoning is invisible; hint stays buffered, _send_text not called
channel._send_text.assert_not_awaited()
assert channel._pending_tool_hints["wx-user"] == ["search 'foo'"]
# Final answer flushes the buffered hint
await channel.send(
type(
"Msg",
(),
{
"chat_id": "wx-user",
"content": "Done",
"media": [],
"metadata": {},
},
)()
)
channel._send_text.assert_any_await("wx-user", "search 'foo'", "ctx-1")
channel._send_text.assert_any_await("wx-user", "Done", "ctx-1")
assert "wx-user" not in channel._pending_tool_hints
@pytest.mark.asyncio
async def test_empty_progress_message_does_not_flush_tool_hints() -> None:
"""Empty progress messages (e.g. after_iteration tool_events) have no
visible content and must NOT act as separators."""
channel, _bus = _make_channel()
channel._client = object()
channel._token = "token"
channel.send_tool_hints = True
channel._context_tokens["wx-user"] = "ctx-1"
channel._context_token_at["wx-user"] = time.time()
channel._send_text = AsyncMock()
# Buffer a tool hint
await channel.send(
type(
"Msg",
(),
{
"chat_id": "wx-user",
"content": "search 'foo'",
"media": [],
"metadata": {"_progress": True, "_tool_hint": True},
},
)()
)
# Send an empty progress message (no content, no media)
await channel.send(
type(
"Msg",
(),
{
"chat_id": "wx-user",
"content": "",
"media": [],
"metadata": {"_progress": True, "_tool_events": [{"phase": "end"}]},
},
)()
)
# Nothing should have been sent yet
channel._send_text.assert_not_awaited()
assert channel._pending_tool_hints["wx-user"] == ["search 'foo'"]
# Final answer flushes the buffered hint
await channel.send(
type(
"Msg",
(),
{
"chat_id": "wx-user",
"content": "Done",
"media": [],
"metadata": {},
},
)()
)
channel._send_text.assert_any_await("wx-user", "search 'foo'", "ctx-1")
channel._send_text.assert_any_await("wx-user", "Done", "ctx-1")
assert "wx-user" not in channel._pending_tool_hints
@pytest.mark.asyncio
async def test_buffer_flush_refreshes_context_token() -> None:
channel, _bus = _make_channel()
channel._client = object()
channel._token = "token"
channel.send_tool_hints = True
channel._context_tokens["wx-user"] = "ctx-old"
channel._context_token_at["wx-user"] = time.time()
channel._refresh_context_token_if_stale = AsyncMock(return_value="ctx-refreshed")
channel._send_text = AsyncMock()
await channel.send(
type(
"Msg",
(),
{
"chat_id": "wx-user",
"content": "hint",
"media": [],
"metadata": {"_progress": True, "_tool_hint": True},
},
)()
)
await channel.send(
type(
"Msg",
(),
{
"chat_id": "wx-user",
"content": "Done",
"media": [],
"metadata": {},
},
)()
)
assert channel._refresh_context_token_if_stale.await_count == 2
channel._refresh_context_token_if_stale.assert_any_await("wx-user", "ctx-old")
channel._send_text.assert_any_await("wx-user", "hint", "ctx-refreshed")
@pytest.mark.asyncio
async def test_buffer_flush_failure_does_not_block_final_answer() -> None:
channel, _bus = _make_channel()
channel._client = object()
channel._token = "token"
channel.send_tool_hints = True
channel._context_tokens["wx-user"] = "ctx-1"
channel._context_token_at["wx-user"] = time.time()
channel._send_text = AsyncMock(side_effect=[RuntimeError("boom"), None])
await channel.send(
type(
"Msg",
(),
{
"chat_id": "wx-user",
"content": "hint",
"media": [],
"metadata": {"_progress": True, "_tool_hint": True},
},
)()
)
await channel.send(
type(
"Msg",
(),
{
"chat_id": "wx-user",
"content": "Done",
"media": [],
"metadata": {},
},
)()
)
assert channel._send_text.await_count == 2
channel._send_text.assert_any_await("wx-user", "hint", "ctx-1")
channel._send_text.assert_any_await("wx-user", "Done", "ctx-1")
@pytest.mark.asyncio
async def test_buffer_flushed_on_stream_end() -> None:
channel, _bus = _make_channel()
channel._client = object()
channel._token = "token"
channel.send_tool_hints = True
channel._context_tokens["wx-user"] = "ctx-1"
channel._context_token_at["wx-user"] = time.time()
channel._send_text = AsyncMock()
await channel.send(
type(
"Msg",
(),
{
"chat_id": "wx-user",
"content": "hint",
"media": [],
"metadata": {"_progress": True, "_tool_hint": True},
},
)()
)
await channel.send_delta("wx-user", "", {"_stream_end": True})
channel._send_text.assert_awaited_once_with("wx-user", "hint", "ctx-1")
assert "wx-user" not in channel._pending_tool_hints
@pytest.mark.asyncio
async def test_stop_clears_buffer() -> None:
channel, _bus = _make_channel()
channel._pending_tool_hints["wx-user"] = ["hint1", "hint2"]
await channel.stop()
assert "wx-user" not in channel._pending_tool_hints
@pytest.mark.asyncio
async def test_send_tool_hints_false_drops_tool_hints() -> None:
channel, _bus = _make_channel()
channel._client = object()
channel._token = "token"
channel.send_tool_hints = False
channel._send_text = AsyncMock()
await channel.send(
type(
"Msg",
(),
{
"chat_id": "wx-user",
"content": "hint",
"media": [],
"metadata": {"_progress": True, "_tool_hint": True},
},
)()
)
channel._send_text.assert_not_awaited()
assert "wx-user" not in channel._pending_tool_hints
+15 -4
View File
@@ -1391,9 +1391,16 @@ def test_kimi_k25_no_extra_body_when_reasoning_effort_none() -> None:
def test_kimi_k25_thinking_enabled_with_openrouter_prefix() -> None:
"""OpenRouter-style model names like moonshotai/kimi-k2.5 must trigger thinking."""
"""OpenRouter-style model names like moonshotai/kimi-k2.5 must trigger thinking.
OR drops upstream-provider `thinking` fields, so the same intent also has
to go through OR's `reasoning.effort` shape (#3851 follow-up).
"""
kw = _build_kwargs_for("openrouter", "moonshotai/kimi-k2.5", reasoning_effort="medium")
assert kw.get("extra_body") == {"thinking": {"type": "enabled"}}
assert kw.get("extra_body") == {
"thinking": {"type": "enabled"},
"reasoning": {"effort": "medium"},
}
def test_kimi_k26_thinking_enabled() -> None:
@@ -1403,9 +1410,13 @@ def test_kimi_k26_thinking_enabled() -> None:
def test_kimi_k26_thinking_enabled_with_openrouter_prefix() -> None:
"""OpenRouter-style names like moonshotai/kimi-k2.6 must trigger thinking."""
"""OpenRouter-style names like moonshotai/kimi-k2.6 must trigger thinking
via both upstream `thinking` and OR's `reasoning.effort`."""
kw = _build_kwargs_for("openrouter", "moonshotai/kimi-k2.6", reasoning_effort="medium")
assert kw.get("extra_body") == {"thinking": {"type": "enabled"}}
assert kw.get("extra_body") == {
"thinking": {"type": "enabled"},
"reasoning": {"effort": "medium"},
}
def test_moonshot_kimi_k26_temperature_override() -> None:
+44 -9
View File
@@ -32,7 +32,7 @@ def _mimo_spec():
def _openrouter_spec():
"""Return the registered OpenRouter ProviderSpec (no thinking_style)."""
"""Return the registered OpenRouter ProviderSpec."""
specs = {s.name: s for s in PROVIDERS}
return specs["openrouter"]
@@ -77,6 +77,13 @@ def test_xiaomi_mimo_uses_thinking_type_style():
assert spec.default_api_base == "https://api.xiaomimimo.com/v1"
def test_openrouter_declares_gateway_reasoning_style():
"""OpenRouter uses its own reasoning.effort field for routed thinking models."""
spec = _openrouter_spec()
assert spec.thinking_style == ""
assert spec.gateway_reasoning_style == "reasoning_effort"
# ---------------------------------------------------------------------------
# _build_kwargs wire-format
# ---------------------------------------------------------------------------
@@ -142,9 +149,11 @@ def test_mimo_reasoning_effort_unset_preserves_provider_default():
def test_mimo_via_openrouter_reasoning_effort_none_disables_thinking():
"""OpenRouter routes MiMo as "xiaomi/mimo-v2.5-pro"; the openrouter spec
has no thinking_style, so the disable signal must come from the
model-name path (#3845)."""
"""OpenRouter routes MiMo as "xiaomi/mimo-v2.5-pro" and does NOT forward
extra_body.thinking to upstream, so a disable signal must also reach OR
in its own `reasoning.effort` shape. Verifies both the upstream-MiMo
payload (#3845) and the OR-native payload (#3851 follow-up) are sent.
"""
provider = _openrouter_provider("xiaomi/mimo-v2.5-pro")
kwargs = provider._build_kwargs(
messages=_simple_messages(),
@@ -152,11 +161,15 @@ def test_mimo_via_openrouter_reasoning_effort_none_disables_thinking():
temperature=0.7, reasoning_effort="none", tool_choice=None,
)
assert "reasoning_effort" not in kwargs
assert kwargs["extra_body"] == {"thinking": {"type": "disabled"}}
assert kwargs["extra_body"] == {
"thinking": {"type": "disabled"},
"reasoning": {"effort": "none"},
}
def test_mimo_via_openrouter_reasoning_effort_medium_enables_thinking():
"""Same as the direct path: any non-none/minimal effort enables thinking."""
"""Non-none/minimal effort enables thinking and the OR `reasoning.effort`
field mirrors the requested effort level."""
provider = _openrouter_provider("xiaomi/mimo-v2.5-pro")
kwargs = provider._build_kwargs(
messages=_simple_messages(),
@@ -164,7 +177,10 @@ def test_mimo_via_openrouter_reasoning_effort_medium_enables_thinking():
temperature=0.7, reasoning_effort="medium", tool_choice=None,
)
assert kwargs.get("reasoning_effort") == "medium"
assert kwargs["extra_body"] == {"thinking": {"type": "enabled"}}
assert kwargs["extra_body"] == {
"thinking": {"type": "enabled"},
"reasoning": {"effort": "medium"},
}
def test_mimo_via_openrouter_bare_slug_also_matches():
@@ -176,12 +192,16 @@ def test_mimo_via_openrouter_bare_slug_also_matches():
tools=None, model=None, max_tokens=100,
temperature=0.7, reasoning_effort="none", tool_choice=None,
)
assert kwargs["extra_body"] == {"thinking": {"type": "disabled"}}
assert kwargs["extra_body"] == {
"thinking": {"type": "disabled"},
"reasoning": {"effort": "none"},
}
def test_mimo_flash_via_openrouter_does_not_inject_thinking():
"""mimo-v2-flash has no thinking mode per Xiaomi docs; the allowlist
excludes it, so no thinking field should be injected on the gateway path."""
excludes it, so neither the upstream `thinking` field nor OR's
`reasoning.effort` should be injected on the gateway path."""
provider = _openrouter_provider("xiaomi/mimo-v2-flash")
kwargs = provider._build_kwargs(
messages=_simple_messages(),
@@ -200,3 +220,18 @@ def test_non_mimo_model_via_openrouter_unaffected():
temperature=0.7, reasoning_effort="none", tool_choice=None,
)
assert "extra_body" not in kwargs
def test_kimi_via_openrouter_also_injects_reasoning_effort():
"""Kimi has the same gateway problem as MiMo: OR drops the upstream
`thinking` field. The same OR-reasoning injection should fire."""
provider = _openrouter_provider("moonshotai/kimi-k2.5")
kwargs = provider._build_kwargs(
messages=_simple_messages(),
tools=None, model=None, max_tokens=100,
temperature=0.7, reasoning_effort="none", tool_choice=None,
)
assert kwargs["extra_body"] == {
"thinking": {"type": "disabled"},
"reasoning": {"effort": "none"},
}
+36
View File
@@ -3,6 +3,8 @@ import subprocess
import sys
from typing import Any
import pytest
from nanobot.agent.tools import (
ArraySchema,
IntegerSchema,
@@ -15,6 +17,7 @@ from nanobot.agent.tools import (
from nanobot.agent.tools.base import Tool
from nanobot.agent.tools.registry import ToolRegistry
from nanobot.agent.tools.shell import ExecTool
from nanobot.security.network import configure_ssrf_whitelist
class SampleTool(Tool):
@@ -218,6 +221,39 @@ def test_exec_extract_absolute_paths_ignores_relative_posix_segments() -> None:
assert "/bin/python" not in paths
def test_exec_extract_absolute_paths_ignores_urls() -> None:
cmd = 'curl -s -o /dev/null -w "%{http_code}" https://www.google.com'
paths = ExecTool._extract_absolute_paths(cmd)
assert paths == ["/dev/null"]
@pytest.mark.parametrize(
"command",
[
'curl -s -o /dev/null -w "%{http_code}" https://www.google.com',
'wget -q -O - http://example.com 2>&1 | head -c 100',
'python3 -c "import urllib.request; print(urllib.request.urlopen(\'http://example.com\').read()[:100])"',
],
)
def test_exec_guard_allows_public_urls(tmp_path, command: str) -> None:
tool = ExecTool(restrict_to_workspace=True)
error = tool._guard_command(command, str(tmp_path))
assert error is None
def test_exec_guard_allows_whitelisted_internal_urls(tmp_path) -> None:
configure_ssrf_whitelist(["10.10.10.0/24"])
try:
tool = ExecTool(restrict_to_workspace=True)
error = tool._guard_command(
'curl -s -H "Authorization: Bearer ..." http://10.10.10.3:8123/api/',
str(tmp_path),
)
assert error is None
finally:
configure_ssrf_whitelist([])
def test_exec_extract_absolute_paths_captures_posix_absolute_paths() -> None:
cmd = "cat /tmp/data.txt > /tmp/out.txt"
paths = ExecTool._extract_absolute_paths(cmd)