Signal and gateway media/attachment delivery features

Fixes on record

Merged to main. A stamp counts the issues a fix is linked to.

• #5602 fix(signal): implement send_image_file, send_voice, and send_video for MEDIA: tag delivery

  merged to main

  files: gateway/platforms/signal.py, tests/gateway/test_signal.py

  Show the diff

  • gateway/platforms/signal.py

    @@ -717,19 +717,27 @@ async def send_image(
                 return SendResult(success=True)
             return SendResult(success=False, error="RPC send with attachment failed")
     
    -    async def send_document(
    +    async def _send_attachment(
             self,
             chat_id: str,
             file_path: str,
    +        media_label: str,
             caption: Optional[str] = None,
    -        filename: Optional[str] = None,
    -        **kwargs,
         ) -> SendResult:
    -        """Send a document/file attachment."""
    +        """Send any file as a Signal attachment via RPC.
    +
    +        Shared implementation for send_document, send_image_file, send_voice,
    +        and send_video — avoids duplicating the validation/routing/RPC logic.
    +        """
             await self._stop_typing_indicator(chat_id)
     
    -        if not Path(file_path).exists():
    -            return SendResult(success=False, error="File not found")
    +        try:
    +            file_size = Path(file_path).stat().st_size
    +        except FileNotFoundError:
    +            return SendResult(success=False, error=f"{media_label} file not found: {file_path}")
    +
    +        if file_size > SIGNAL_MAX_ATTACHMENT_SIZE:
    +            return SendResult(success=False, error=f"{media_label} too large ({file_size} bytes)")
     
             params: Dict[str, Any] = {
                 "account": self.account,
    @@ -746,7 +754,59 @@ async def send_document(
             if result is not None:
                 self._track_sent_timestamp(result)
                 return SendResult(success=True)
    -        return SendResult(success=False, error="RPC send document failed")
    +        return SendResult(success=False, error=f"RPC send {media_label.lower()} failed")
    +
    +    async def send_document(
    +        self,
    +        chat_id: str,
    +        file_path: str,
    +        caption: Optional[str] = None,
    +        filename: Optional[str] = None,
    +        **kwargs,
    +    ) -> SendResult:
    +        """Send a document/file attachment."""
    +        return await self._send_attachment(chat_id, file_path, "File", caption)
    +
    +    async def send_image_file(
    +        self,
    +        chat_id: str,
    +        image_path: str,
    +        caption: Optional[str] = None,
    +        reply_to: Optional[str] = None,
    +        **kwargs,
    +    ) -> SendResult:
    +        """Send a local image file as a native Signal attachment.
    +
    +        Called by the gateway media delivery flow when MEDIA: tags containing
    +        image paths are extracted from agent responses.
    +        """
    +        return await self._send_attachment(chat_id, image_path, "Image", caption)
    +
    +    async def send_voice(
    +        self,
    +        chat_id: str,
    +        audio_path: str,
    +        caption: Optional[str] = None,
    +        reply_to: Optional[str] = None,
    +        **kwargs,
    +    ) -> SendResult:
    +        """Send an audio file as a Signal attachment.
    +
    +        Signal does not distinguish voice messages from file attachments at
    +        the API level, so this routes through the same RPC send path.
    +        """
    +        return await self._send_attachment(chat_id, audio_path, "Audio", caption)
    +
    +    async def send_video(
    +        self,
    +        chat_id: str,
    +        video_path: str,
    +        caption: Optional[str] = None,
    +        reply_to: Optional[str] = None,
    +        **kwargs,
    +    ) -> SendResult:
    +        """Send a video file as a Signal attachment."""
    +        return await self._send_attachment(chat_id, video_path, "Video", caption)
     
         # ------------------------------------------------------------------
         # Typing Indicators

  • tests/gateway/test_signal.py

    @@ -2,6 +2,7 @@
     import base64
     import json
     import pytest
    +from pathlib import Path
     from unittest.mock import MagicMock, patch, AsyncMock
     from urllib.parse import quote
     
    @@ -368,3 +369,341 @@ def test_signal_in_platform_map(self):
             # Just verify the import works and Signal is a valid platform
             from gateway.config import Platform
             assert Platform.SIGNAL.value == "signal"
    +
    +
    +# ---------------------------------------------------------------------------
    +# send_image_file method (#5105)
    +# ---------------------------------------------------------------------------
    +
    +class TestSignalSendImageFile:
    +    @pytest.mark.asyncio
    +    async def test_send_image_file_sends_via_rpc(self, monkeypatch, tmp_path):
    +        """send_image_file should send image as attachment via signal-cli RPC."""
    +        adapter = _make_signal_adapter(monkeypatch)
    +        mock_rpc, captured = _stub_rpc({"timestamp": 1234567890})
    +        adapter._rpc = mock_rpc
    +        adapter._stop_typing_indicator = AsyncMock()
    +
    +        img_path = tmp_path / "chart.png"
    +        img_path.write_bytes(b"\x89PNG" + b"\x00" * 100)
    +
    +        result = await adapter.send_image_file(chat_id="+155****4567", image_path=str(img_path))
    +
    +        assert result.success is True
    +        assert len(captured) == 1
    +        assert captured[0]["method"] == "send"
    +        assert captured[0]["params"]["account"] == adapter.account
    +        assert captured[0]["params"]["recipient"] == ["+155****4567"]
    +        assert captured[0]["params"]["attachments"] == [str(img_path)]
    +        assert captured[0]["params"]["message"] == ""  # caption=None → ""
    +        # Typing indicator must be stopped before sending
    +        adapter._stop_typing_indicator.assert_awaited_once_with("+155****4567")
    +        # Timestamp must be tracked for echo-back prevention
    +        assert 1234567890 in adapter._recent_sent_timestamps
    +
    +    @pytest.mark.asyncio
    +    async def test_send_image_file_to_group(self, monkeypatch, tmp_path):
    +        """send_image_file should route group chats via groupId."""
    +        adapter = _make_signal_adapter(monkeypatch)
    +        mock_rpc, captured = _stub_rpc({"timestamp": 1234567890})
    +        adapter._rpc = mock_rpc
    +        adapter._stop_typing_indicator = AsyncMock()
    +
    +        img_path = tmp_path / "photo.jpg"
    +        img_path.write_bytes(b"\xff\xd8" + b"\x00" * 100)
    +
    +        result = await adapter.send_image_file(
    +            chat_id="group:abc123==", image_path=str(img_path), caption="Here's the chart"
    +        )
    +
    +        assert result.success is True
    +        assert captured[0]["params"]["groupId"] == "abc123=="
    +        assert captured[0]["params"]["message"] == "Here's the chart"
    +
    +    @pytest.mark.asyncio
    +    async def test_send_image_file_missing(self, monkeypatch):
    +        """send_image_file should fail gracefully for nonexistent files."""
    +        adapter = _make_signal_adapter(monkeypatch)
    +        adapter._stop_typing_indicator = AsyncMock()
    +
    +        result = await adapter.send_image_file(chat_id="+155****4567", image_path="/nonexistent.png")
    +
    +        assert result.success is False
    +        assert "not found" in result.error.lower()
    +
    +    @pytest.mark.asyncio
    +    async def test_send_image_file_too_large(self, monkeypatch, tmp_path):
    +        """send_image_file should reject files over 100MB."""
    +        adapter = _make_signal_adapter(monkeypatch)
    +        adapter._stop_typing_indicator = AsyncMock()
    +
    +        img_path = tmp_path / "huge.png"
    +        img_path.write_bytes(b"x")
    +
    +        def mock_stat(self, **kwargs):
    +            class FakeStat:
    +                st_size = 200 * 1024 * 1024  # 200 MB
    +            return FakeStat()
    +
    +        with patch.object(Path, "stat", mock_stat):
    +            result = await adapter.send_image_file(chat_id="+155****4567", image_path=str(img_path))
    +
    +        assert result.success is False
    +        assert "too large" in result.error.lower()
    +
    +    @pytest.mark.asyncio
    +    async def test_send_image_file_rpc_failure(self, monkeypatch, tmp_path):
    +        """send_image_file should return error when RPC returns None."""
    +        adapter = _make_signal_adapter(monkeypatch)
    +        mock_rpc, _ = _stub_rpc(None)
    +        adapter._rpc = mock_rpc
    +        adapter._stop_typing_indicator = AsyncMock()
    +
    +        img_path = tmp_path / "test.png"
    +        img_path.write_bytes(b"\x89PNG" + b"\x00" * 100)
    +
    +        result = await adapter.send_image_file(chat_id="+155****4567", image_path=str(img_path))
    +
    +        assert result.success is False
    +        assert "failed" in result.error.lower()
    +
    +
    +# ---------------------------------------------------------------------------
    +# send_voice method (#5105)
    +# ---------------------------------------------------------------------------
    +
    +class TestSignalSendVoice:
    +    @pytest.mark.asyncio
    +    async def test_send_voice_sends_via_rpc(self, monkeypatch, tmp_path):
    +        """send_voice should send audio as attachment via signal-cli RPC."""
    +        adapter = _make_signal_adapter(monkeypatch)
    +        mock_rpc, captured = _stub_rpc({"timestamp": 1234567890})
    +        adapter._rpc = mock_rpc
    +        adapter._stop_typing_indicator = AsyncMock()
    +
    +        audio_path = tmp_path / "reply.ogg"
    +        audio_path.write_bytes(b"OggS" + b"\x00" * 100)
    +
    +        result = await adapter.send_voice(chat_id="+155****4567", audio_path=str(audio_path))
    +
    +        assert result.success is True
    +        assert captured[0]["method"] == "send"
    +        assert captured[0]["params"]["attachments"] == [str(audio_path)]
    +        assert captured[0]["params"]["message"] == ""  # caption=None → ""
    +        adapter._stop_typing_indicator.assert_awaited_once_with("+155****4567")
    +        assert 1234567890 in adapter._recent_sent_timestamps
    +
    +    @pytest.mark.asyncio
    +    async def test_send_voice_missing_file(self, monkeypatch):
    +        """send_voice should fail for nonexistent audio."""
    +        adapter = _make_signal_adapter(monkeypatch)
    +        adapter._stop_typing_indicator = AsyncMock()
    +
    +        result = await adapter.send_voice(chat_id="+155****4567", audio_path="/missing.ogg")
    +
    +        assert result.success is False
    +        assert "not found" in result.error.lower()
    +
    +    @pytest.mark.asyncio
    +    async def test_send_voice_to_group(self, monkeypatch, tmp_path):
    +        """send_voice should route group chats correctly."""
    +        adapter = _make_signal_adapter(monkeypatch)
    +        mock_rpc, captured = _stub_rpc({"timestamp": 9999})
    +        adapter._rpc = mock_rpc
    +        adapter._stop_typing_indicator = AsyncMock()
    +
    +        audio_path = tmp_path / "note.mp3"
    +        audio_path.write_bytes(b"\xff\xe0" + b"\x00" * 100)
    +
    +        result = await adapter.send_voice(chat_id="group:grp1==", audio_path=str(audio_path))
    +
    +        assert result.success is True
    +        assert captured[0]["params"]["groupId"] == "grp1=="
    +
    +    @pytest.mark.asyncio
    +    async def test_send_voice_too_large(self, monkeypatch, tmp_path):
    +        """send_voice should reject files over 100MB."""
    +        adapter = _make_signal_adapter(monkeypatch)
    +        adapter._stop_typing_indicator = AsyncMock()
    +
    +        audio_path = tmp_path / "huge.ogg"
    +        audio_path.write_bytes(b"x")
    +
    +        def mock_stat(self, **kwargs):
    +            class FakeStat:
    +                st_size = 200 * 1024 * 1024
    +            return FakeStat()
    +
    +        with patch.object(Path, "stat", mock_stat):
    +            result = await adapter.send_voice(chat_id="+155****4567", audio_path=str(audio_path))
    +
    +        assert result.success is False
    +        assert "too large" in result.error.lower()
    +
    +    @pytest.mark.asyncio
    +    async def test_send_voice_rpc_failure(self, monkeypatch, tmp_path):
    +        """send_voice should return error when RPC returns None."""
    +        adapter = _make_signal_adapter(monkeypatch)
    +        mock_rpc, _ = _stub_rpc(None)
    +        adapter._rpc = mock_rpc
    +        adapter._stop_typing_indicator = AsyncMock()
    +
    +        audio_path = tmp_path / "reply.ogg"
    +        audio_path.write_bytes(b"OggS" + b"\x00" * 100)
    +
    +        result = await adapter.send_voice(chat_id="+155****4567", audio_path=str(audio_path))
    +
    +        assert result.success is False
    +        assert "failed" in result.error.lower()
    +
    +
    +# ---------------------------------------------------------------------------
    +# send_video method (#5105)
    +# ---------------------------------------------------------------------------
    +
    +class TestSignalSendVideo:
    +    @pytest.mark.asyncio
    +    async def test_send_video_sends_via_rpc(self, monkeypatch, tmp_path):
    +        """send_video should send video as attachment via signal-cli RPC."""
    +        adapter = _make_signal_adapter(monkeypatch)
    +        mock_rpc, captured = _stub_rpc({"timestamp": 1234567890})
    +        adapter._rpc = mock_rpc
    +        adapter._stop_typing_indicator = AsyncMock()
    +
    +        vid_path = tmp_path / "demo.mp4"
    +        vid_path.write_bytes(b"\x00\x00\x00\x18ftyp" + b"\x00" * 100)
    +
    +        result = await adapter.send_video(chat_id="+155****4567", video_path=str(vid_path))
    +
    +        assert result.success is True
    +        assert captured[0]["method"] == "send"
    +        assert captured[0]["params"]["attachments"] == [str(vid_path)]
    +        assert captured[0]["params"]["message"] == ""  # caption=None → ""
    +        adapter._stop_typing_indicator.assert_awaited_once_with("+155****4567")
    +        assert 1234567890 in adapter._recent_sent_timestamps
    +
    +    @pytest.mark.asyncio
    +    async def test_send_video_missing_file(self, monkeypatch):
    +        """send_video should fail for nonexistent video."""
    +        adapter = _make_signal_adapter(monkeypatch)
    +        adapter._stop_typing_indicator = AsyncMock()
    +
    +        result = await adapter.send_video(chat_id="+155****4567", video_path="/missing.mp4")
    +
    +        assert result.success is False
    +        assert "not found" in result.error.lower()
    +
    +    @pytest.mark.asyncio
    +    async def test_send_video_too_large(self, monkeypatch, tmp_path):
    +        """send_video should reject files over 100MB."""
    +        adapter = _make_signal_adapter(monkeypatch)
    +        adapter._stop_typing_indicator = AsyncMock()
    +
    +        vid_path = tmp_path / "huge.mp4"
    +        vid_path.write_bytes(b"x")
    +
    +        def mock_stat(self, **kwargs):
    +            class FakeStat:
    +                st_size = 200 * 1024 * 1024
    +            return FakeStat()
    +
    +        with patch.object(Path, "stat", mock_stat):
    +            result = await adapter.send_video(chat_id="+155****4567", video_path=str(vid_path))
    +
    +        assert result.success is False
    +        assert "too large" in result.error.lower()
    +
    +    @pytest.mark.asyncio
    +    async def test_send_video_rpc_failure(self, monkeypatch, tmp_path):
    +        """send_video should return error when RPC returns None."""
    +        adapter = _make_signal_adapter(monkeypatch)
    +        mock_rpc, _ = _stub_rpc(None)
    +        adapter._rpc = mock_rpc
    +        adapter._stop_typing_indicator = AsyncMock()
    +
    +        vid_path = tmp_path / "demo.mp4"
    +        vid_path.write_bytes(b"\x00\x00\x00\x18ftyp" + b"\x00" * 100)
    +
    +        result = await adapter.send_video(chat_id="+155****4567", video_path=str(vid_path))
    +
    +        assert result.success is False
    +        assert "failed" in result.error.lower()
    +
    +
    +# ---------------------------------------------------------------------------
    +# MEDIA: tag extraction integration
    +# ---------------------------------------------------------------------------
    +
    +class TestSignalMediaExtraction:
    +    """Verify the full pipeline: MEDIA: tag → extract → send_image_file/send_voice."""
    +
    +    def test_extract_media_finds_image_tag(self):
    +        """BasePlatformAdapter.extract_media should find MEDIA: image paths."""
    +        from gateway.platforms.base import BasePlatformAdapter
    +        media, cleaned = BasePlatformAdapter.extract_media(
    +            "Here's the chart.\nMEDIA:/tmp/price_graph.png"
    +        )
    +        assert len(media) == 1
    +        assert media[0][0] == "/tmp/price_graph.png"
    +        assert "MEDIA:" not in cleaned
    +
    +    def test_extract_media_finds_audio_tag(self):
    +        """BasePlatformAdapter.extract_media should find MEDIA: audio paths."""
    +        from gateway.platforms.base import BasePlatformAdapter
    +        media, cleaned = BasePlatformAdapter.extract_media(
    +            "[[audio_as_voice]]\nMEDIA:/tmp/reply.ogg"
    +        )
    +        assert len(media) == 1
    +        assert media[0][0] == "/tmp/reply.ogg"
    +        assert media[0][1] is True  # is_voice flag
    +
    +    def test_signal_has_all_media_methods(self, monkeypatch):
    +        """SignalAdapter must override all media send methods used by gateway."""
    +        adapter = _make_signal_adapter(monkeypatch)
    +        from gateway.platforms.base import BasePlatformAdapter
    +
    +        # These methods must NOT be the base class defaults (which just send text)
    +        assert type(adapter).send_image_file is not BasePlatformAdapter.send_image_file
    +        assert type(adapter).send_voice is not BasePlatformAdapter.send_voice
    +        assert type(adapter).send_video is not BasePlatformAdapter.send_video
    +        assert type(adapter).send_document is not BasePlatformAdapter.send_document
    +        assert type(adapter).send_image is not BasePlatformAdapter.send_image
    +
    +
    +# ---------------------------------------------------------------------------
    +# send_document now routes through _send_attachment (#5105 bonus)
    +# ---------------------------------------------------------------------------
    +
    +class TestSignalSendDocumentViaHelper:
    +    """Verify send_document gained size check and path-in-error via _send_attachment."""
    +
    +    @pytest.mark.asyncio
    +    async def test_send_document_too_large(self, monkeypatch, tmp_path):
    +        """send_document should now reject files over 100MB (was previously missing)."""
    +        adapter = _make_signal_adapter(monkeypatch)
    +        adapter._stop_typing_indicator = AsyncMock()
    +
    +        doc_path = tmp_path / "huge.pdf"
    +        doc_path.write_bytes(b"x")
    +
    +        def mock_stat(self, **kwargs):
    +            class FakeStat:
    +                st_size = 200 * 1024 * 1024
    +            return FakeStat()
    +
    +        with patch.object(Path, "stat", mock_stat):
    +            result = await adapter.send_document(chat_id="+155****4567", file_path=str(doc_path))
    +
    +        assert result.success is False
    +        assert "too large" in result.error.lower()
    +
    +    @pytest.mark.asyncio
    +    async def test_send_document_error_includes_path(self, monkeypatch):
    +        """send_document error message should include the file path."""
    +        adapter = _make_signal_adapter(monkeypatch)
    +        adapter._stop_typing_indicator = AsyncMock()
    +
    +        result = await adapter.send_document(chat_id="+155****4567", file_path="/nonexistent.pdf")
    +
    +        assert result.success is False
    +        assert "/nonexistent.pdf" in result.error