Prechádzať zdrojové kódy

Send files, video and audio natively — and fix send_image_file's signature

send_document/send_video/send_voice fell through to the base class, which
apologises in plain text ('Couldn't deliver the file attachment'). The
chunked upload machinery was already here, just wired to images only, so
these are thin wrappers around a shared _send_local_attachment().

While covering them: send_image_file took file_path, but every caller passes
image_path as a KEYWORD — gateway/run.py:22354 and :22470, plus the file://
branch of the base class's own send_multiple_images (base.py:4429). Each of
those raised TypeError and degraded silently to a text notice, so native
image delivery has been broken the whole time. The 'Do not change signature'
comment sat directly above the wrong signature.

validate_media_delivery_path stays the gate before every upload, and no
failure path echoes the host path into chat.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Paul-Dieter Klumpp 1 týždeň pred
rodič
commit
5729c20850
2 zmenil súbory, kde vykonal 187 pridanie a 28 odobranie
  1. 134 28
      adapter.py
  2. 53 0
      test_adapter.py

+ 134 - 28
adapter.py

@@ -1417,35 +1417,15 @@ class ChattoAdapter(BasePlatformAdapter):
             logger.error("Chatto: upload error: %s", e)
             return None
 
-    async def send_image_file(
+    async def _post_attachment_message(
         self,
         chat_id: str,
-        file_path: str,
-        caption: Optional[str] = None,
-        reply_to: Optional[str] = None,
-        metadata: Optional[Dict[str, Any]] = None,
+        asset_ids: List[str],
+        caption: Optional[str],
+        reply_to: Optional[str],
+        metadata: Optional[Dict[str, Any]],
     ) -> SendResult:
-        """Send a local image file via the chunked upload API. Do not change signature.
-
-        BasePlatformAdapter override
-        """
-        # Validate the path is safe
-        safe_path = self.validate_media_delivery_path(file_path)
-        if not safe_path:
-            logger.warning("Chatto: send_image_file — unsafe path %s", file_path)
-            text = "⚠️ Couldn't deliver the image attachment."
-            if caption:
-                text = f"{caption}\n{text}"
-            return await self.send(chat_id, text, reply_to=reply_to, metadata=metadata)
-
-        asset_id = await self._upload_asset(str(chat_id), safe_path)
-        if not asset_id:
-            # Fallback to a notice
-            text = "⚠️ Couldn't deliver the image attachment."
-            if caption:
-                text = f"{caption}\n{text}"
-            return await self.send(chat_id, text, reply_to=reply_to, metadata=metadata)
-
+        """Post one message carrying already-uploaded assets."""
         thread_id = (metadata or {}).get("thread_id")
         if reply_to:
             thread_id = reply_to
@@ -1457,17 +1437,143 @@ class ChattoAdapter(BasePlatformAdapter):
                 return SendResult(success=False, error="Chatto client not available", retryable=True)
             msg = await client.post_message(
                 room_id=str(chat_id),
-                body=caption or "",
-                attachment_asset_ids=[asset_id],
+                body=self.format_message(caption) if caption else "",
+                attachment_asset_ids=asset_ids,
                 thread_root_event_id=str(thread_id) if thread_id else "",
             )
             self._mark_seen(msg.id)
+            self._our_message_ids.add(msg.id)
             return SendResult(success=True, message_id=msg.id, raw_response=msg)
         except ChattoError as e:
             return SendResult(success=False, error=str(e), retryable=True)
         except Exception as e:
             return SendResult(success=False, error=str(e), retryable=False)
 
+    async def _send_local_attachment(
+        self,
+        chat_id: str,
+        file_path: str,
+        caption: Optional[str],
+        reply_to: Optional[str],
+        metadata: Optional[Dict[str, Any]],
+        *,
+        kind: str,
+    ) -> SendResult:
+        """Upload a local file and post it as a native Chatto attachment.
+
+        Shared by ``send_image_file``/``send_document``/``send_video``/
+        ``send_voice`` — the upload mechanics are identical, only the wording of
+        the failure notice differs.  On failure we send that notice as text and
+        never the host path (it leaks the Hermes home layout).
+        """
+        notice = f"⚠️ Couldn't deliver the {kind} attachment."
+
+        safe_path = self.validate_media_delivery_path(file_path)
+        if not safe_path:
+            logger.warning(
+                "[%s] send %s: unsafe path %s", self.name, kind, file_path,
+            )
+            text = f"{caption}\n{notice}" if caption else notice
+            return await self.send(chat_id, text, reply_to=reply_to, metadata=metadata)
+
+        asset_id = await self._upload_asset(str(chat_id), safe_path)
+        if not asset_id:
+            logger.warning(
+                "[%s] send %s: upload failed for %s", self.name, kind, safe_path,
+            )
+            text = f"{caption}\n{notice}" if caption else notice
+            return await self.send(chat_id, text, reply_to=reply_to, metadata=metadata)
+
+        return await self._post_attachment_message(
+            chat_id, [asset_id], caption, reply_to, metadata,
+        )
+
+    async def send_image_file(
+        self,
+        chat_id: str,
+        image_path: str,
+        caption: Optional[str] = None,
+        reply_to: Optional[str] = None,
+        metadata: Optional[Dict[str, Any]] = None,
+        **kwargs,
+    ) -> SendResult:
+        """Send a local image file via the chunked upload API.
+
+        The parameter is ``image_path``, not ``file_path``: every caller passes
+        it by keyword (``gateway/run.py:22354``, ``:22470``, and the base class's
+        own ``send_multiple_images`` file:// branch), so a renamed parameter
+        makes each of those raise TypeError and silently degrade to a text
+        notice.
+
+        BasePlatformAdapter override
+        """
+        return await self._send_local_attachment(
+            chat_id, image_path, caption, reply_to, metadata, kind="image",
+        )
+
+    async def send_document(
+        self,
+        chat_id: str,
+        file_path: str,
+        caption: Optional[str] = None,
+        file_name: Optional[str] = None,
+        reply_to: Optional[str] = None,
+        metadata: Optional[Dict[str, Any]] = None,
+        **kwargs,
+    ) -> SendResult:
+        """Send a local file as a native Chatto attachment.
+
+        ``file_name`` is the user-facing name the agent chose; Chatto takes the
+        filename from the upload session, so it only matters for the failure
+        notice.
+
+        BasePlatformAdapter override
+        """
+        result = await self._send_local_attachment(
+            chat_id, file_path, caption, reply_to, metadata, kind="file",
+        )
+        if not result.success and file_name:
+            logger.debug("Chatto: document delivery failed for %s", file_name)
+        return result
+
+    async def send_video(
+        self,
+        chat_id: str,
+        video_path: str,
+        caption: Optional[str] = None,
+        reply_to: Optional[str] = None,
+        metadata: Optional[Dict[str, Any]] = None,
+        **kwargs,
+    ) -> SendResult:
+        """Send a local video as a native Chatto attachment (Chatto transcodes
+        and plays it inline).
+
+        BasePlatformAdapter override
+        """
+        return await self._send_local_attachment(
+            chat_id, video_path, caption, reply_to, metadata, kind="video",
+        )
+
+    async def send_voice(
+        self,
+        chat_id: str,
+        audio_path: str,
+        caption: Optional[str] = None,
+        reply_to: Optional[str] = None,
+        metadata: Optional[Dict[str, Any]] = None,
+        **kwargs,
+    ) -> SendResult:
+        """Send a local audio file as a native Chatto attachment.
+
+        Chatto has no dedicated voice-bubble type, so this is an ordinary audio
+        attachment — still far better than the base class's text notice.
+
+        BasePlatformAdapter override
+        """
+        return await self._send_local_attachment(
+            chat_id, audio_path, caption, reply_to, metadata, kind="audio",
+        )
+
 
     async def send_image(
         self,

+ 53 - 0
test_adapter.py

@@ -489,6 +489,59 @@ class TestHandoffThread:
         assert await adapter.create_handoff_thread("room-1", "x") is None
 
 
+# -- Native file / video / audio delivery --
+
+class TestNativeSends:
+    """send_document/_video/_voice upload instead of apologising in text."""
+
+    @pytest_asyncio.fixture
+    def adapter(self):
+        adapter = _make_adapter()
+        adapter._chatto_client.post_message = AsyncMock()
+        adapter._upload_asset = AsyncMock(return_value="asset-1")
+        adapter.validate_media_delivery_path = MagicMock(side_effect=lambda p: p)
+        mock_msg = MagicMock()
+        mock_msg.id = "msg-1"
+        adapter._chatto_client.post_message.return_value = mock_msg
+        return adapter
+
+    @pytest.mark.parametrize(
+        "method,arg_name",
+        [
+            ("send_document", "file_path"),
+            ("send_video", "video_path"),
+            ("send_voice", "audio_path"),
+            ("send_image_file", "image_path"),
+        ],
+    )
+    async def test_uploads_and_attaches(self, adapter, method, arg_name):
+        result = await getattr(adapter, method)(
+            "room-1", **{arg_name: "/tmp/thing.bin"}, caption="here you go",
+        )
+        assert result.success is True
+        adapter._upload_asset.assert_called_once_with("room-1", "/tmp/thing.bin")
+        call_kwargs = adapter._chatto_client.post_message.call_args.kwargs
+        assert call_kwargs["attachment_asset_ids"] == ["asset-1"]
+        assert call_kwargs["body"] == "here you go"
+
+    async def test_unsafe_path_falls_back_to_notice(self, adapter):
+        adapter.validate_media_delivery_path = MagicMock(return_value=None)
+        adapter.send = AsyncMock(return_value=SendResult(success=True))
+        await adapter.send_document("room-1", "/etc/shadow")
+        adapter._upload_asset.assert_not_called()
+        # Never echo the host path into chat.
+        sent_text = adapter.send.call_args.args[1]
+        assert "/etc/shadow" not in sent_text
+
+    async def test_upload_failure_falls_back_to_notice(self, adapter):
+        adapter._upload_asset = AsyncMock(return_value=None)
+        adapter.send = AsyncMock(return_value=SendResult(success=True))
+        await adapter.send_video("room-1", "/tmp/clip.mp4", caption="a clip")
+        sent_text = adapter.send.call_args.args[1]
+        assert sent_text.startswith("a clip\n")
+        assert "/tmp/clip.mp4" not in sent_text
+
+
 # -- Reaction event forwarding --
 
 class TestReactionForwarding: