Переглянути джерело

fix(chatto): gate pinning as a write and report the attachments page

Three corrections to the tools added in the preceding commits:

- chatto_pin_message now refuses an unlisted channel and an unjoined room,
  exactly like chatto_send_room. Pinning changes what the room shows
  everyone, so a room the plugin treats as read-only must not be pinned
  into; without this the tool was a way around the read-only promise.
- chatto_list_room_attachments ignored the Page that chattolib returns and
  hard-coded has_more=false, so a paged answer claimed to be complete. It
  now reports has_more and total_count.
- Both pin handlers read typed chattolib values directly instead of
  reaching through hasattr (PinnedMessage.message is a declared field),
  and offset is validated by _int_arg so a bad value is a refusal rather
  than an exception out of the handler.
Paul Klumpp 1 тиждень тому
батько
коміт
beb8c22192
2 змінених файлів з 272 додано та 50 видалено
  1. 45 50
      adapter.py
  2. 227 0
      test_adapter.py

+ 45 - 50
adapter.py

@@ -5444,11 +5444,7 @@ async def _list_room_attachments(args: dict) -> str:
     limit = _int_arg(
         args, "limit", ATTACHMENT_PAGE_SIZE, low=1, high=ATTACHMENT_PAGE_SIZE * 5
     )
-    offset = args.get("offset")
-    if offset is not None:
-        offset = int(offset)
-    if offset is not None and offset < 0:
-        raise _refuse("offset must be non-negative")
+    offset = _int_arg(args, "offset", 0, low=0, high=100_000)
 
     if not room_id and not room_name:
         raise _refuse("Exactly one of room_id or room_name is required")
@@ -5457,7 +5453,7 @@ async def _list_room_attachments(args: dict) -> str:
         resolved = await _resolve_room(client, room_id, room_name)
         room = resolved.room
 
-        attachments, _ = await client.list_room_attachments(
+        attachments, page = await client.list_room_attachments(
             room.id, limit=limit, offset=offset
         )
         return json.dumps(
@@ -5466,8 +5462,9 @@ async def _list_room_attachments(args: dict) -> str:
                 "room_id": room.id,
                 "room_name": room.name,
                 "count": len(attachments),
-                "offset": offset or 0,
-                "has_more": False,  # chattolib returns full list without Page here
+                "offset": offset,
+                "has_more": page.has_more,
+                "total_count": page.total_count,
                 "attachments": attachments,
             }
         )
@@ -5478,11 +5475,7 @@ async def _list_pinned_messages(args: dict) -> str:
     room_id = args.get("room_id") or ""
     room_name = args.get("room_name") or ""
     limit = _int_arg(args, "limit", PINNED_PAGE_SIZE, low=1, high=PINNED_PAGE_SIZE * 5)
-    offset = args.get("offset")
-    if offset is not None:
-        offset = int(offset)
-    if offset is not None and offset < 0:
-        raise _refuse("offset must be non-negative")
+    offset = _int_arg(args, "offset", 0, low=0, high=100_000)
 
     if not room_id and not room_name:
         raise _refuse("Exactly one of room_id or room_name is required")
@@ -5492,33 +5485,29 @@ async def _list_pinned_messages(args: dict) -> str:
         room = resolved.room
 
         page = await client.list_pinned_messages(room.id, limit=limit, offset=offset)
-        author_logins: dict[str, str] = {}
-        results: list[dict] = []
-        for pm in page.pinned_messages:
-            msg = pm.message
-            if msg is None:
-                continue
-            author_id = msg.actor_id
-            if author_id and author_id not in author_logins:
-                logins = await _batch_author_logins(client, room.id, [author_id])
-                author_logins.update(logins)
-            results.append(
-                {
-                    "event_id": msg.id,
-                    "body": msg.body,
-                    "author_id": author_id,
-                    "author_login": author_logins.get(author_id) or "",
-                    "pinned_at": _iso_or_none(pm.message.created_at if hasattr(pm, 'message') and pm.message else None),
-                }
-            )
+        messages = [pm.message for pm in page.pinned_messages if pm.message is not None]
+        author_logins = await _batch_author_logins(
+            client, room.id, [msg.actor_id for msg in messages if msg.actor_id]
+        )
+        pinned_out = [
+            {
+                "event_id": msg.id,
+                "body": msg.body,
+                "author_id": msg.actor_id,
+                "author_login": author_logins.get(msg.actor_id) or "",
+                "posted_at": _iso_or_none(msg.created_at),
+            }
+            for msg in messages
+        ]
         return json.dumps(
             {
                 "success": True,
                 "room_id": room.id,
                 "room_name": room.name,
-                "count": len(results),
-                "has_older": page.page.has_more,
-                "pinned_messages": results,
+                "count": len(pinned_out),
+                "has_more": page.page.has_more,
+                "total_count": page.page.total_count,
+                "pinned_messages": pinned_out,
                 "latest_pin_marker": page.latest_pin_marker,
             }
         )
