fix(talk): address PR #741 reviewer feedback

Four targeted fixes from the AI code review:

1. TalkConversation.description: drop the misleading `str | None`
   union (spreed always sends `""`, never null) — type is now `str`
   with default `""`.

2. get_messages: guard the X-Chat-Last-Given int parse with
   try/except so a misbehaving proxy can't crash the read flow;
   logs a warning and falls back to None.

3. get_messages: clamp `limit` to [1, 200] in the client (spreed
   caps server-side at 200 and silently truncates) so the returned
   `count` always matches what was actually requested. Both client
   and server-tool docstrings updated to state the valid range.

4. Add an integration test covering the 32000-char message ceiling
   in talk_send_message — the empty-message case was already tested,
   the over-length case was not.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

nextcloud_mcp_server/client/talk.py

@@ -11,6 +11,7 @@ require ``OCS-APIRequest: true`` and respond as JSON when ``Accept:
 application/json`` is sent.
 """
 
+import logging
 from typing import Any
 
 from nextcloud_mcp_server.client.base import BaseNextcloudClient
@@ -20,6 +21,8 @@ from nextcloud_mcp_server.models.talk import (
     TalkParticipant,
 )
 
+logger = logging.getLogger(__name__)
+
 
 class TalkClient(BaseNextcloudClient):
     """Client for Nextcloud Talk (spreed) app operations."""
@@ -124,7 +127,10 @@ class TalkClient(BaseNextcloudClient):
 
         Args:
             token: Conversation token.
-            limit: Max messages to return (spreed caps this at 200).
+            limit: Max messages to return. spreed caps this server-side
+                at 200; values outside ``[1, 200]`` are clamped here so
+                callers don't get a confusing mismatch between the
+                requested limit and the returned ``count``.
             last_known_message_id: Pagination cursor — pass the value
                 from the previous response's ``X-Chat-Last-Given`` header.
             look_into_future: When False (default), return *older*
@@ -140,10 +146,12 @@ class TalkClient(BaseNextcloudClient):
         Returns:
             ``(messages, x_chat_last_given)`` where the integer is the
             value of the ``X-Chat-Last-Given`` response header (or None
-            if the header was absent), suitable for pagination.
+            if the header was absent or unparseable), suitable for
+            pagination.
         """
+        clamped_limit = min(max(1, limit), 200)
         params: dict[str, Any] = {
-            "limit": limit,
+            "limit": clamped_limit,
             "lookIntoFuture": 1 if look_into_future else 0,
             "setReadMarker": 1 if set_read_marker else 0,
             "includeLastKnown": 1 if include_last_known else 0,
@@ -161,7 +169,17 @@ class TalkClient(BaseNextcloudClient):
         # spreed returns 200 with an empty data list when there's nothing
         # new, so we trust the JSON body here.
         last_given_header = response.headers.get("X-Chat-Last-Given")
-        last_given = int(last_given_header) if last_given_header else None
+        last_given: int | None = None
+        if last_given_header:
+            try:
+                last_given = int(last_given_header)
+            except ValueError:
+                # Defensive: spreed always sends an int, but a misbehaving
+                # proxy could mangle the header. Don't crash the read flow.
+                logger.warning(
+                    "Invalid X-Chat-Last-Given header from spreed: %r",
+                    last_given_header,
+                )
         data = response.json()["ocs"]["data"]
         return [TalkMessage(**msg) for msg in data], last_given
 

nextcloud_mcp_server/models/talk.py

@@ -53,7 +53,7 @@ class TalkConversation(BaseModel):
     type: int
     name: str
     displayName: str
-    description: str | None = ""
+    description: str = ""
     participantType: int | None = None
     unreadMessages: int = 0
     unreadMention: bool = False

nextcloud_mcp_server/server/talk.py

@@ -105,7 +105,9 @@ def configure_talk_tools(mcp: FastMCP) -> None:
 
         Args:
             token: Conversation token.
-            limit: Max messages per page (spreed caps at 200).
+            limit: Max messages per page. Valid range is 1-200 (spreed
+                caps server-side at 200); values outside this range are
+                clamped. Default 50.
             last_known_message_id: Pagination cursor — pass the
                 ``last_known_message_id`` from the previous response to
                 fetch the next (older) page.

tests/server/test_talk_mcp.py

@@ -141,3 +141,18 @@ async def test_talk_send_message_validation_empty_text(
         "talk_send_message", {"token": token, "message": ""}
     )
     assert result.isError is True, "Expected validation error for empty message"
+
+
+async def test_talk_send_message_validation_too_long(
+    nc_mcp_client: ClientSession, temporary_conversation: dict
+):
+    """A message exceeding the 32000-char ceiling is rejected client-side."""
+    token = temporary_conversation["token"]
+
+    result = await nc_mcp_client.call_tool(
+        "talk_send_message",
+        {"token": token, "message": "x" * 32001},
+    )
+    assert result.isError is True, (
+        "Expected validation error for message longer than 32000 characters"
+    )