From 4e06c00b461b0ff12bc788c8d8df99255f7f7813 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E7=AB=A5=E5=A4=A9=E7=AB=8B?= Date: Mon, 23 Mar 2026 21:22:31 +0800 Subject: [PATCH 01/33] fix: add origin_message_id support for spawn and message deduplication --- nanobot/agent/tools/spawn.py | 6 ++++ nanobot/channels/manager.py | 32 +++++++++++++++++++++ tests/tools/test_message_tool_suppress.py | 34 ++++++++++++++++++++++- 3 files changed, 71 insertions(+), 1 deletion(-) diff --git a/nanobot/agent/tools/spawn.py b/nanobot/agent/tools/spawn.py index beda058a8..927823f63 100644 --- a/nanobot/agent/tools/spawn.py +++ b/nanobot/agent/tools/spawn.py @@ -25,6 +25,7 @@ class SpawnTool(Tool): self._origin_channel: ContextVar[str] = ContextVar("spawn_origin_channel", default="cli") self._origin_chat_id: ContextVar[str] = ContextVar("spawn_origin_chat_id", default="direct") self._session_key: ContextVar[str] = ContextVar("spawn_session_key", default="cli:direct") + self._origin_message_id: str | None = None def set_context(self, channel: str, chat_id: str, effective_key: str | None = None) -> None: """Set the origin context for subagent announcements.""" @@ -32,6 +33,10 @@ class SpawnTool(Tool): self._origin_chat_id.set(chat_id) self._session_key.set(effective_key or f"{channel}:{chat_id}") + def set_origin_message_id(self, message_id: str | None) -> None: + """Set the source message id for downstream deduplication.""" + self._origin_message_id = message_id + @property def name(self) -> str: return "spawn" @@ -54,4 +59,5 @@ class SpawnTool(Tool): origin_channel=self._origin_channel.get(), origin_chat_id=self._origin_chat_id.get(), session_key=self._session_key.get(), + origin_message_id=self._origin_message_id, ) diff --git a/nanobot/channels/manager.py b/nanobot/channels/manager.py index 14a6b2a5e..2f893a1fa 100644 --- a/nanobot/channels/manager.py +++ b/nanobot/channels/manager.py @@ -3,6 +3,7 @@ from __future__ import annotations import asyncio +import hashlib from pathlib import Path from typing import TYPE_CHECKING, Any @@ -37,6 +38,12 @@ _BOOL_CAMEL_ALIASES: dict[str, str] = { } +@dataclass +class _RecentOutbound: + fingerprint: str + ts: float + + class ChannelManager: """ Manages chat channels and coordinates message routing. @@ -59,6 +66,7 @@ class ChannelManager: self._session_manager = session_manager self.channels: dict[str, BaseChannel] = {} self._dispatch_task: asyncio.Task | None = None + self._recent_outbound: dict[tuple[str, str], _RecentOutbound] = {} self._init_channels() @@ -233,6 +241,25 @@ class ChannelManager: except Exception as e: logger.error("Error stopping {}: {}", name, e) + @staticmethod + def _fingerprint_content(content: str) -> str: + normalized = " ".join(content.split()) + return hashlib.sha1(normalized.encode("utf-8")).hexdigest() if normalized else "" + + def _should_suppress_outbound(self, msg: OutboundMessage) -> bool: + if msg.metadata.get("_progress"): + return False + fingerprint = self._fingerprint_content(msg.content) + if not fingerprint: + return False + key = (msg.channel, msg.chat_id) + recent = self._recent_outbound.get(key) + now = asyncio.get_running_loop().time() + if recent and recent.fingerprint == fingerprint and now - recent.ts <= 8.0: + return True + self._recent_outbound[key] = _RecentOutbound(fingerprint=fingerprint, ts=now) + return False + async def _dispatch_outbound(self) -> None: """Dispatch outbound messages to the appropriate channel.""" logger.info("Outbound dispatcher started") @@ -273,6 +300,11 @@ class ChannelManager: channel = self.channels.get(msg.channel) if channel: + # Duplicate suppression (non-streaming only) + if not msg.metadata.get("_stream_delta") and not msg.metadata.get("_stream_end") and not msg.metadata.get("_streamed"): + if self._should_suppress_outbound(msg): + logger.info("Suppressing duplicate outbound message to {}:{}", msg.channel, msg.chat_id) + continue await self._send_with_retry(channel, msg) else: logger.warning("Unknown channel: {}", msg.channel) diff --git a/tests/tools/test_message_tool_suppress.py b/tests/tools/test_message_tool_suppress.py index 213a8be63..195f86abd 100644 --- a/tests/tools/test_message_tool_suppress.py +++ b/tests/tools/test_message_tool_suppress.py @@ -2,7 +2,7 @@ import asyncio from pathlib import Path -from unittest.mock import AsyncMock, MagicMock +from unittest.mock import AsyncMock, MagicMock, patch import pytest @@ -166,3 +166,35 @@ class TestMessageToolTurnTracking: tool._sent_in_turn = True tool.start_turn() assert not tool._sent_in_turn + + +class TestSystemReplySuppression: + @pytest.mark.asyncio + async def test_subagent_system_reply_suppressed_when_duplicate(self, tmp_path: Path) -> None: + with patch("nanobot.agent.loop.ContextBuilder"), \ + patch("nanobot.agent.loop.SessionManager") as MockSessionManager, \ + patch("nanobot.agent.loop.SubagentManager"): + session = MagicMock() + session.get_history.return_value = [] + MockSessionManager.return_value.get_or_create.return_value = session + + bus = MessageBus() + provider = MagicMock() + provider.get_default_model.return_value = "test-model" + loop = AgentLoop(bus=bus, provider=provider, workspace=tmp_path, model="test-model", memory_window=10) + + loop._remember_visible_reply("feishu:chat123", "Done") + loop._run_agent_loop = AsyncMock(return_value=("Done", [], [])) + loop._save_turn = MagicMock() + loop.sessions.save = MagicMock() + + msg = InboundMessage( + channel="system", + sender_id="subagent", + chat_id="feishu:chat123", + content="background result", + metadata={"source": "subagent"}, + ) + + result = await loop._process_message(msg) + assert result is None From 61a8ad27d952fa4e9917fb65069ad8aac21c6724 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E7=AB=A5=E5=A4=A9=E7=AB=8B?= Date: Mon, 23 Mar 2026 21:26:24 +0800 Subject: [PATCH 02/33] fix: add origin_message_id parameter to SubagentManager.spawn() --- nanobot/agent/subagent.py | 24 +++++++++++++++--------- 1 file changed, 15 insertions(+), 9 deletions(-) diff --git a/nanobot/agent/subagent.py b/nanobot/agent/subagent.py index e64dc8f97..8b9463b89 100644 --- a/nanobot/agent/subagent.py +++ b/nanobot/agent/subagent.py @@ -114,6 +114,7 @@ class SubagentManager: origin_channel: str = "cli", origin_chat_id: str = "direct", session_key: str | None = None, + origin_message_id: str | None = None, ) -> str: """Spawn a subagent to execute a task in the background.""" task_id = str(uuid.uuid4())[:8] @@ -129,7 +130,7 @@ class SubagentManager: self._task_statuses[task_id] = status bg_task = asyncio.create_task( - self._run_subagent(task_id, task, display_label, origin, status) + self._run_subagent(task_id, task, display_label, origin, status, origin_message_id) ) self._running_tasks[task_id] = bg_task if session_key: @@ -155,6 +156,7 @@ class SubagentManager: label: str, origin: dict[str, str], status: SubagentStatus, + origin_message_id: str | None = None, ) -> None: """Execute the subagent task and announce the result.""" logger.info("Subagent [{}] starting task: {}", task_id, label) @@ -224,24 +226,24 @@ class SubagentManager: await self._announce_result( task_id, label, task, self._format_partial_progress(result), - origin, "error", + origin, "error", origin_message_id, ) elif result.stop_reason == "error": await self._announce_result( task_id, label, task, result.error or "Error: subagent execution failed.", - origin, "error", + origin, "error", origin_message_id, ) else: final_result = result.final_content or "Task completed but no final response was generated." logger.info("Subagent [{}] completed successfully", task_id) - await self._announce_result(task_id, label, task, final_result, origin, "ok") + await self._announce_result(task_id, label, task, final_result, origin, "ok", origin_message_id) except Exception as e: status.phase = "error" status.error = str(e) logger.error("Subagent [{}] failed: {}", task_id, e) - await self._announce_result(task_id, label, task, f"Error: {e}", origin, "error") + await self._announce_result(task_id, label, task, f"Error: {e}", origin, "error", origin_message_id) async def _announce_result( self, @@ -251,6 +253,7 @@ class SubagentManager: result: str, origin: dict[str, str], status: str, + origin_message_id: str | None = None, ) -> None: """Announce the subagent result to the main agent via the message bus.""" status_text = "completed successfully" if status == "ok" else "failed" @@ -269,16 +272,19 @@ class SubagentManager: # routed to the correct pending queue (mid-turn injection) instead of # being dispatched as a competing independent task. override = origin.get("session_key") or f"{origin['channel']}:{origin['chat_id']}" + metadata: dict[str, Any] = { + "injected_event": "subagent_result", + "subagent_task_id": task_id, + } + if origin_message_id: + metadata["origin_message_id"] = origin_message_id msg = InboundMessage( channel="system", sender_id="subagent", chat_id=f"{origin['channel']}:{origin['chat_id']}", content=announce_content, session_key_override=override, - metadata={ - "injected_event": "subagent_result", - "subagent_task_id": task_id, - }, + metadata=metadata, ) await self.bus.publish_inbound(msg) From 306958d6e6ee08cf3508b4803087a7f259306781 Mon Sep 17 00:00:00 2001 From: Xubin Ren Date: Fri, 1 May 2026 10:46:31 +0000 Subject: [PATCH 03/33] add native Bedrock Converse provider Made-with: Cursor --- docs/configuration.md | 178 +++++ nanobot/config/schema.py | 12 +- nanobot/providers/__init__.py | 3 + nanobot/providers/bedrock_provider.py | 730 ++++++++++++++++++++ nanobot/providers/factory.py | 16 + nanobot/providers/registry.py | 25 +- nanobot/session/manager.py | 2 +- pyproject.toml | 1 + tests/agent/test_session_manager_history.py | 6 + tests/providers/test_bedrock_provider.py | 254 +++++++ tests/providers/test_providers_init.py | 3 + 11 files changed, 1226 insertions(+), 4 deletions(-) create mode 100644 nanobot/providers/bedrock_provider.py create mode 100644 tests/providers/test_bedrock_provider.py diff --git a/docs/configuration.md b/docs/configuration.md index f295b50c5..f20cec5f0 100644 --- a/docs/configuration.md +++ b/docs/configuration.md @@ -63,6 +63,7 @@ IMAP_PASSWORD=your-password-here | `byteplus` | LLM (VolcEngine international, pay-per-use) | [Coding Plan](https://www.byteplus.com/en/activity/codingplan?utm_campaign=nanobot&utm_content=nanobot&utm_medium=devrel&utm_source=OWO&utm_term=nanobot) · [byteplus.com](https://www.byteplus.com) | | `anthropic` | LLM (Claude direct) | [console.anthropic.com](https://console.anthropic.com) | | `azure_openai` | LLM (Azure OpenAI) | [portal.azure.com](https://portal.azure.com) | +| `bedrock` | LLM (AWS Bedrock Converse, Claude/Nova/Llama/etc.) | [aws.amazon.com/bedrock](https://aws.amazon.com/bedrock/) | | `openai` | LLM + Voice transcription (Whisper) | [platform.openai.com](https://platform.openai.com) | | `deepseek` | LLM (DeepSeek direct) | [platform.deepseek.com](https://platform.deepseek.com) | | `groq` | LLM + Voice transcription (Whisper, default) | [console.groq.com](https://console.groq.com) | @@ -85,6 +86,183 @@ IMAP_PASSWORD=your-password-here | `github_copilot` | LLM (GitHub Copilot, OAuth) | `nanobot provider login github-copilot` | | `qianfan` | LLM (Baidu Qianfan) | [cloud.baidu.com](https://cloud.baidu.com/doc/qianfan/s/Hmh4suq26) | +
+AWS Bedrock (Converse API) + +Bedrock uses the native `bedrock-runtime` Converse API, so it can call Bedrock model IDs such as Claude Opus 4.7, Claude Sonnet, Amazon Nova, Meta Llama, Mistral, Qwen, and other models that support Converse. It supports normal chat, streaming, tool calling, tool results, token usage, and Bedrock error metadata. + +This provider is for Bedrock's native Converse API, not Bedrock's OpenAI-compatible `/openai/v1` endpoint. For OpenAI-compatible Bedrock models, you can still use `custom` if you specifically want that API surface. + +**1. Configure credentials** + +Use the normal AWS credential chain (`AWS_ACCESS_KEY_ID` / `AWS_SECRET_ACCESS_KEY`, an AWS profile, or an IAM role). The IAM identity needs: + +```json +{ + "Effect": "Allow", + "Action": [ + "bedrock:InvokeModel", + "bedrock:InvokeModelWithResponseStream" + ], + "Resource": "*" +} +``` + +You can also set `providers.bedrock.apiKey` to a Bedrock API key; nanobot exports it as `AWS_BEARER_TOKEN_BEDROCK` for the AWS SDK. + +Credential options: + +- **AWS CLI/default profile**: leave `apiKey` and `profile` empty, then run `aws configure` or provide `AWS_ACCESS_KEY_ID` / `AWS_SECRET_ACCESS_KEY`. +- **Named AWS profile**: set `profile` to a profile from `~/.aws/config` or `~/.aws/credentials`. +- **IAM role**: on EC2/ECS/Lambda, leave `apiKey` and `profile` empty and attach a role with Bedrock permissions. +- **Bedrock API key**: set `apiKey` or `AWS_BEARER_TOKEN_BEDROCK`; `profile` can stay `null`. + +**2. Minimal config** + +For a non-Anthropic model such as Amazon Nova: + +```json +{ + "providers": { + "bedrock": { + "region": "us-east-1" + } + }, + "agents": { + "defaults": { + "provider": "bedrock", + "model": "bedrock/amazon.nova-lite-v1:0", + "reasoningEffort": null + } + } +} +``` + +With a Bedrock API key: + +```json +{ + "providers": { + "bedrock": { + "region": "us-east-1", + "apiKey": "${AWS_BEARER_TOKEN_BEDROCK}" + } + }, + "agents": { + "defaults": { + "provider": "bedrock", + "model": "bedrock/amazon.nova-lite-v1:0", + "reasoningEffort": null + } + } +} +``` + +With a named AWS profile: + +```json +{ + "providers": { + "bedrock": { + "region": "us-east-1", + "profile": "my-bedrock-profile" + } + }, + "agents": { + "defaults": { + "provider": "bedrock", + "model": "bedrock/amazon.nova-lite-v1:0" + } + } +} +``` + +**3. Claude Opus 4.7 example** + +```json +{ + "providers": { + "bedrock": { + "region": "us-east-1" + } + }, + "agents": { + "defaults": { + "provider": "bedrock", + "model": "bedrock/global.anthropic.claude-opus-4-7", + "reasoningEffort": "medium", + "maxTokens": 8192 + } + } +} +``` + +For regional routing, use one of Bedrock's inference IDs, for example `bedrock/us.anthropic.claude-opus-4-7`, `bedrock/eu.anthropic.claude-opus-4-7`, or `bedrock/jp.anthropic.claude-opus-4-7`. + +Claude Opus 4.7 does not accept `temperature`, `top_p`, or `top_k`; nanobot omits `temperature` automatically for this model. If `reasoningEffort` is set to `low`, `medium`, `high`, `max`, or `adaptive`, nanobot sends Bedrock's adaptive thinking parameter. + +Anthropic models on Bedrock can also require Anthropic use-case registration and are subject to Anthropic-supported country/region restrictions. If Claude fails with a `ValidationException` about unsupported countries or regions, try a non-Anthropic Bedrock model such as Amazon Nova to verify the provider setup. + +**4. Model IDs** + +Use Bedrock model IDs or inference profile IDs with a `bedrock/` prefix in nanobot config. nanobot removes the prefix before calling AWS. + +Examples: + +- `bedrock/amazon.nova-micro-v1:0` +- `bedrock/amazon.nova-lite-v1:0` +- `bedrock/global.anthropic.claude-opus-4-7` +- `bedrock/us.anthropic.claude-opus-4-7` +- `bedrock/openai.gpt-oss-20b-1:0` +- `bedrock/meta.llama...` +- `bedrock/mistral...` + +Check the Bedrock console for the exact model ID and region availability. Some models require cross-region inference profile IDs such as `us.*`, `eu.*`, or `global.*`. + +**5. Advanced model fields** + +Model-specific fields can be supplied with `extraBody`; nanobot merges it into Converse `additionalModelRequestFields`: + +```json +{ + "providers": { + "bedrock": { + "region": "us-east-1", + "extraBody": { + "thinking": { + "type": "adaptive", + "effort": "medium", + "display": "summarized" + } + } + } + } +} +``` + +Use `apiBase` only for a custom Bedrock Runtime endpoint URL, such as a VPC endpoint or proxy. It is not needed for normal AWS regions. + +Current scope: nanobot passes `messages`, `system`, `inferenceConfig`, `toolConfig`, and `additionalModelRequestFields`. Bedrock Prompt Management, Guardrails, `serviceTier`, and other top-level Converse options are not first-class config fields yet. + +**6. Quick checks** + +```bash +# For AWS credential-chain usage: +aws sts get-caller-identity + +# For API-key usage: +export AWS_BEARER_TOKEN_BEDROCK="your-bedrock-api-key" +export AWS_REGION="us-east-1" +``` + +Then run: + +```bash +nanobot agent -m "Reply with one short sentence." +``` + +
+
OpenAI Codex (OAuth) diff --git a/nanobot/config/schema.py b/nanobot/config/schema.py index a6c9d10c4..210767bfe 100644 --- a/nanobot/config/schema.py +++ b/nanobot/config/schema.py @@ -119,11 +119,19 @@ class ProviderConfig(Base): extra_body: dict[str, Any] | None = None # Extra fields merged into every request body +class BedrockProviderConfig(ProviderConfig): + """AWS Bedrock Runtime provider configuration.""" + + region: str | None = None # AWS region, falls back to AWS_REGION/AWS_DEFAULT_REGION/profile + profile: str | None = None # Optional AWS shared config profile + + class ProvidersConfig(Base): """Configuration for LLM providers.""" custom: ProviderConfig = Field(default_factory=ProviderConfig) # Any OpenAI-compatible endpoint azure_openai: ProviderConfig = Field(default_factory=ProviderConfig) # Azure OpenAI (model = deployment name) + bedrock: BedrockProviderConfig = Field(default_factory=BedrockProviderConfig) # AWS Bedrock Converse anthropic: ProviderConfig = Field(default_factory=ProviderConfig) openai: ProviderConfig = Field(default_factory=ProviderConfig) openrouter: ProviderConfig = Field(default_factory=ProviderConfig) @@ -287,14 +295,14 @@ class Config(BaseSettings): for spec in PROVIDERS: p = getattr(self.providers, spec.name, None) if p and model_prefix and normalized_prefix == spec.name: - if spec.is_oauth or spec.is_local or p.api_key: + if spec.is_oauth or spec.is_local or spec.is_direct or p.api_key: return p, spec.name # Match by keyword (order follows PROVIDERS registry) for spec in PROVIDERS: p = getattr(self.providers, spec.name, None) if p and any(_kw_matches(kw) for kw in spec.keywords): - if spec.is_oauth or spec.is_local or p.api_key: + if spec.is_oauth or spec.is_local or spec.is_direct or p.api_key: return p, spec.name # Fallback: configured local providers can route models without diff --git a/nanobot/providers/__init__.py b/nanobot/providers/__init__.py index ce2378707..6150788a7 100644 --- a/nanobot/providers/__init__.py +++ b/nanobot/providers/__init__.py @@ -15,6 +15,7 @@ __all__ = [ "OpenAICodexProvider", "GitHubCopilotProvider", "AzureOpenAIProvider", + "BedrockProvider", ] _LAZY_IMPORTS = { @@ -23,11 +24,13 @@ _LAZY_IMPORTS = { "OpenAICodexProvider": ".openai_codex_provider", "GitHubCopilotProvider": ".github_copilot_provider", "AzureOpenAIProvider": ".azure_openai_provider", + "BedrockProvider": ".bedrock_provider", } if TYPE_CHECKING: from nanobot.providers.anthropic_provider import AnthropicProvider from nanobot.providers.azure_openai_provider import AzureOpenAIProvider + from nanobot.providers.bedrock_provider import BedrockProvider from nanobot.providers.github_copilot_provider import GitHubCopilotProvider from nanobot.providers.openai_compat_provider import OpenAICompatProvider from nanobot.providers.openai_codex_provider import OpenAICodexProvider diff --git a/nanobot/providers/bedrock_provider.py b/nanobot/providers/bedrock_provider.py new file mode 100644 index 000000000..479637916 --- /dev/null +++ b/nanobot/providers/bedrock_provider.py @@ -0,0 +1,730 @@ +"""AWS Bedrock Converse provider.""" + +from __future__ import annotations + +import asyncio +import base64 +import json +import os +import re +from collections.abc import Awaitable, Callable, Iterator +from typing import Any + +import json_repair + +from nanobot.providers.base import LLMProvider, LLMResponse, ToolCallRequest + +_IMAGE_DATA_URL = re.compile(r"^data:image/([a-zA-Z0-9.+-]+);base64,(.*)$", re.DOTALL) +_TEXT_BLOCK_TYPES = {"text", "input_text", "output_text"} +_TEMPERATURE_UNSUPPORTED_MODEL_TOKENS = ("claude-opus-4-7",) +_ADAPTIVE_THINKING_ONLY_MODEL_TOKENS = ("claude-opus-4-7",) + + +def _deep_merge(base: dict[str, Any], override: dict[str, Any]) -> dict[str, Any]: + merged = dict(base) + for key, value in override.items(): + if key in merged and isinstance(merged[key], dict) and isinstance(value, dict): + merged[key] = _deep_merge(merged[key], value) + else: + merged[key] = value + return merged + + +def _next_or_none(iterator: Iterator[dict[str, Any]]) -> dict[str, Any] | None: + try: + return next(iterator) + except StopIteration: + return None + + +class BedrockProvider(LLMProvider): + """LLM provider using AWS Bedrock Runtime's Converse APIs.""" + + def __init__( + self, + api_key: str | None = None, + api_base: str | None = None, + default_model: str = "bedrock/global.anthropic.claude-opus-4-7", + *, + region: str | None = None, + profile: str | None = None, + extra_body: dict[str, Any] | None = None, + client: Any | None = None, + ): + super().__init__(api_key, api_base) + self.default_model = default_model + self.region = region or os.environ.get("AWS_REGION") or os.environ.get("AWS_DEFAULT_REGION") + self.profile = profile + self._extra_body = extra_body or {} + self._client = client if client is not None else self._make_client() + + def _make_client(self) -> Any: + if self.api_key: + os.environ["AWS_BEARER_TOKEN_BEDROCK"] = self.api_key + try: + import boto3 + except ImportError as exc: # pragma: no cover - exercised only without boto3 installed + raise RuntimeError( + "AWS Bedrock provider requires boto3. Install it with `pip install boto3`." + ) from exc + + session_kwargs: dict[str, Any] = {} + if self.profile: + session_kwargs["profile_name"] = self.profile + session = boto3.Session(**session_kwargs) + + client_kwargs: dict[str, Any] = {} + if self.region: + client_kwargs["region_name"] = self.region + if self.api_base: + client_kwargs["endpoint_url"] = self.api_base + return session.client("bedrock-runtime", **client_kwargs) + + @staticmethod + def _strip_prefix(model: str) -> str: + if model.startswith("bedrock/"): + return model[len("bedrock/"):] + return model + + @staticmethod + def _matches_model_token(model: str, tokens: tuple[str, ...]) -> bool: + model_lower = model.lower() + return any(token in model_lower for token in tokens) + + @classmethod + def _supports_temperature(cls, model: str) -> bool: + return not cls._matches_model_token(model, _TEMPERATURE_UNSUPPORTED_MODEL_TOKENS) + + @classmethod + def _uses_adaptive_thinking_only(cls, model: str) -> bool: + return cls._matches_model_token(model, _ADAPTIVE_THINKING_ONLY_MODEL_TOKENS) + + @staticmethod + def _image_url_block(block: dict[str, Any]) -> dict[str, Any] | None: + url = (block.get("image_url") or {}).get("url", "") + if not isinstance(url, str) or not url: + return None + match = _IMAGE_DATA_URL.match(url) + if not match: + return {"text": f"(image URL: {url})"} + fmt = match.group(1).lower() + if fmt == "jpg": + fmt = "jpeg" + try: + data = base64.b64decode(match.group(2), validate=False) + except Exception: + return {"text": "(invalid image data)"} + return {"image": {"format": fmt, "source": {"bytes": data}}} + + @classmethod + def _content_blocks(cls, content: Any, *, for_tool_result: bool = False) -> list[dict[str, Any]]: + if isinstance(content, str) or content is None: + return [{"text": content or "(empty)"}] + if not isinstance(content, list): + if for_tool_result and isinstance(content, dict): + return [{"json": content}] + return [{"text": str(content)}] + + blocks: list[dict[str, Any]] = [] + for item in content: + if not isinstance(item, dict): + blocks.append({"text": str(item)}) + continue + + item_type = item.get("type") + if item_type in _TEXT_BLOCK_TYPES or "text" in item: + text = item.get("text") + if text: + blocks.append({"text": str(text)}) + continue + if item_type == "image_url": + converted = cls._image_url_block(item) + if converted: + blocks.append(converted) + continue + + # Preserve already-Bedrock-shaped content where possible. + for key in ("text", "image", "document", "video", "json", "searchResult"): + if key in item: + blocks.append({key: item[key]}) + break + else: + blocks.append({"json": item} if for_tool_result else {"text": json.dumps(item)}) + + return blocks or [{"text": "(empty)"}] + + @classmethod + def _system_blocks(cls, content: Any) -> list[dict[str, Any]]: + return [ + block for block in cls._content_blocks(content) + if "text" in block or "cachePoint" in block or "guardContent" in block + ] + + @classmethod + def _tool_result_block(cls, msg: dict[str, Any]) -> dict[str, Any]: + return { + "toolResult": { + "toolUseId": str(msg.get("tool_call_id") or ""), + "content": cls._content_blocks(msg.get("content"), for_tool_result=True), + "status": "success", + } + } + + @staticmethod + def _tool_use_block(tool_call: dict[str, Any]) -> dict[str, Any] | None: + function = tool_call.get("function") + if not isinstance(function, dict): + return None + args = function.get("arguments", {}) + if isinstance(args, str): + try: + args = json_repair.loads(args) if args.strip() else {} + except Exception: + args = {} + if not isinstance(args, dict): + args = {} + return { + "toolUse": { + "toolUseId": str(tool_call.get("id") or ""), + "name": str(function.get("name") or ""), + "input": args, + } + } + + @staticmethod + def _reasoning_block(block: dict[str, Any]) -> dict[str, Any] | None: + if block.get("type") not in {"thinking", "reasoning", "redacted_thinking"}: + return None + text = block.get("thinking") or block.get("text") + signature = block.get("signature") + if text and signature: + return { + "reasoningContent": { + "reasoningText": {"text": str(text), "signature": str(signature)} + } + } + redacted = block.get("redactedContent") + if redacted is None and isinstance(block.get("redactedContentBase64"), str): + try: + redacted = base64.b64decode(block["redactedContentBase64"]) + except Exception: + redacted = None + if redacted is not None: + return {"reasoningContent": {"redactedContent": redacted}} + return None + + @classmethod + def _assistant_blocks(cls, msg: dict[str, Any]) -> list[dict[str, Any]]: + blocks: list[dict[str, Any]] = [] + + for thinking in msg.get("thinking_blocks") or []: + if isinstance(thinking, dict): + reasoning = cls._reasoning_block(thinking) + if reasoning: + blocks.append(reasoning) + + content = msg.get("content") + if isinstance(content, str) and content: + blocks.append({"text": content}) + elif isinstance(content, list): + blocks.extend(block for block in cls._content_blocks(content) if "text" in block) + + for tool_call in msg.get("tool_calls") or []: + if isinstance(tool_call, dict): + block = cls._tool_use_block(tool_call) + if block: + blocks.append(block) + + return blocks or [{"text": ""}] + + @staticmethod + def _has_tool_use(msg: dict[str, Any]) -> bool: + content = msg.get("content") + return isinstance(content, list) and any( + isinstance(block, dict) and "toolUse" in block for block in content + ) + + @staticmethod + def _merge_consecutive(messages: list[dict[str, Any]]) -> list[dict[str, Any]]: + merged: list[dict[str, Any]] = [] + for msg in messages: + if merged and merged[-1].get("role") == msg.get("role"): + prev = merged[-1].setdefault("content", []) + cur = msg.get("content") or [] + if not isinstance(prev, list): + prev = [{"text": str(prev)}] + merged[-1]["content"] = prev + if isinstance(cur, list): + prev.extend(cur) + else: + prev.append({"text": str(cur)}) + else: + merged.append(msg) + + last_popped: dict[str, Any] | None = None + while merged and merged[-1].get("role") == "assistant": + last_popped = merged.pop() + if not merged and last_popped is not None and not BedrockProvider._has_tool_use(last_popped): + merged.append({"role": "user", "content": last_popped.get("content") or [{"text": "(empty)"}]}) + if merged and merged[0].get("role") == "assistant" and not BedrockProvider._has_tool_use(merged[0]): + merged.insert(0, {"role": "user", "content": [{"text": "(conversation continued)"}]}) + return merged + + def _convert_messages( + self, + messages: list[dict[str, Any]], + ) -> tuple[list[dict[str, Any]], list[dict[str, Any]]]: + system: list[dict[str, Any]] = [] + converted: list[dict[str, Any]] = [] + + for msg in messages: + role = msg.get("role") + content = msg.get("content") + if role == "system": + system.extend(self._system_blocks(content)) + continue + if role == "tool": + block = self._tool_result_block(msg) + if converted and converted[-1].get("role") == "user": + converted[-1].setdefault("content", []).append(block) + else: + converted.append({"role": "user", "content": [block]}) + continue + if role == "assistant": + converted.append({"role": "assistant", "content": self._assistant_blocks(msg)}) + continue + if role == "user": + converted.append({"role": "user", "content": self._content_blocks(content)}) + + return system, self._merge_consecutive(converted) + + @staticmethod + def _convert_tools(tools: list[dict[str, Any]] | None) -> list[dict[str, Any]] | None: + if not tools: + return None + result: list[dict[str, Any]] = [] + for tool in tools: + func = tool.get("function") if isinstance(tool.get("function"), dict) else tool + if not isinstance(func, dict): + continue + name = str(func.get("name") or "") + if not name: + continue + spec: dict[str, Any] = { + "name": name, + "inputSchema": { + "json": func.get("parameters") or {"type": "object", "properties": {}} + }, + } + description = func.get("description") + if description: + spec["description"] = str(description) + strict = func.get("strict", tool.get("strict")) + if isinstance(strict, bool): + spec["strict"] = strict + result.append({"toolSpec": spec}) + return result or None + + @staticmethod + def _convert_tool_choice( + tool_choice: str | dict[str, Any] | None, + ) -> dict[str, Any] | None: + if tool_choice is None or tool_choice == "auto": + return {"auto": {}} + if tool_choice == "required": + return {"any": {}} + if tool_choice == "none": + return None + if isinstance(tool_choice, dict): + name = tool_choice.get("function", {}).get("name") + if name: + return {"tool": {"name": str(name)}} + return {"auto": {}} + + @staticmethod + def _adaptive_thinking(reasoning_effort: str | None) -> dict[str, Any] | None: + if not reasoning_effort: + return None + effort = reasoning_effort.lower() + if effort == "none": + return None + thinking: dict[str, Any] = {"type": "adaptive"} + if effort != "adaptive": + thinking["effort"] = effort + return thinking + + def _build_kwargs( + self, + messages: list[dict[str, Any]], + tools: list[dict[str, Any]] | None, + model: str | None, + max_tokens: int, + temperature: float, + reasoning_effort: str | None, + tool_choice: str | dict[str, Any] | None, + ) -> dict[str, Any]: + model_id = self._strip_prefix(model or self.default_model) + system, bedrock_messages = self._convert_messages(self._sanitize_empty_content(messages)) + if not bedrock_messages: + bedrock_messages = [{"role": "user", "content": [{"text": "(empty)"}]}] + + kwargs: dict[str, Any] = { + "modelId": model_id, + "messages": bedrock_messages, + "inferenceConfig": {"maxTokens": max(1, max_tokens)}, + } + if system: + kwargs["system"] = system + if self._supports_temperature(model_id): + kwargs["inferenceConfig"]["temperature"] = temperature + + additional: dict[str, Any] = {} + if self._uses_adaptive_thinking_only(model_id): + thinking = self._adaptive_thinking(reasoning_effort) + if thinking: + additional["thinking"] = thinking + if self._extra_body: + additional = _deep_merge(additional, self._extra_body) + if additional: + kwargs["additionalModelRequestFields"] = additional + + bedrock_tools = self._convert_tools(tools) + if bedrock_tools: + tool_config: dict[str, Any] = {"tools": bedrock_tools} + choice = self._convert_tool_choice(tool_choice) + if choice: + tool_config["toolChoice"] = choice + kwargs["toolConfig"] = tool_config + + return kwargs + + @staticmethod + def _finish_reason(stop_reason: str | None) -> str: + return { + "end_turn": "stop", + "tool_use": "tool_calls", + "max_tokens": "length", + }.get(stop_reason or "", stop_reason or "stop") + + @staticmethod + def _usage(usage: dict[str, Any] | None) -> dict[str, int]: + if not usage: + return {} + prompt = int(usage.get("inputTokens") or 0) + completion = int(usage.get("outputTokens") or 0) + total = int(usage.get("totalTokens") or prompt + completion) + result = { + "prompt_tokens": prompt, + "completion_tokens": completion, + "total_tokens": total, + } + cache_read = int(usage.get("cacheReadInputTokens") or 0) + cache_write = int(usage.get("cacheWriteInputTokens") or 0) + if cache_read: + result["cached_tokens"] = cache_read + result["cache_read_input_tokens"] = cache_read + if cache_write: + result["cache_creation_input_tokens"] = cache_write + return result + + @staticmethod + def _parse_reasoning(block: dict[str, Any]) -> tuple[str | None, dict[str, Any] | None]: + reasoning = block.get("reasoningContent") + if not isinstance(reasoning, dict): + return None, None + text_obj = reasoning.get("reasoningText") + if isinstance(text_obj, dict): + text = text_obj.get("text") + if isinstance(text, str): + return text, { + "type": "thinking", + "thinking": text, + "signature": text_obj.get("signature", ""), + } + redacted = reasoning.get("redactedContent") + if redacted is not None: + if isinstance(redacted, (bytes, bytearray)): + encoded = base64.b64encode(bytes(redacted)).decode("ascii") + return None, {"type": "redacted_thinking", "redactedContentBase64": encoded} + return None, {"type": "redacted_thinking", "redactedContent": redacted} + return None, None + + @classmethod + def _parse_response(cls, response: dict[str, Any]) -> LLMResponse: + content_parts: list[str] = [] + reasoning_parts: list[str] = [] + tool_calls: list[ToolCallRequest] = [] + thinking_blocks: list[dict[str, Any]] = [] + message = (response.get("output") or {}).get("message") or {} + + for block in message.get("content") or []: + if not isinstance(block, dict): + continue + if isinstance(block.get("text"), str): + content_parts.append(block["text"]) + tool_use = block.get("toolUse") + if isinstance(tool_use, dict): + arguments = tool_use.get("input") if isinstance(tool_use.get("input"), dict) else {} + tool_calls.append(ToolCallRequest( + id=str(tool_use.get("toolUseId") or ""), + name=str(tool_use.get("name") or ""), + arguments=arguments, + )) + reasoning_text, thinking = cls._parse_reasoning(block) + if reasoning_text: + reasoning_parts.append(reasoning_text) + if thinking: + thinking_blocks.append(thinking) + + return LLMResponse( + content="".join(content_parts) or None, + tool_calls=tool_calls, + finish_reason=cls._finish_reason(response.get("stopReason")), + usage=cls._usage(response.get("usage")), + reasoning_content="".join(reasoning_parts) or None, + thinking_blocks=thinking_blocks or None, + ) + + @classmethod + def _parse_stream_event( + cls, + event: dict[str, Any], + *, + content_parts: list[str], + reasoning_parts: list[str], + thinking_blocks: list[dict[str, Any]], + tool_buffers: dict[int, dict[str, Any]], + state: dict[str, Any], + ) -> str | None: + if "contentBlockStart" in event: + data = event["contentBlockStart"] + idx = int(data.get("contentBlockIndex") or 0) + start = data.get("start") or {} + tool_use = start.get("toolUse") + if isinstance(tool_use, dict): + tool_buffers[idx] = { + "id": str(tool_use.get("toolUseId") or ""), + "name": str(tool_use.get("name") or ""), + "input": "", + } + return None + + if "contentBlockDelta" in event: + data = event["contentBlockDelta"] + idx = int(data.get("contentBlockIndex") or 0) + delta = data.get("delta") or {} + text = delta.get("text") + if isinstance(text, str): + content_parts.append(text) + return text + tool_delta = delta.get("toolUse") + if isinstance(tool_delta, dict): + buf = tool_buffers.setdefault(idx, {"id": "", "name": "", "input": ""}) + if isinstance(tool_delta.get("input"), str): + buf["input"] += tool_delta["input"] + reasoning = delta.get("reasoningContent") + if isinstance(reasoning, dict): + buf = state.setdefault("reasoning_buffers", {}).setdefault( + idx, {"text": "", "signature": "", "redactedContent": None} + ) + if isinstance(reasoning.get("text"), str): + buf["text"] += reasoning["text"] + reasoning_parts.append(reasoning["text"]) + if isinstance(reasoning.get("signature"), str): + buf["signature"] = reasoning["signature"] + if reasoning.get("redactedContent") is not None: + buf["redactedContent"] = reasoning["redactedContent"] + return None + + if "contentBlockStop" in event: + idx = int((event["contentBlockStop"] or {}).get("contentBlockIndex") or 0) + reasoning_buf = state.setdefault("reasoning_buffers", {}).pop(idx, None) + if reasoning_buf: + if reasoning_buf.get("text"): + thinking_blocks.append({ + "type": "thinking", + "thinking": reasoning_buf["text"], + "signature": reasoning_buf.get("signature", ""), + }) + elif reasoning_buf.get("redactedContent") is not None: + redacted = reasoning_buf["redactedContent"] + if isinstance(redacted, (bytes, bytearray)): + redacted_block = { + "type": "redacted_thinking", + "redactedContentBase64": base64.b64encode(bytes(redacted)).decode("ascii"), + } + else: + redacted_block = { + "type": "redacted_thinking", + "redactedContent": redacted, + } + thinking_blocks.append({ + **redacted_block, + }) + return None + + if "messageStop" in event: + state["stop_reason"] = (event["messageStop"] or {}).get("stopReason") + return None + + if "metadata" in event: + metadata = event["metadata"] or {} + if isinstance(metadata.get("usage"), dict): + state["usage"] = metadata["usage"] + return None + + return None + + @classmethod + def _stream_result( + cls, + *, + content_parts: list[str], + reasoning_parts: list[str], + thinking_blocks: list[dict[str, Any]], + tool_buffers: dict[int, dict[str, Any]], + state: dict[str, Any], + ) -> LLMResponse: + tool_calls: list[ToolCallRequest] = [] + for buf in tool_buffers.values(): + args: Any = {} + if buf.get("input"): + try: + args = json_repair.loads(buf["input"]) + except Exception: + args = {} + tool_calls.append(ToolCallRequest( + id=buf.get("id") or "", + name=buf.get("name") or "", + arguments=args if isinstance(args, dict) else {}, + )) + return LLMResponse( + content="".join(content_parts) or None, + tool_calls=tool_calls, + finish_reason=cls._finish_reason(state.get("stop_reason")), + usage=cls._usage(state.get("usage")), + reasoning_content="".join(reasoning_parts) or None, + thinking_blocks=thinking_blocks or None, + ) + + @classmethod + def _handle_error(cls, e: Exception) -> LLMResponse: + response = getattr(e, "response", None) + metadata = response.get("ResponseMetadata", {}) if isinstance(response, dict) else {} + headers = metadata.get("HTTPHeaders") if isinstance(metadata, dict) else None + error_obj = response.get("Error", {}) if isinstance(response, dict) else {} + message = error_obj.get("Message") if isinstance(error_obj, dict) else None + code = error_obj.get("Code") if isinstance(error_obj, dict) else None + status_code = metadata.get("HTTPStatusCode") if isinstance(metadata, dict) else None + body = message or str(e) + retry_after = cls._extract_retry_after_from_headers(headers) + if retry_after is None: + retry_after = cls._extract_retry_after(body) + + error_name = e.__class__.__name__.lower() + error_kind = None + if "timeout" in error_name: + error_kind = "timeout" + elif "connection" in error_name or "endpoint" in error_name: + error_kind = "connection" + + code_text = str(code or "").lower() + should_retry = None + if status_code is not None: + should_retry = int(status_code) == 429 or int(status_code) >= 500 + if any(token in code_text for token in ("throttl", "timeout", "unavailable", "modelnotready")): + should_retry = True + + return LLMResponse( + content=f"Error: {str(body).strip()[:500]}", + finish_reason="error", + retry_after=retry_after, + error_status_code=int(status_code) if status_code is not None else None, + error_kind=error_kind, + error_type=code_text or None, + error_code=code_text or None, + error_retry_after_s=retry_after, + error_should_retry=should_retry, + ) + + async def chat( + self, + messages: list[dict[str, Any]], + tools: list[dict[str, Any]] | None = None, + model: str | None = None, + max_tokens: int = 4096, + temperature: float = 0.7, + reasoning_effort: str | None = None, + tool_choice: str | dict[str, Any] | None = None, + ) -> LLMResponse: + try: + kwargs = self._build_kwargs( + messages, tools, model, max_tokens, temperature, reasoning_effort, tool_choice + ) + response = await asyncio.to_thread(self._client.converse, **kwargs) + return self._parse_response(response) + except Exception as e: + return self._handle_error(e) + + async def chat_stream( + self, + messages: list[dict[str, Any]], + tools: list[dict[str, Any]] | None = None, + model: str | None = None, + max_tokens: int = 4096, + temperature: float = 0.7, + reasoning_effort: str | None = None, + tool_choice: str | dict[str, Any] | None = None, + on_content_delta: Callable[[str], Awaitable[None]] | None = None, + ) -> LLMResponse: + idle_timeout_s = int(os.environ.get("NANOBOT_STREAM_IDLE_TIMEOUT_S", "90")) + content_parts: list[str] = [] + reasoning_parts: list[str] = [] + thinking_blocks: list[dict[str, Any]] = [] + tool_buffers: dict[int, dict[str, Any]] = {} + state: dict[str, Any] = {} + + try: + kwargs = self._build_kwargs( + messages, tools, model, max_tokens, temperature, reasoning_effort, tool_choice + ) + response = await asyncio.to_thread(self._client.converse_stream, **kwargs) + stream = iter(response.get("stream") or []) + while True: + event = await asyncio.wait_for( + asyncio.to_thread(_next_or_none, stream), + timeout=idle_timeout_s, + ) + if event is None: + break + delta = self._parse_stream_event( + event, + content_parts=content_parts, + reasoning_parts=reasoning_parts, + thinking_blocks=thinking_blocks, + tool_buffers=tool_buffers, + state=state, + ) + if delta and on_content_delta: + await on_content_delta(delta) + return self._stream_result( + content_parts=content_parts, + reasoning_parts=reasoning_parts, + thinking_blocks=thinking_blocks, + tool_buffers=tool_buffers, + state=state, + ) + except asyncio.TimeoutError: + return LLMResponse( + content=( + f"Error calling LLM: stream stalled for more than " + f"{idle_timeout_s} seconds" + ), + finish_reason="error", + error_kind="timeout", + ) + except Exception as e: + return self._handle_error(e) + + def get_default_model(self) -> str: + return self.default_model diff --git a/nanobot/providers/factory.py b/nanobot/providers/factory.py index 5f97b04a3..d71390940 100644 --- a/nanobot/providers/factory.py +++ b/nanobot/providers/factory.py @@ -60,6 +60,17 @@ def make_provider(config: Config) -> LLMProvider: default_model=model, extra_headers=p.extra_headers if p else None, ) + elif backend == "bedrock": + from nanobot.providers.bedrock_provider import BedrockProvider + + provider = BedrockProvider( + api_key=p.api_key if p else None, + api_base=p.api_base if p else None, + default_model=model, + region=getattr(p, "region", None) if p else None, + profile=getattr(p, "profile", None) if p else None, + extra_body=p.extra_body if p else None, + ) else: from nanobot.providers.openai_compat_provider import OpenAICompatProvider @@ -85,12 +96,17 @@ def provider_signature(config: Config) -> tuple[object, ...]: """Return the config fields that affect the primary LLM provider.""" model = config.agents.defaults.model defaults = config.agents.defaults + p = config.get_provider(model) return ( model, defaults.provider, config.get_provider_name(model), config.get_api_key(model), config.get_api_base(model), + p.extra_headers if p else None, + p.extra_body if p else None, + getattr(p, "region", None) if p else None, + getattr(p, "profile", None) if p else None, defaults.max_tokens, defaults.temperature, defaults.reasoning_effort, diff --git a/nanobot/providers/registry.py b/nanobot/providers/registry.py index 6f947bab0..a64041688 100644 --- a/nanobot/providers/registry.py +++ b/nanobot/providers/registry.py @@ -34,7 +34,7 @@ class ProviderSpec: display_name: str = "" # shown in `nanobot status` # which provider implementation to use - # "openai_compat" | "anthropic" | "azure_openai" | "openai_codex" | "github_copilot" + # "openai_compat" | "anthropic" | "azure_openai" | "openai_codex" | "github_copilot" | "bedrock" backend: str = "openai_compat" # extra env vars, e.g. (("ZHIPUAI_API_KEY", "{api_key}"),) @@ -105,6 +105,29 @@ PROVIDERS: tuple[ProviderSpec, ...] = ( backend="azure_openai", is_direct=True, ), + # === AWS Bedrock (native Converse API via bedrock-runtime) ============= + ProviderSpec( + name="bedrock", + keywords=( + "bedrock", + "anthropic.claude", + "amazon.nova", + "meta.", + "mistral.", + "cohere.", + "qwen.", + "deepseek.", + "openai.gpt-oss", + "ai21.", + "moonshot.", + "writer.", + "zai.", + ), + env_key="AWS_BEARER_TOKEN_BEDROCK", + display_name="AWS Bedrock", + backend="bedrock", + is_direct=True, + ), # === Gateways (detected by api_key / api_base, not model name) ========= # Gateways can route any model, so they win in fallback. # OpenRouter: global gateway, keys start with "sk-or-" diff --git a/nanobot/session/manager.py b/nanobot/session/manager.py index de97135d4..fb1d6cf62 100644 --- a/nanobot/session/manager.py +++ b/nanobot/session/manager.py @@ -118,7 +118,7 @@ class Session: if include_timestamps: content = self._annotate_message_time(message, content) entry: dict[str, Any] = {"role": message["role"], "content": content} - for key in ("tool_calls", "tool_call_id", "name", "reasoning_content"): + for key in ("tool_calls", "tool_call_id", "name", "reasoning_content", "thinking_blocks"): if key in message: entry[key] = message[key] out.append(entry) diff --git a/pyproject.toml b/pyproject.toml index 36185f39f..ff3b2a349 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -61,6 +61,7 @@ dependencies = [ "openpyxl>=3.1.0,<4.0.0", "python-pptx>=1.0.0,<2.0.0", "filelock>=3.25.2", + "boto3>=1.43.0", ] [project.optional-dependencies] diff --git a/tests/agent/test_session_manager_history.py b/tests/agent/test_session_manager_history.py index eff260b7c..b80c774a1 100644 --- a/tests/agent/test_session_manager_history.py +++ b/tests/agent/test_session_manager_history.py @@ -180,6 +180,7 @@ def test_get_history_preserves_reasoning_content(): "role": "assistant", "content": "done", "reasoning_content": "hidden chain of thought", + "thinking_blocks": [{"type": "thinking", "thinking": "hidden chain of thought", "signature": "sig"}], }) history = session.get_history(max_messages=500) @@ -190,6 +191,11 @@ def test_get_history_preserves_reasoning_content(): "role": "assistant", "content": "done", "reasoning_content": "hidden chain of thought", + "thinking_blocks": [{ + "type": "thinking", + "thinking": "hidden chain of thought", + "signature": "sig", + }], }, ] diff --git a/tests/providers/test_bedrock_provider.py b/tests/providers/test_bedrock_provider.py new file mode 100644 index 000000000..e86b8426d --- /dev/null +++ b/tests/providers/test_bedrock_provider.py @@ -0,0 +1,254 @@ +"""Tests for the native AWS Bedrock Converse provider.""" + +from __future__ import annotations + +from typing import Any + +import pytest + +from nanobot.config.schema import Config, ProvidersConfig +from nanobot.providers.bedrock_provider import BedrockProvider +from nanobot.providers.registry import find_by_name + + +class FakeClient: + def __init__( + self, + *, + response: dict[str, Any] | None = None, + stream_events: list[dict[str, Any]] | None = None, + error: Exception | None = None, + ) -> None: + self.response = response + self.stream_events = stream_events or [] + self.error = error + self.calls: list[dict[str, Any]] = [] + self.stream_calls: list[dict[str, Any]] = [] + + def converse(self, **kwargs): + self.calls.append(kwargs) + if self.error: + raise self.error + return self.response or {} + + def converse_stream(self, **kwargs): + self.stream_calls.append(kwargs) + if self.error: + raise self.error + return {"stream": iter(self.stream_events)} + + +class FakeBedrockError(Exception): + def __init__(self) -> None: + super().__init__("too many requests") + self.response = { + "ResponseMetadata": { + "HTTPStatusCode": 429, + "HTTPHeaders": {"retry-after": "3"}, + }, + "Error": { + "Code": "ThrottlingException", + "Message": "Rate exceeded", + }, + } + + +def test_bedrock_provider_is_registered_and_matches_without_api_key() -> None: + spec = find_by_name("bedrock") + assert spec is not None + assert spec.backend == "bedrock" + assert spec.is_direct is True + assert hasattr(ProvidersConfig(), "bedrock") + + cfg = Config.model_validate({ + "agents": {"defaults": {"model": "bedrock/global.anthropic.claude-opus-4-7"}}, + "providers": {"bedrock": {"region": "us-east-1"}}, + }) + + assert cfg.get_provider_name() == "bedrock" + assert cfg.get_provider().region == "us-east-1" + + +def test_opus_47_uses_adaptive_thinking_and_omits_temperature() -> None: + provider = BedrockProvider(region="us-east-1", client=FakeClient()) + + kwargs = provider._build_kwargs( + messages=[{"role": "user", "content": "hi"}], + tools=None, + model="bedrock/global.anthropic.claude-opus-4-7", + max_tokens=2048, + temperature=0.1, + reasoning_effort="medium", + tool_choice=None, + ) + + assert kwargs["modelId"] == "global.anthropic.claude-opus-4-7" + assert kwargs["inferenceConfig"] == {"maxTokens": 2048} + assert kwargs["additionalModelRequestFields"]["thinking"] == { + "type": "adaptive", + "effort": "medium", + } + + +def test_generic_bedrock_model_keeps_temperature_and_skips_anthropic_thinking() -> None: + provider = BedrockProvider(region="us-east-1", client=FakeClient()) + + kwargs = provider._build_kwargs( + messages=[{"role": "user", "content": "hi"}], + tools=None, + model="bedrock/amazon.nova-lite-v1:0", + max_tokens=1024, + temperature=0.3, + reasoning_effort="medium", + tool_choice=None, + ) + + assert kwargs["modelId"] == "amazon.nova-lite-v1:0" + assert kwargs["inferenceConfig"] == {"maxTokens": 1024, "temperature": 0.3} + assert "additionalModelRequestFields" not in kwargs + + +def test_build_kwargs_converts_messages_tools_and_tool_results() -> None: + provider = BedrockProvider(region="us-east-1", client=FakeClient()) + tools = [{ + "type": "function", + "function": { + "name": "read_file", + "description": "Read a file", + "parameters": {"type": "object", "properties": {"path": {"type": "string"}}}, + }, + }] + messages = [ + {"role": "system", "content": "You are helpful."}, + {"role": "user", "content": "read x"}, + { + "role": "assistant", + "content": "", + "tool_calls": [{ + "id": "toolu_1", + "type": "function", + "function": {"name": "read_file", "arguments": '{"path": "x"}'}, + }], + }, + {"role": "tool", "tool_call_id": "toolu_1", "name": "read_file", "content": "ok"}, + {"role": "user", "content": "continue"}, + ] + + kwargs = provider._build_kwargs( + messages=messages, + tools=tools, + model="bedrock/anthropic.claude-opus-4-7", + max_tokens=1024, + temperature=0.7, + reasoning_effort=None, + tool_choice="required", + ) + + assert kwargs["system"] == [{"text": "You are helpful."}] + assert kwargs["messages"][1]["content"] == [{ + "toolUse": { + "toolUseId": "toolu_1", + "name": "read_file", + "input": {"path": "x"}, + } + }] + assert kwargs["messages"][2]["role"] == "user" + assert kwargs["messages"][2]["content"][0]["toolResult"]["toolUseId"] == "toolu_1" + assert kwargs["messages"][2]["content"][1] == {"text": "continue"} + tool_spec = kwargs["toolConfig"]["tools"][0]["toolSpec"] + assert tool_spec["name"] == "read_file" + assert kwargs["toolConfig"]["toolChoice"] == {"any": {}} + + +def test_parse_response_maps_text_tools_reasoning_usage_and_stop_reason() -> None: + response = { + "output": { + "message": { + "role": "assistant", + "content": [ + {"reasoningContent": {"reasoningText": {"text": "think", "signature": "sig"}}}, + {"text": "hello"}, + {"toolUse": {"toolUseId": "t1", "name": "search", "input": {"q": "x"}}}, + ], + } + }, + "stopReason": "tool_use", + "usage": { + "inputTokens": 10, + "outputTokens": 5, + "totalTokens": 15, + "cacheReadInputTokens": 2, + }, + } + + result = BedrockProvider._parse_response(response) + + assert result.content == "hello" + assert result.finish_reason == "tool_calls" + assert result.usage["prompt_tokens"] == 10 + assert result.usage["cached_tokens"] == 2 + assert result.reasoning_content == "think" + assert result.thinking_blocks == [{"type": "thinking", "thinking": "think", "signature": "sig"}] + assert result.tool_calls[0].id == "t1" + assert result.tool_calls[0].arguments == {"q": "x"} + + +@pytest.mark.asyncio +async def test_chat_stream_aggregates_text_tool_use_and_usage() -> None: + client = FakeClient(stream_events=[ + {"contentBlockDelta": {"contentBlockIndex": 0, "delta": {"text": "he"}}}, + {"contentBlockDelta": {"contentBlockIndex": 0, "delta": {"text": "llo"}}}, + { + "contentBlockStart": { + "contentBlockIndex": 1, + "start": {"toolUse": {"toolUseId": "t1", "name": "search"}}, + } + }, + { + "contentBlockDelta": { + "contentBlockIndex": 1, + "delta": {"toolUse": {"input": '{"q":'}}, + } + }, + { + "contentBlockDelta": { + "contentBlockIndex": 1, + "delta": {"toolUse": {"input": '"x"}'}}, + } + }, + {"contentBlockStop": {"contentBlockIndex": 1}}, + {"messageStop": {"stopReason": "tool_use"}}, + {"metadata": {"usage": {"inputTokens": 3, "outputTokens": 4, "totalTokens": 7}}}, + ]) + provider = BedrockProvider(region="us-east-1", client=client) + deltas: list[str] = [] + + result = await provider.chat_stream( + messages=[{"role": "user", "content": "hi"}], + model="bedrock/anthropic.claude-opus-4-7", + on_content_delta=lambda text: _append_delta(deltas, text), + ) + + assert deltas == ["he", "llo"] + assert result.content == "hello" + assert result.finish_reason == "tool_calls" + assert result.usage == {"prompt_tokens": 3, "completion_tokens": 4, "total_tokens": 7} + assert result.tool_calls[0].name == "search" + assert result.tool_calls[0].arguments == {"q": "x"} + + +async def _append_delta(deltas: list[str], text: str) -> None: + deltas.append(text) + + +@pytest.mark.asyncio +async def test_chat_error_maps_retry_metadata() -> None: + provider = BedrockProvider(region="us-east-1", client=FakeClient(error=FakeBedrockError())) + + result = await provider.chat(messages=[{"role": "user", "content": "hi"}]) + + assert result.finish_reason == "error" + assert result.error_status_code == 429 + assert result.error_should_retry is True + assert result.error_code == "throttlingexception" + assert result.retry_after == 3 diff --git a/tests/providers/test_providers_init.py b/tests/providers/test_providers_init.py index 620cf8daf..707a3f683 100644 --- a/tests/providers/test_providers_init.py +++ b/tests/providers/test_providers_init.py @@ -13,6 +13,7 @@ def test_importing_providers_package_is_lazy(monkeypatch) -> None: monkeypatch.delitem(sys.modules, "nanobot.providers.openai_codex_provider", raising=False) monkeypatch.delitem(sys.modules, "nanobot.providers.github_copilot_provider", raising=False) monkeypatch.delitem(sys.modules, "nanobot.providers.azure_openai_provider", raising=False) + monkeypatch.delitem(sys.modules, "nanobot.providers.bedrock_provider", raising=False) providers = importlib.import_module("nanobot.providers") @@ -21,6 +22,7 @@ def test_importing_providers_package_is_lazy(monkeypatch) -> None: assert "nanobot.providers.openai_codex_provider" not in sys.modules assert "nanobot.providers.github_copilot_provider" not in sys.modules assert "nanobot.providers.azure_openai_provider" not in sys.modules + assert "nanobot.providers.bedrock_provider" not in sys.modules assert providers.__all__ == [ "LLMProvider", "LLMResponse", @@ -29,6 +31,7 @@ def test_importing_providers_package_is_lazy(monkeypatch) -> None: "OpenAICodexProvider", "GitHubCopilotProvider", "AzureOpenAIProvider", + "BedrockProvider", ] From 830730f82d1ce5e6cd3f6cdf059ec13d00a2628f Mon Sep 17 00:00:00 2001 From: chengyongru <2755839590@qq.com> Date: Wed, 29 Apr 2026 22:39:38 +0800 Subject: [PATCH 04/33] feat(skills): add update-setup wizard skill --- nanobot/skills/update-setup/SKILL.md | 83 ++++++++++++++++++++++++++++ 1 file changed, 83 insertions(+) create mode 100644 nanobot/skills/update-setup/SKILL.md diff --git a/nanobot/skills/update-setup/SKILL.md b/nanobot/skills/update-setup/SKILL.md new file mode 100644 index 000000000..b2661d869 --- /dev/null +++ b/nanobot/skills/update-setup/SKILL.md @@ -0,0 +1,83 @@ +--- +name: update-setup +description: One-time setup wizard for the nanobot upgrade skill. Triggers: setup update, configure update, 切设置更新, 初始化更新. +--- + +# Update Setup + +Generate a personalized upgrade skill for this workspace. + +## Step 1: Check Existing + +Use `read_file` to check if `skills/update/SKILL.md` already exists in the workspace. + +If it exists, use `ask_user` to ask: "An upgrade skill already exists. Reconfigure?" with options ["yes", "no"]. If no, stop here. + +## Step 2: Current Version + +Use `exec` to run `nanobot --version`. Tell the user the current version. + +## Step 3: Ask Questions + +Use `ask_user` three times, one question per call. + +**Question 1 — Install method:** + +``` +question: "How did you install nanobot?" +options: ["uv", "pipx", "pip", "source (git clone)"] +``` + +**Question 2 — Optional dependencies:** + +``` +question: "Which optional dependencies do you need? List names separated by spaces, or reply 'none'. Available: wecom, weixin, msteams, matrix, discord, langsmith, pdf" +``` + +Parse the reply. If the user says "none" or similar, set extras to empty. Otherwise collect the valid names. + +**Question 3 — Proxy:** + +``` +question: "Do you need an HTTP proxy to reach PyPI or GitHub?" +options: ["no", "yes"] +``` + +If yes, ask one more time for the proxy URL: `question: "Enter proxy URL (e.g. http://127.0.0.1:7890):"`. + +## Step 4: Generate Skill + +Build the extras string. If the user selected dependencies, format as `[dep1,dep2,...]`. Otherwise omit the brackets entirely. + +Determine the upgrade command from the install method: + +| Method | Command | +|--------|---------| +| uv | `uv tool install nanobot-ai[EXTRAS] --force` | +| pipx | `pipx install nanobot-ai[EXTRAS] --upgrade` | +| pip | `pip install --upgrade "nanobot-ai[EXTRAS]"` | +| source | `git pull && uv sync` | + +For source installs, ignore extras (uv sync handles them). + +Build the skill content. If proxy is configured, add `export http_proxy=URL` and `export https_proxy=URL` lines before the upgrade command. + +Use `write_file` to write `skills/update/SKILL.md` with this content: + +``` +--- +name: update +description: "Upgrade nanobot to the latest version. Triggers: upgrade nanobot, update nanobot, 升级nanobot, 更新nanobot." +--- + +# Update Nanobot + +1. (If proxy configured) Set proxy: `export http_proxy=URL && export https_proxy=URL` +2. Use `exec` to run the upgrade command: +3. Use `exec` to verify: `nanobot --version` +4. Tell the user the new version. Say: "Restart nanobot to apply the update." +``` + +## Step 5: Confirm + +Tell the user: "Upgrade skill created. Say 'upgrade nanobot' when you want to update." From 6891a7a4d481289f96201f7bccba8124cd53a48d Mon Sep 17 00:00:00 2001 From: Xubin Ren Date: Fri, 1 May 2026 11:00:01 +0000 Subject: [PATCH 05/33] fix(skills): correct update setup commands Made-with: Cursor --- nanobot/skills/update-setup/SKILL.md | 17 ++++++++++------- 1 file changed, 10 insertions(+), 7 deletions(-) diff --git a/nanobot/skills/update-setup/SKILL.md b/nanobot/skills/update-setup/SKILL.md index b2661d869..215a43c9b 100644 --- a/nanobot/skills/update-setup/SKILL.md +++ b/nanobot/skills/update-setup/SKILL.md @@ -19,7 +19,7 @@ Use `exec` to run `nanobot --version`. Tell the user the current version. ## Step 3: Ask Questions -Use `ask_user` three times, one question per call. +Use `ask_user` for the questions below, one question per call. **Question 1 — Install method:** @@ -28,10 +28,13 @@ question: "How did you install nanobot?" options: ["uv", "pipx", "pip", "source (git clone)"] ``` +If the user selected `source (git clone)`, ask for the local checkout path: +`question: "Where is your nanobot source checkout? Enter an absolute path or a path relative to this workspace:"`. + **Question 2 — Optional dependencies:** ``` -question: "Which optional dependencies do you need? List names separated by spaces, or reply 'none'. Available: wecom, weixin, msteams, matrix, discord, langsmith, pdf" +question: "Which optional dependencies do you need? List names separated by spaces, or reply 'none'. Available: api, wecom, weixin, msteams, matrix, discord, langsmith, pdf" ``` Parse the reply. If the user says "none" or similar, set extras to empty. Otherwise collect the valid names. @@ -53,12 +56,12 @@ Determine the upgrade command from the install method: | Method | Command | |--------|---------| -| uv | `uv tool install nanobot-ai[EXTRAS] --force` | -| pipx | `pipx install nanobot-ai[EXTRAS] --upgrade` | -| pip | `pip install --upgrade "nanobot-ai[EXTRAS]"` | -| source | `git pull && uv sync` | +| uv | `uv tool install "nanobot-ai[EXTRAS]" --force` | +| pipx | `pipx install --force "nanobot-ai[EXTRAS]"` | +| pip | `python -m pip install --upgrade "nanobot-ai[EXTRAS]"` | +| source | `cd && git pull && python -m pip install -e ".[EXTRAS]"` | -For source installs, ignore extras (uv sync handles them). +For source installs, include extras in the editable install command when selected. Quote the source checkout path if it contains spaces. Build the skill content. If proxy is configured, add `export http_proxy=URL` and `export https_proxy=URL` lines before the upgrade command. From 58ae2d5b7e78369ffb17a6c03cee5afdff6ec671 Mon Sep 17 00:00:00 2001 From: LZDQ <1486701401@qq.com> Date: Fri, 1 May 2026 18:58:41 +0800 Subject: [PATCH 06/33] Claude: replace module-level file read states with per-loop per-session state class. fixes #3571 --- nanobot/agent/loop.py | 20 ++- nanobot/agent/memory.py | 9 +- nanobot/agent/subagent.py | 16 +- nanobot/agent/tools/file_state.py | 205 ++++++++++++++++---------- nanobot/agent/tools/filesystem.py | 29 ++-- tests/tools/test_edit_enhancements.py | 12 +- tests/tools/test_read_enhancements.py | 31 ++++ 7 files changed, 214 insertions(+), 108 deletions(-) diff --git a/nanobot/agent/loop.py b/nanobot/agent/loop.py index cbddfc286..756391995 100644 --- a/nanobot/agent/loop.py +++ b/nanobot/agent/loop.py @@ -28,6 +28,7 @@ from nanobot.agent.tools.ask import ( pending_ask_user_id, ) from nanobot.agent.tools.cron import CronTool +from nanobot.agent.tools.file_state import FileStates from nanobot.agent.tools.filesystem import EditFileTool, ListDirTool, ReadFileTool, WriteFileTool from nanobot.agent.tools.message import MessageTool from nanobot.agent.tools.notebook import NotebookEditTool @@ -247,6 +248,10 @@ class AgentLoop: self.context = ContextBuilder(workspace, timezone=timezone, disabled_skills=disabled_skills) self.sessions = session_manager or SessionManager(workspace) self.tools = ToolRegistry() + # Per-session file-read/write tracker (issue #3571) — shared across + # the filesystem tools registered below so this AgentLoop does not + # leak read-dedup state into another loop's tools. + self._file_states = FileStates() self.runner = AgentRunner(provider) self.subagents = SubagentManager( provider=provider, @@ -348,17 +353,24 @@ class AgentLoop: self.workspace if (self.restrict_to_workspace or self.exec_config.sandbox) else None ) extra_read = [BUILTIN_SKILLS_DIR] if allowed_dir else None + # One FileStates per session so read-dedup and read-before-edit + # tracking does not leak across sessions sharing this process + # (issue #3571). + file_states = self._file_states self.tools.register(AskUserTool()) self.tools.register( ReadFileTool( - workspace=self.workspace, allowed_dir=allowed_dir, extra_allowed_dirs=extra_read + workspace=self.workspace, + allowed_dir=allowed_dir, + extra_allowed_dirs=extra_read, + file_states=file_states, ) ) for cls in (WriteFileTool, EditFileTool, ListDirTool): - self.tools.register(cls(workspace=self.workspace, allowed_dir=allowed_dir)) + self.tools.register(cls(workspace=self.workspace, allowed_dir=allowed_dir, file_states=file_states)) for cls in (GlobTool, GrepTool): - self.tools.register(cls(workspace=self.workspace, allowed_dir=allowed_dir)) - self.tools.register(NotebookEditTool(workspace=self.workspace, allowed_dir=allowed_dir)) + self.tools.register(cls(workspace=self.workspace, allowed_dir=allowed_dir, file_states=file_states)) + self.tools.register(NotebookEditTool(workspace=self.workspace, allowed_dir=allowed_dir, file_states=file_states)) if self.exec_config.enable: self.tools.register( ExecTool( diff --git a/nanobot/agent/memory.py b/nanobot/agent/memory.py index 4bf79c356..80f6c580f 100644 --- a/nanobot/agent/memory.py +++ b/nanobot/agent/memory.py @@ -753,23 +753,28 @@ class Dream: def _build_tools(self) -> ToolRegistry: """Build a minimal tool registry for the Dream agent.""" from nanobot.agent.skills import BUILTIN_SKILLS_DIR + from nanobot.agent.tools.file_state import FileStates from nanobot.agent.tools.filesystem import EditFileTool, ReadFileTool, WriteFileTool tools = ToolRegistry() workspace = self.store.workspace # Allow reading builtin skills for reference during skill creation extra_read = [BUILTIN_SKILLS_DIR] if BUILTIN_SKILLS_DIR.exists() else None + # Dream gets its own FileStates so its caches stay isolated from the + # main loop's sessions (issue #3571). + file_states = FileStates() tools.register(ReadFileTool( workspace=workspace, allowed_dir=workspace, extra_allowed_dirs=extra_read, + file_states=file_states, )) - tools.register(EditFileTool(workspace=workspace, allowed_dir=workspace)) + tools.register(EditFileTool(workspace=workspace, allowed_dir=workspace, file_states=file_states)) # write_file resolves relative paths from workspace root, but can only # write under skills/ so the prompt can safely use skills//SKILL.md. skills_dir = workspace / "skills" skills_dir.mkdir(parents=True, exist_ok=True) - tools.register(WriteFileTool(workspace=workspace, allowed_dir=skills_dir)) + tools.register(WriteFileTool(workspace=workspace, allowed_dir=skills_dir, file_states=file_states)) return tools # -- skill listing -------------------------------------------------------- diff --git a/nanobot/agent/subagent.py b/nanobot/agent/subagent.py index e64dc8f97..6d16c7699 100644 --- a/nanobot/agent/subagent.py +++ b/nanobot/agent/subagent.py @@ -168,12 +168,16 @@ class SubagentManager: tools = ToolRegistry() allowed_dir = self.workspace if (self.restrict_to_workspace or self.exec_config.sandbox) else None extra_read = [BUILTIN_SKILLS_DIR] if allowed_dir else None - tools.register(ReadFileTool(workspace=self.workspace, allowed_dir=allowed_dir, extra_allowed_dirs=extra_read)) - tools.register(WriteFileTool(workspace=self.workspace, allowed_dir=allowed_dir)) - tools.register(EditFileTool(workspace=self.workspace, allowed_dir=allowed_dir)) - tools.register(ListDirTool(workspace=self.workspace, allowed_dir=allowed_dir)) - tools.register(GlobTool(workspace=self.workspace, allowed_dir=allowed_dir)) - tools.register(GrepTool(workspace=self.workspace, allowed_dir=allowed_dir)) + # Subagent gets its own FileStates so its read-dedup cache is + # isolated from the parent loop's sessions (issue #3571). + from nanobot.agent.tools.file_state import FileStates + file_states = FileStates() + tools.register(ReadFileTool(workspace=self.workspace, allowed_dir=allowed_dir, extra_allowed_dirs=extra_read, file_states=file_states)) + tools.register(WriteFileTool(workspace=self.workspace, allowed_dir=allowed_dir, file_states=file_states)) + tools.register(EditFileTool(workspace=self.workspace, allowed_dir=allowed_dir, file_states=file_states)) + tools.register(ListDirTool(workspace=self.workspace, allowed_dir=allowed_dir, file_states=file_states)) + tools.register(GlobTool(workspace=self.workspace, allowed_dir=allowed_dir, file_states=file_states)) + tools.register(GrepTool(workspace=self.workspace, allowed_dir=allowed_dir, file_states=file_states)) if self.exec_config.enable: tools.register(ExecTool( working_dir=str(self.workspace), diff --git a/nanobot/agent/tools/file_state.py b/nanobot/agent/tools/file_state.py index 74bb9bf65..018581ac9 100644 --- a/nanobot/agent/tools/file_state.py +++ b/nanobot/agent/tools/file_state.py @@ -17,9 +17,6 @@ class ReadState: can_dedup: bool -_state: dict[str, ReadState] = {} - - def _hash_file(p: str) -> str | None: try: return hashlib.sha256(Path(p).read_bytes()).hexdigest() @@ -27,93 +24,141 @@ def _hash_file(p: str) -> str | None: return None +class FileStates: + """Per-session read/write tracker. + + Owns its own state dict so read-dedup ("File unchanged since last read") + and read-before-edit warnings stay scoped to one agent session and do + not leak across sessions sharing this process. + """ + + __slots__ = ("_state",) + + def __init__(self) -> None: + self._state: dict[str, ReadState] = {} + + def record_read(self, path: str | Path, offset: int = 1, limit: int | None = None) -> None: + """Record that a file was read (called after successful read).""" + p = str(Path(path).resolve()) + try: + mtime = os.path.getmtime(p) + except OSError: + return + self._state[p] = ReadState( + mtime=mtime, + offset=offset, + limit=limit, + content_hash=_hash_file(p), + can_dedup=True, + ) + + def record_write(self, path: str | Path) -> None: + """Record that a file was written (updates mtime in state).""" + p = str(Path(path).resolve()) + try: + mtime = os.path.getmtime(p) + except OSError: + self._state.pop(p, None) + return + self._state[p] = ReadState( + mtime=mtime, + offset=1, + limit=None, + content_hash=_hash_file(p), + can_dedup=False, + ) + + def check_read(self, path: str | Path) -> str | None: + """Check if a file has been read and is fresh. + + Returns None if OK, or a warning string. + When mtime changed but file content is identical (e.g. touch, editor save), + the check passes to avoid false-positive staleness warnings. + """ + p = str(Path(path).resolve()) + entry = self._state.get(p) + if entry is None: + return "Warning: file has not been read yet. Read it first to verify content before editing." + try: + current_mtime = os.path.getmtime(p) + except OSError: + return None + if current_mtime != entry.mtime: + if entry.content_hash and _hash_file(p) == entry.content_hash: + entry.mtime = current_mtime + return None + return "Warning: file has been modified since last read. Re-read to verify content before editing." + # mtime unchanged - still check content hash to detect quick modifications + if entry.content_hash and _hash_file(p) != entry.content_hash: + return "Warning: file has been modified since last read. Re-read to verify content before editing." + return None + + def is_unchanged(self, path: str | Path, offset: int = 1, limit: int | None = None) -> bool: + """Return True if file was previously read with same params and content is unchanged.""" + p = str(Path(path).resolve()) + entry = self._state.get(p) + if entry is None: + return False + if not entry.can_dedup: + return False + if entry.offset != offset or entry.limit != limit: + return False + try: + current_mtime = os.path.getmtime(p) + except OSError: + return False + if current_mtime != entry.mtime: + # mtime changed - check if content also changed + current_hash = _hash_file(p) + if current_hash != entry.content_hash: + # Content actually changed - don't dedup + entry.can_dedup = False + return False + # Content identical despite mtime change (e.g. touch) - mark as not dedupable to force full read next time + entry.can_dedup = False + return True + # mtime unchanged - content must be identical + return True + + def get(self, path: str | Path) -> ReadState | None: + """Return the raw ReadState entry for a path, or None.""" + return self._state.get(str(Path(path).resolve())) + + def clear(self) -> None: + """Clear all tracked state (useful for testing).""" + self._state.clear() + + +# Module-level default instance, retained for backward compatibility with +# tests and callers that reach in directly. Per-session callers should hold +# their own FileStates instance instead of touching this one. +_default = FileStates() + + def record_read(path: str | Path, offset: int = 1, limit: int | None = None) -> None: - """Record that a file was read (called after successful read).""" - p = str(Path(path).resolve()) - try: - mtime = os.path.getmtime(p) - except OSError: - return - _state[p] = ReadState( - mtime=mtime, - offset=offset, - limit=limit, - content_hash=_hash_file(p), - can_dedup=True, - ) + _default.record_read(path, offset=offset, limit=limit) def record_write(path: str | Path) -> None: - """Record that a file was written (updates mtime in state).""" - p = str(Path(path).resolve()) - try: - mtime = os.path.getmtime(p) - except OSError: - _state.pop(p, None) - return - _state[p] = ReadState( - mtime=mtime, - offset=1, - limit=None, - content_hash=_hash_file(p), - can_dedup=False, - ) + _default.record_write(path) def check_read(path: str | Path) -> str | None: - """Check if a file has been read and is fresh. - - Returns None if OK, or a warning string. - When mtime changed but file content is identical (e.g. touch, editor save), - the check passes to avoid false-positive staleness warnings. - """ - p = str(Path(path).resolve()) - entry = _state.get(p) - if entry is None: - return "Warning: file has not been read yet. Read it first to verify content before editing." - try: - current_mtime = os.path.getmtime(p) - except OSError: - return None - if current_mtime != entry.mtime: - if entry.content_hash and _hash_file(p) == entry.content_hash: - entry.mtime = current_mtime - return None - return "Warning: file has been modified since last read. Re-read to verify content before editing." - # mtime unchanged - still check content hash to detect quick modifications - if entry.content_hash and _hash_file(p) != entry.content_hash: - return "Warning: file has been modified since last read. Re-read to verify content before editing." - return None + return _default.check_read(path) def is_unchanged(path: str | Path, offset: int = 1, limit: int | None = None) -> bool: - """Return True if file was previously read with same params and content is unchanged.""" - p = str(Path(path).resolve()) - entry = _state.get(p) - if entry is None: - return False - if not entry.can_dedup: - return False - if entry.offset != offset or entry.limit != limit: - return False - try: - current_mtime = os.path.getmtime(p) - except OSError: - return False - if current_mtime != entry.mtime: - # mtime changed - check if content also changed - current_hash = _hash_file(p) - if current_hash != entry.content_hash: - # Content actually changed - don't dedup - entry.can_dedup = False - return False - # Content identical despite mtime change (e.g. touch) - mark as not dedupable to force full read next time - entry.can_dedup = False - return True - # mtime unchanged - content must be identical - return True + return _default.is_unchanged(path, offset=offset, limit=limit) def clear() -> None: - """Clear all tracked state (useful for testing).""" - _state.clear() + _default.clear() + + +# Legacy attribute for callers that reached into the module-level dict +# directly (filesystem.py used to do this). Kept as a property-like accessor +# so existing imports keep working. +def __getattr__(name: str): + if name == "_state": + return _default._state + raise AttributeError(name) diff --git a/nanobot/agent/tools/filesystem.py b/nanobot/agent/tools/filesystem.py index c0d628a71..500f0123b 100644 --- a/nanobot/agent/tools/filesystem.py +++ b/nanobot/agent/tools/filesystem.py @@ -9,7 +9,7 @@ from typing import Any from nanobot.agent.tools.base import Tool, tool_parameters from nanobot.agent.tools.schema import BooleanSchema, IntegerSchema, StringSchema, tool_parameters_schema -from nanobot.agent.tools import file_state +from nanobot.agent.tools.file_state import FileStates, _hash_file from nanobot.utils.helpers import build_image_content_blocks, detect_image_mime from nanobot.config.paths import get_media_dir @@ -49,10 +49,15 @@ class _FsTool(Tool): workspace: Path | None = None, allowed_dir: Path | None = None, extra_allowed_dirs: list[Path] | None = None, + file_states: FileStates | None = None, ): self._workspace = workspace self._allowed_dir = allowed_dir self._extra_allowed_dirs = extra_allowed_dirs + # Read-dedup / read-before-edit state is scoped to one FileStates + # so it does not leak across sessions sharing this process + # (issue #3571). Bare constructions get a private instance. + self._file_states: FileStates = file_states if file_states is not None else FileStates() def _resolve(self, path: str) -> Path: return _resolve_path(path, self._workspace, self._allowed_dir, self._extra_allowed_dirs) @@ -184,7 +189,7 @@ class ReadFileTool(_FsTool): # Read dedup: same path + offset + limit + unchanged mtime → stub # Always check for external modifications before dedup - entry = file_state._state.get(str(fp.resolve())) + entry = self._file_states.get(fp) try: current_mtime = os.path.getmtime(fp) except OSError: @@ -193,21 +198,21 @@ class ReadFileTool(_FsTool): if current_mtime != entry.mtime: # File was modified externally - force full read and mark as not dedupable entry.can_dedup = False - file_state.record_read(fp, offset=offset, limit=limit) # Update state with new mtime + self._file_states.record_read(fp, offset=offset, limit=limit) # Update state with new mtime # Continue to read full content (don't return dedup message) else: # File unchanged - return dedup message # But only if content is actually unchanged (not just mtime) - current_hash = file_state._hash_file(str(fp)) + current_hash = _hash_file(str(fp)) if current_hash == entry.content_hash: return f"[File unchanged since last read: {path}]" else: # Content changed despite same mtime - force full read entry.can_dedup = False - file_state.record_read(fp, offset=offset, limit=limit) + self._file_states.record_read(fp, offset=offset, limit=limit) else: # No previous state or marked as not dedupable - read full content - file_state.record_read(fp, offset=offset, limit=limit) + self._file_states.record_read(fp, offset=offset, limit=limit) # Force full read by setting can_dedup to False for this read if entry: entry.can_dedup = False @@ -256,7 +261,7 @@ class ReadFileTool(_FsTool): result += f"\n\n(Showing lines {offset}-{end} of {total}. Use offset={end + 1} to continue.)" else: result += f"\n\n(End of file — {total} lines total)" - file_state.record_read(fp, offset=offset, limit=limit) + self._file_states.record_read(fp, offset=offset, limit=limit) return result except PermissionError as e: return f"Error: {e}" @@ -365,7 +370,7 @@ class WriteFileTool(_FsTool): fp = self._resolve(path) fp.parent.mkdir(parents=True, exist_ok=True) fp.write_text(content, encoding="utf-8") - file_state.record_write(fp) + self._file_states.record_write(fp) return f"Successfully wrote {len(content)} characters to {fp}" except PermissionError as e: return f"Error: {e}" @@ -699,7 +704,7 @@ class EditFileTool(_FsTool): if old_text == "": fp.parent.mkdir(parents=True, exist_ok=True) fp.write_text(new_text, encoding="utf-8") - file_state.record_write(fp) + self._file_states.record_write(fp) return f"Successfully created {fp}" return self._file_not_found_msg(path, fp) @@ -718,11 +723,11 @@ class EditFileTool(_FsTool): if content.strip(): return f"Error: Cannot create file — {path} already exists and is not empty." fp.write_text(new_text, encoding="utf-8") - file_state.record_write(fp) + self._file_states.record_write(fp) return f"Successfully edited {fp}" # Read-before-edit check - warning = file_state.check_read(fp) + warning = self._file_states.check_read(fp) raw = fp.read_bytes() uses_crlf = b"\r\n" in raw @@ -767,7 +772,7 @@ class EditFileTool(_FsTool): new_content = new_content.replace("\n", "\r\n") fp.write_bytes(new_content.encode("utf-8")) - file_state.record_write(fp) + self._file_states.record_write(fp) msg = f"Successfully edited {fp}" if warning: msg = f"{warning}\n{msg}" diff --git a/tests/tools/test_edit_enhancements.py b/tests/tools/test_edit_enhancements.py index 7ad098960..1f22c963b 100644 --- a/tests/tools/test_edit_enhancements.py +++ b/tests/tools/test_edit_enhancements.py @@ -27,12 +27,16 @@ class TestEditReadTracking: """edit_file should warn when file hasn't been read first.""" @pytest.fixture() - def read_tool(self, tmp_path): - return ReadFileTool(workspace=tmp_path) + def file_states(self): + return file_state.FileStates() @pytest.fixture() - def edit_tool(self, tmp_path): - return EditFileTool(workspace=tmp_path) + def read_tool(self, tmp_path, file_states): + return ReadFileTool(workspace=tmp_path, file_states=file_states) + + @pytest.fixture() + def edit_tool(self, tmp_path, file_states): + return EditFileTool(workspace=tmp_path, file_states=file_states) @pytest.mark.asyncio async def test_edit_warns_if_file_not_read_first(self, edit_tool, tmp_path): diff --git a/tests/tools/test_read_enhancements.py b/tests/tools/test_read_enhancements.py index f7a62f05b..b5577f058 100644 --- a/tests/tools/test_read_enhancements.py +++ b/tests/tools/test_read_enhancements.py @@ -97,6 +97,37 @@ class TestReadDedup: assert isinstance(second, list) +# --------------------------------------------------------------------------- +# Cross-session isolation (issue #3571) +# --------------------------------------------------------------------------- +# Each session must keep its own read cache. When session A reads a file, +# session B reading the same file must still receive the full content, not +# the "[File unchanged since last read]" dedup stub. The stub is only valid +# within the session that first cached the read. + +class TestReadDedupSessionIsolation: + + @pytest.mark.asyncio + async def test_separate_sessions_do_not_share_dedup_state(self, tmp_path): + f = tmp_path / "shared.txt" + f.write_text("\n".join(f"line {i}" for i in range(10)), encoding="utf-8") + + session_a_tool = ReadFileTool(workspace=tmp_path) + session_b_tool = ReadFileTool(workspace=tmp_path) + + first = await session_a_tool.execute(path=str(f)) + assert "line 0" in first + + # Session B has never read this file before — it must see the full + # content, not the dedup stub from session A. + second = await session_b_tool.execute(path=str(f)) + assert "unchanged" not in second.lower(), ( + "Session B should not inherit session A's read-dedup state. " + f"Got: {second!r}" + ) + assert "line 0" in second + + # --------------------------------------------------------------------------- # PDF support # --------------------------------------------------------------------------- From fae38319ca5e9e266616773420722ce758aac5fe Mon Sep 17 00:00:00 2001 From: Xubin Ren Date: Fri, 1 May 2026 11:11:21 +0000 Subject: [PATCH 07/33] fix(tools): scope file state by session Made-with: Cursor --- nanobot/agent/loop.py | 71 +++++++++++++++------------ nanobot/agent/tools/file_state.py | 21 ++++++++ nanobot/agent/tools/filesystem.py | 17 +++++-- tests/tools/test_read_enhancements.py | 30 +++++++++++ 4 files changed, 102 insertions(+), 37 deletions(-) diff --git a/nanobot/agent/loop.py b/nanobot/agent/loop.py index 756391995..490172af6 100644 --- a/nanobot/agent/loop.py +++ b/nanobot/agent/loop.py @@ -28,7 +28,7 @@ from nanobot.agent.tools.ask import ( pending_ask_user_id, ) from nanobot.agent.tools.cron import CronTool -from nanobot.agent.tools.file_state import FileStates +from nanobot.agent.tools.file_state import FileStates, bind_file_states, reset_file_states from nanobot.agent.tools.filesystem import EditFileTool, ListDirTool, ReadFileTool, WriteFileTool from nanobot.agent.tools.message import MessageTool from nanobot.agent.tools.notebook import NotebookEditTool @@ -248,10 +248,9 @@ class AgentLoop: self.context = ContextBuilder(workspace, timezone=timezone, disabled_skills=disabled_skills) self.sessions = session_manager or SessionManager(workspace) self.tools = ToolRegistry() - # Per-session file-read/write tracker (issue #3571) — shared across - # the filesystem tools registered below so this AgentLoop does not - # leak read-dedup state into another loop's tools. - self._file_states = FileStates() + # One file-read/write tracker per logical session. The tool registry is + # shared by this loop, so tools resolve the active state via contextvars. + self._file_states_by_session: dict[str, FileStates] = {} self.runner = AgentRunner(provider) self.subagents = SubagentManager( provider=provider, @@ -313,6 +312,14 @@ class AgentLoop: self.commands = CommandRouter() register_builtin_commands(self.commands) + def _file_states_for_session(self, session_key: str | None) -> FileStates: + key = session_key or "__default__" + states = self._file_states_by_session.get(key) + if states is None: + states = FileStates() + self._file_states_by_session[key] = states + return states + def _sync_subagent_runtime_limits(self) -> None: """Keep subagent runtime limits aligned with mutable loop settings.""" self.subagents.max_iterations = self.max_iterations @@ -353,24 +360,19 @@ class AgentLoop: self.workspace if (self.restrict_to_workspace or self.exec_config.sandbox) else None ) extra_read = [BUILTIN_SKILLS_DIR] if allowed_dir else None - # One FileStates per session so read-dedup and read-before-edit - # tracking does not leak across sessions sharing this process - # (issue #3571). - file_states = self._file_states self.tools.register(AskUserTool()) self.tools.register( ReadFileTool( workspace=self.workspace, allowed_dir=allowed_dir, extra_allowed_dirs=extra_read, - file_states=file_states, ) ) for cls in (WriteFileTool, EditFileTool, ListDirTool): - self.tools.register(cls(workspace=self.workspace, allowed_dir=allowed_dir, file_states=file_states)) + self.tools.register(cls(workspace=self.workspace, allowed_dir=allowed_dir)) for cls in (GlobTool, GrepTool): - self.tools.register(cls(workspace=self.workspace, allowed_dir=allowed_dir, file_states=file_states)) - self.tools.register(NotebookEditTool(workspace=self.workspace, allowed_dir=allowed_dir, file_states=file_states)) + self.tools.register(cls(workspace=self.workspace, allowed_dir=allowed_dir)) + self.tools.register(NotebookEditTool(workspace=self.workspace, allowed_dir=allowed_dir)) if self.exec_config.enable: self.tools.register( ExecTool( @@ -630,25 +632,30 @@ class AgentLoop: return items - result = await self.runner.run(AgentRunSpec( - initial_messages=initial_messages, - tools=self.tools, - model=self.model, - max_iterations=self.max_iterations, - max_tool_result_chars=self.max_tool_result_chars, - hook=hook, - error_message="Sorry, I encountered an error calling the AI model.", - concurrent_tools=True, - workspace=self.workspace, - session_key=session.key if session else None, - context_window_tokens=self.context_window_tokens, - context_block_limit=self.context_block_limit, - provider_retry_mode=self.provider_retry_mode, - progress_callback=on_progress, - retry_wait_callback=on_retry_wait, - checkpoint_callback=_checkpoint, - injection_callback=_drain_pending, - )) + active_session_key = session.key if session else session_key + file_state_token = bind_file_states(self._file_states_for_session(active_session_key)) + try: + result = await self.runner.run(AgentRunSpec( + initial_messages=initial_messages, + tools=self.tools, + model=self.model, + max_iterations=self.max_iterations, + max_tool_result_chars=self.max_tool_result_chars, + hook=hook, + error_message="Sorry, I encountered an error calling the AI model.", + concurrent_tools=True, + workspace=self.workspace, + session_key=session.key if session else None, + context_window_tokens=self.context_window_tokens, + context_block_limit=self.context_block_limit, + provider_retry_mode=self.provider_retry_mode, + progress_callback=on_progress, + retry_wait_callback=on_retry_wait, + checkpoint_callback=_checkpoint, + injection_callback=_drain_pending, + )) + finally: + reset_file_states(file_state_token) self._last_usage = result.usage if result.stop_reason == "max_iterations": logger.warning("Max iterations ({}) reached", self.max_iterations) diff --git a/nanobot/agent/tools/file_state.py b/nanobot/agent/tools/file_state.py index 018581ac9..4cf5af873 100644 --- a/nanobot/agent/tools/file_state.py +++ b/nanobot/agent/tools/file_state.py @@ -4,6 +4,7 @@ from __future__ import annotations import hashlib import os +from contextvars import ContextVar, Token from dataclasses import dataclass from pathlib import Path @@ -129,6 +130,26 @@ class FileStates: self._state.clear() +_current_file_states: ContextVar[FileStates | None] = ContextVar( + "nanobot_file_states", + default=None, +) + + +def current_file_states(default: FileStates) -> FileStates: + """Return the FileStates bound to the current agent task, or a fallback.""" + return _current_file_states.get() or default + + +def bind_file_states(file_states: FileStates) -> Token[FileStates | None]: + """Bind file read/write state for the current async task.""" + return _current_file_states.set(file_states) + + +def reset_file_states(token: Token[FileStates | None]) -> None: + _current_file_states.reset(token) + + # Module-level default instance, retained for backward compatibility with # tests and callers that reach in directly. Per-session callers should hold # their own FileStates instance instead of touching this one. diff --git a/nanobot/agent/tools/filesystem.py b/nanobot/agent/tools/filesystem.py index 500f0123b..587a149f2 100644 --- a/nanobot/agent/tools/filesystem.py +++ b/nanobot/agent/tools/filesystem.py @@ -9,7 +9,7 @@ from typing import Any from nanobot.agent.tools.base import Tool, tool_parameters from nanobot.agent.tools.schema import BooleanSchema, IntegerSchema, StringSchema, tool_parameters_schema -from nanobot.agent.tools.file_state import FileStates, _hash_file +from nanobot.agent.tools.file_state import FileStates, _hash_file, current_file_states from nanobot.utils.helpers import build_image_content_blocks, detect_image_mime from nanobot.config.paths import get_media_dir @@ -54,10 +54,17 @@ class _FsTool(Tool): self._workspace = workspace self._allowed_dir = allowed_dir self._extra_allowed_dirs = extra_allowed_dirs - # Read-dedup / read-before-edit state is scoped to one FileStates - # so it does not leak across sessions sharing this process - # (issue #3571). Bare constructions get a private instance. - self._file_states: FileStates = file_states if file_states is not None else FileStates() + # Explicit state is used by isolated runners like Dream/subagents. + # Main AgentLoop tools leave this unset and resolve state from the + # current async task, which keeps shared tool instances session-safe. + self._explicit_file_states = file_states + self._fallback_file_states = FileStates() + + @property + def _file_states(self) -> FileStates: + if self._explicit_file_states is not None: + return self._explicit_file_states + return current_file_states(self._fallback_file_states) def _resolve(self, path: str) -> Path: return _resolve_path(path, self._workspace, self._allowed_dir, self._extra_allowed_dirs) diff --git a/tests/tools/test_read_enhancements.py b/tests/tools/test_read_enhancements.py index b5577f058..490623d5b 100644 --- a/tests/tools/test_read_enhancements.py +++ b/tests/tools/test_read_enhancements.py @@ -127,6 +127,36 @@ class TestReadDedupSessionIsolation: ) assert "line 0" in second + @pytest.mark.asyncio + async def test_shared_loop_tool_uses_bound_session_state(self, tmp_path): + f = tmp_path / "shared.txt" + f.write_text("\n".join(f"line {i}" for i in range(10)), encoding="utf-8") + + # AgentLoop registers one shared ReadFileTool instance. The session + # boundary is the task-local FileStates binding, not the tool object. + shared_tool = ReadFileTool(workspace=tmp_path) + session_a = file_state.FileStates() + session_b = file_state.FileStates() + + token = file_state.bind_file_states(session_a) + try: + first = await shared_tool.execute(path=str(f)) + repeat = await shared_tool.execute(path=str(f)) + finally: + file_state.reset_file_states(token) + + assert "line 0" in first + assert "unchanged" in repeat.lower() + + token = file_state.bind_file_states(session_b) + try: + second_session_read = await shared_tool.execute(path=str(f)) + finally: + file_state.reset_file_states(token) + + assert "unchanged" not in second_session_read.lower() + assert "line 0" in second_session_read + # --------------------------------------------------------------------------- # PDF support From 39c38b593f74e301937ee44085ff6ad0d570c423 Mon Sep 17 00:00:00 2001 From: Xubin Ren Date: Fri, 1 May 2026 11:13:59 +0000 Subject: [PATCH 08/33] refactor(tools): move file state lookup out of loop Made-with: Cursor --- nanobot/agent/loop.py | 14 +++----------- nanobot/agent/tools/file_state.py | 20 ++++++++++++++++++++ 2 files changed, 23 insertions(+), 11 deletions(-) diff --git a/nanobot/agent/loop.py b/nanobot/agent/loop.py index 490172af6..b18bfe37c 100644 --- a/nanobot/agent/loop.py +++ b/nanobot/agent/loop.py @@ -28,7 +28,7 @@ from nanobot.agent.tools.ask import ( pending_ask_user_id, ) from nanobot.agent.tools.cron import CronTool -from nanobot.agent.tools.file_state import FileStates, bind_file_states, reset_file_states +from nanobot.agent.tools.file_state import FileStateStore, bind_file_states, reset_file_states from nanobot.agent.tools.filesystem import EditFileTool, ListDirTool, ReadFileTool, WriteFileTool from nanobot.agent.tools.message import MessageTool from nanobot.agent.tools.notebook import NotebookEditTool @@ -250,7 +250,7 @@ class AgentLoop: self.tools = ToolRegistry() # One file-read/write tracker per logical session. The tool registry is # shared by this loop, so tools resolve the active state via contextvars. - self._file_states_by_session: dict[str, FileStates] = {} + self._file_state_store = FileStateStore() self.runner = AgentRunner(provider) self.subagents = SubagentManager( provider=provider, @@ -312,14 +312,6 @@ class AgentLoop: self.commands = CommandRouter() register_builtin_commands(self.commands) - def _file_states_for_session(self, session_key: str | None) -> FileStates: - key = session_key or "__default__" - states = self._file_states_by_session.get(key) - if states is None: - states = FileStates() - self._file_states_by_session[key] = states - return states - def _sync_subagent_runtime_limits(self) -> None: """Keep subagent runtime limits aligned with mutable loop settings.""" self.subagents.max_iterations = self.max_iterations @@ -633,7 +625,7 @@ class AgentLoop: return items active_session_key = session.key if session else session_key - file_state_token = bind_file_states(self._file_states_for_session(active_session_key)) + file_state_token = bind_file_states(self._file_state_store.for_session(active_session_key)) try: result = await self.runner.run(AgentRunSpec( initial_messages=initial_messages, diff --git a/nanobot/agent/tools/file_state.py b/nanobot/agent/tools/file_state.py index 4cf5af873..33673b3ef 100644 --- a/nanobot/agent/tools/file_state.py +++ b/nanobot/agent/tools/file_state.py @@ -130,6 +130,26 @@ class FileStates: self._state.clear() +class FileStateStore: + """Lookup table for per-session file read/write state.""" + + __slots__ = ("_states_by_key",) + + def __init__(self) -> None: + self._states_by_key: dict[str, FileStates] = {} + + def for_session(self, session_key: str | None) -> FileStates: + key = session_key or "__default__" + states = self._states_by_key.get(key) + if states is None: + states = FileStates() + self._states_by_key[key] = states + return states + + def clear(self) -> None: + self._states_by_key.clear() + + _current_file_states: ContextVar[FileStates | None] = ContextVar( "nanobot_file_states", default=None, From 1c24f10236ecb605c008af39d4dae57386cee506 Mon Sep 17 00:00:00 2001 From: Xubin Ren Date: Fri, 1 May 2026 11:18:47 +0000 Subject: [PATCH 09/33] fix(skills): update restart instructions in upgrade process --- nanobot/skills/update-setup/SKILL.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/nanobot/skills/update-setup/SKILL.md b/nanobot/skills/update-setup/SKILL.md index 215a43c9b..104afcd5f 100644 --- a/nanobot/skills/update-setup/SKILL.md +++ b/nanobot/skills/update-setup/SKILL.md @@ -78,7 +78,7 @@ description: "Upgrade nanobot to the latest version. Triggers: upgrade nanobot, 1. (If proxy configured) Set proxy: `export http_proxy=URL && export https_proxy=URL` 2. Use `exec` to run the upgrade command: 3. Use `exec` to verify: `nanobot --version` -4. Tell the user the new version. Say: "Restart nanobot to apply the update." +4. Tell the user the new version. Say: "Run `/restart` to restart nanobot and apply the update. If `/restart` is unavailable in this channel, restart the nanobot process manually." ``` ## Step 5: Confirm From d9800ecdd2f38b6f8ea95170ce4ab4cbd14820aa Mon Sep 17 00:00:00 2001 From: Jack Lu <46274946+JackLuguibin@users.noreply.github.com> Date: Fri, 1 May 2026 01:42:31 +0800 Subject: [PATCH 10/33] refactor: replace try-except blocks with contextlib.suppress for cleaner error handling across multiple files --- nanobot/agent/context.py | 5 +-- nanobot/agent/loop.py | 6 +-- nanobot/agent/memory.py | 18 +++----- nanobot/agent/runner.py | 5 +-- nanobot/agent/tools/mcp.py | 6 +-- nanobot/agent/tools/search.py | 5 +-- nanobot/agent/tools/shell.py | 6 +-- nanobot/channels/discord.py | 13 ++---- nanobot/channels/email.py | 5 +-- nanobot/channels/feishu.py | 6 +-- nanobot/channels/manager.py | 5 +-- nanobot/channels/matrix.py | 17 +++---- nanobot/channels/mochat.py | 9 ++-- nanobot/channels/msteams.py | 6 +-- nanobot/channels/qq.py | 13 ++---- nanobot/channels/telegram.py | 22 ++++----- nanobot/channels/weixin.py | 45 +++++-------------- nanobot/channels/whatsapp.py | 5 +-- nanobot/cli/commands.py | 30 ++++--------- nanobot/command/builtin.py | 14 +++--- nanobot/providers/base.py | 5 +-- nanobot/providers/github_copilot_provider.py | 5 +-- nanobot/security/network.py | 5 +-- nanobot/session/manager.py | 13 ++---- .../skill-creator/scripts/package_skill.py | 12 +++-- nanobot/utils/helpers.py | 7 +-- nanobot/utils/restart.py | 5 +-- 27 files changed, 98 insertions(+), 195 deletions(-) diff --git a/nanobot/agent/context.py b/nanobot/agent/context.py index 38945ea2c..22cb14b57 100644 --- a/nanobot/agent/context.py +++ b/nanobot/agent/context.py @@ -3,6 +3,7 @@ import base64 import mimetypes import platform +from contextlib import suppress from importlib.resources import files as pkg_files from pathlib import Path from typing import Any @@ -121,12 +122,10 @@ class ContextBuilder: @staticmethod def _is_template_content(content: str, template_path: str) -> bool: """Check if *content* is identical to the bundled template (user hasn't customized it).""" - try: + with suppress(Exception): tpl = pkg_files("nanobot") / "templates" / template_path if tpl.is_file(): return content.strip() == tpl.read_text(encoding="utf-8").strip() - except Exception: - pass return False def build_messages( diff --git a/nanobot/agent/loop.py b/nanobot/agent/loop.py index b18bfe37c..e463ff373 100644 --- a/nanobot/agent/loop.py +++ b/nanobot/agent/loop.py @@ -7,7 +7,7 @@ import dataclasses import json import os import time -from contextlib import AsyncExitStack, nullcontext +from contextlib import AsyncExitStack, nullcontext, suppress from pathlib import Path from typing import TYPE_CHECKING, Any, Awaitable, Callable @@ -492,10 +492,8 @@ class AgentLoop: tasks = self._active_tasks.pop(key, []) cancelled = sum(1 for t in tasks if not t.done() and t.cancel()) for t in tasks: - try: + with suppress(asyncio.CancelledError, Exception): await t - except (asyncio.CancelledError, Exception): - pass sub_cancelled = await self.subagents.cancel_by_session(key) return cancelled + sub_cancelled diff --git a/nanobot/agent/memory.py b/nanobot/agent/memory.py index 80f6c580f..756449cbd 100644 --- a/nanobot/agent/memory.py +++ b/nanobot/agent/memory.py @@ -7,6 +7,7 @@ import json import os import re import weakref +from contextlib import suppress import tiktoken from datetime import datetime from pathlib import Path @@ -296,10 +297,8 @@ class MemoryStore: def _next_cursor(self) -> int: """Read the current cursor counter and return the next value.""" if self._cursor_file.exists(): - try: + with suppress(ValueError, OSError): return int(self._cursor_file.read_text(encoding="utf-8").strip()) + 1 - except (ValueError, OSError): - pass # Fast path: trust the tail when intact. Otherwise scan the whole # file and take ``max`` — that stays correct even if the monotonic # invariant was broken by external writes. @@ -328,7 +327,7 @@ class MemoryStore: def _read_entries(self) -> list[dict[str, Any]]: """Read all entries from history.jsonl.""" entries: list[dict[str, Any]] = [] - try: + with suppress(FileNotFoundError): with open(self.history_file, "r", encoding="utf-8") as f: for line in f: line = line.strip() @@ -337,8 +336,7 @@ class MemoryStore: entries.append(json.loads(line)) except json.JSONDecodeError: continue - except FileNotFoundError: - pass + return entries def _read_last_entry(self) -> dict[str, Any] | None: @@ -374,14 +372,12 @@ class MemoryStore: # On Windows, opening a directory with O_RDONLY raises # PermissionError — skip the dir sync there (NTFS # journals metadata synchronously). - try: + with suppress(PermissionError): fd = os.open(str(self.history_file.parent), os.O_RDONLY) try: os.fsync(fd) finally: os.close(fd) - except PermissionError: - pass # Windows — directory fsync not supported except BaseException: tmp_path.unlink(missing_ok=True) raise @@ -390,10 +386,8 @@ class MemoryStore: def get_last_dream_cursor(self) -> int: if self._dream_cursor_file.exists(): - try: + with suppress(ValueError, OSError): return int(self._dream_cursor_file.read_text(encoding="utf-8").strip()) - except (ValueError, OSError): - pass return 0 def set_last_dream_cursor(self, cursor: int) -> None: diff --git a/nanobot/agent/runner.py b/nanobot/agent/runner.py index c7cf126c3..d002d8989 100644 --- a/nanobot/agent/runner.py +++ b/nanobot/agent/runner.py @@ -5,6 +5,7 @@ from __future__ import annotations import asyncio import inspect import os +from contextlib import suppress from dataclasses import dataclass, field from pathlib import Path from typing import Any @@ -752,12 +753,10 @@ class AgentRunner: prepare_call = getattr(spec.tools, "prepare_call", None) tool, params, prep_error = None, tool_call.arguments, None if callable(prepare_call): - try: + with suppress(Exception): prepared = prepare_call(tool_call.name, tool_call.arguments) if isinstance(prepared, tuple) and len(prepared) == 3: tool, params, prep_error = prepared - except Exception: - pass if prep_error: event = { "name": tool_call.name, diff --git a/nanobot/agent/tools/mcp.py b/nanobot/agent/tools/mcp.py index 0e5b008f5..580020a64 100644 --- a/nanobot/agent/tools/mcp.py +++ b/nanobot/agent/tools/mcp.py @@ -4,7 +4,7 @@ import asyncio import os import re import shutil -from contextlib import AsyncExitStack +from contextlib import AsyncExitStack, suppress from typing import Any import httpx @@ -609,10 +609,8 @@ async def connect_mcp_servers( "only JSON-RPC to stdout and sends logs/debug output to stderr instead." ) logger.error("MCP server '{}': failed to connect: {}{}", name, e, hint) - try: + with suppress(Exception): await server_stack.aclose() - except Exception: - pass return name, None server_stacks: dict[str, AsyncExitStack] = {} diff --git a/nanobot/agent/tools/search.py b/nanobot/agent/tools/search.py index 9c1024694..405a89c76 100644 --- a/nanobot/agent/tools/search.py +++ b/nanobot/agent/tools/search.py @@ -5,6 +5,7 @@ from __future__ import annotations import fnmatch import os import re +from contextlib import suppress from pathlib import Path, PurePosixPath from typing import Any, Iterable, TypeVar @@ -92,10 +93,8 @@ class _SearchTool(_FsTool): def _display_path(self, target: Path, root: Path) -> str: if self._workspace: - try: + with suppress(ValueError): return target.relative_to(self._workspace).as_posix() - except ValueError: - pass return target.relative_to(root).as_posix() def _iter_files(self, root: Path) -> Iterable[Path]: diff --git a/nanobot/agent/tools/shell.py b/nanobot/agent/tools/shell.py index 9484c73f7..059428aa1 100644 --- a/nanobot/agent/tools/shell.py +++ b/nanobot/agent/tools/shell.py @@ -5,6 +5,7 @@ import os import re import shutil import sys +from contextlib import suppress from pathlib import Path from typing import Any @@ -212,9 +213,8 @@ class ExecTool(Tool): """Kill a subprocess and reap it to prevent zombies.""" process.kill() try: - await asyncio.wait_for(process.wait(), timeout=5.0) - except asyncio.TimeoutError: - pass + with suppress(asyncio.TimeoutError): + await asyncio.wait_for(process.wait(), timeout=5.0) finally: if not _IS_WINDOWS: try: diff --git a/nanobot/channels/discord.py b/nanobot/channels/discord.py index 94be6a907..bb39b66b7 100644 --- a/nanobot/channels/discord.py +++ b/nanobot/channels/discord.py @@ -5,6 +5,7 @@ from __future__ import annotations import asyncio import importlib.util import time +from contextlib import suppress from dataclasses import dataclass from pathlib import Path from typing import TYPE_CHECKING, Any, Literal @@ -564,10 +565,8 @@ class DiscordChannel(BaseChannel): # Delayed working indicator (cosmetic — not tied to subagent lifecycle) async def _delayed_working_emoji() -> None: await asyncio.sleep(self.config.working_emoji_delay) - try: + with suppress(Exception): await message.add_reaction(self.config.working_emoji) - except Exception: - pass self._working_emoji_tasks[channel_id] = asyncio.create_task(_delayed_working_emoji()) @@ -771,10 +770,8 @@ class DiscordChannel(BaseChannel): if task is None: return task.cancel() - try: + with suppress(asyncio.CancelledError): await task - except asyncio.CancelledError: - pass async def _clear_reactions(self, chat_id: str) -> None: """Remove all pending reactions after bot replies.""" @@ -788,10 +785,8 @@ class DiscordChannel(BaseChannel): return bot_user = self._client.user if self._client else None for emoji in (self.config.read_receipt_emoji, self.config.working_emoji): - try: + with suppress(Exception): await msg_obj.remove_reaction(emoji, bot_user) - except Exception: - pass async def _cancel_all_typing(self) -> None: """Stop all typing tasks.""" diff --git a/nanobot/channels/email.py b/nanobot/channels/email.py index 5b5856560..36cafc995 100644 --- a/nanobot/channels/email.py +++ b/nanobot/channels/email.py @@ -6,6 +6,7 @@ import imaplib import re import smtplib import ssl +from contextlib import suppress from datetime import date from email import policy from email.header import decode_header, make_header @@ -460,10 +461,8 @@ class EmailChannel(BaseChannel): if mark_seen: client.store(imap_id, "+FLAGS", "\\Seen") finally: - try: + with suppress(Exception): client.logout() - except Exception: - pass def _collect_self_addresses(self) -> set[str]: """Return normalized email addresses owned by this channel instance.""" diff --git a/nanobot/channels/feishu.py b/nanobot/channels/feishu.py index 7ddb8506d..f617b93db 100644 --- a/nanobot/channels/feishu.py +++ b/nanobot/channels/feishu.py @@ -9,6 +9,7 @@ import threading import time import uuid from collections import OrderedDict +from contextlib import suppress from dataclasses import dataclass from typing import Any, Literal @@ -612,12 +613,11 @@ class FeishuChannel(BaseChannel): """Callback: store reaction_id after background add-reaction completes.""" if task.cancelled(): return - try: + # Failures already logged by _on_background_task_done. + with suppress(Exception): reaction_id = task.result() if reaction_id: self._reaction_ids[message_id] = reaction_id - except Exception: - pass # already logged by _on_background_task_done # Trim cache to prevent unbounded growth if len(self._reaction_ids) > 500: self._reaction_ids.pop(next(iter(self._reaction_ids))) diff --git a/nanobot/channels/manager.py b/nanobot/channels/manager.py index 14a6b2a5e..4bf2be6e8 100644 --- a/nanobot/channels/manager.py +++ b/nanobot/channels/manager.py @@ -3,6 +3,7 @@ from __future__ import annotations import asyncio +from contextlib import suppress from pathlib import Path from typing import TYPE_CHECKING, Any @@ -220,10 +221,8 @@ class ChannelManager: # Stop dispatcher if self._dispatch_task: self._dispatch_task.cancel() - try: + with suppress(asyncio.CancelledError): await self._dispatch_task - except asyncio.CancelledError: - pass # Stop all channels for name, channel in self.channels.items(): diff --git a/nanobot/channels/matrix.py b/nanobot/channels/matrix.py index 8ce08ae61..a234e8cef 100644 --- a/nanobot/channels/matrix.py +++ b/nanobot/channels/matrix.py @@ -5,6 +5,7 @@ import json import logging import mimetypes import time +from contextlib import suppress from dataclasses import dataclass from pathlib import Path from typing import Any, Literal, TypeAlias @@ -341,10 +342,8 @@ class MatrixChannel(BaseChannel): timeout=self.config.sync_stop_grace_seconds) except (asyncio.TimeoutError, asyncio.CancelledError): self._sync_task.cancel() - try: + with suppress(asyncio.CancelledError): await self._sync_task - except asyncio.CancelledError: - pass if self.client: await self.client.close() @@ -609,13 +608,11 @@ class MatrixChannel(BaseChannel): """Best-effort typing indicator update.""" if not self.client: return - try: + with suppress(Exception): response = await self.client.room_typing(room_id=room_id, typing_state=typing, timeout=TYPING_NOTICE_TIMEOUT_MS) if isinstance(response, RoomTypingError): logger.debug("Matrix typing failed for {}: {}", room_id, response) - except Exception: - pass async def _start_typing_keepalive(self, room_id: str) -> None: """Start periodic typing refresh (spec-recommended keepalive).""" @@ -625,22 +622,18 @@ class MatrixChannel(BaseChannel): return async def loop() -> None: - try: + with suppress(asyncio.CancelledError): while self._running: await asyncio.sleep(TYPING_KEEPALIVE_INTERVAL_MS / 1000) await self._set_typing(room_id, True) - except asyncio.CancelledError: - pass self._typing_tasks[room_id] = asyncio.create_task(loop()) async def _stop_typing_keepalive(self, room_id: str, *, clear_typing: bool) -> None: if task := self._typing_tasks.pop(room_id, None): task.cancel() - try: + with suppress(asyncio.CancelledError): await task - except asyncio.CancelledError: - pass if clear_typing: await self._set_typing(room_id, False) diff --git a/nanobot/channels/mochat.py b/nanobot/channels/mochat.py index 0b02aec62..110b454cc 100644 --- a/nanobot/channels/mochat.py +++ b/nanobot/channels/mochat.py @@ -5,6 +5,7 @@ from __future__ import annotations import asyncio import json from collections import deque +from contextlib import suppress from dataclasses import dataclass, field from datetime import datetime from typing import Any @@ -330,10 +331,8 @@ class MochatChannel(BaseChannel): await self._cancel_delay_timers() if self._socket: - try: + with suppress(Exception): await self._socket.disconnect() - except Exception: - pass self._socket = None if self._cursor_save_task: @@ -460,10 +459,8 @@ class MochatChannel(BaseChannel): return True except Exception as e: logger.error("Failed to connect Mochat websocket: {}", e) - try: + with suppress(Exception): await client.disconnect() - except Exception: - pass self._socket = None return False diff --git a/nanobot/channels/msteams.py b/nanobot/channels/msteams.py index eb7b8c819..f30b1af61 100644 --- a/nanobot/channels/msteams.py +++ b/nanobot/channels/msteams.py @@ -20,7 +20,7 @@ import re import tempfile import threading import time -from contextlib import contextmanager +from contextlib import contextmanager, suppress from dataclasses import dataclass from http.server import BaseHTTPRequestHandler, ThreadingHTTPServer from typing import TYPE_CHECKING, Any @@ -712,10 +712,8 @@ class MSTeamsChannel(BaseChannel): os.replace(tmp_path, path) finally: if tmp_path and os.path.exists(tmp_path): - try: + with suppress(OSError): os.unlink(tmp_path) - except OSError: - pass def _save_refs_locked(self, *, prune: bool = True) -> None: """Persist conversation references (caller must hold _refs_guard).""" diff --git a/nanobot/channels/qq.py b/nanobot/channels/qq.py index f109f6da6..00338229a 100644 --- a/nanobot/channels/qq.py +++ b/nanobot/channels/qq.py @@ -25,6 +25,7 @@ import os import re import time from collections import deque +from contextlib import suppress from pathlib import Path from typing import TYPE_CHECKING, Any, Literal from urllib.parse import unquote, urlparse @@ -221,17 +222,13 @@ class QQChannel(BaseChannel): """Stop bot and cleanup resources.""" self._running = False if self._client: - try: + with suppress(Exception): await self._client.close() - except Exception: - pass self._client = None if self._http: - try: + with suppress(Exception): await self._http.close() - except Exception: - pass self._http = None logger.info("QQ bot stopped") @@ -683,7 +680,5 @@ class QQChannel(BaseChannel): finally: # Cleanup partial file if tmp_path is not None: - try: + with suppress(Exception): tmp_path.unlink(missing_ok=True) - except Exception: - pass diff --git a/nanobot/channels/telegram.py b/nanobot/channels/telegram.py index cbf9f6427..793419917 100644 --- a/nanobot/channels/telegram.py +++ b/nanobot/channels/telegram.py @@ -6,6 +6,7 @@ import asyncio import re import time import unicodedata +from contextlib import suppress from dataclasses import dataclass from pathlib import Path from typing import Any, Literal @@ -462,10 +463,8 @@ class TelegramChannel(BaseChannel): if not msg.metadata.get("_progress", False): self._stop_typing(msg.chat_id) if reply_to_message_id := msg.metadata.get("message_id"): - try: + with suppress(ValueError): await self._remove_reaction(msg.chat_id, int(reply_to_message_id)) - except ValueError: - pass try: chat_id = int(msg.chat_id) @@ -642,10 +641,8 @@ class TelegramChannel(BaseChannel): return self._stop_typing(chat_id) if reply_to_message_id := meta.get("message_id"): - try: + with suppress(ValueError): await self._remove_reaction(chat_id, int(reply_to_message_id)) - except ValueError: - pass thread_kwargs = {} if message_thread_id := meta.get("message_thread_id"): thread_kwargs["message_thread_id"] = message_thread_id @@ -1162,11 +1159,10 @@ class TelegramChannel(BaseChannel): async def _typing_loop(self, chat_id: str) -> None: """Repeatedly send 'typing' action until cancelled.""" try: - while self._app: - await self._app.bot.send_chat_action(chat_id=int(chat_id), action="typing") - await asyncio.sleep(4) - except asyncio.CancelledError: - pass + with suppress(asyncio.CancelledError): + while self._app: + await self._app.bot.send_chat_action(chat_id=int(chat_id), action="typing") + await asyncio.sleep(4) except Exception as e: logger.debug("Typing indicator stopped for {}: {}", chat_id, e) @@ -1265,10 +1261,8 @@ class TelegramChannel(BaseChannel): button_label = query.data or "" await query.answer() if query.message: - try: + with suppress(Exception): await query.message.edit_reply_markup(reply_markup=None) - except Exception: - pass logger.debug("Inline button tap from {}: {}", sender_id, button_label) self._start_typing(str(chat_id)) await self._handle_message( diff --git a/nanobot/channels/weixin.py b/nanobot/channels/weixin.py index fbe84bcf8..68fbed85d 100644 --- a/nanobot/channels/weixin.py +++ b/nanobot/channels/weixin.py @@ -19,6 +19,7 @@ import re import time import uuid from collections import OrderedDict +from contextlib import suppress from pathlib import Path from typing import Any from urllib.parse import quote @@ -211,7 +212,7 @@ class WeixinChannel(BaseChannel): def _save_state(self) -> None: state_file = self._get_state_dir() / "account.json" - try: + with suppress(Exception): data = { "token": self._token, "get_updates_buf": self._get_updates_buf, @@ -220,8 +221,6 @@ class WeixinChannel(BaseChannel): "base_url": self.config.base_url, } state_file.write_text(json.dumps(data, ensure_ascii=False)) - except Exception: - pass # ------------------------------------------------------------------ # HTTP helpers (matches api.ts buildHeaders / apiFetch) @@ -576,10 +575,8 @@ class WeixinChannel(BaseChannel): # Process messages (WeixinMessage[] from types.ts) msgs: list[dict] = data.get("msgs", []) or [] for msg in msgs: - try: + with suppress(Exception): await self._process_message(msg) - except Exception: - pass # ------------------------------------------------------------------ # Inbound message processing (matches inbound.ts + process-message.ts) @@ -932,10 +929,8 @@ class WeixinChannel(BaseChannel): await asyncio.sleep(TYPING_KEEPALIVE_INTERVAL_S) if stop_event.is_set(): break - try: + with suppress(Exception): await self._send_typing(user_id, typing_ticket, TYPING_STATUS_TYPING) - except Exception: - pass finally: pass @@ -962,16 +957,12 @@ class WeixinChannel(BaseChannel): return typing_ticket = "" - try: + with suppress(Exception): typing_ticket = await self._get_typing_ticket(msg.chat_id, ctx_token) - except Exception: - typing_ticket = "" if typing_ticket: - try: + with suppress(Exception): await self._send_typing(msg.chat_id, typing_ticket, TYPING_STATUS_TYPING) - except Exception: - pass typing_keepalive_stop = asyncio.Event() typing_keepalive_task: asyncio.Task | None = None @@ -1043,16 +1034,12 @@ class WeixinChannel(BaseChannel): if typing_keepalive_task: typing_keepalive_stop.set() typing_keepalive_task.cancel() - try: + with suppress(asyncio.CancelledError): await typing_keepalive_task - except asyncio.CancelledError: - pass if typing_ticket and not is_progress: - try: + with suppress(Exception): await self._send_typing(msg.chat_id, typing_ticket, TYPING_STATUS_CANCEL) - except Exception: - pass async def _start_typing(self, chat_id: str, context_token: str = "") -> None: """Start typing indicator immediately when a message is received.""" @@ -1076,10 +1063,8 @@ class WeixinChannel(BaseChannel): await asyncio.sleep(TYPING_KEEPALIVE_INTERVAL_S) if stop_event.is_set(): break - try: + with suppress(Exception): await self._send_typing(chat_id, ticket, TYPING_STATUS_TYPING) - except Exception: - pass finally: pass @@ -1095,10 +1080,8 @@ class WeixinChannel(BaseChannel): if stop_event: stop_event.set() task.cancel() - try: + with suppress(asyncio.CancelledError): await task - except asyncio.CancelledError: - pass if not clear_remote: return entry = self._typing_tickets.get(chat_id) @@ -1339,13 +1322,11 @@ def _encrypt_aes_ecb(data: bytes, aes_key_b64: str) -> bytes: pad_len = 16 - len(data) % 16 padded = data + bytes([pad_len] * pad_len) - try: + with suppress(ImportError): from Crypto.Cipher import AES cipher = AES.new(key, AES.MODE_ECB) return cipher.encrypt(padded) - except ImportError: - pass try: from cryptography.hazmat.primitives.ciphers import Cipher, algorithms, modes @@ -1371,13 +1352,11 @@ def _decrypt_aes_ecb(data: bytes, aes_key_b64: str) -> bytes: decrypted: bytes | None = None - try: + with suppress(ImportError): from Crypto.Cipher import AES cipher = AES.new(key, AES.MODE_ECB) decrypted = cipher.decrypt(data) - except ImportError: - pass if decrypted is None: try: diff --git a/nanobot/channels/whatsapp.py b/nanobot/channels/whatsapp.py index e2485da72..74d53203f 100644 --- a/nanobot/channels/whatsapp.py +++ b/nanobot/channels/whatsapp.py @@ -8,6 +8,7 @@ import os import secrets import shutil import subprocess +from contextlib import suppress from collections import OrderedDict from pathlib import Path from typing import Any, Literal @@ -47,10 +48,8 @@ def _load_or_create_bridge_token(path: Path) -> str: path.parent.mkdir(parents=True, exist_ok=True) token = secrets.token_urlsafe(32) path.write_text(token, encoding="utf-8") - try: + with suppress(OSError): path.chmod(0o600) - except OSError: - pass return token diff --git a/nanobot/cli/commands.py b/nanobot/cli/commands.py index 903555b47..952742ea4 100644 --- a/nanobot/cli/commands.py +++ b/nanobot/cli/commands.py @@ -5,7 +5,7 @@ import os import select import signal import sys -from contextlib import nullcontext +from contextlib import nullcontext, suppress from pathlib import Path from typing import Any @@ -14,11 +14,9 @@ if sys.platform == "win32": if sys.stdout.encoding != "utf-8": os.environ["PYTHONIOENCODING"] = "utf-8" # Re-open stdout/stderr with UTF-8 encoding - try: + with suppress(Exception): sys.stdout.reconfigure(encoding="utf-8", errors="replace") sys.stderr.reconfigure(encoding="utf-8", errors="replace") - except Exception: - pass import typer from loguru import logger @@ -83,35 +81,29 @@ def _flush_pending_tty_input() -> None: except Exception: return - try: + with suppress(Exception): import termios termios.tcflush(fd, termios.TCIFLUSH) return - except Exception: - pass - try: + with suppress(Exception): while True: ready, _, _ = select.select([fd], [], [], 0) if not ready: break if not os.read(fd, 4096): break - except Exception: - return def _restore_terminal() -> None: """Restore terminal to its original state (echo, line buffering, etc.).""" if _SAVED_TERM_ATTRS is None: return - try: + with suppress(Exception): import termios termios.tcsetattr(sys.stdin.fileno(), termios.TCSADRAIN, _SAVED_TERM_ATTRS) - except Exception: - pass def _init_prompt_session() -> None: @@ -119,12 +111,10 @@ def _init_prompt_session() -> None: global _PROMPT_SESSION, _SAVED_TERM_ATTRS # Save terminal state so we can restore it on exit - try: + with suppress(Exception): import termios _SAVED_TERM_ATTRS = termios.tcgetattr(sys.stdin.fileno()) - except Exception: - pass from nanobot.config.paths import get_cli_history_path @@ -936,10 +926,8 @@ def _run_gateway( config.gateway.host or "127.0.0.1", port ) writer.close() - try: + with suppress(Exception): await writer.wait_closed() - except Exception: - pass break except OSError: await asyncio.sleep(0.1) @@ -1520,10 +1508,8 @@ def _login_openai_codex() -> None: from oauth_cli_kit import get_token, login_oauth_interactive token = None - try: + with suppress(Exception): token = get_token() - except Exception: - pass if not (token and token.access): console.print("[cyan]Starting interactive OAuth login...[/cyan]\n") token = login_oauth_interactive( diff --git a/nanobot/command/builtin.py b/nanobot/command/builtin.py index 299fa97b3..32444a4ba 100644 --- a/nanobot/command/builtin.py +++ b/nanobot/command/builtin.py @@ -5,6 +5,7 @@ from __future__ import annotations import asyncio import os import sys +from contextlib import suppress from nanobot import __version__ from nanobot.bus.events import OutboundMessage @@ -50,16 +51,15 @@ async def cmd_status(ctx: CommandContext) -> OutboundMessage: loop = ctx.loop session = ctx.session or loop.sessions.get_or_create(ctx.key) ctx_est = 0 - try: + with suppress(Exception): ctx_est, _ = loop.consolidator.estimate_session_prompt_tokens(session) - except Exception: - pass if ctx_est <= 0: ctx_est = loop._last_usage.get("prompt_tokens", 0) # Fetch web search provider usage (best-effort, never blocks the response) search_usage_text: str | None = None - try: + # Never let usage fetch break /status + with suppress(Exception): from nanobot.utils.searchusage import fetch_search_usage web_cfg = getattr(loop, "web_config", None) search_cfg = getattr(web_cfg, "search", None) if web_cfg else None @@ -68,14 +68,10 @@ async def cmd_status(ctx: CommandContext) -> OutboundMessage: api_key = getattr(search_cfg, "api_key", "") or None usage = await fetch_search_usage(provider=provider, api_key=api_key) search_usage_text = usage.format() - except Exception: - pass # Never let usage fetch break /status active_tasks = loop._active_tasks.get(ctx.key, []) task_count = sum(1 for t in active_tasks if not t.done()) - try: + with suppress(Exception): task_count += loop.subagents.get_running_count_by_session(ctx.key) - except Exception: - pass return OutboundMessage( channel=ctx.msg.channel, chat_id=ctx.msg.chat_id, diff --git a/nanobot/providers/base.py b/nanobot/providers/base.py index a60ea09eb..1d598f20a 100644 --- a/nanobot/providers/base.py +++ b/nanobot/providers/base.py @@ -4,6 +4,7 @@ import asyncio import json import re from abc import ABC, abstractmethod +from contextlib import suppress from collections.abc import Awaitable, Callable from dataclasses import dataclass, field from datetime import datetime, timezone @@ -643,14 +644,12 @@ class LLMProvider(ABC): return value return None - try: + with suppress(TypeError, ValueError): retry_ms = _header_value("retry-after-ms") if retry_ms is not None: value = float(retry_ms) / 1000.0 if value > 0: return value - except (TypeError, ValueError): - pass retry_after = _header_value("retry-after") if retry_after is None: diff --git a/nanobot/providers/github_copilot_provider.py b/nanobot/providers/github_copilot_provider.py index 8d50006a0..dbc49e73e 100644 --- a/nanobot/providers/github_copilot_provider.py +++ b/nanobot/providers/github_copilot_provider.py @@ -5,6 +5,7 @@ from __future__ import annotations import time import webbrowser from collections.abc import Callable +from contextlib import suppress import httpx from oauth_cli_kit.models import OAuthToken @@ -86,10 +87,8 @@ def login_github_copilot( printer(f"Open: {verify_url}") printer(f"Code: {user_code}") if verify_complete: - try: + with suppress(Exception): webbrowser.open(verify_complete) - except Exception: - pass deadline = time.time() + expires_in current_interval = interval diff --git a/nanobot/security/network.py b/nanobot/security/network.py index 970702b98..54676b5d9 100644 --- a/nanobot/security/network.py +++ b/nanobot/security/network.py @@ -5,6 +5,7 @@ from __future__ import annotations import ipaddress import re import socket +from contextlib import suppress from urllib.parse import urlparse _BLOCKED_NETWORKS = [ @@ -30,10 +31,8 @@ def configure_ssrf_whitelist(cidrs: list[str]) -> None: global _allowed_networks nets = [] for cidr in cidrs: - try: + with suppress(ValueError): nets.append(ipaddress.ip_network(cidr, strict=False)) - except ValueError: - pass _allowed_networks = nets diff --git a/nanobot/session/manager.py b/nanobot/session/manager.py index fb1d6cf62..06c7317d0 100644 --- a/nanobot/session/manager.py +++ b/nanobot/session/manager.py @@ -3,6 +3,7 @@ import json import os import shutil +from contextlib import suppress from dataclasses import dataclass, field from datetime import datetime from pathlib import Path @@ -362,15 +363,11 @@ class SessionManager: if data.get("_type") == "metadata": metadata = data.get("metadata", {}) if data.get("created_at"): - try: + with suppress(ValueError, TypeError): created_at = datetime.fromisoformat(data["created_at"]) - except (ValueError, TypeError): - pass if data.get("updated_at"): - try: + with suppress(ValueError, TypeError): updated_at = datetime.fromisoformat(data["updated_at"]) - except (ValueError, TypeError): - pass last_consolidated = data.get("last_consolidated", 0) else: messages.append(data) @@ -440,14 +437,12 @@ class SessionManager: # On Windows, opening a directory with O_RDONLY raises # PermissionError — skip the dir sync there (NTFS # journals metadata synchronously). - try: + with suppress(PermissionError): fd = os.open(str(path.parent), os.O_RDONLY) try: os.fsync(fd) finally: os.close(fd) - except PermissionError: - pass # Windows — directory fsync not supported except BaseException: tmp_path.unlink(missing_ok=True) raise diff --git a/nanobot/skills/skill-creator/scripts/package_skill.py b/nanobot/skills/skill-creator/scripts/package_skill.py index 48fcbbe5e..7e3a8f17f 100755 --- a/nanobot/skills/skill-creator/scripts/package_skill.py +++ b/nanobot/skills/skill-creator/scripts/package_skill.py @@ -12,25 +12,23 @@ Example: import sys import zipfile +from contextlib import suppress from pathlib import Path from quick_validate import validate_skill def _is_within(path: Path, root: Path) -> bool: - try: + with suppress(ValueError): path.relative_to(root) return True - except ValueError: - return False + return False def _cleanup_partial_archive(skill_filename: Path) -> None: - try: - if skill_filename.exists(): + if skill_filename.exists(): + with suppress(OSError): skill_filename.unlink() - except OSError: - pass def package_skill(skill_path, output_dir=None): diff --git a/nanobot/utils/helpers.py b/nanobot/utils/helpers.py index 74c80c110..5a6dc129d 100644 --- a/nanobot/utils/helpers.py +++ b/nanobot/utils/helpers.py @@ -6,6 +6,7 @@ import re import shutil import time import uuid +from contextlib import suppress from datetime import datetime from pathlib import Path from typing import Any @@ -416,14 +417,10 @@ def estimate_prompt_tokens_chain( """Estimate prompt tokens via provider counter first, then tiktoken fallback.""" provider_counter = getattr(provider, "estimate_prompt_tokens", None) if callable(provider_counter): - try: + with suppress(Exception): tokens, source = provider_counter(messages, tools, model) if isinstance(tokens, (int, float)) and tokens > 0: return int(tokens), str(source or "provider_counter") - except Exception: - pass - - estimated = estimate_prompt_tokens(messages, tools) if estimated > 0: return int(estimated), "tiktoken" return 0, "none" diff --git a/nanobot/utils/restart.py b/nanobot/utils/restart.py index 871667f07..962928e74 100644 --- a/nanobot/utils/restart.py +++ b/nanobot/utils/restart.py @@ -5,6 +5,7 @@ from __future__ import annotations import json import os import time +from contextlib import suppress from dataclasses import dataclass, field from typing import Any @@ -26,11 +27,9 @@ def format_restart_completed_message(started_at_raw: str) -> str: """Build restart completion text and include elapsed time when available.""" elapsed_suffix = "" if started_at_raw: - try: + with suppress(ValueError): elapsed_s = max(0.0, time.time() - float(started_at_raw)) elapsed_suffix = f" in {elapsed_s:.1f}s" - except ValueError: - pass return f"Restart completed{elapsed_suffix}." From 15007afd4ab00a90373e895b1615a1f13b21a4f4 Mon Sep 17 00:00:00 2001 From: coldxiangyu Date: Fri, 1 May 2026 18:54:32 +0800 Subject: [PATCH 11/33] fix(matrix): skip events received before bot startup Matrix sync replays the room timeline on each startup or `/restart`, causing already-handled messages to be reprocessed (#3553). Even with `store_sync_tokens=True`, the sync token isn't reliably re-injected when restoring a session via access_token + load_store(), so the client re-reads recent timeline entries. Filter `event.server_timestamp` against the process start time so old events are dropped at the `_on_message` / `_on_media_message` entry points. Trade-off: messages received during downtime won't be processed, which matches the issue reporter's expectation. Closes #3553 Co-Authored-By: Claude Opus 4.7 (1M context) --- nanobot/channels/matrix.py | 24 +++++++++++- tests/channels/test_matrix_channel.py | 56 +++++++++++++++++++++++++++ 2 files changed, 78 insertions(+), 2 deletions(-) diff --git a/nanobot/channels/matrix.py b/nanobot/channels/matrix.py index a234e8cef..2e0b4de6d 100644 --- a/nanobot/channels/matrix.py +++ b/nanobot/channels/matrix.py @@ -252,11 +252,13 @@ class MatrixChannel(BaseChannel): self._server_upload_limit_bytes: int | None = None self._server_upload_limit_checked = False self._stream_bufs: dict[str, _StreamBuf] = {} + self._started_at_ms: int = 0 async def start(self) -> None: """Start Matrix client and begin sync loop.""" self._running = True + self._started_at_ms = int(time.time() * 1000) _configure_nio_logging_bridge() self.store_path = get_data_dir() / "matrix-store" @@ -667,6 +669,16 @@ class MatrixChannel(BaseChannel): return True return bool(self.config.allow_room_mentions and mentions.get("room") is True) + def _is_pre_startup_event(self, event: RoomMessage) -> bool: + """Skip events that landed in the timeline before this process started. + + Matrix sync replays the room timeline on each startup/restart; without + this filter old messages would be re-handled as if they were fresh + (#3553). + """ + ts = getattr(event, "server_timestamp", None) + return isinstance(ts, int) and ts < self._started_at_ms + def _should_process_message(self, room: MatrixRoom, event: RoomMessage) -> bool: """Apply sender and room policy checks.""" if not self.is_allowed(event.sender): @@ -851,7 +863,11 @@ class MatrixChannel(BaseChannel): return meta async def _on_message(self, room: MatrixRoom, event: RoomMessageText) -> None: - if event.sender == self.config.user_id or not self._should_process_message(room, event): + if ( + event.sender == self.config.user_id + or self._is_pre_startup_event(event) + or not self._should_process_message(room, event) + ): return await self._start_typing_keepalive(room.room_id) try: @@ -864,7 +880,11 @@ class MatrixChannel(BaseChannel): raise async def _on_media_message(self, room: MatrixRoom, event: MatrixMediaEvent) -> None: - if event.sender == self.config.user_id or not self._should_process_message(room, event): + if ( + event.sender == self.config.user_id + or self._is_pre_startup_event(event) + or not self._should_process_message(room, event) + ): return attachment, marker = await self._fetch_media_attachment(room, event) parts: list[str] = [] diff --git a/tests/channels/test_matrix_channel.py b/tests/channels/test_matrix_channel.py index 27b7e1255..bf7a09e23 100644 --- a/tests/channels/test_matrix_channel.py +++ b/tests/channels/test_matrix_channel.py @@ -380,6 +380,62 @@ async def test_on_message_skips_typing_for_self_message() -> None: assert client.typing_calls == [] +@pytest.mark.asyncio +async def test_on_message_skips_pre_startup_event() -> None: + channel = MatrixChannel(_make_config(), MessageBus()) + client = _FakeAsyncClient("", "", "", None) + channel.client = client + channel._started_at_ms = 1_000_000 + + handled: list[str] = [] + + async def _fake_handle_message(**kwargs) -> None: + handled.append(kwargs["sender_id"]) + + channel._handle_message = _fake_handle_message # type: ignore[method-assign] + + room = SimpleNamespace(room_id="!room:matrix.org", display_name="Test room") + old_event = SimpleNamespace( + sender="@alice:matrix.org", body="old", source={}, server_timestamp=999_999 + ) + fresh_event = SimpleNamespace( + sender="@alice:matrix.org", body="fresh", source={}, server_timestamp=1_000_001 + ) + + await channel._on_message(room, old_event) + await channel._on_message(room, fresh_event) + + assert handled == ["@alice:matrix.org"] + assert client.typing_calls == [ + ("!room:matrix.org", True, TYPING_NOTICE_TIMEOUT_MS), + ] + + +@pytest.mark.asyncio +async def test_on_media_message_skips_pre_startup_event() -> None: + channel = MatrixChannel(_make_config(), MessageBus()) + client = _FakeAsyncClient("", "", "", None) + channel.client = client + channel._started_at_ms = 1_000_000 + + handled: list[str] = [] + + async def _fake_handle_message(**kwargs) -> None: + handled.append(kwargs["sender_id"]) + + channel._handle_message = _fake_handle_message # type: ignore[method-assign] + + room = SimpleNamespace(room_id="!room:matrix.org", display_name="Test room") + old_event = SimpleNamespace( + sender="@alice:matrix.org", body="old", source={}, server_timestamp=999_999 + ) + + await channel._on_media_message(room, old_event) + + assert handled == [] + assert client.typing_calls == [] + + @pytest.mark.asyncio async def test_on_message_skips_typing_for_denied_sender() -> None: channel = MatrixChannel(_make_config(allow_from=["@bob:matrix.org"]), MessageBus()) From 0284174df9267774bcd804a423961b6df2569348 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Thu, 30 Apr 2026 20:43:18 +0000 Subject: [PATCH 12/33] fix: prevent empty Matrix messages when progress callback sends empty content Agent-Logs-Url: https://github.com/halldorjanetzko/nanobot/sessions/df528c59-8214-41a0-9b79-9d1d41857107 Co-authored-by: halldorjanetzko <158819146+halldorjanetzko@users.noreply.github.com> --- nanobot/channels/matrix.py | 2 +- tests/channels/test_matrix_channel.py | 38 +++++++++++++++++++++++++++ 2 files changed, 39 insertions(+), 1 deletion(-) diff --git a/nanobot/channels/matrix.py b/nanobot/channels/matrix.py index 2e0b4de6d..3a0fba8f1 100644 --- a/nanobot/channels/matrix.py +++ b/nanobot/channels/matrix.py @@ -524,7 +524,7 @@ class MatrixChannel(BaseChannel): failures.append(fail) if failures: text = f"{text.rstrip()}\n{chr(10).join(failures)}" if text.strip() else "\n".join(failures) - if text or not candidates: + if text.strip(): content = _build_matrix_text_content(text) if relates_to: content["m.relates_to"] = relates_to diff --git a/tests/channels/test_matrix_channel.py b/tests/channels/test_matrix_channel.py index bf7a09e23..8fbe2c119 100644 --- a/tests/channels/test_matrix_channel.py +++ b/tests/channels/test_matrix_channel.py @@ -1246,6 +1246,44 @@ async def test_send_progress_keeps_typing_keepalive_running() -> None: await channel.stop() +@pytest.mark.asyncio +async def test_send_empty_content_does_not_call_room_send() -> None: + """Progress messages with empty content must not produce an empty body: '' event.""" + channel = MatrixChannel(_make_config(), MessageBus()) + client = _FakeAsyncClient("", "", "", None) + channel.client = client + + await channel.send( + OutboundMessage( + channel="matrix", + chat_id="!room:matrix.org", + content="", + metadata={"_progress": True}, + ) + ) + + assert client.room_send_calls == [] + + +@pytest.mark.asyncio +async def test_send_whitespace_only_content_does_not_call_room_send() -> None: + """Progress messages with whitespace-only content must not produce an empty message.""" + channel = MatrixChannel(_make_config(), MessageBus()) + client = _FakeAsyncClient("", "", "", None) + channel.client = client + + await channel.send( + OutboundMessage( + channel="matrix", + chat_id="!room:matrix.org", + content=" \n\n ", + metadata={"_progress": True}, + ) + ) + + assert client.room_send_calls == [] + + @pytest.mark.asyncio async def test_send_clears_typing_when_send_fails() -> None: channel = MatrixChannel(_make_config(), MessageBus()) From ad952e0da2e6852821afd64a0d06febbcd8d81d2 Mon Sep 17 00:00:00 2001 From: hinotoi-agent Date: Fri, 1 May 2026 12:33:24 +0800 Subject: [PATCH 13/33] fix(dingtalk): block SSRF in outbound media fetches --- nanobot/channels/dingtalk.py | 187 ++++++++++++++--- tests/channels/test_dingtalk_channel.py | 268 +++++++++++++++++++++++- 2 files changed, 420 insertions(+), 35 deletions(-) diff --git a/nanobot/channels/dingtalk.py b/nanobot/channels/dingtalk.py index a863ba0df..609a7fa54 100644 --- a/nanobot/channels/dingtalk.py +++ b/nanobot/channels/dingtalk.py @@ -9,7 +9,7 @@ import zipfile from io import BytesIO from pathlib import Path from typing import Any -from urllib.parse import unquote, urlparse +from urllib.parse import unquote, urljoin, urlparse import httpx from loguru import logger @@ -19,6 +19,10 @@ from nanobot.bus.events import OutboundMessage from nanobot.bus.queue import MessageBus from nanobot.channels.base import BaseChannel from nanobot.config.schema import Base +from nanobot.security.network import validate_resolved_url, validate_url_target + +DINGTALK_MAX_REMOTE_MEDIA_BYTES = 20 * 1024 * 1024 +DINGTALK_MAX_REMOTE_MEDIA_REDIRECTS = 3 try: from dingtalk_stream import ( @@ -155,6 +159,8 @@ class DingTalkConfig(Base): client_id: str = "" client_secret: str = "" allow_from: list[str] = Field(default_factory=list) + allow_remote_media_redirects: bool = False + remote_media_redirect_allowed_hosts: list[str] = Field(default_factory=list) class DingTalkChannel(BaseChannel): @@ -281,9 +287,12 @@ class DingTalkChannel(BaseChannel): def _guess_upload_type(self, media_ref: str) -> str: ext = Path(urlparse(media_ref).path).suffix.lower() - if ext in self._IMAGE_EXTS: return "image" - if ext in self._AUDIO_EXTS: return "voice" - if ext in self._VIDEO_EXTS: return "video" + if ext in self._IMAGE_EXTS: + return "image" + if ext in self._AUDIO_EXTS: + return "voice" + if ext in self._VIDEO_EXTS: + return "video" return "file" def _guess_filename(self, media_ref: str, upload_type: str) -> str: @@ -315,6 +324,146 @@ class DingTalkChannel(BaseChannel): return self._zip_bytes(filename, data) return data, filename, content_type + def _validate_remote_media_url(self, media_ref: str) -> bool: + ok, err = validate_url_target(media_ref) + if not ok: + logger.warning("DingTalk remote media URL blocked ref={} reason={}", media_ref, err) + return False + return True + + def _redirect_host_allowed(self, current_url: str, next_url: str) -> bool: + current_host = (urlparse(current_url).hostname or "").lower() + next_host = (urlparse(next_url).hostname or "").lower() + if not next_host: + return False + if next_host == current_host: + return True + allowed_hosts = {host.lower() for host in self.config.remote_media_redirect_allowed_hosts} + return next_host in allowed_hosts + + def _next_remote_media_url(self, current_url: str, location: str | None) -> str | None: + if not self.config.allow_remote_media_redirects: + logger.warning("DingTalk media download redirect refused ref={}", current_url) + return None + if not location: + logger.warning("DingTalk media download redirect without Location ref={}", current_url) + return None + next_url = urljoin(current_url, location) + if not self._redirect_host_allowed(current_url, next_url): + logger.warning( + "DingTalk media download cross-host redirect refused ref={} next={}", + current_url, + next_url, + ) + return None + if not self._validate_remote_media_url(next_url): + return None + return next_url + + async def _fetch_remote_media_bytes( + self, + media_ref: str, + ) -> tuple[bytes | None, str | None]: + """Fetch a remote media URL with SSRF, redirect, and size checks.""" + if not self._http: + return None, None + + if not self._validate_remote_media_url(media_ref): + return None, None + + try: + # Prefer streaming with a running byte cap so large responses are not + # materialized before the limit is enforced. Test fakes may only + # implement get(), so keep a small compatibility fallback below. + stream = getattr(self._http, "stream", None) + if stream is not None: + current_url = media_ref + for _ in range(DINGTALK_MAX_REMOTE_MEDIA_REDIRECTS + 1): + async with stream("GET", current_url, follow_redirects=False) as resp: + final_ok, final_err = validate_resolved_url(str(resp.url)) + if not final_ok: + logger.warning( + "DingTalk remote media redirect blocked ref={} final={} reason={}", + media_ref, + resp.url, + final_err, + ) + return None, None + if 300 <= resp.status_code < 400: + next_url = self._next_remote_media_url( + str(resp.url), resp.headers.get("location") + ) + if not next_url: + return None, None + current_url = next_url + continue + if resp.status_code >= 400: + logger.warning( + "DingTalk media download failed status={} ref={}", + resp.status_code, + current_url, + ) + return None, None + chunks: list[bytes] = [] + total = 0 + async for chunk in resp.aiter_bytes(): + total += len(chunk) + if total > DINGTALK_MAX_REMOTE_MEDIA_BYTES: + logger.warning( + "DingTalk media download too large ref={} bytes>{}", + current_url, + DINGTALK_MAX_REMOTE_MEDIA_BYTES, + ) + return None, None + chunks.append(chunk) + return b"".join(chunks), (resp.headers.get("content-type") or "") + logger.warning("DingTalk media download exceeded redirect limit ref={}", media_ref) + return None, None + + current_url = media_ref + for _ in range(DINGTALK_MAX_REMOTE_MEDIA_REDIRECTS + 1): + resp = await self._http.get(current_url, follow_redirects=False) + final_ok, final_err = validate_resolved_url(str(getattr(resp, "url", current_url))) + if not final_ok: + logger.warning( + "DingTalk remote media redirect blocked ref={} final={} reason={}", + media_ref, + getattr(resp, "url", current_url), + final_err, + ) + return None, None + if 300 <= resp.status_code < 400: + next_url = self._next_remote_media_url( + str(getattr(resp, "url", current_url)), resp.headers.get("location") + ) + if not next_url: + return None, None + current_url = next_url + continue + if resp.status_code >= 400: + logger.warning( + "DingTalk media download failed status={} ref={}", + resp.status_code, + current_url, + ) + return None, None + if len(resp.content) > DINGTALK_MAX_REMOTE_MEDIA_BYTES: + logger.warning( + "DingTalk media download too large ref={} bytes>{}", + current_url, + DINGTALK_MAX_REMOTE_MEDIA_BYTES, + ) + return None, None + return resp.content, (resp.headers.get("content-type") or "") + logger.warning("DingTalk media download exceeded redirect limit ref={}", media_ref) + return None, None + except httpx.TransportError as e: + logger.error("DingTalk media download network error ref={} err={}", media_ref, e) + raise + except Exception as e: + logger.error("DingTalk media download error ref={} err={}", media_ref, e) + return None, None + async def _read_media_bytes( self, media_ref: str, @@ -323,26 +472,12 @@ class DingTalkChannel(BaseChannel): return None, None, None if self._is_http_url(media_ref): - if not self._http: - return None, None, None - try: - resp = await self._http.get(media_ref, follow_redirects=True) - if resp.status_code >= 400: - logger.warning( - "DingTalk media download failed status={} ref={}", - resp.status_code, - media_ref, - ) - return None, None, None - content_type = (resp.headers.get("content-type") or "").split(";")[0].strip() - filename = self._guess_filename(media_ref, self._guess_upload_type(media_ref)) - return resp.content, filename, content_type or None - except httpx.TransportError as e: - logger.error("DingTalk media download network error ref={} err={}", media_ref, e) - raise - except Exception as e: - logger.error("DingTalk media download error ref={} err={}", media_ref, e) + data, raw_content_type = await self._fetch_remote_media_bytes(media_ref) + if data is None: return None, None, None + content_type = (raw_content_type or "").split(";")[0].strip() + filename = self._guess_filename(media_ref, self._guess_upload_type(media_ref)) + return data, filename, content_type or None try: if media_ref.startswith("file://"): @@ -435,8 +570,10 @@ class DingTalkChannel(BaseChannel): if resp.status_code != 200: logger.error("DingTalk send failed msgKey={} status={} body={}", msg_key, resp.status_code, body[:500]) return False - try: result = resp.json() - except Exception: result = {} + try: + result = resp.json() + except Exception: + result = {} errcode = result.get("errcode") if errcode not in (None, 0): logger.error("DingTalk send api error msgKey={} errcode={} body={}", msg_key, errcode, body[:500]) diff --git a/tests/channels/test_dingtalk_channel.py b/tests/channels/test_dingtalk_channel.py index 86de99bb5..f14c81302 100644 --- a/tests/channels/test_dingtalk_channel.py +++ b/tests/channels/test_dingtalk_channel.py @@ -2,7 +2,6 @@ import asyncio import zipfile from io import BytesIO from types import SimpleNamespace -from unittest.mock import AsyncMock import httpx import pytest @@ -17,19 +16,27 @@ except ImportError: if not DINGTALK_AVAILABLE: pytest.skip("DingTalk dependencies not installed (dingtalk-stream)", allow_module_level=True) -from nanobot.bus.queue import MessageBus import nanobot.channels.dingtalk as dingtalk_module -from nanobot.channels.dingtalk import DingTalkChannel, NanobotDingTalkHandler -from nanobot.channels.dingtalk import DingTalkConfig +from nanobot.bus.queue import MessageBus +from nanobot.channels.dingtalk import DingTalkChannel, DingTalkConfig, NanobotDingTalkHandler class _FakeResponse: - def __init__(self, status_code: int = 200, json_body: dict | None = None) -> None: + def __init__( + self, + status_code: int = 200, + json_body: dict | None = None, + *, + content: bytes = b"", + headers: dict[str, str] | None = None, + url: str = "https://example.com/file", + ) -> None: self.status_code = status_code self._json_body = json_body or {} - self.text = "{}" - self.content = b"" - self.headers = {"content-type": "application/json"} + self.text = content.decode("utf-8", errors="replace") if content else "{}" + self.content = content + self.headers = headers or {"content-type": "application/json"} + self.url = httpx.URL(url) def json(self) -> dict: return self._json_body @@ -46,11 +53,13 @@ class _FakeHttp: return _FakeResponse() async def post(self, url: str, json=None, headers=None, **kwargs): - self.calls.append({"method": "POST", "url": url, "json": json, "headers": headers}) + self.calls.append( + {"method": "POST", "url": url, "json": json, "headers": headers, "kwargs": kwargs} + ) return self._next_response() async def get(self, url: str, **kwargs): - self.calls.append({"method": "GET", "url": url}) + self.calls.append({"method": "GET", "url": url, "kwargs": kwargs}) return self._next_response() @@ -242,6 +251,245 @@ async def test_download_dingtalk_file(tmp_path, monkeypatch) -> None: assert channel._http.calls[1]["method"] == "GET" +@pytest.mark.asyncio +async def test_read_media_bytes_rejects_private_http_target_before_fetch() -> None: + """Remote media fetches must not reach loopback/private addresses.""" + channel = DingTalkChannel( + DingTalkConfig(client_id="app", client_secret="secret", allow_from=["*"]), + MessageBus(), + ) + channel._http = _FakeHttp( + responses=[ + _FakeResponse( + 200, + content=b"internal secret", + headers={"content-type": "text/plain"}, + url="http://127.0.0.1/admin.txt", + ) + ] + ) + + data, filename, content_type = await channel._read_media_bytes("http://127.0.0.1/admin.txt") + + assert (data, filename, content_type) == (None, None, None) + assert channel._http.calls == [] + + +@pytest.mark.asyncio +async def test_read_media_bytes_rejects_private_redirect_result() -> None: + """A public-looking media URL must not be accepted after redirecting private.""" + channel = DingTalkChannel( + DingTalkConfig(client_id="app", client_secret="secret", allow_from=["*"]), + MessageBus(), + ) + channel._http = _FakeHttp( + responses=[ + _FakeResponse( + 200, + content=b"metadata bytes", + headers={"content-type": "text/plain"}, + url="http://127.0.0.1/metadata", + ) + ] + ) + + data, filename, content_type = await channel._read_media_bytes("https://example.com/safe.txt") + + assert (data, filename, content_type) == (None, None, None) + assert len(channel._http.calls) == 1 + + +@pytest.mark.asyncio +async def test_read_media_bytes_rejects_oversized_remote_response(monkeypatch) -> None: + """DingTalk media downloads should enforce a byte cap before upload.""" + monkeypatch.setattr(dingtalk_module, "DINGTALK_MAX_REMOTE_MEDIA_BYTES", 8, raising=False) + channel = DingTalkChannel( + DingTalkConfig(client_id="app", client_secret="secret", allow_from=["*"]), + MessageBus(), + ) + channel._http = _FakeHttp( + responses=[ + _FakeResponse( + 200, + content=b"123456789", + headers={"content-type": "text/plain"}, + url="https://example.com/large.txt", + ) + ] + ) + + data, filename, content_type = await channel._read_media_bytes("https://example.com/large.txt") + + assert (data, filename, content_type) == (None, None, None) + + +@pytest.mark.asyncio +async def test_read_media_bytes_does_not_follow_remote_redirects_by_default() -> None: + """Redirects are refused by default instead of followed into internal networks.""" + channel = DingTalkChannel( + DingTalkConfig(client_id="app", client_secret="secret", allow_from=["*"]), + MessageBus(), + ) + channel._http = _FakeHttp( + responses=[ + _FakeResponse( + 302, + headers={"location": "http://127.0.0.1/metadata"}, + url="https://example.com/redirect.txt", + ) + ] + ) + + data, filename, content_type = await channel._read_media_bytes("https://example.com/redirect.txt") + + assert (data, filename, content_type) == (None, None, None) + assert channel._http.calls[0]["kwargs"]["follow_redirects"] is False + + +@pytest.mark.asyncio +async def test_read_media_bytes_follows_safe_redirect_when_explicitly_enabled() -> None: + """Operators can opt in to public redirects without enabling private redirects.""" + channel = DingTalkChannel( + DingTalkConfig( + client_id="app", + client_secret="secret", + allow_from=["*"], + allow_remote_media_redirects=True, + ), + MessageBus(), + ) + channel._http = _FakeHttp( + responses=[ + _FakeResponse( + 302, + headers={"location": "https://example.com/final.txt"}, + url="https://example.com/redirect.txt", + ), + _FakeResponse( + 200, + content=b"redirected media", + headers={"content-type": "text/plain"}, + url="https://example.com/final.txt", + ), + ] + ) + + data, filename, content_type = await channel._read_media_bytes("https://example.com/redirect.txt") + + assert (data, filename, content_type) == (b"redirected media", "redirect.txt", "text/plain") + assert [call["url"] for call in channel._http.calls] == [ + "https://example.com/redirect.txt", + "https://example.com/final.txt", + ] + assert all(call["kwargs"]["follow_redirects"] is False for call in channel._http.calls) + + +@pytest.mark.asyncio +async def test_read_media_bytes_blocks_cross_host_redirect_without_allowlist() -> None: + """Redirect opt-in should not allow arbitrary cross-host redirects by default.""" + channel = DingTalkChannel( + DingTalkConfig( + client_id="app", + client_secret="secret", + allow_from=["*"], + allow_remote_media_redirects=True, + ), + MessageBus(), + ) + channel._http = _FakeHttp( + responses=[ + _FakeResponse( + 302, + headers={"location": "https://example.org/final.txt"}, + url="https://example.com/redirect.txt", + ), + _FakeResponse( + 200, + content=b"cross-host media", + headers={"content-type": "text/plain"}, + url="https://example.org/final.txt", + ), + ] + ) + + data, filename, content_type = await channel._read_media_bytes("https://example.com/redirect.txt") + + assert (data, filename, content_type) == (None, None, None) + assert [call["url"] for call in channel._http.calls] == ["https://example.com/redirect.txt"] + + +@pytest.mark.asyncio +async def test_read_media_bytes_allows_cross_host_redirect_when_allowlisted() -> None: + """Operators can explicitly allow a known CDN/download host for redirects.""" + channel = DingTalkChannel( + DingTalkConfig( + client_id="app", + client_secret="secret", + allow_from=["*"], + allow_remote_media_redirects=True, + remote_media_redirect_allowed_hosts=["example.org"], + ), + MessageBus(), + ) + channel._http = _FakeHttp( + responses=[ + _FakeResponse( + 302, + headers={"location": "https://example.org/final.txt"}, + url="https://example.com/redirect.txt", + ), + _FakeResponse( + 200, + content=b"cross-host media", + headers={"content-type": "text/plain"}, + url="https://example.org/final.txt", + ), + ] + ) + + data, filename, content_type = await channel._read_media_bytes("https://example.com/redirect.txt") + + assert (data, filename, content_type) == (b"cross-host media", "redirect.txt", "text/plain") + assert [call["url"] for call in channel._http.calls] == [ + "https://example.com/redirect.txt", + "https://example.org/final.txt", + ] + + +@pytest.mark.asyncio +async def test_read_media_bytes_blocks_private_redirect_even_when_redirects_enabled() -> None: + """Redirect opt-in must still validate each hop before fetching it.""" + channel = DingTalkChannel( + DingTalkConfig( + client_id="app", + client_secret="secret", + allow_from=["*"], + allow_remote_media_redirects=True, + ), + MessageBus(), + ) + channel._http = _FakeHttp( + responses=[ + _FakeResponse( + 302, + headers={"location": "http://127.0.0.1/metadata"}, + url="https://example.com/redirect.txt", + ), + _FakeResponse( + 200, + content=b"internal secret", + headers={"content-type": "text/plain"}, + url="http://127.0.0.1/metadata", + ), + ] + ) + + data, filename, content_type = await channel._read_media_bytes("https://example.com/redirect.txt") + + assert (data, filename, content_type) == (None, None, None) + assert [call["url"] for call in channel._http.calls] == ["https://example.com/redirect.txt"] + + def test_normalize_upload_payload_zips_html_attachment() -> None: channel = DingTalkChannel( DingTalkConfig(client_id="app", client_secret="secret", allow_from=["*"]), From 73840b0af66631333b821d75c90486125be242d1 Mon Sep 17 00:00:00 2001 From: liuZhou <2047379328@qq.com> Date: Thu, 30 Apr 2026 23:58:45 +0800 Subject: [PATCH 14/33] fix(matrix): remove tuple default from allow_room_mentions --- nanobot/channels/matrix.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/nanobot/channels/matrix.py b/nanobot/channels/matrix.py index 3a0fba8f1..b53ae8607 100644 --- a/nanobot/channels/matrix.py +++ b/nanobot/channels/matrix.py @@ -215,7 +215,7 @@ class MatrixConfig(Base): allow_from: list[str] = Field(default_factory=list) group_policy: Literal["open", "mention", "allowlist"] = "open" group_allow_from: list[str] = Field(default_factory=list) - allow_room_mentions: bool = False, + allow_room_mentions: bool = False streaming: bool = False From 1040124ede021db8650867e028914e06d577bce7 Mon Sep 17 00:00:00 2001 From: hanyuanling Date: Thu, 30 Apr 2026 17:53:18 +0800 Subject: [PATCH 15/33] Fix API stream lifecycle for tool-backed requests --- CONTRIBUTING.md | 20 +++++++++ nanobot/api/server.py | 32 +++++++++++--- tests/test_api_stream.py | 95 +++++++++++++++++++++++++++++++++++++--- 3 files changed, 135 insertions(+), 12 deletions(-) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 51406a09f..7c9fe5f9e 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -43,6 +43,26 @@ We use a two-branch model to balance stability and exploration: **When in doubt, target `nightly`.** It is easier to move a stable idea from `nightly` to `main` than to undo a risky change after it lands in the stable branch. +### Starting Work + +Before making changes, sync the target branch and create a topic branch from it. +For stable bug fixes and documentation-only changes, start from the latest `main`. +For experimental work, start from the latest `nightly`. + +```bash +git fetch upstream +git switch main +git pull --ff-only upstream main +git switch -c your-topic-branch +``` + +Use your primary HKUDS/nanobot remote in place of `upstream` if your checkout +uses a different remote name. + +Keep unrelated local changes out of the topic branch. If your checkout already has +work in progress, use a separate worktree or finish that work before starting a +new branch. + ### How Does Nightly Get Merged to Main? We don't merge the entire `nightly` branch. Instead, stable features are **cherry-picked** from `nightly` into individual PRs targeting `main`: diff --git a/nanobot/api/server.py b/nanobot/api/server.py index 92e7be90c..0a79fa097 100644 --- a/nanobot/api/server.py +++ b/nanobot/api/server.py @@ -7,6 +7,7 @@ All requests route to a single persistent API session. from __future__ import annotations import asyncio +import contextlib import json as _json import time import uuid @@ -18,8 +19,12 @@ from loguru import logger from nanobot.config.paths import get_media_dir from nanobot.utils.helpers import safe_filename from nanobot.utils.media_decode import ( - FileSizeExceeded as _FileSizeExceeded, MAX_FILE_SIZE, +) +from nanobot.utils.media_decode import ( + FileSizeExceeded as _FileSizeExceeded, +) +from nanobot.utils.media_decode import ( save_base64_data_url as _save_base64_data_url, ) from nanobot.utils.runtime import EMPTY_FINAL_RESPONSE_MESSAGE @@ -240,18 +245,25 @@ async def handle_chat_completions(request: web.Request) -> web.Response: chunk_id = f"chatcmpl-{uuid.uuid4().hex[:12]}" queue: asyncio.Queue[str | None] = asyncio.Queue() stream_failed = False + emitted_content = False async def _on_stream(token: str) -> None: + nonlocal emitted_content + if token: + emitted_content = True await queue.put(token) async def _on_stream_end(*_a: Any, **_kw: Any) -> None: - await queue.put(None) + # Agent stream-end callbacks mark generation segment boundaries. + # Tool-backed requests may continue after a segment ends, so the + # HTTP SSE stream is closed only when process_direct returns. + return None async def _run() -> None: nonlocal stream_failed try: async with session_lock: - await asyncio.wait_for( + response = await asyncio.wait_for( agent_loop.process_direct( content=text, media=media_paths if media_paths else None, @@ -263,9 +275,14 @@ async def handle_chat_completions(request: web.Request) -> web.Response: ), timeout=timeout_s, ) + if not emitted_content: + response_text = _response_text(response) + if response_text.strip(): + await queue.put(response_text) except Exception: stream_failed = True logger.exception("Streaming error for session {}", session_key) + finally: await queue.put(None) task = asyncio.create_task(_run()) @@ -276,7 +293,10 @@ async def handle_chat_completions(request: web.Request) -> web.Response: break await resp.write(_sse_chunk(token, model_name, chunk_id)) finally: - task.cancel() + if not task.done(): + task.cancel() + with contextlib.suppress(asyncio.CancelledError): + await task if not stream_failed: await resp.write(_sse_chunk("", model_name, chunk_id, finish_reason="stop")) @@ -284,7 +304,7 @@ async def handle_chat_completions(request: web.Request) -> web.Response: return resp # -- non-streaming path (original logic) -- - _FALLBACK = EMPTY_FINAL_RESPONSE_MESSAGE + fallback = EMPTY_FINAL_RESPONSE_MESSAGE try: async with session_lock: @@ -316,7 +336,7 @@ async def handle_chat_completions(request: web.Request) -> web.Response: response_text = _response_text(retry_response) if not response_text or not response_text.strip(): logger.warning("Empty response after retry, using fallback") - response_text = _FALLBACK + response_text = fallback except asyncio.TimeoutError: return _error_json(504, f"Request timed out after {timeout_s}s") diff --git a/tests/test_api_stream.py b/tests/test_api_stream.py index 75d866525..cc7935a82 100644 --- a/tests/test_api_stream.py +++ b/tests/test_api_stream.py @@ -10,8 +10,8 @@ import pytest import pytest_asyncio from nanobot.api.server import ( - _sse_chunk, _SSE_DONE, + _sse_chunk, create_app, ) @@ -111,13 +111,13 @@ async def test_stream_true_returns_sse(aiohttp_client) -> None: assert resp.content_type == "text/event-stream" body = await resp.text() - lines = [l for l in body.split("\n") if l.startswith("data: ")] + lines = [line for line in body.split("\n") if line.startswith("data: ")] # Should have: 2 token chunks + 1 finish chunk + [DONE] - data_lines = [l[len("data: "):] for l in lines] + data_lines = [line[len("data: "):] for line in lines] assert data_lines[-1] == "[DONE]" - chunks = [json.loads(l) for l in data_lines[:-1]] + chunks = [json.loads(line) for line in data_lines[:-1]] assert chunks[0]["choices"][0]["delta"]["content"] == "Hello" assert chunks[1]["choices"][0]["delta"]["content"] == " world" # Last chunk before [DONE] should have finish_reason=stop @@ -181,8 +181,12 @@ async def test_stream_sse_chunk_ids_are_consistent(aiohttp_client) -> None: json={"messages": [{"role": "user", "content": "go"}], "stream": True}, ) body = await resp.text() - data_lines = [l[len("data: "):] for l in body.split("\n") if l.startswith("data: ") and l != "data: [DONE]"] - chunks = [json.loads(l) for l in data_lines] + data_lines = [ + line[len("data: "):] + for line in body.split("\n") + if line.startswith("data: ") and line != "data: [DONE]" + ] + chunks = [json.loads(line) for line in data_lines] chunk_ids = {c["id"] for c in chunks} assert len(chunk_ids) == 1, f"Expected single chunk id, got {chunk_ids}" @@ -218,6 +222,85 @@ async def test_stream_passes_on_stream_callbacks(aiohttp_client) -> None: assert captured_kwargs.get("on_stream_end") is not None +@pytest.mark.skipif(not HAS_AIOHTTP, reason="aiohttp not installed") +@pytest.mark.asyncio +async def test_stream_segment_end_does_not_close_sse(aiohttp_client) -> None: + """Intermediate stream-end callbacks should not terminate the HTTP stream.""" + agent = MagicMock() + + async def fake_process_direct(*, on_stream=None, on_stream_end=None, **kwargs): + assert on_stream is not None + assert on_stream_end is not None + await on_stream("planning") + await on_stream_end(resuming=True) + await asyncio.sleep(0.05) + await on_stream(" final") + await on_stream_end(resuming=False) + return "planning final" + + agent.process_direct = fake_process_direct + agent._connect_mcp = AsyncMock() + agent.close_mcp = AsyncMock() + + app = create_app(agent, model_name="m") + client = await aiohttp_client(app) + + resp = await client.post( + "/v1/chat/completions", + json={"messages": [{"role": "user", "content": "use a tool"}], "stream": True}, + ) + + assert resp.status == 200 + body = await resp.text() + data_lines = [ + line[len("data: "):] for line in body.split("\n") if line.startswith("data: ") + ] + assert data_lines[-1] == "[DONE]" + + chunks = [json.loads(line) for line in data_lines[:-1]] + deltas = [c["choices"][0]["delta"].get("content", "") for c in chunks] + assert "planning" in deltas + assert " final" in deltas + assert chunks[-1]["choices"][0]["finish_reason"] == "stop" + + +@pytest.mark.skipif(not HAS_AIOHTTP, reason="aiohttp not installed") +@pytest.mark.asyncio +async def test_stream_uses_final_response_when_no_deltas(aiohttp_client) -> None: + """stream=true should not return an empty stream when the agent returns content.""" + agent = MagicMock() + + async def fake_process_direct(*, on_stream=None, on_stream_end=None, **kwargs): + assert on_stream is not None + assert on_stream_end is not None + await on_stream_end(resuming=False) + return "plain final" + + agent.process_direct = fake_process_direct + agent._connect_mcp = AsyncMock() + agent.close_mcp = AsyncMock() + + app = create_app(agent, model_name="m") + client = await aiohttp_client(app) + + resp = await client.post( + "/v1/chat/completions", + json={"messages": [{"role": "user", "content": "hi"}], "stream": True}, + ) + + assert resp.status == 200 + body = await resp.text() + data_lines = [ + line[len("data: "):] for line in body.split("\n") if line.startswith("data: ") + ] + chunks = [json.loads(line) for line in data_lines[:-1]] + deltas = [c["choices"][0]["delta"].get("content", "") for c in chunks] + + assert "plain final" in deltas + assert data_lines[-1] == "[DONE]" + assert chunks[-1]["choices"][0]["finish_reason"] == "stop" + + @pytest.mark.skipif(not HAS_AIOHTTP, reason="aiohttp not installed") @pytest.mark.asyncio async def test_stream_with_session_id(aiohttp_client) -> None: From c4170fa9bab120901e879af59f73511ebb2cf63d Mon Sep 17 00:00:00 2001 From: yorkhellen Date: Thu, 30 Apr 2026 15:25:13 +0800 Subject: [PATCH 16/33] feat: Add sender_id to LLM runtime context --- nanobot/agent/context.py | 7 +++++-- nanobot/agent/loop.py | 2 ++ nanobot/agent/memory.py | 1 + 3 files changed, 8 insertions(+), 2 deletions(-) diff --git a/nanobot/agent/context.py b/nanobot/agent/context.py index 22cb14b57..ccd1b882e 100644 --- a/nanobot/agent/context.py +++ b/nanobot/agent/context.py @@ -83,12 +83,14 @@ class ContextBuilder: @staticmethod def _build_runtime_context( channel: str | None, chat_id: str | None, timezone: str | None = None, - session_summary: str | None = None, + session_summary: str | None = None, sender_id: str | None = None, ) -> str: """Build untrusted runtime metadata block for injection before the user message.""" lines = [f"Current Time: {current_time_str(timezone)}"] if channel and chat_id: lines += [f"Channel: {channel}", f"Chat ID: {chat_id}"] + if sender_id: + lines += [f"Sender ID: {sender_id}"] if session_summary: lines += ["", "[Resumed Session]", session_summary] return ContextBuilder._RUNTIME_CONTEXT_TAG + "\n" + "\n".join(lines) + "\n" + ContextBuilder._RUNTIME_CONTEXT_END @@ -138,9 +140,10 @@ class ContextBuilder: chat_id: str | None = None, current_role: str = "user", session_summary: str | None = None, + sender_id: str | None = None, ) -> list[dict[str, Any]]: """Build the complete message list for an LLM call.""" - runtime_ctx = self._build_runtime_context(channel, chat_id, self.timezone, session_summary=session_summary) + runtime_ctx = self._build_runtime_context(channel, chat_id, self.timezone, session_summary=session_summary, sender_id=sender_id) user_content = self._build_user_content(current_message, media) # Merge runtime context and user content into a single user message diff --git a/nanobot/agent/loop.py b/nanobot/agent/loop.py index e463ff373..d9dd4585c 100644 --- a/nanobot/agent/loop.py +++ b/nanobot/agent/loop.py @@ -929,6 +929,7 @@ class AgentLoop: chat_id=chat_id, session_summary=pending, current_role=current_role, + sender_id=msg.sender_id, ) final_content, _, all_msgs, stop_reason, _ = await self._run_agent_loop( messages, session=session, channel=channel, chat_id=chat_id, @@ -1024,6 +1025,7 @@ class AgentLoop: media=msg.media if msg.media else None, channel=msg.channel, chat_id=self._runtime_chat_id(msg), + sender_id=msg.sender_id, ) async def _bus_progress( diff --git a/nanobot/agent/memory.py b/nanobot/agent/memory.py index 756449cbd..85cc5ab4a 100644 --- a/nanobot/agent/memory.py +++ b/nanobot/agent/memory.py @@ -518,6 +518,7 @@ class Consolidator: channel=channel, chat_id=chat_id, session_summary=session_summary, + sender_id=None, ) return estimate_prompt_tokens_chain( self.provider, From 08f326ec5565739961f0292f56ff5e77f3f81cd6 Mon Sep 17 00:00:00 2001 From: yorkhellen Date: Thu, 30 Apr 2026 15:27:29 +0800 Subject: [PATCH 17/33] test: Add tests for sender_id runtime context injection --- tests/agent/test_context_prompt_cache.py | 36 ++++++++++++++++++++++++ 1 file changed, 36 insertions(+) diff --git a/tests/agent/test_context_prompt_cache.py b/tests/agent/test_context_prompt_cache.py index ec1ab5430..bf06b8b2d 100644 --- a/tests/agent/test_context_prompt_cache.py +++ b/tests/agent/test_context_prompt_cache.py @@ -87,6 +87,42 @@ def test_runtime_context_is_separate_untrusted_user_message(tmp_path) -> None: assert "Return exactly: OK" in user_content +def test_runtime_context_includes_sender_id_when_provided(tmp_path) -> None: + """Sender ID should be included in runtime context when provided.""" + workspace = _make_workspace(tmp_path) + builder = ContextBuilder(workspace) + + messages = builder.build_messages( + history=[], + current_message="Return exactly: OK", + channel="cli", + chat_id="direct", + sender_id="user-12345", + ) + + user_content = messages[-1]["content"] + assert isinstance(user_content, str) + assert "Sender ID: user-12345" in user_content + + +def test_runtime_context_excludes_sender_id_when_not_provided(tmp_path) -> None: + """Sender ID should not be present in runtime context when not provided.""" + workspace = _make_workspace(tmp_path) + builder = ContextBuilder(workspace) + + messages = builder.build_messages( + history=[], + current_message="Return exactly: OK", + channel="cli", + chat_id="direct", + sender_id=None, + ) + + user_content = messages[-1]["content"] + assert isinstance(user_content, str) + assert "Sender ID:" not in user_content + + def test_unprocessed_history_injected_into_system_prompt(tmp_path) -> None: """Entries in history.jsonl not yet consumed by Dream appear with timestamps.""" workspace = _make_workspace(tmp_path) From 8ca575bdeb4d9d4c81e8ecd22b5ada7e17362cdc Mon Sep 17 00:00:00 2001 From: Jiajun Xie Date: Thu, 30 Apr 2026 12:52:45 +0000 Subject: [PATCH 18/33] fix: adjust DeepSeek reasoning mode check condition - Modified _drop_deepseek_incomplete_reasoning_history to properly handle reasoning mode detection - Fixes issue #3554 --- nanobot/providers/openai_compat_provider.py | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/nanobot/providers/openai_compat_provider.py b/nanobot/providers/openai_compat_provider.py index 6712b68bc..251568afc 100644 --- a/nanobot/providers/openai_compat_provider.py +++ b/nanobot/providers/openai_compat_provider.py @@ -457,11 +457,16 @@ class OpenAICompatProvider(LLMProvider): if ( not self._spec or self._spec.name != "deepseek" - or not reasoning_effort - or reasoning_effort.lower() == "none" ): return messages + # For DeepSeek models, always check for incomplete reasoning history + # when thinking mode might be active. reasoning_effort may not be set + # explicitly but the model could still be using thinking mode by default. + # Only skip this check when reasoning_effort is explicitly set to "none". + if reasoning_effort is not None and reasoning_effort.lower() == "none": + return messages + bad_idx = None for idx, msg in enumerate(messages): if ( From 43a58335f6e7f22daee64ce9839432dea85b06fb Mon Sep 17 00:00:00 2001 From: Xubin Ren Date: Fri, 1 May 2026 11:48:57 +0000 Subject: [PATCH 19/33] fix(provider): narrow DeepSeek reasoning history cleanup Made-with: Cursor --- nanobot/providers/openai_compat_provider.py | 18 +++++-- tests/providers/test_litellm_kwargs.py | 58 +++++++++++++++++++-- 2 files changed, 67 insertions(+), 9 deletions(-) diff --git a/nanobot/providers/openai_compat_provider.py b/nanobot/providers/openai_compat_provider.py index 251568afc..decdccb3a 100644 --- a/nanobot/providers/openai_compat_provider.py +++ b/nanobot/providers/openai_compat_provider.py @@ -452,6 +452,7 @@ class OpenAICompatProvider(LLMProvider): def _drop_deepseek_incomplete_reasoning_history( self, messages: list[dict[str, Any]], + model_name: str, reasoning_effort: str | None, ) -> list[dict[str, Any]]: if ( @@ -460,13 +461,19 @@ class OpenAICompatProvider(LLMProvider): ): return messages - # For DeepSeek models, always check for incomplete reasoning history - # when thinking mode might be active. reasoning_effort may not be set - # explicitly but the model could still be using thinking mode by default. - # Only skip this check when reasoning_effort is explicitly set to "none". - if reasoning_effort is not None and reasoning_effort.lower() == "none": + semantic_effort = reasoning_effort.lower() if isinstance(reasoning_effort, str) else None + if semantic_effort in {"none", "minimal", "minimum"}: return messages + # DeepSeek-V4 can require reasoning_content even when the config did + # not explicitly request reasoning_effort. Keep that implicit-thinking + # cleanup scoped to known thinking-capable DeepSeek models so normal + # deepseek-chat history is not trimmed. + if semantic_effort is None: + model_lower = model_name.lower() + if not any(token in model_lower for token in ("deepseek-v4", "deepseek-reasoner")): + return messages + bad_idx = None for idx, msg in enumerate(messages): if ( @@ -537,6 +544,7 @@ class OpenAICompatProvider(LLMProvider): messages = self._drop_deepseek_incomplete_reasoning_history( messages, + model_name, reasoning_effort, ) kwargs: dict[str, Any] = { diff --git a/tests/providers/test_litellm_kwargs.py b/tests/providers/test_litellm_kwargs.py index d3b03c60d..a3b624171 100644 --- a/tests/providers/test_litellm_kwargs.py +++ b/tests/providers/test_litellm_kwargs.py @@ -913,8 +913,8 @@ def test_deepseek_backfills_reasoning_content_on_legacy_tool_call_messages() -> assert msg["reasoning_content"] == "" -def test_backfill_does_not_touch_messages_when_thinking_off() -> None: - """When reasoning_effort is None or minimal, legacy messages must NOT be altered.""" +def test_backfill_does_not_touch_messages_when_thinking_explicitly_off() -> None: + """When thinking is explicitly disabled, legacy messages must NOT be altered.""" spec = find_by_name("deepseek") with patch("nanobot.providers.openai_compat_provider.AsyncOpenAI"): p = OpenAICompatProvider(api_key="k", default_model="deepseek-v4-pro", spec=spec) @@ -926,7 +926,7 @@ def test_backfill_does_not_touch_messages_when_thinking_off() -> None: {"role": "tool", "tool_call_id": "tc1", "content": "result"}, {"role": "user", "content": "thanks"}, ] - for effort in (None, "minimal"): + for effort in ("minimal", "none"): kw = p._build_kwargs( messages=list(messages), tools=None, model="deepseek-v4-pro", max_tokens=1024, temperature=0.7, @@ -937,6 +937,56 @@ def test_backfill_does_not_touch_messages_when_thinking_off() -> None: assert "reasoning_content" not in msg +def test_deepseek_v4_drops_incomplete_reasoning_history_when_effort_implicit() -> None: + """DeepSeek-V4 may default to thinking, so incomplete legacy history is trimmed.""" + spec = find_by_name("deepseek") + with patch("nanobot.providers.openai_compat_provider.AsyncOpenAI"): + p = OpenAICompatProvider(api_key="k", default_model="deepseek-v4-pro", spec=spec) + messages = [ + {"role": "system", "content": "system"}, + {"role": "user", "content": "hi"}, + {"role": "assistant", "content": "", "tool_calls": [ + {"id": "tc1", "type": "function", "function": {"name": "web_search", "arguments": "{}"}} + ]}, + {"role": "tool", "tool_call_id": "tc1", "content": "result"}, + {"role": "user", "content": "thanks"}, + ] + + kw = p._build_kwargs( + messages=list(messages), tools=None, model="deepseek-v4-pro", + max_tokens=1024, temperature=0.7, + reasoning_effort=None, tool_choice=None, + ) + + assert [msg["role"] for msg in kw["messages"]] == ["system", "user"] + assert kw["messages"][-1]["content"] == "thanks" + + +def test_deepseek_chat_keeps_tool_history_when_effort_implicit() -> None: + """Implicit cleanup must not trim non-thinking DeepSeek chat models.""" + spec = find_by_name("deepseek") + with patch("nanobot.providers.openai_compat_provider.AsyncOpenAI"): + p = OpenAICompatProvider(api_key="k", default_model="deepseek-chat", spec=spec) + messages = [ + {"role": "user", "content": "hi"}, + {"role": "assistant", "content": "", "tool_calls": [ + {"id": "tc1", "type": "function", "function": {"name": "web_search", "arguments": "{}"}} + ]}, + {"role": "tool", "tool_call_id": "tc1", "content": "result"}, + {"role": "user", "content": "thanks"}, + ] + + kw = p._build_kwargs( + messages=list(messages), tools=None, model="deepseek-chat", + max_tokens=1024, temperature=0.7, + reasoning_effort=None, tool_choice=None, + ) + + roles = [msg["role"] for msg in kw["messages"]] + assert roles == ["user", "assistant", "tool", "user"] + assert kw["messages"][1]["tool_calls"] + + def test_deepseek_coerces_list_content_to_string() -> None: """DeepSeek chat endpoint expects message.content to be a string.""" spec = find_by_name("deepseek") @@ -1061,7 +1111,7 @@ def test_kimi_k2_thinking_series_no_thinking_injection() -> None: # --------------------------------------------------------------------------- -# reasoning_effort="none" — treated as thinking disabled +# reasoning_effort="none" — treated as thinking disabled # --------------------------------------------------------------------------- def test_deepseek_thinking_disabled_for_none_string() -> None: From 5dc96505e87be46bf46095e570a98d467621de7d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E5=BD=AD=E6=98=9F=E6=9D=B0?= <1198425718@qq.com> Date: Wed, 29 Apr 2026 14:07:25 +0800 Subject: [PATCH 20/33] fix(web_fetch): sanitize URL to strip markdown backticks and quotes before validation LLM-generated tool calls may wrap URLs in markdown backticks or quotes (e.g. \https://example.com\), causing urlparse to produce empty scheme and netloc, which leads to all fetch attempts failing silently. Add URL cleaning at the top of WebFetchTool.execute to strip whitespace, backticks, double quotes, and single quotes, plus an early rejection guard for non-http(s) URLs after cleaning. --- nanobot/agent/tools/web.py | 3 + .../tools/test_web_fetch_url_sanitization.py | 139 ++++++++++++++++++ 2 files changed, 142 insertions(+) create mode 100644 tests/tools/test_web_fetch_url_sanitization.py diff --git a/nanobot/agent/tools/web.py b/nanobot/agent/tools/web.py index e0372bead..e25e196c7 100644 --- a/nanobot/agent/tools/web.py +++ b/nanobot/agent/tools/web.py @@ -388,6 +388,9 @@ class WebFetchTool(Tool): max_chars: int | None = None, **kwargs: Any, ) -> Any: + url = url.strip().strip("`").strip('"').strip("'") + if not (url.startswith("http://") or url.startswith("https://")): + return json.dumps({"error": "Invalid URL after cleaning", "url": url}, ensure_ascii=False) extract_mode = kwargs.pop("extractMode", extract_mode) max_chars = kwargs.pop("maxChars", max_chars) or self.max_chars is_valid, error_msg = _validate_url_safe(url) diff --git a/tests/tools/test_web_fetch_url_sanitization.py b/tests/tools/test_web_fetch_url_sanitization.py new file mode 100644 index 000000000..e44b206a3 --- /dev/null +++ b/tests/tools/test_web_fetch_url_sanitization.py @@ -0,0 +1,139 @@ +"""Tests for web_fetch URL sanitization (backtick/quote stripping).""" + +from __future__ import annotations + +import json +from unittest.mock import patch + +import pytest + +from nanobot.agent.tools.web import WebFetchTool, _validate_url + + +def _fake_resolve_public(hostname, port, family=0, type_=0): + import socket + return [(socket.AF_INET, socket.SOCK_STREAM, 0, "", ("93.184.216.34", 0))] + + +class FakeResponse: + status_code = 200 + url = "https://example.com/page" + text = "T