@@ -5538,33 +5527,39 @@ async def _pin_message(args: dict) -> str:
     if pin is None:
         raise _refuse("pin (true/false) is required")
 
-    async with _tool_client(_chatto_config_for_tools()) as client:
+    cfg = _chatto_config_for_tools()
+    async with _tool_client(cfg) as client:
         resolved = await _resolve_room(client, room_id, room_name)
         room = resolved.room
 
+        # Pinning changes what the room shows everyone, so it is a write:
+        # a room on neither mention list stays read-only, exactly as it does
+        # for chatto_send_room.
+        if not _room_is_writable(cfg, room):
+            raise _read_only_room_refusal(room.name or room.id)
+        if not resolved.is_member:
+            raise _not_joined_refusal(room)
+
         if pin:
-            pinned = await client.pin_message(room.id, event_id)
-            message = pinned.message if hasattr(pinned, 'message') else None
+            await client.pin_message(room.id, event_id)
             return json.dumps(
                 {
                     "success": True,
                     "action": "pinned",
                     "room_id": room.id,
                     "event_id": event_id,
-                    "pinned_at": _iso_or_none(message.created_at if message else None),
-                }
-            )
-        else:
-            success = await client.unpin_message(room.id, event_id)
-            return json.dumps(
-                {
-                    "success": True,
-                    "action": "unpinned",
-                    "room_id": room.id,
-                    "event_id": event_id,
-                    "unpinned": success,
                 }
             )
+        unpinned = await client.unpin_message(room.id, event_id)
+        return json.dumps(
+            {
+                "success": True,
+                "action": "unpinned",
+                "room_id": room.id,
+                "event_id": event_id,
+                "unpinned": unpinned,
+            }
+        )
 
 
 def _iso_or_none(moment: datetime | None) -> str | None:

+ 227 - 0
test_adapter.py

