From f34462c076095751b089e74065e5eb0fec52a54c Mon Sep 17 00:00:00 2001 From: Hua Date: Fri, 20 Mar 2026 11:33:47 +0800 Subject: [PATCH] fix(qq): allow file_data uploads without media url --- AGENTS.md | 2 +- README.md | 22 ++++----- nanobot/channels/qq.py | 79 ++++++++++++++++++++------------ nanobot/utils/delivery.py | 9 ++-- tests/test_qq_channel.py | 96 ++++++++++++++++++++++++++++++++++++--- 5 files changed, 154 insertions(+), 54 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index fd92be6..f96cdad 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -35,7 +35,7 @@ Do not commit real API keys, tokens, chat logs, or workspace data. Keep local se - Agent runtime config should be hot-reloaded from the active `config.json` for safe in-process fields such as `tools.mcpServers`, `tools.web.*`, `tools.exec.*`, `tools.restrictToWorkspace`, `agents.defaults.model`, `agents.defaults.maxToolIterations`, `agents.defaults.contextWindowTokens`, `agents.defaults.maxTokens`, `agents.defaults.temperature`, `agents.defaults.reasoningEffort`, `channels.sendProgress`, and `channels.sendToolHints`. Channel connection settings and provider credentials still require a restart. - nanobot does not expose local files over HTTP. If a feature needs a public URL for local files, provide your own static file server and point config such as `mediaBaseUrl` at it. - Generated screenshots, downloads, and other temporary user-delivery artifacts should be written under `workspace/out`, not the workspace root. Treat that as the generic delivery-artifact root for tools, MCP servers, and skills. -- QQ outbound media sends remote `http(s)` image URLs directly. For local QQ images, prefer the documented rich-media `file_data` upload path together with the public `url`, and keep the URL-only flow as a fallback for SDK/runtime compatibility. QQ consumes delivery artifacts produced elsewhere; `mediaBaseUrl` must expose those generated files through your own static file server. +- QQ outbound media sends remote `http(s)` image URLs directly. For local QQ images, try `file_data` upload first. If `mediaBaseUrl` is configured, keep the URL-based path available as a fallback for SDK/runtime compatibility; without it, there is no URL fallback. - `/skill` shells out to `npx clawhub@latest`; it requires Node.js/`npx` at runtime. - `/skill uninstall` runs in a non-interactive context, so keep passing `--yes` when shelling out to ClawHub. - Treat empty `/skill search` output as a user-visible "no results" case rather than a silent success. Surface npm/registry failures directly to the user. diff --git a/README.md b/README.md index ad35d5e..fb5e778 100644 --- a/README.md +++ b/README.md @@ -706,11 +706,10 @@ Uses **botpy SDK** with WebSocket — no public IP required. Currently supports } ``` -`mediaBaseUrl` is optional, but it is required if you want nanobot to send local screenshots or -other local image files through QQ. nanobot does not serve local files over HTTP, so -`mediaBaseUrl` must point to your own static file server. Generated delivery artifacts should be -written under `workspace/out`, and `mediaBaseUrl` should expose that directory with matching -relative paths. +`mediaBaseUrl` is optional. For local QQ images, nanobot will first try direct `file_data` upload +from generated delivery artifacts under `workspace/out`. Configuring `mediaBaseUrl` is still +recommended, because nanobot can then map those files onto your own static file server and fall +back to the URL-based rich-media flow when needed. Multi-bot example: @@ -747,14 +746,11 @@ nanobot gateway Now send a message to the bot from QQ — it should respond! Outbound QQ media sends remote `http(s)` images through the QQ rich-media `url` flow directly. -For local image files, nanobot first publishes or maps the file to a public URL, then tries the -documented `file_data` upload path together with that URL; if the installed QQ SDK/runtime path -does not accept that upload, nanobot falls back to the existing URL-only rich-media flow. -nanobot does not serve local files itself, so `mediaBaseUrl` must point to your own HTTP server -that exposes generated delivery artifacts. Tools and skills should write deliverable files under -`workspace/out`; QQ maps local image paths from that directory onto `mediaBaseUrl` using the same -relative path. Files outside `workspace/out` are rejected. Without that publishing config, local -files still fall back to a text notice. +For local image files, nanobot always tries `file_data` upload first. When `mediaBaseUrl` is +configured, nanobot also maps the same local file onto that public URL and can fall back to the +existing URL-only rich-media flow if direct upload fails. Without `mediaBaseUrl`, nanobot still +attempts direct upload, but there is no URL fallback path. Tools and skills should write +deliverable files under `workspace/out`; QQ accepts only local image files from that directory. When an agent uses shell/browser tools to create screenshots or other temporary files for delivery, it should write them under `workspace/out` instead of the workspace root so channel publishing rules diff --git a/nanobot/channels/qq.py b/nanobot/channels/qq.py index 7fa5a0c..29c42c7 100644 --- a/nanobot/channels/qq.py +++ b/nanobot/channels/qq.py @@ -36,11 +36,12 @@ if TYPE_CHECKING: def _make_bot_class(channel: "QQChannel") -> "type[botpy.Client]": """Create a botpy Client subclass bound to the given channel.""" intents = botpy.Intents(public_messages=True, direct_message=True) + http_timeout_seconds = 20 class _Bot(botpy.Client): def __init__(self): # Disable botpy's file log — nanobot uses loguru; default "botpy.log" fails on read-only fs - super().__init__(intents=intents, ext_handlers=False) + super().__init__(intents=intents, timeout=http_timeout_seconds, ext_handlers=False) async def on_ready(self): logger.info("QQ bot ready: {}", self.robot.name) @@ -96,16 +97,17 @@ class QQChannel(BaseChannel): """Return the active workspace root used by QQ publishing.""" return (self._workspace or Path.cwd()).resolve(strict=False) - async def _publish_local_media(self, media_path: str) -> tuple[str | None, str | None]: - """Map a local delivery artifact to its served URL.""" - _, media_url, error = resolve_delivery_media( + async def _publish_local_media( + self, + media_path: str, + ) -> tuple[Path | None, str | None, str | None]: + """Resolve a local delivery artifact and optionally map it to its served URL.""" + local_path, media_url, error = resolve_delivery_media( media_path, self._workspace_root(), self.config.media_base_url, ) - if error: - return None, error - return media_url, None + return local_path, media_url, error def _next_msg_seq(self) -> int: """Return the next QQ message sequence number.""" @@ -174,21 +176,22 @@ class QQChannel(BaseChannel): self, chat_id: str, msg_type: str, - media_url: str, + media_url: str | None, local_path: Path, content: str | None, msg_id: str | None, ) -> None: - """Upload a local QQ image using the documented file_data field, then send it.""" + """Upload a local QQ image using file_data and, when available, a public URL.""" if not self._client or Route is None: raise RuntimeError("QQ client not initialized") payload = { "file_type": 1, - "url": media_url, "file_data": self._encode_file_data(local_path), "srv_send_msg": False, } + if media_url: + payload["url"] = media_url if msg_type == "group": route = Route("POST", "/v2/groups/{group_openid}/files", group_openid=chat_id) media = await self._client.api._http.request(route, json=payload) @@ -265,9 +268,10 @@ class QQChannel(BaseChannel): resolved_media = media_path local_media_path: Path | None = None if not self._is_remote_media(media_path): - local_media_path = Path(media_path).expanduser() - resolved_media, publish_error = await self._publish_local_media(media_path) - if not resolved_media: + local_media_path, resolved_media, publish_error = await self._publish_local_media( + media_path + ) + if local_media_path is None: logger.warning( "QQ outbound local media could not be published: {} ({})", media_path, @@ -278,11 +282,12 @@ class QQChannel(BaseChannel): ) continue - ok, error = validate_url_target(resolved_media) - if not ok: - logger.warning("QQ outbound media blocked by URL validation: {}", error) - fallback_lines.append(self._failed_media_notice(media_path, error)) - continue + if resolved_media: + ok, error = validate_url_target(resolved_media) + if not ok: + logger.warning("QQ outbound media blocked by URL validation: {}", error) + fallback_lines.append(self._failed_media_notice(media_path, error)) + continue try: if local_media_path is not None: @@ -296,18 +301,32 @@ class QQChannel(BaseChannel): msg_id, ) except Exception as local_upload_error: - logger.warning( - "QQ local file_data upload failed for {}: {}, falling back to URL-only upload", - local_media_path, - local_upload_error, - ) - await self._post_remote_media_message( - msg.chat_id, - msg_type, - resolved_media, - msg.content if msg.content and not content_sent else None, - msg_id, - ) + if resolved_media: + logger.warning( + "QQ local file_data upload failed for {}: {}, falling back to URL-only upload", + local_media_path, + local_upload_error, + ) + await self._post_remote_media_message( + msg.chat_id, + msg_type, + resolved_media, + msg.content if msg.content and not content_sent else None, + msg_id, + ) + else: + logger.warning( + "QQ local file_data upload failed for {} without mediaBaseUrl fallback: {}", + local_media_path, + local_upload_error, + ) + fallback_lines.append( + self._failed_media_notice( + media_path, + "QQ local file_data upload failed", + ) + ) + continue else: await self._post_remote_media_message( msg.chat_id, diff --git a/nanobot/utils/delivery.py b/nanobot/utils/delivery.py index 025903e..5749806 100644 --- a/nanobot/utils/delivery.py +++ b/nanobot/utils/delivery.py @@ -28,11 +28,9 @@ def is_image_file(path: Path) -> bool: def resolve_delivery_media( media_path: str | Path, workspace: Path, - media_base_url: str, + media_base_url: str = "", ) -> tuple[Path | None, str | None, str | None]: - """Resolve a local delivery artifact to a public URL under media_base_url.""" - if not media_base_url: - return None, None, "local media publishing is not configured" + """Resolve a local delivery artifact and optionally map it to a public URL.""" source = Path(media_path).expanduser() try: @@ -55,6 +53,9 @@ def resolve_delivery_media( if not is_image_file(resolved): return None, None, "local delivery media must be an image" + if not media_base_url: + return resolved, None, None + media_url = urljoin( f"{media_base_url.rstrip('/')}/", quote(relative_path.as_posix(), safe="/"), diff --git a/tests/test_qq_channel.py b/tests/test_qq_channel.py index 476355d..8da68c1 100644 --- a/tests/test_qq_channel.py +++ b/tests/test_qq_channel.py @@ -5,7 +5,7 @@ import pytest from nanobot.bus.events import OutboundMessage from nanobot.bus.queue import MessageBus -from nanobot.channels.qq import QQChannel +from nanobot.channels.qq import QQChannel, _make_bot_class from nanobot.config.schema import QQConfig @@ -54,6 +54,23 @@ class _FakeClient: self.api = _FakeApi() +def test_make_bot_class_uses_longer_http_timeout(monkeypatch) -> None: + if not hasattr(__import__("nanobot.channels.qq", fromlist=["botpy"]).botpy, "Client"): + pytest.skip("botpy not installed") + + captured: dict[str, object] = {} + + def fake_init(self, *args, **kwargs) -> None: # noqa: ARG001 + captured["kwargs"] = kwargs + + monkeypatch.setattr("nanobot.channels.qq.botpy.Client.__init__", fake_init) + bot_cls = _make_bot_class(SimpleNamespace(_on_message=None)) + bot_cls() + + assert captured["kwargs"]["timeout"] == 20 + assert captured["kwargs"]["ext_handlers"] is False + + @pytest.mark.asyncio async def test_on_group_message_routes_to_group_chat_id() -> None: channel = QQChannel(QQConfig(app_id="app", secret="secret", allow_from=["user1"]), MessageBus()) @@ -164,8 +181,21 @@ async def test_send_group_remote_media_url_uses_file_api_then_media_message(monk @pytest.mark.asyncio -async def test_send_local_media_falls_back_to_text_notice_when_publishing_not_configured() -> None: - channel = QQChannel(QQConfig(app_id="app", secret="secret", allow_from=["*"]), MessageBus()) +async def test_send_local_media_without_media_base_url_uses_file_data_only( + tmp_path, +) -> None: + workspace = tmp_path / "workspace" + workspace.mkdir() + out_dir = workspace / "out" + out_dir.mkdir() + source = out_dir / "demo.png" + source.write_bytes(b"\x89PNG\r\n\x1a\nfake-png") + + channel = QQChannel( + QQConfig(app_id="app", secret="secret", allow_from=["*"]), + MessageBus(), + workspace=workspace, + ) channel._client = _FakeClient() await channel.send( @@ -173,18 +203,31 @@ async def test_send_local_media_falls_back_to_text_notice_when_publishing_not_co channel="qq", chat_id="user123", content="hello", - media=["/tmp/demo.png"], + media=[str(source)], metadata={"message_id": "msg1"}, ) ) assert channel._client.api.c2c_file_calls == [] assert channel._client.api.group_file_calls == [] + assert channel._client.api.raw_file_upload_calls == [ + { + "method": "POST", + "path": "/v2/users/{openid}/files", + "params": {"openid": "user123"}, + "json": { + "file_type": 1, + "file_data": b64encode(b"\x89PNG\r\n\x1a\nfake-png").decode("ascii"), + "srv_send_msg": False, + }, + } + ] assert channel._client.api.c2c_calls == [ { "openid": "user123", - "msg_type": 0, - "content": "hello\n[Failed to send: demo.png - local media publishing is not configured]", + "msg_type": 7, + "content": "hello", + "media": {"file_info": "c2c-file-info", "file_uuid": "c2c-file", "ttl": 60}, "msg_id": "msg1", "msg_seq": 2, } @@ -420,6 +463,47 @@ async def test_send_local_media_falls_back_to_url_only_upload_when_file_data_upl ] +@pytest.mark.asyncio +async def test_send_local_media_without_media_base_url_falls_back_to_text_notice_when_file_data_upload_fails( + tmp_path, +) -> None: + workspace = tmp_path / "workspace" + workspace.mkdir() + out_dir = workspace / "out" + out_dir.mkdir() + source = out_dir / "demo.png" + source.write_bytes(b"\x89PNG\r\n\x1a\nfake-png") + + channel = QQChannel( + QQConfig(app_id="app", secret="secret", allow_from=["*"]), + MessageBus(), + workspace=workspace, + ) + channel._client = _FakeClient() + channel._client.api.raise_on_raw_file_upload = True + + await channel.send( + OutboundMessage( + channel="qq", + chat_id="user123", + content="hello", + media=[str(source)], + metadata={"message_id": "msg1"}, + ) + ) + + assert channel._client.api.c2c_file_calls == [] + assert channel._client.api.c2c_calls == [ + { + "openid": "user123", + "msg_type": 0, + "content": "hello\n[Failed to send: demo.png - QQ local file_data upload failed]", + "msg_id": "msg1", + "msg_seq": 2, + } + ] + + @pytest.mark.asyncio async def test_send_local_media_symlink_to_outside_out_dir_is_rejected( monkeypatch,