ok

" + headers = {"content-type": "text/html"} + def raise_for_status(self): pass + def json(self): return {} + + +class FakeStreamResponse: + headers = {"content-type": "text/html"} + url = "https://example.com/page" + async def __aenter__(self): return self + async def __aexit__(self, *a): return False + + +class FakeClient: + def __init__(self, *a, **kw): pass + async def __aenter__(self): return self + async def __aexit__(self, *a): return False + def stream(self, method, url, **kw): + return FakeStreamResponse() + async def get(self, url, **kw): + return FakeResponse() + + +def _patch_env(): + return patch("nanobot.security.network.socket.getaddrinfo", _fake_resolve_public), \ + patch("nanobot.agent.tools.web.httpx.AsyncClient", FakeClient) + + +# --- urlparse / _validate_url level tests --- + +@pytest.mark.parametrize("dirty_url", [ + "`https://example.com/page`", + " `https://example.com/page` ", + '"https://example.com/page"', + "'https://example.com/page'", + ' "https://example.com/page" ', +]) +def test_dirty_urls_fail_validation(dirty_url): + is_valid, msg = _validate_url(dirty_url) + assert not is_valid + + +def test_clean_url_passes_validation(): + is_valid, msg = _validate_url("https://example.com/page") + assert is_valid + + +def test_backtick_url_produces_empty_scheme_in_urlparse(): + from urllib.parse import urlparse + p = urlparse("`https://example.com/page`") + assert p.scheme == "" + assert p.netloc == "" + + +# --- WebFetchTool.execute integration tests --- + +@pytest.mark.asyncio +async def test_execute_strips_backticks_and_succeeds(): + tool = WebFetchTool() + with _patch_env()[0], _patch_env()[1]: + result = await tool.execute(url="`https://example.com/page`") + data = json.loads(result) + assert "error" not in data, f"unexpected error: {data}" + + +@pytest.mark.asyncio +async def test_execute_strips_double_quotes_and_succeeds(): + tool = WebFetchTool() + with _patch_env()[0], _patch_env()[1]: + result = await tool.execute(url='"https://example.com/page"') + data = json.loads(result) + assert "error" not in data, f"unexpected error: {data}" + + +@pytest.mark.asyncio +async def test_execute_strips_single_quotes_and_succeeds(): + tool = WebFetchTool() + with _patch_env()[0], _patch_env()[1]: + result = await tool.execute(url="'https://example.com/page'") + data = json.loads(result) + assert "error" not in data, f"unexpected error: {data}" + + +@pytest.mark.asyncio +async def test_execute_strips_space_and_backticks(): + tool = WebFetchTool() + with _patch_env()[0], _patch_env()[1]: + result = await tool.execute(url=" `https://example.com/page` ") + data = json.loads(result) + assert "error" not in data, f"unexpected error: {data}" + + +# --- startswith guard tests --- + +@pytest.mark.asyncio +async def test_execute_rejects_non_http_url_after_cleaning(): + tool = WebFetchTool() + result = await tool.execute(url="ftp://example.com/file") + data = json.loads(result) + assert "error" in data + assert "Invalid URL" in data["error"] + + +@pytest.mark.asyncio +async def test_execute_rejects_garbage_after_cleaning(): + tool = WebFetchTool() + result = await tool.execute(url="`not a url at all`") + data = json.loads(result) + assert "error" in data + assert "Invalid URL" in data["error"] + + +@pytest.mark.asyncio +async def test_execute_rejects_bare_domain_after_cleaning(): + tool = WebFetchTool() + result = await tool.execute(url="`example.com/page`") + data = json.loads(result) + assert "error" in data + assert "Invalid URL" in data["error"] From aea5948b118a798798ecdbc737d1929100207696 Mon Sep 17 00:00:00 2001 From: Xubin Ren Date: Fri, 1 May 2026 11:56:27 +0000 Subject: [PATCH 21/33] fix(tools): tighten web fetch URL cleaning Made-with: Cursor --- nanobot/agent/tools/web.py | 4 +--- .../tools/test_web_fetch_url_sanitization.py | 24 ++++++++++++++++--- 2 files changed, 22 insertions(+), 6 deletions(-) diff --git a/nanobot/agent/tools/web.py b/nanobot/agent/tools/web.py index e25e196c7..6378a7979 100644 --- a/nanobot/agent/tools/web.py +++ b/nanobot/agent/tools/web.py @@ -388,9 +388,7 @@ class WebFetchTool(Tool): max_chars: int | None = None, **kwargs: Any, ) -> Any: - url = url.strip().strip("`").strip('"').strip("'") - if not (url.startswith("http://") or url.startswith("https://")): - return json.dumps({"error": "Invalid URL after cleaning", "url": url}, ensure_ascii=False) + url = url.strip(" \t\r\n`\"'") extract_mode = kwargs.pop("extractMode", extract_mode) max_chars = kwargs.pop("maxChars", max_chars) or self.max_chars is_valid, error_msg = _validate_url_safe(url) diff --git a/tests/tools/test_web_fetch_url_sanitization.py b/tests/tools/test_web_fetch_url_sanitization.py index e44b206a3..8a24338c0 100644 --- a/tests/tools/test_web_fetch_url_sanitization.py +++ b/tests/tools/test_web_fetch_url_sanitization.py @@ -110,6 +110,24 @@ async def test_execute_strips_space_and_backticks(): assert "error" not in data, f"unexpected error: {data}" +@pytest.mark.asyncio +async def test_execute_strips_mixed_markdown_and_quotes(): + tool = WebFetchTool() + with _patch_env()[0], _patch_env()[1]: + result = await tool.execute(url='"`https://example.com/page`"') + data = json.loads(result) + assert "error" not in data, f"unexpected error: {data}" + + +@pytest.mark.asyncio +async def test_execute_keeps_case_insensitive_http_scheme(): + tool = WebFetchTool() + with _patch_env()[0], _patch_env()[1]: + result = await tool.execute(url="HTTPS://example.com/page") + data = json.loads(result) + assert "error" not in data, f"unexpected error: {data}" + + # --- startswith guard tests --- @pytest.mark.asyncio @@ -118,7 +136,7 @@ async def test_execute_rejects_non_http_url_after_cleaning(): result = await tool.execute(url="ftp://example.com/file") data = json.loads(result) assert "error" in data - assert "Invalid URL" in data["error"] + assert "URL validation failed" in data["error"] @pytest.mark.asyncio @@ -127,7 +145,7 @@ async def test_execute_rejects_garbage_after_cleaning(): result = await tool.execute(url="`not a url at all`") data = json.loads(result) assert "error" in data - assert "Invalid URL" in data["error"] + assert "URL validation failed" in data["error"] @pytest.mark.asyncio @@ -136,4 +154,4 @@ async def test_execute_rejects_bare_domain_after_cleaning(): result = await tool.execute(url="`example.com/page`") data = json.loads(result) assert "error" in data - assert "Invalid URL" in data["error"] + assert "URL validation failed" in data["error"] From 2c397ad442abe758ed4777e4052f4004984b65df Mon Sep 17 00:00:00 2001 From: bravel Date: Fri, 1 May 2026 19:16:00 +0800 Subject: [PATCH 22/33] fix: strip partial think tags in streaming output --- nanobot/utils/helpers.py | 10 ++++++++++ tests/agent/test_runner.py | 21 +++++++++++++++++++++ tests/utils/test_strip_think.py | 8 ++++++++ 3 files changed, 39 insertions(+) diff --git a/nanobot/utils/helpers.py b/nanobot/utils/helpers.py index 5a6dc129d..3ce15ad62 100644 --- a/nanobot/utils/helpers.py +++ b/nanobot/utils/helpers.py @@ -32,6 +32,8 @@ def strip_think(text: str) -> str: explanatory prose that mentions these tokens. 5. Orphan closing tags `` / `` **at the very start or end of the text** only, for the same reason. + 6. Trailing partial control tags split across stream chunks, such as + ` str: text = re.sub(r"\s*\s*$", "", text) # Edge-only channel markers (harmony / Gemma 4 variant leaks). text = re.sub(r"^\s*<\|?channel\|?>\s*", "", text) + # Stream chunks may end in the middle of a control tag. Strip only known + # control-token prefixes at the very end. + partial_control_tag = ( + r"hiddenWorld") + return LLMResponse(content="Hello hiddenWorld", tool_calls=[], usage={}) + + loop.provider.chat_stream_with_retry = chat_stream_with_retry + + async def on_stream(delta: str) -> None: + deltas.append(delta) + + final_content, _, _, _, _ = await loop._run_agent_loop([], on_stream=on_stream) + + assert final_content == "Hello World" + assert deltas == ["Hello", " World"] + + @pytest.mark.asyncio async def test_loop_retries_think_only_final_response(tmp_path): loop = _make_loop(tmp_path) diff --git a/tests/utils/test_strip_think.py b/tests/utils/test_strip_think.py index 1eda89eb5..7adbe7bce 100644 --- a/tests/utils/test_strip_think.py +++ b/tests/utils/test_strip_think.py @@ -102,6 +102,14 @@ class TestStripThinkMalformedLeaks: assert strip_think("喷泉策略：09:00 开启") == ("喷泉策略：09:00 开启") assert strip_think("<|channel|>answer") == "answer" + def test_partial_trailing_think_tag_after_visible_text(self): + assert strip_think("喷泉策略说明 Date: Fri, 1 May 2026 12:04:03 +0000 Subject: [PATCH 23/33] fix(utils): cover complete trailing think markers Made-with: Cursor --- nanobot/utils/helpers.py | 4 ++-- tests/agent/test_runner.py | 21 +++++++++++++++++++++ tests/utils/test_strip_think.py | 2 ++ 3 files changed, 25 insertions(+), 2 deletions(-) diff --git a/nanobot/utils/helpers.py b/nanobot/utils/helpers.py index 3ce15ad62..ae008becd 100644 --- a/nanobot/utils/helpers.py +++ b/nanobot/utils/helpers.py @@ -63,8 +63,8 @@ def strip_think(text: str) -> str: # Stream chunks may end in the middle of a control tag. Strip only known # control-token prefixes at the very end. partial_control_tag = ( - r"?" + r"|<\|?(?:c|ch|cha|chan|chann|channe|channel)(?:\|?>?)?" ) text = re.sub(rf"(?:{partial_control_tag})$", "", text) text = re.sub(r"^\s*<\|?$", "", text) diff --git a/tests/agent/test_runner.py b/tests/agent/test_runner.py index 79af0a674..10d25348a 100644 --- a/tests/agent/test_runner.py +++ b/tests/agent/test_runner.py @@ -993,6 +993,27 @@ async def test_loop_stream_filter_hides_partial_trailing_think_prefix(tmp_path): assert deltas == ["Hello", " World"] +@pytest.mark.asyncio +async def test_loop_stream_filter_hides_complete_trailing_think_tag(tmp_path): + loop = _make_loop(tmp_path) + deltas: list[str] = [] + + async def chat_stream_with_retry(*, on_content_delta, **kwargs): + await on_content_delta("Hello ") + await on_content_delta("hiddenWorld") + return LLMResponse(content="Hello hiddenWorld", tool_calls=[], usage={}) + + loop.provider.chat_stream_with_retry = chat_stream_with_retry + + async def on_stream(delta: str) -> None: + deltas.append(delta) + + final_content, _, _, _, _ = await loop._run_agent_loop([], on_stream=on_stream) + + assert final_content == "Hello World" + assert deltas == ["Hello", " World"] + + @pytest.mark.asyncio async def test_loop_retries_think_only_final_response(tmp_path): loop = _make_loop(tmp_path) diff --git a/tests/utils/test_strip_think.py b/tests/utils/test_strip_think.py index 7adbe7bce..5db93e658 100644 --- a/tests/utils/test_strip_think.py +++ b/tests/utils/test_strip_think.py @@ -105,10 +105,12 @@ class TestStripThinkMalformedLeaks: def test_partial_trailing_think_tag_after_visible_text(self): assert strip_think("喷泉策略说明 ") == "answer" def test_partial_trailing_channel_marker_after_visible_text(self): assert strip_think("喷泉策略说明 <|chan") == "喷泉策略说明" assert strip_think("answer ") == "answer" class TestStripThinkConservativePreserve: From 539d82eadcbe80321a8f09ed667a4d6b7ef1fbd4 Mon Sep 17 00:00:00 2001 From: Xubin Ren Date: Fri, 1 May 2026 12:08:27 +0000 Subject: [PATCH 24/33] test(tools): accept spawn origin message context Made-with: Cursor --- tests/test_tool_contextvars.py | 33 ++++++++++++++++++++++++++++++--- 1 file changed, 30 insertions(+), 3 deletions(-) diff --git a/tests/test_tool_contextvars.py b/tests/test_tool_contextvars.py index 1303f0001..a1e7bd8c0 100644 --- a/tests/test_tool_contextvars.py +++ b/tests/test_tool_contextvars.py @@ -49,7 +49,16 @@ async def test_spawn_tool_keeps_task_local_context() -> None: release = asyncio.Event() class _Manager: - async def spawn(self, *, task: str, label: str | None, origin_channel: str, origin_chat_id: str, session_key: str) -> str: + async def spawn( + self, + *, + task: str, + label: str | None, + origin_channel: str, + origin_chat_id: str, + session_key: str, + origin_message_id: str | None = None, + ) -> str: seen.append((origin_channel, origin_chat_id, session_key)) return f"{origin_channel}:{origin_chat_id}:{task}" @@ -147,7 +156,16 @@ async def test_spawn_tool_basic_set_context_and_execute() -> None: seen: list[tuple[str, str, str]] = [] class _Manager: - async def spawn(self, *, task, label, origin_channel, origin_chat_id, session_key): + async def spawn( + self, + *, + task, + label, + origin_channel, + origin_chat_id, + session_key, + origin_message_id=None, + ): seen.append((origin_channel, origin_chat_id, session_key)) return f"ok: {task}" @@ -165,7 +183,16 @@ async def test_spawn_tool_default_values_without_set_context() -> None: seen: list[tuple[str, str, str]] = [] class _Manager: - async def spawn(self, *, task, label, origin_channel, origin_chat_id, session_key): + async def spawn( + self, + *, + task, + label, + origin_channel, + origin_chat_id, + session_key, + origin_message_id=None, + ): seen.append((origin_channel, origin_chat_id, session_key)) return "ok" From 4860a9a6c9c4c2ba83e9f7e11c48007dc55a474b Mon Sep 17 00:00:00 2001 From: coldxiangyu Date: Fri, 1 May 2026 20:40:09 +0800 Subject: [PATCH 25/33] fix(matrix): stop sync loop on irrecoverable auth errors MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When the Matrix homeserver returns M_UNKNOWN_TOKEN / M_FORBIDDEN / M_UNAUTHORIZED (or soft_logout), the previous _sync_loop kept retrying sync_forever every 2 seconds forever, spamming the homeserver and filling logs (#1851). The auth state cannot recover by retrying, so this is pure noise and a soft DoS on the homeserver. - Extract `_is_fatal_auth_response()` helper - In `_on_sync_error`, on fatal auth: set `_running=False` and call `stop_sync_forever()` so the loop exits cleanly - Add exponential backoff (2s → 60s cap) to the generic exception path in `_sync_loop` so transient network blips also stop hammering Closes #1851 Co-Authored-By: Claude Opus 4.7 (1M context) --- nanobot/channels/matrix.py | 24 +++++++++-- tests/channels/test_matrix_channel.py | 57 ++++++++++++++++++++++++++- 2 files changed, 76 insertions(+), 5 deletions(-) diff --git a/nanobot/channels/matrix.py b/nanobot/channels/matrix.py index b53ae8607..0d1989b03 100644 --- a/nanobot/channels/matrix.py +++ b/nanobot/channels/matrix.py @@ -590,15 +590,26 @@ class MatrixChannel(BaseChannel): self.client.add_response_callback(self._on_join_error, JoinError) self.client.add_response_callback(self._on_send_error, RoomSendError) - def _log_response_error(self, label: str, response: Any) -> None: - """Log Matrix response errors — auth errors at ERROR level, rest at WARNING.""" + def _is_fatal_auth_response(self, response: Any) -> bool: code = getattr(response, "status_code", None) is_auth = code in {"M_UNKNOWN_TOKEN", "M_FORBIDDEN", "M_UNAUTHORIZED"} - is_fatal = is_auth or getattr(response, "soft_logout", False) + return is_auth or bool(getattr(response, "soft_logout", False)) + + def _log_response_error(self, label: str, response: Any) -> None: + """Log Matrix response errors — auth errors at ERROR level, rest at WARNING.""" + is_fatal = self._is_fatal_auth_response(response) (logger.error if is_fatal else logger.warning)("Matrix {} failed: {}", label, response) async def _on_sync_error(self, response: SyncError) -> None: self._log_response_error("sync", response) + if self._is_fatal_auth_response(response): + # Auth errors won't recover by retry; stop the sync loop instead of + # spamming the homeserver every 2s (#1851). + logger.error("Matrix authentication failed irrecoverably; stopping sync loop") + self._running = False + if self.client: + with suppress(Exception): + self.client.stop_sync_forever() async def _on_join_error(self, response: JoinError) -> None: self._log_response_error("join", response) @@ -640,13 +651,18 @@ class MatrixChannel(BaseChannel): await self._set_typing(room_id, False) async def _sync_loop(self) -> None: + backoff = 2.0 while self._running: try: await self.client.sync_forever(timeout=30000, full_state=True) + backoff = 2.0 except asyncio.CancelledError: break except Exception: - await asyncio.sleep(2) + if not self._running: + break + await asyncio.sleep(backoff) + backoff = min(backoff * 2, 60.0) async def _on_room_invite(self, room: MatrixRoom, event: InviteEvent) -> None: if self.is_allowed(event.sender): diff --git a/tests/channels/test_matrix_channel.py b/tests/channels/test_matrix_channel.py index 8fbe2c119..8bd9f8154 100644 --- a/tests/channels/test_matrix_channel.py +++ b/tests/channels/test_matrix_channel.py @@ -7,7 +7,7 @@ import pytest pytest.importorskip("nio") pytest.importorskip("nh3") pytest.importorskip("mistune") -from nio import RoomSendResponse +from nio import RoomSendResponse, SyncError from nanobot.channels.matrix import _build_matrix_text_content @@ -266,6 +266,61 @@ async def test_start_disables_e2ee_when_configured( await channel.stop() +@pytest.mark.asyncio +async def test_on_sync_error_stops_loop_on_unknown_token() -> None: + channel = MatrixChannel(_make_config(), MessageBus()) + client = _FakeAsyncClient("", "", "", None) + channel.client = client + channel._running = True + + await channel._on_sync_error(SyncError(message="bad", status_code="M_UNKNOWN_TOKEN")) + + assert channel._running is False + assert client.stop_sync_forever_called is True + + +@pytest.mark.asyncio +async def test_on_sync_error_keeps_running_on_transient_error() -> None: + channel = MatrixChannel(_make_config(), MessageBus()) + client = _FakeAsyncClient("", "", "", None) + channel.client = client + channel._running = True + + await channel._on_sync_error(SyncError(message="oops", status_code="M_LIMIT_EXCEEDED")) + + assert channel._running is True + assert client.stop_sync_forever_called is False + + +@pytest.mark.asyncio +async def test_sync_loop_backs_off_on_repeated_errors(monkeypatch) -> None: + channel = MatrixChannel(_make_config(), MessageBus()) + + sleeps: list[float] = [] + + async def _fake_sleep(delay: float) -> None: + sleeps.append(delay) + + monkeypatch.setattr(matrix_module.asyncio, "sleep", _fake_sleep) + + call_count = {"n": 0} + + class _BoomClient: + async def sync_forever(self, **_kwargs) -> None: + call_count["n"] += 1 + if call_count["n"] > 4: + channel._running = False + return + raise RuntimeError("boom") + + channel.client = _BoomClient() + channel._running = True + + await channel._sync_loop() + + assert sleeps == [2.0, 4.0, 8.0, 16.0] + + @pytest.mark.asyncio async def test_stop_stops_sync_forever_before_close(monkeypatch) -> None: channel = MatrixChannel(_make_config(device_id="DEVICE"), MessageBus()) From 4c54a2b15342481a3e18f5571b3f0cd6850b2a59 Mon Sep 17 00:00:00 2001 From: coldxiangyu Date: Fri, 1 May 2026 21:40:20 +0800 Subject: [PATCH 26/33] fix(anthropic): auto-fallback to stream on long-request error The Anthropic SDK raises a client-side ValueError when a non-streaming `messages.create` call could exceed the 10-minute server timeout (e.g. high `max_tokens` combined with extended thinking budget). The error text "Streaming is required for operations that may take longer than 10 minutes" was bubbling up to the user as an opaque LLM error in channels that use the non-stream path (e.g. wecom in #2709). Detect this specific ValueError in `chat()` and transparently retry through `chat_stream()` (without `on_content_delta` so behavior matches the non-stream contract). Other ValueErrors continue to flow through `_handle_error` unchanged. Closes #2709 Co-Authored-By: Claude Opus 4.7 (1M context) --- nanobot/providers/anthropic_provider.py | 22 ++++ .../test_anthropic_long_request_fallback.py | 105 ++++++++++++++++++ 2 files changed, 127 insertions(+) create mode 100644 tests/providers/test_anthropic_long_request_fallback.py diff --git a/nanobot/providers/anthropic_provider.py b/nanobot/providers/anthropic_provider.py index a46cc32e9..2c6aa531e 100644 --- a/nanobot/providers/anthropic_provider.py +++ b/nanobot/providers/anthropic_provider.py @@ -537,6 +537,13 @@ class AnthropicProvider(LLMProvider): # Public API # ------------------------------------------------------------------ + @staticmethod + def _is_streaming_required_error(e: Exception) -> bool: + """Anthropic SDK rejects long non-stream requests with a ValueError + whose message starts with 'Streaming is required'. Match defensively + on substring so a future SDK message tweak doesn't break detection.""" + return isinstance(e, ValueError) and "streaming is required" in str(e).lower() + async def chat( self, messages: list[dict[str, Any]], @@ -555,6 +562,21 @@ class AnthropicProvider(LLMProvider): response = await self._client.messages.create(**kwargs) return self._parse_response(response) except Exception as e: + if self._is_streaming_required_error(e): + # Anthropic SDK refuses non-stream calls when max_tokens (plus + # extended thinking budget) could push the request past the + # 10-minute server-side timeout (#2709). Transparently retry + # via the streaming path so callers don't need to know the + # provider-specific limit. + return await self.chat_stream( + messages=messages, + tools=tools, + model=model, + max_tokens=max_tokens, + temperature=temperature, + reasoning_effort=reasoning_effort, + tool_choice=tool_choice, + ) return self._handle_error(e) async def chat_stream( diff --git a/tests/providers/test_anthropic_long_request_fallback.py b/tests/providers/test_anthropic_long_request_fallback.py new file mode 100644 index 000000000..6af968e6f --- /dev/null +++ b/tests/providers/test_anthropic_long_request_fallback.py @@ -0,0 +1,105 @@ +"""Regression test for #2709: Anthropic non-stream long-request fallback. + +When ``messages.create`` raises the Anthropic SDK's client-side +``ValueError("Streaming is required for operations that may take longer +than 10 minutes...")``, ``AnthropicProvider.chat`` should transparently +retry via ``chat_stream`` instead of surfacing the error. +""" + +from __future__ import annotations + +from typing import Any +from unittest.mock import AsyncMock, MagicMock + +import pytest + +from nanobot.providers.anthropic_provider import AnthropicProvider +from nanobot.providers.base import LLMResponse + + +_LONG_REQUEST_MESSAGE = ( + "Streaming is required for operations that may take longer than 10 minutes. " + "See https://github.com/anthropics/anthropic-sdk-python#long-requests for more details" +) + + +def _make_provider() -> AnthropicProvider: + provider = AnthropicProvider(api_key="test-key") + provider._client = MagicMock() + return provider + + +def test_is_streaming_required_error_matches_value_error() -> None: + assert AnthropicProvider._is_streaming_required_error( + ValueError(_LONG_REQUEST_MESSAGE) + ) is True + + +def test_is_streaming_required_error_ignores_other_value_errors() -> None: + assert AnthropicProvider._is_streaming_required_error( + ValueError("something else went wrong") + ) is False + + +def test_is_streaming_required_error_ignores_other_exception_types() -> None: + assert AnthropicProvider._is_streaming_required_error( + RuntimeError(_LONG_REQUEST_MESSAGE) + ) is False + + +@pytest.mark.asyncio +async def test_chat_falls_back_to_stream_on_long_request_error() -> None: + provider = _make_provider() + provider._client.messages.create = AsyncMock( + side_effect=ValueError(_LONG_REQUEST_MESSAGE) + ) + + expected = LLMResponse(content="streamed result", finish_reason="stop") + captured: dict[str, Any] = {} + + async def _fake_chat_stream(**kwargs: Any) -> LLMResponse: + captured.update(kwargs) + return expected + + provider.chat_stream = _fake_chat_stream # type: ignore[method-assign] + + result = await provider.chat( + messages=[{"role": "user", "content": "hi"}], + max_tokens=64_000, + temperature=0.5, + reasoning_effort="high", + tool_choice="auto", + ) + + assert result is expected + assert captured["messages"] == [{"role": "user", "content": "hi"}] + assert captured["max_tokens"] == 64_000 + assert captured["temperature"] == 0.5 + assert captured["reasoning_effort"] == "high" + assert captured["tool_choice"] == "auto" + # The fallback must NOT pass an on_content_delta — chat() callers don't + # expect streaming side-effects. + assert "on_content_delta" not in captured + + +@pytest.mark.asyncio +async def test_chat_does_not_fall_back_on_unrelated_value_error() -> None: + provider = _make_provider() + provider._client.messages.create = AsyncMock( + side_effect=ValueError("some other validation failure") + ) + + called = False + + async def _should_not_be_called(**_kwargs: Any) -> LLMResponse: + nonlocal called + called = True + return LLMResponse(content="x", finish_reason="stop") + + provider.chat_stream = _should_not_be_called # type: ignore[method-assign] + + result = await provider.chat(messages=[{"role": "user", "content": "hi"}]) + + assert called is False + # Generic ValueError flows through _handle_error and surfaces as an error response. + assert result.finish_reason == "error" or "Error" in (result.content or "") From fd1a5a62675836c979fdef59fe869d9577d9d1c9 Mon Sep 17 00:00:00 2001 From: Xubin Ren Date: Fri, 1 May 2026 15:42:58 +0000 Subject: [PATCH 27/33] test(provider): tidy Anthropic fallback imports Co-authored-by: Cursor --- tests/providers/test_anthropic_long_request_fallback.py | 1 - 1 file changed, 1 deletion(-) diff --git a/tests/providers/test_anthropic_long_request_fallback.py b/tests/providers/test_anthropic_long_request_fallback.py index 6af968e6f..0cfdee922 100644 --- a/tests/providers/test_anthropic_long_request_fallback.py +++ b/tests/providers/test_anthropic_long_request_fallback.py @@ -16,7 +16,6 @@ import pytest from nanobot.providers.anthropic_provider import AnthropicProvider from nanobot.providers.base import LLMResponse - _LONG_REQUEST_MESSAGE = ( "Streaming is required for operations that may take longer than 10 minutes. " "See https://github.com/anthropics/anthropic-sdk-python#long-requests for more details" From ee364c6ac1634c3b5fd89c9947d8f446b665df68 Mon Sep 17 00:00:00 2001 From: yorkhellen Date: Fri, 1 May 2026 23:57:15 +0800 Subject: [PATCH 28/33] fix(helpers): restore tiktoken fallback in estimate_prompt_tokens_chain --- nanobot/utils/helpers.py | 1 + 1 file changed, 1 insertion(+) diff --git a/nanobot/utils/helpers.py b/nanobot/utils/helpers.py index ae008becd..0afe193cc 100644 --- a/nanobot/utils/helpers.py +++ b/nanobot/utils/helpers.py @@ -431,6 +431,7 @@ def estimate_prompt_tokens_chain( tokens, source = provider_counter(messages, tools, model) if isinstance(tokens, (int, float)) and tokens > 0: return int(tokens), str(source or "provider_counter") + estimated = estimate_prompt_tokens(messages, tools) if estimated > 0: return int(estimated), "tiktoken" return 0, "none" From 051037ff087c18156237e5385f7cc6b5d032ec6f Mon Sep 17 00:00:00 2001 From: moranfong <274257964+zijiefang@users.noreply.github.com> Date: Mon, 13 Apr 2026 23:45:26 +0800 Subject: [PATCH 29/33] feat(provider): add LongCat via OpenAI-compatible backend --- docs/configuration.md | 29 +++++++++++++++++++++ nanobot/config/schema.py | 1 + nanobot/providers/registry.py | 9 +++++++ tests/cli/test_commands.py | 33 ++++++++++++++++++++++++ tests/providers/test_longcat_provider.py | 29 +++++++++++++++++++++ 5 files changed, 101 insertions(+) create mode 100644 tests/providers/test_longcat_provider.py diff --git a/docs/configuration.md b/docs/configuration.md index f20cec5f0..51e9f32d8 100644 --- a/docs/configuration.md +++ b/docs/configuration.md @@ -76,6 +76,7 @@ IMAP_PASSWORD=your-password-here | `moonshot` | LLM (Moonshot/Kimi) | [platform.moonshot.cn](https://platform.moonshot.cn) | | `zhipu` | LLM (Zhipu GLM) | [open.bigmodel.cn](https://open.bigmodel.cn) | | `mimo` | LLM (MiMo) | [platform.xiaomimimo.com](https://platform.xiaomimimo.com) | +| `longcat` | LLM (LongCat) | [longcat.chat](https://longcat.chat/platform/docs/zh/) | | `ollama` | LLM (local, Ollama) | — | | `lm_studio` | LLM (local, LM Studio) | — | | `mistral` | LLM | [docs.mistral.ai](https://docs.mistral.ai/) | @@ -339,6 +340,34 @@ nanobot agent -c ~/.nanobot-telegram/config.json -w /tmp/nanobot-telegram-test -
+
+LongCat (OpenAI-compatible) + +LongCat is available through nanobot's built-in OpenAI-compatible provider flow. +The default API base already points to `https://api.longcat.chat/openai`, so you +usually only need to set `apiKey`. + +```json +{ + "providers": { + "longcat": { + "apiKey": "${LONGCAT_API_KEY}" + } + }, + "agents": { + "defaults": { + "provider": "longcat", + "model": "LongCat-Flash-Chat" + } + } +} +``` + +Official model names include `LongCat-Flash-Chat`, `LongCat-Flash-Thinking`, +`LongCat-Flash-Thinking-2601`, and `LongCat-Flash-Lite`. + +
+
Custom Provider (Any OpenAI-compatible API) diff --git a/nanobot/config/schema.py b/nanobot/config/schema.py index 210767bfe..bb7ed0731 100644 --- a/nanobot/config/schema.py +++ b/nanobot/config/schema.py @@ -151,6 +151,7 @@ class ProvidersConfig(Base): mistral: ProviderConfig = Field(default_factory=ProviderConfig) stepfun: ProviderConfig = Field(default_factory=ProviderConfig) # Step Fun (阶跃星辰) xiaomi_mimo: ProviderConfig = Field(default_factory=ProviderConfig) # Xiaomi MIMO (小米) + longcat: ProviderConfig = Field(default_factory=ProviderConfig) # LongCat aihubmix: ProviderConfig = Field(default_factory=ProviderConfig) # AiHubMix API gateway siliconflow: ProviderConfig = Field(default_factory=ProviderConfig) # SiliconFlow (硅基流动) volcengine: ProviderConfig = Field(default_factory=ProviderConfig) # VolcEngine (火山引擎) diff --git a/nanobot/providers/registry.py b/nanobot/providers/registry.py index a64041688..44fcd1ee3 100644 --- a/nanobot/providers/registry.py +++ b/nanobot/providers/registry.py @@ -376,6 +376,15 @@ PROVIDERS: tuple[ProviderSpec, ...] = ( backend="openai_compat", default_api_base="https://api.xiaomimimo.com/v1", ), + # LongCat: OpenAI-compatible API + ProviderSpec( + name="longcat", + keywords=("longcat",), + env_key="LONGCAT_API_KEY", + display_name="LongCat", + backend="openai_compat", + default_api_base="https://api.longcat.chat/openai", + ), # === Local deployment (matched by config key, NOT by api_base) ========= # vLLM / any OpenAI-compatible local server ProviderSpec( diff --git a/tests/cli/test_commands.py b/tests/cli/test_commands.py index 6439c73f3..2360ac288 100644 --- a/tests/cli/test_commands.py +++ b/tests/cli/test_commands.py @@ -285,6 +285,39 @@ def test_find_by_name_accepts_camel_case_and_hyphen_aliases(): assert find_by_name("volcengineCodingPlan").name == "volcengine_coding_plan" assert find_by_name("github-copilot") is not None assert find_by_name("github-copilot").name == "github_copilot" + assert find_by_name("longcat") is not None + assert find_by_name("longcat").name == "longcat" + + +def test_config_explicit_longcat_provider_resolves_provider_name(): + config = Config.model_validate( + { + "agents": { + "defaults": { + "provider": "longcat", + "model": "LongCat-Flash-Chat", + } + }, + "providers": { + "longcat": { + "apiKey": "test-key", + } + }, + } + ) + + assert config.get_provider_name() == "longcat" + + +def test_config_auto_detects_longcat_from_model_keyword(): + config = Config.model_validate( + { + "agents": {"defaults": {"provider": "auto", "model": "longcat/LongCat-Flash-Chat"}}, + "providers": {"longcat": {"apiKey": "test-key"}}, + } + ) + + assert config.get_provider_name() == "longcat" def test_config_explicit_xiaomi_mimo_provider_uses_default_api_base(): diff --git a/tests/providers/test_longcat_provider.py b/tests/providers/test_longcat_provider.py new file mode 100644 index 000000000..20190098d --- /dev/null +++ b/tests/providers/test_longcat_provider.py @@ -0,0 +1,29 @@ +"""Tests for the LongCat provider registration.""" + +from nanobot.config.schema import ProvidersConfig +from nanobot.providers.registry import PROVIDERS, find_by_name + + +def test_longcat_config_field_exists(): + """ProvidersConfig should have a longcat field.""" + config = ProvidersConfig() + assert hasattr(config, "longcat") + + +def test_longcat_provider_in_registry(): + """LongCat should be registered in the provider registry.""" + specs = {s.name: s for s in PROVIDERS} + assert "longcat" in specs + + longcat = specs["longcat"] + assert longcat.backend == "openai_compat" + assert longcat.env_key == "LONGCAT_API_KEY" + assert longcat.default_api_base == "https://api.longcat.chat/openai" + + +def test_find_by_name_longcat(): + """find_by_name should resolve the LongCat provider.""" + spec = find_by_name("longcat") + + assert spec is not None + assert spec.name == "longcat" From 861fbb0dde20db6875b3913e8a5faf5950fc0291 Mon Sep 17 00:00:00 2001 From: Xubin Ren Date: Fri, 1 May 2026 17:26:23 +0000 Subject: [PATCH 30/33] fix(provider): correct LongCat OpenAI base URL Use the SDK-ready /v1 base so LongCat chat completions hit the documented endpoint. Co-authored-by: Cursor --- docs/configuration.md | 2 +- nanobot/providers/registry.py | 2 +- tests/cli/test_commands.py | 1 + tests/providers/test_longcat_provider.py | 2 +- 4 files changed, 4 insertions(+), 3 deletions(-) diff --git a/docs/configuration.md b/docs/configuration.md index 51e9f32d8..ec889c758 100644 --- a/docs/configuration.md +++ b/docs/configuration.md @@ -344,7 +344,7 @@ nanobot agent -c ~/.nanobot-telegram/config.json -w /tmp/nanobot-telegram-test - LongCat (OpenAI-compatible) LongCat is available through nanobot's built-in OpenAI-compatible provider flow. -The default API base already points to `https://api.longcat.chat/openai`, so you +The default API base already points to `https://api.longcat.chat/openai/v1`, so you usually only need to set `apiKey`. ```json diff --git a/nanobot/providers/registry.py b/nanobot/providers/registry.py index 44fcd1ee3..2e2bdbc50 100644 --- a/nanobot/providers/registry.py +++ b/nanobot/providers/registry.py @@ -383,7 +383,7 @@ PROVIDERS: tuple[ProviderSpec, ...] = ( env_key="LONGCAT_API_KEY", display_name="LongCat", backend="openai_compat", - default_api_base="https://api.longcat.chat/openai", + default_api_base="https://api.longcat.chat/openai/v1", ), # === Local deployment (matched by config key, NOT by api_base) ========= # vLLM / any OpenAI-compatible local server diff --git a/tests/cli/test_commands.py b/tests/cli/test_commands.py index 2360ac288..50ede9095 100644 --- a/tests/cli/test_commands.py +++ b/tests/cli/test_commands.py @@ -307,6 +307,7 @@ def test_config_explicit_longcat_provider_resolves_provider_name(): ) assert config.get_provider_name() == "longcat" + assert config.get_api_base() == "https://api.longcat.chat/openai/v1" def test_config_auto_detects_longcat_from_model_keyword(): diff --git a/tests/providers/test_longcat_provider.py b/tests/providers/test_longcat_provider.py index 20190098d..0e25b6aee 100644 --- a/tests/providers/test_longcat_provider.py +++ b/tests/providers/test_longcat_provider.py @@ -18,7 +18,7 @@ def test_longcat_provider_in_registry(): longcat = specs["longcat"] assert longcat.backend == "openai_compat" assert longcat.env_key == "LONGCAT_API_KEY" - assert longcat.default_api_base == "https://api.longcat.chat/openai" + assert longcat.default_api_base == "https://api.longcat.chat/openai/v1" def test_find_by_name_longcat(): From fde530de017436584313e94cef8575869d4d4bca Mon Sep 17 00:00:00 2001 From: Xubin Ren Date: Sat, 2 May 2026 07:40:29 +0000 Subject: [PATCH 31/33] refactor(setup): enhance SKILL.md for upgrade process clarity --- nanobot/skills/update-setup/SKILL.md | 53 +++++++++++++++++++++++----- 1 file changed, 45 insertions(+), 8 deletions(-) diff --git a/nanobot/skills/update-setup/SKILL.md b/nanobot/skills/update-setup/SKILL.md index 104afcd5f..7e9d5cc60 100644 --- a/nanobot/skills/update-setup/SKILL.md +++ b/nanobot/skills/update-setup/SKILL.md @@ -13,21 +13,45 @@ Use `read_file` to check if `skills/update/SKILL.md` already exists in the works If it exists, use `ask_user` to ask: "An upgrade skill already exists. Reconfigure?" with options ["yes", "no"]. If no, stop here. -## Step 2: Current Version +## Step 2: Current Version and Install Clues Use `exec` to run `nanobot --version`. Tell the user the current version. -## Step 3: Ask Questions +Then collect install clues with `exec`. These commands are best-effort; if one fails, +keep going and show the useful output: -Use `ask_user` for the questions below, one question per call. +``` +command -v nanobot || true +python -m pip show nanobot-ai || true +pipx list | sed -n '/nanobot-ai/,+3p' || true +uv tool list | sed -n '/nanobot-ai/,+3p' || true +``` + +Summarize what you found in one short paragraph. Use the clues only to suggest a +likely install method. Do not treat them as confirmation. + +## Step 3: Confirm Required Inputs + +CRITICAL: Do not write `skills/update/SKILL.md` until the install method is +explicitly confirmed by the user. The install method must come from a user +answer or confirmation, not from inference alone. If you cannot get a clear +answer, stop and ask the user to rerun this setup when they know how nanobot was +installed. + +Use `ask_user` for the questions below, one question per call. If `ask_user` is +not available or cannot collect the answer, ask in normal chat and stop without +writing the skill. **Question 1 — Install method:** ``` -question: "How did you install nanobot?" -options: ["uv", "pipx", "pip", "source (git clone)"] +question: "I found these install clues: . Which update method should this workspace use?" +options: ["uv", "pipx", "pip", "source (git clone)", "not sure"] ``` +If the user selected `not sure`, explain the difference between the options and +stop. Do not generate the upgrade skill. + If the user selected `source (git clone)`, ask for the local checkout path: `question: "Where is your nanobot source checkout? Enter an absolute path or a path relative to this workspace:"`. @@ -63,6 +87,18 @@ Determine the upgrade command from the install method: For source installs, include extras in the editable install command when selected. Quote the source checkout path if it contains spaces. +Determine the preflight check from the install method: + +| Method | Preflight check | +|--------|-----------------| +| uv | `command -v uv` | +| pipx | `command -v pipx` | +| pip | `python -m pip --version` | +| source | `test -d && test -d /.git && test -f /pyproject.toml` | + +For source installs, quote the source checkout path in the preflight check if it +contains spaces. + Build the skill content. If proxy is configured, add `export http_proxy=URL` and `export https_proxy=URL` lines before the upgrade command. Use `write_file` to write `skills/update/SKILL.md` with this content: @@ -76,9 +112,10 @@ description: "Upgrade nanobot to the latest version. Triggers: upgrade nanobot, # Update Nanobot 1. (If proxy configured) Set proxy: `export http_proxy=URL && export https_proxy=URL` -2. Use `exec` to run the upgrade command: -3. Use `exec` to verify: `nanobot --version` -4. Tell the user the new version. Say: "Run `/restart` to restart nanobot and apply the update. If `/restart` is unavailable in this channel, restart the nanobot process manually." +2. Use `exec` to run the preflight check: . If it fails, stop and tell the user to rerun `update-setup` because the saved install method no longer matches this environment. +3. Use `exec` to run the upgrade command: +4. Use `exec` to verify: `nanobot --version` +5. Tell the user the new version. Say: "Run `/restart` to restart nanobot and apply the update. If `/restart` is unavailable in this channel, restart the nanobot process manually." ``` ## Step 5: Confirm From 2fa15ccf1bee318802243d3986db471df462b309 Mon Sep 17 00:00:00 2001 From: Xubin Ren Date: Sat, 2 May 2026 11:37:07 +0000 Subject: [PATCH 32/33] fix: improve media failure diagnostics and token fallback coverage --- nanobot/channels/slack.py | 13 ++++++++--- tests/channels/test_slack_channel.py | 8 +++++++ tests/utils/test_token_estimation.py | 32 ++++++++++++++++++++++++++++ 3 files changed, 50 insertions(+), 3 deletions(-) create mode 100644 tests/utils/test_token_estimation.py diff --git a/nanobot/channels/slack.py b/nanobot/channels/slack.py index 66ef6f3b6..0bdeedc78 100644 --- a/nanobot/channels/slack.py +++ b/nanobot/channels/slack.py @@ -435,9 +435,9 @@ class SlackChannel(BaseChannel): marker = f"[{marker_type}: {name}]" url = str(file_info.get("url_private_download") or file_info.get("url_private") or "") if not url: - return None, f"[{marker_type}: {name}: missing download url]" + return None, self._download_failure_marker(marker_type, name, "missing download url") if not self.config.bot_token: - return None, f"[{marker_type}: {name}: missing bot token]" + return None, self._download_failure_marker(marker_type, name, "missing bot token") filename = safe_filename(f"{file_id}_{name}") path = Path(get_media_dir("slack")) / filename @@ -454,7 +454,14 @@ class SlackChannel(BaseChannel): return str(path), marker except Exception as e: logger.warning("Failed to download Slack file {}: {}", file_id, e) - return None, f"[{marker_type}: {name}: download failed]" + return None, self._download_failure_marker(marker_type, name, "download failed") + + @staticmethod + def _download_failure_marker(marker_type: str, name: str, reason: str) -> str: + return ( + f"[{marker_type}: {name}: {reason}; not available to nanobot. " + "Check Slack files:read scope, reinstall the Slack app, and ensure the bot can access the file.]" + ) @staticmethod def _looks_like_html_download(response: httpx.Response) -> bool: diff --git a/tests/channels/test_slack_channel.py b/tests/channels/test_slack_channel.py index 49ae3908e..630685eed 100644 --- a/tests/channels/test_slack_channel.py +++ b/tests/channels/test_slack_channel.py @@ -643,6 +643,14 @@ def test_slack_download_rejects_login_html() -> None: assert SlackChannel._looks_like_html_download(markdown_response) is False +def test_slack_download_failure_marker_is_actionable() -> None: + marker = SlackChannel._download_failure_marker("image", "screenshot.png", "download failed") + + assert "not available to nanobot" in marker + assert "files:read" in marker + assert "reinstall the Slack app" in marker + + def test_slack_channel_uses_channel_aware_allow_policy() -> None: channel = SlackChannel(SlackConfig(enabled=True, allow_from=[]), MessageBus()) assert channel.is_allowed("U1") is True diff --git a/tests/utils/test_token_estimation.py b/tests/utils/test_token_estimation.py new file mode 100644 index 000000000..c3257b317 --- /dev/null +++ b/tests/utils/test_token_estimation.py @@ -0,0 +1,32 @@ +from nanobot.utils.helpers import estimate_prompt_tokens_chain + + +class _NoCounterProvider: + pass + + +class _BrokenCounterProvider: + def estimate_prompt_tokens(self, messages, tools=None, model=None): + raise RuntimeError("counter unavailable") + + +def test_estimate_prompt_tokens_chain_falls_back_without_provider_counter() -> None: + tokens, source = estimate_prompt_tokens_chain( + _NoCounterProvider(), + "test-model", + [{"role": "user", "content": "hello"}], + ) + + assert tokens > 0 + assert source == "tiktoken" + + +def test_estimate_prompt_tokens_chain_falls_back_when_provider_counter_fails() -> None: + tokens, source = estimate_prompt_tokens_chain( + _BrokenCounterProvider(), + "test-model", + [{"role": "user", "content": "hello"}], + ) + + assert tokens > 0 + assert source == "tiktoken" From 5853d5dfda8a3660ad4da4eddf99353edb06496f Mon Sep 17 00:00:00 2001 From: chengyongru <61816729+chengyongru@users.noreply.github.com> Date: Sun, 3 May 2026 00:27:17 +0800 Subject: [PATCH 33/33] fix: allow_patterns take priority over deny_patterns in ExecTool (#3594) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fix: allow_patterns take priority over deny_patterns in ExecTool Previously deny_patterns were checked first with no bypass, meaning allow_patterns could never exempt commands from the built-in deny list. This made it impossible to whitelist destructive commands for specific directories (e.g. build/cleanup tasks). Changes: - shell.py: check allow_patterns first; if matched, skip deny check - shell.py: deny_patterns now appends to built-in list (not replaces) - schema.py: add allow_patterns/deny_patterns to ExecToolConfig - loop.py/subagent.py: pass allow_patterns/deny_patterns to ExecTool - Add test_exec_allow_patterns.py covering priority semantics * fix: separate deny pattern errors from workspace violation detection The deny pattern error message "Command blocked by safety guard" was included in _WORKSPACE_BLOCK_MARKERS, causing deny_pattern blocks to be misclassified as fatal workspace violations. This meant LLMs had no chance to retry with a different command — the turn was aborted immediately. Changes: - shell.py: deny/allowlist error messages now use distinct phrasing ("blocked by deny pattern filter" / "blocked by allowlist filter") - runner.py: remove "blocked by safety guard" from _WORKSPACE_BLOCK_MARKERS so deny_pattern errors are treated as normal tool errors (LLM can retry) instead of fatal violations - workspace path errors still use "blocked by safety guard" and remain fatal as intended * fix: update test assertions to match new deny pattern error message * fix: indentation error in test file * fix: restore SSRF fatal classification and tidy exec pattern plumbing Address review feedback on the deny/allow_patterns rework: - runner.py: re-add "internal/private url detected" to _WORKSPACE_BLOCK_MARKERS. The earlier marker removal also stripped fatal classification from SSRF / internal-URL rejections (whose message still says "blocked by safety guard"), turning a hard security boundary into something the LLM could retry. - loop.py / subagent.py: drop `or None` between ExecToolConfig and ExecTool. The schema default is an empty list and ExecTool already normalizes None back to [], so the indirection was a no-op. - shell.py: extract `explicitly_allowed` flag in _guard_command so allow_patterns are scanned once instead of twice and the control flow no longer relies on a no-op `pass + else` branch. - tests/agent/test_runner.py: add a regression test asserting that the SSRF block message is treated as fatal, while deny/allowlist filter messages are deliberately non-fatal. * fix: remove unused exec allow-pattern test import Keep the new ExecTool allow-pattern coverage clean under ruff. Co-authored-by: Cursor --------- Co-authored-by: Xubin Ren Co-authored-by: Cursor --- nanobot/agent/loop.py | 2 + nanobot/agent/runner.py | 2 +- nanobot/agent/subagent.py | 2 + nanobot/agent/tools/shell.py | 20 ++++++--- nanobot/config/schema.py | 2 + tests/agent/test_runner.py | 21 +++++++++ tests/tools/test_exec_allow_patterns.py | 58 +++++++++++++++++++++++++ tests/tools/test_exec_security.py | 2 +- 8 files changed, 100 insertions(+), 9 deletions(-) create mode 100644 tests/tools/test_exec_allow_patterns.py diff --git a/nanobot/agent/loop.py b/nanobot/agent/loop.py index b74c314bb..1a8042fed 100644 --- a/nanobot/agent/loop.py +++ b/nanobot/agent/loop.py @@ -374,6 +374,8 @@ class AgentLoop: sandbox=self.exec_config.sandbox, path_append=self.exec_config.path_append, allowed_env_keys=self.exec_config.allowed_env_keys, + allow_patterns=self.exec_config.allow_patterns, + deny_patterns=self.exec_config.deny_patterns, ) ) if self.web_config.enable: diff --git a/nanobot/agent/runner.py b/nanobot/agent/runner.py index d002d8989..3d941f382 100644 --- a/nanobot/agent/runner.py +++ b/nanobot/agent/runner.py @@ -833,13 +833,13 @@ class AgentRunner: # Markers identifying tool results that represent a workspace / safety boundary rejection. _WORKSPACE_BLOCK_MARKERS: tuple[str, ...] = ( - "blocked by safety guard", "outside the configured workspace", "outside allowed directory", "working_dir is outside", "working_dir could not be resolved", "path traversal detected", "path outside working dir", + "internal/private url detected", ) @classmethod diff --git a/nanobot/agent/subagent.py b/nanobot/agent/subagent.py index 0dd549fe8..18f0bd53b 100644 --- a/nanobot/agent/subagent.py +++ b/nanobot/agent/subagent.py @@ -188,6 +188,8 @@ class SubagentManager: sandbox=self.exec_config.sandbox, path_append=self.exec_config.path_append, allowed_env_keys=self.exec_config.allowed_env_keys, + allow_patterns=self.exec_config.allow_patterns, + deny_patterns=self.exec_config.deny_patterns, )) if self.web_config.enable: tools.register( diff --git a/nanobot/agent/tools/shell.py b/nanobot/agent/tools/shell.py index 059428aa1..b7f841a5c 100644 --- a/nanobot/agent/tools/shell.py +++ b/nanobot/agent/tools/shell.py @@ -52,7 +52,7 @@ class ExecTool(Tool): self.timeout = timeout self.working_dir = working_dir self.sandbox = sandbox - self.deny_patterns = deny_patterns or [ + self.deny_patterns = (deny_patterns or []) + [ r"\brm\s+-[rf]{1,2}\b", # rm -r, rm -rf, rm -fr r"\bdel\s+/[fq]\b", # del /f, del /q r"\brmdir\s+/s\b", # rmdir /s @@ -273,13 +273,19 @@ class ExecTool(Tool): cmd = command.strip() lower = cmd.lower() - for pattern in self.deny_patterns: - if re.search(pattern, lower): - return "Error: Command blocked by safety guard (dangerous pattern detected)" + # allow_patterns take priority over deny_patterns so that users can + # exempt specific commands (e.g. "rm -rf" inside a build directory) + # from the hardcoded deny list via configuration. + explicitly_allowed = bool(self.allow_patterns) and any( + re.search(p, lower) for p in self.allow_patterns + ) + if not explicitly_allowed: + for pattern in self.deny_patterns: + if re.search(pattern, lower): + return "Error: Command blocked by deny pattern filter" - if self.allow_patterns: - if not any(re.search(p, lower) for p in self.allow_patterns): - return "Error: Command blocked by safety guard (not in allowlist)" + if self.allow_patterns: + return "Error: Command blocked by allowlist filter (not in allowlist)" from nanobot.security.network import contains_internal_url if contains_internal_url(cmd): diff --git a/nanobot/config/schema.py b/nanobot/config/schema.py index bb7ed0731..be4eb7202 100644 --- a/nanobot/config/schema.py +++ b/nanobot/config/schema.py @@ -223,6 +223,8 @@ class ExecToolConfig(Base): path_append: str = "" sandbox: str = "" # sandbox backend: "" (none) or "bwrap" allowed_env_keys: list[str] = Field(default_factory=list) # Env var names to pass through to subprocess (e.g. ["GOPATH", "JAVA_HOME"]) + allow_patterns: list[str] = Field(default_factory=list) # Regex patterns that bypass deny_patterns (e.g. [r"rm\s+-rf\s+/tmp/"]) + deny_patterns: list[str] = Field(default_factory=list) # Extra regex patterns to block (appended to built-in list) class MCPServerConfig(Base): """MCP server connection configuration (stdio or HTTP).""" diff --git a/tests/agent/test_runner.py b/tests/agent/test_runner.py index 10d25348a..aa558b4ff 100644 --- a/tests/agent/test_runner.py +++ b/tests/agent/test_runner.py @@ -352,6 +352,27 @@ async def test_runner_stops_on_workspace_violation_without_fail_on_tool_error(): ] +def test_is_workspace_violation_recognizes_ssrf_block(): + """Internal/private URL block must be classified as a fatal workspace violation. + + Regression guard: the deny/allowlist filter messages were intentionally split + out of `_WORKSPACE_BLOCK_MARKERS` so the LLM can retry, but SSRF rejections + are a hard security boundary and must remain fatal. + """ + from nanobot.agent.runner import AgentRunner + + ssrf_msg = "Error: Command blocked by safety guard (internal/private URL detected)" + assert AgentRunner._is_workspace_violation(ssrf_msg) is True + + # Sanity: deny/allowlist filter messages are deliberately *not* fatal. + assert AgentRunner._is_workspace_violation( + "Error: Command blocked by deny pattern filter" + ) is False + assert AgentRunner._is_workspace_violation( + "Error: Command blocked by allowlist filter (not in allowlist)" + ) is False + + @pytest.mark.asyncio async def test_runner_persists_large_tool_results_for_follow_up_calls(tmp_path): from nanobot.agent.runner import AgentRunSpec, AgentRunner diff --git a/tests/tools/test_exec_allow_patterns.py b/tests/tools/test_exec_allow_patterns.py new file mode 100644 index 000000000..1a2a90521 --- /dev/null +++ b/tests/tools/test_exec_allow_patterns.py @@ -0,0 +1,58 @@ +"""Tests for allow_patterns priority over deny_patterns.""" + +from __future__ import annotations + +from nanobot.agent.tools.shell import ExecTool + + +def test_deny_patterns_block_rm_rf(): + """Baseline: rm -rf is blocked by default deny list.""" + tool = ExecTool() + result = tool._guard_command("rm -rf /tmp/build", "/tmp") + assert result is not None + assert "deny pattern filter" in result.lower() + + +def test_allow_patterns_bypass_deny(): + """allow_patterns take priority: matching command skips deny check.""" + tool = ExecTool(allow_patterns=[r"rm\s+-rf\s+/tmp/"]) + result = tool._guard_command("rm -rf /tmp/build", "/tmp") + assert result is None + + +def test_allow_patterns_must_match_to_bypass(): + """Non-matching allow_patterns do NOT bypass deny.""" + tool = ExecTool(allow_patterns=[r"rm\s+-rf\s+/opt/"]) + result = tool._guard_command("rm -rf /tmp/build", "/tmp") + assert result is not None + assert "deny pattern filter" in result.lower() + + +def test_extra_deny_patterns_from_config(): + """User-supplied deny patterns are appended to built-in list.""" + tool = ExecTool(deny_patterns=[r"\bping\b"]) + # ping is blocked by extra deny + assert tool._guard_command("ping example.com", "/tmp") is not None + # rm -rf still blocked by built-in deny + assert tool._guard_command("rm -rf /tmp/x", "/tmp") is not None + + +def test_allow_patterns_bypass_extra_deny(): + """allow_patterns also bypasses user-supplied deny patterns.""" + tool = ExecTool( + deny_patterns=[r"\bping\b"], + allow_patterns=[r"\bping\s+example\.com\b"], + ) + result = tool._guard_command("ping example.com", "/tmp") + assert result is None + + +def test_allow_patterns_is_whitelist_only(): + """When allow_patterns is set, non-matching non-denied commands are blocked.""" + tool = ExecTool(allow_patterns=[r"\becho\b"]) + # echo matches allow → ok + assert tool._guard_command("echo hello", "/tmp") is None + # ls does not match allow and is not in deny → blocked by allowlist + result = tool._guard_command("ls /tmp", "/tmp") + assert result is not None + assert "allowlist" in result.lower() diff --git a/tests/tools/test_exec_security.py b/tests/tools/test_exec_security.py index 20687dcbf..64dc49563 100644 --- a/tests/tools/test_exec_security.py +++ b/tests/tools/test_exec_security.py @@ -94,7 +94,7 @@ def test_exec_blocks_writes_to_history_jsonl(command): tool = ExecTool() result = tool._guard_command(command, "/tmp") assert result is not None - assert "dangerous pattern" in result.lower() + assert "deny pattern filter" in result.lower() @pytest.mark.parametrize(