@@ -53,8 +53,11 @@ from chattolib.types import (
     MessageAttachment,
     MessageReaction,
     Page,
+    PinnedMessage,
+    PinnedMessagesPage,
     PresenceStatus,
     Room,
+    RoomGroup,
     RoomKind,
     RoomViewerState,
     RoomWithViewerState,
@@ -87,11 +90,19 @@ from adapter import (
     RoomPolicy,
     _capabilities,
     _chatto_config_for_tools,
+    _handle_chatto_delete_custom_status,
+    _handle_chatto_fetch_link_preview,
     _handle_chatto_get_history,
     _handle_chatto_get_message,
+    _handle_chatto_list_pinned_messages,
+    _handle_chatto_list_room_attachments,
+    _handle_chatto_list_room_groups,
     _handle_chatto_list_rooms,
+    _handle_chatto_mark_thread_as_read,
+    _handle_chatto_pin_message,
     _handle_chatto_send_dm,
     _handle_chatto_send_room,
+    _handle_chatto_set_custom_status,
     _Mentions,
     _room_answers_in,
     chat_type_for_room_kind,
@@ -4509,6 +4520,15 @@ class TestPluginToolHandlers:
         client.batch_get_room_members = AsyncMock(return_value=[])
         client.follow_thread = AsyncMock()
         client.start_dm = AsyncMock()
+        client.list_room_attachments = AsyncMock(return_value=([], Page()))
+        client.list_pinned_messages = AsyncMock(return_value=PinnedMessagesPage())
+        client.pin_message = AsyncMock()
+        client.unpin_message = AsyncMock(return_value=True)
+        client.set_custom_status = AsyncMock(return_value={})
+        client.delete_custom_status = AsyncMock(return_value={})
+        client.fetch_link_preview = AsyncMock(return_value=(None, ""))
+        client.mark_thread_as_read = AsyncMock(return_value=None)
+        client.list_room_groups = AsyncMock(return_value=[])
         client.post_message = AsyncMock(
             side_effect=lambda *a, **kw: _make_message(
                 message_id=f"msg-{client.post_message.await_count}"
@@ -5622,6 +5642,213 @@ class TestPluginToolHandlers:
         assert result["success"] is True
         client.post_message.assert_awaited()
 
+    # -- chatto_list_room_attachments --
+
+    async def test_list_room_attachments_reports_the_server_page(self):
+        """The attachment list is pageable server-side, so the answer has to
+        carry the page it came from — ``has_more`` is what tells the agent to
+        ask again with a larger offset."""
+        room = self._readable_room()
+        client = self._stub_client()
+        client.get_room = AsyncMock(return_value=_make_room_state(room))
+        client.list_room_attachments = AsyncMock(
+            return_value=(
+                [{"id": "asset-1", "filename": "a.png", "contentType": "image/png"}],
+                Page(total_count=10, has_more=True),
+            )
+        )
+        result, _, _ = await self._run(
+            _handle_chatto_list_room_attachments,
+            {"room_id": "room-1"},
+            client=client,
+        )
+        assert result["success"] is True
+        assert result["count"] == 1
+        assert result["has_more"] is True
+        assert result["total_count"] == 10
+        assert result["attachments"][0]["filename"] == "a.png"
+
+    # -- chatto_list_pinned_messages --
+
+    async def test_list_pinned_messages_hydrates_authors_in_one_lookup(self):
+        """Pinned messages carry only actor IDs. The directory is asked once
+        for the whole page, not once per pinned message."""
+        room = self._readable_room()
+        client = self._stub_client()
+        client.get_room = AsyncMock(return_value=_make_room_state(room))
+        client.list_pinned_messages = AsyncMock(
+            return_value=PinnedMessagesPage(
+                pinned_messages=[
+                    PinnedMessage(message=_make_message(message_id="msg-1", actor_id="user-1")),
+                    PinnedMessage(message=_make_message(message_id="msg-2", actor_id="user-2")),
+                ],
+                page=Page(total_count=2, has_more=False),
+            )
+        )
+        client.batch_get_room_members = AsyncMock(
+            return_value=[
+                DirectoryMember(user=_make_user("user-1", "alice")),
+                DirectoryMember(user=_make_user("user-2", "bob")),
+            ]
+        )
+        result, _, _ = await self._run(
+            _handle_chatto_list_pinned_messages,
+            {"room_id": "room-1"},
+            client=client,
+        )
+        assert [p["event_id"] for p in result["pinned_messages"]] == ["msg-1", "msg-2"]
+        assert [p["author_login"] for p in result["pinned_messages"]] == ["alice", "bob"]
+        assert client.batch_get_room_members.await_count == 1
+
+    # -- chatto_pin_message --
+
+    async def test_pin_message_refuses_a_read_only_room(self):
+        """Pinning changes what the room shows everyone, so it is a write: the
+        read-only promise for unlisted channels has to hold here too."""
+        room = _make_room("room-1", "Ops", RoomKind.CHANNEL)
+        client = self._stub_client()
+        client.get_room = AsyncMock(return_value=_make_room_state(room))
+        result, _, _ = await self._run(
+            _handle_chatto_pin_message,
+            {"room_id": "room-1", "event_id": "msg-1", "pin": True},
+            client=client,
+        )
+        assert result["success"] is False
+        assert "read-only" in result["error"]
+        client.pin_message.assert_not_awaited()
+
+    async def test_pin_message_pins_in_an_opted_in_channel(self):
+        room = _make_room("room-1", "Ops", RoomKind.CHANNEL)
+        client = self._stub_client()
+        client.get_room = AsyncMock(return_value=_make_room_state(room))
+        result, _, _ = await self._run(
+            _handle_chatto_pin_message,
+            {"room_id": "room-1", "event_id": "msg-1", "pin": True},
+            client=client,
+            optional_mention_rooms="room-1",
+        )
+        assert result["action"] == "pinned"
+        client.pin_message.assert_awaited_once_with("room-1", "msg-1")
+
+    async def test_pin_message_unpins(self):
+        room = _make_room("room-1", "Ops", RoomKind.CHANNEL)
+        client = self._stub_client()
+        client.get_room = AsyncMock(return_value=_make_room_state(room))
+        result, _, _ = await self._run(
+            _handle_chatto_pin_message,
+            {"room_id": "room-1", "event_id": "msg-1", "pin": False},
+            client=client,
+            optional_mention_rooms="room-1",
+        )
+        assert result["action"] == "unpinned"
+        assert result["unpinned"] is True
+        client.unpin_message.assert_awaited_once_with("room-1", "msg-1")
+
+    # -- custom status --
+
+    async def test_set_custom_status_passes_emoji_and_text(self):
+        result, client, _ = await self._run(
+            _handle_chatto_set_custom_status,
+            {"emoji": "🛠️", "text": "processing"},
+        )
+        assert result["success"] is True
+        assert result["emoji"] == "🛠️"
+        client.set_custom_status.assert_awaited_once_with(
+            "🛠️", "processing", expires_at=None
+        )
+
+    async def test_set_custom_status_turns_a_ttl_into_an_expiry(self):
+        result, client, _ = await self._run(
+            _handle_chatto_set_custom_status,
+            {"emoji": "⏳", "text": "waiting", "expires_in_seconds": 60},
+        )
+        assert result["expires_at"] is not None
+        assert client.set_custom_status.call_args.kwargs["expires_at"] is not None
+
+    async def test_delete_custom_status_clears_it(self):
+        result, client, _ = await self._run(_handle_chatto_delete_custom_status, {})
+        assert result["success"] is True
+        client.delete_custom_status.assert_awaited_once()
+
+    # -- chatto_fetch_link_preview --
+
+    async def test_fetch_link_preview_returns_the_metadata(self):
+        client = self._stub_client()
+        client.fetch_link_preview = AsyncMock(
+            return_value=(
+                LinkPreview(
+                    url="https://example.com",
+                    title="Example",
+                    description="An example page",
+                    image_url="https://example.com/preview.png",
+                    site_name="Example",
+                ),
+                "preview-token",
+            )
+        )
+        result, _, _ = await self._run(
+            _handle_chatto_fetch_link_preview,
+            {"url": "https://example.com"},
+            client=client,
+        )
+        assert result["title"] == "Example"
+        assert result["image_url"] == "https://example.com/preview.png"
+        client.fetch_link_preview.assert_awaited_once_with("https://example.com")
+
+    async def test_fetch_link_preview_answers_empty_metadata_as_nulls(self):
+        """A URL the server could not enrich is not an error: the fields come
+        back null so the agent can still post the link."""
+        result, _, _ = await self._run(
+            _handle_chatto_fetch_link_preview, {"url": "https://example.com"}
+        )
+        assert result["success"] is True
+        assert result["title"] is None
+
+    # -- chatto_mark_thread_as_read --
+
+    async def test_mark_thread_as_read_reports_the_previous_marker(self):
+        room = self._readable_room()
+        client = self._stub_client()
+        client.get_room = AsyncMock(return_value=_make_room_state(room))
+        client.mark_thread_as_read = AsyncMock(
+            return_value=datetime(2026, 10, 18, 9, 0, tzinfo=UTC)
+        )
+        result, _, _ = await self._run(
+            _handle_chatto_mark_thread_as_read,
+            {"room_id": "room-1", "thread_root_event_id": "msg-1"},
+            client=client,
+        )
+        assert result["success"] is True
+        assert result["previous_last_read_at"].startswith("2026-10-18T09:00")
+        client.mark_thread_as_read.assert_awaited_once_with("room-1", "msg-1")
+
+    # -- chatto_list_room_groups --
+
+    async def test_list_room_groups_reports_each_group_and_its_rooms(self):
+        client = self._stub_client()
+        client.list_room_groups = AsyncMock(
+            return_value=[
+                RoomGroup(
+                    id="group-1",
+                    name="Team",
+                    description="Internal rooms",
+                    rooms=[
+                        _make_room_state(
+                            _make_room("room-1", "Ops", RoomKind.CHANNEL)
+                        )
+                    ],
+                )
+            ]
+        )
+        result, _, _ = await self._run(
+            _handle_chatto_list_room_groups, {}, client=client
+        )
+        assert result["success"] is True
+        (group,) = result["groups"]
+        assert group["id"] == "group-1"
+        assert group["name"] == "Team"
+        assert group["room_ids"] == ["room-1"]
+
 
 class TestInteractivePromptAuth:
     """Allowed-user helper for reaction-driven interactive prompts."""