fix: address PR review feedback (round 7) and fix CI

- Rename collectives_update_collective to collectives_set_collective_emoji
  (more precise since only emoji is settable)
- Use standard JSON-RPC error code -32603 (INTERNAL_ERROR) instead of -1
- Handle UnicodeDecodeError when reading page content via WebDAV
- Replace brittle 'Welcome' content assertion with length check

Fixes CI: test_update_operations_not_idempotent no longer matches the
renamed tool, which is correctly idempotent (no ETag involved).

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

nextcloud_mcp_server/server/collectives.py

@@ -34,8 +34,8 @@ logger = logging.getLogger(__name__)
 def _handle_collectives_error(e: OCSError | HTTPStatusError) -> McpError:
     """Convert OCS or HTTP errors to McpError."""
     if isinstance(e, OCSError):
-        return McpError(ErrorData(code=-1, message=e.message))
-    return McpError(ErrorData(code=-1, message=str(e)))
+        return McpError(ErrorData(code=-32603, message=e.message))
+    return McpError(ErrorData(code=-32603, message=str(e)))
 
 
 def configure_collectives_tools(mcp: FastMCP):
@@ -124,7 +124,7 @@ def configure_collectives_tools(mcp: FastMCP):
             try:
                 file_bytes, _ = await client.webdav.read_file(webdav_path)
                 content = file_bytes.decode("utf-8")
-            except (HTTPStatusError, OSError) as e:
+            except (HTTPStatusError, OSError, UnicodeDecodeError) as e:
                 logger.warning(
                     "Failed to read page content via WebDAV: %s: %s",
                     webdav_path,
@@ -235,21 +235,21 @@ def configure_collectives_tools(mcp: FastMCP):
         )
 
     @mcp.tool(
-        title="Update Collective",
+        title="Set Collective Emoji",
         annotations=ToolAnnotations(idempotentHint=True, openWorldHint=True),
     )
     @require_scopes("collectives:write")
     @instrument_tool
-    async def collectives_update_collective(
-        ctx: Context, collective_id: int, emoji: str | None = None
+    async def collectives_set_collective_emoji(
+        ctx: Context, collective_id: int, emoji: str
     ) -> CollectiveOperationResponse:
-        """Update a Nextcloud Collective (emoji).
+        """Set the emoji on a Nextcloud Collective.
 
-        At least one field must be provided.
+        Setting the same emoji twice produces the same result (idempotent).
 
         Args:
             collective_id: ID of the collective
-            emoji: New emoji for the collective
+            emoji: Emoji to set on the collective
         """
         client = await get_client(ctx)
         try:
@@ -262,7 +262,7 @@ def configure_collectives_tools(mcp: FastMCP):
         return CollectiveOperationResponse(
             collective_id=collective.id,
             status_code=200,
-            message=f"Collective updated (emoji: {collective.emoji})",
+            message=f"Collective emoji set to: {collective.emoji}",
         )
 
     @mcp.tool(

tests/server/test_collectives_mcp.py

@@ -71,7 +71,7 @@ async def test_collectives_tools_available(nc_mcp_client: ClientSession):
     expected_tools = [
         "collectives_get_collectives",
         "collectives_create_collective",
-        "collectives_update_collective",
+        "collectives_set_collective_emoji",
         "collectives_trash_collective",
         "collectives_delete_collective",
         "collectives_get_pages",
@@ -116,12 +116,12 @@ async def test_collectives_list(
     logger.info(f"Found {data['total']} collectives")
 
 
-async def test_collectives_update_emoji(
+async def test_collectives_set_collective_emoji(
     nc_mcp_client: ClientSession, temporary_collective: dict
 ):
-    """Test updating a collective's emoji."""
+    """Test setting a collective's emoji."""
     result = await nc_mcp_client.call_tool(
-        "collectives_update_collective",
+        "collectives_set_collective_emoji",
         {"collective_id": temporary_collective["id"], "emoji": "📖"},
     )
     assert result.isError is False
@@ -242,7 +242,7 @@ async def test_collectives_get_landing_page_content(
     assert data["content"] is not None, (
         "Landing page should have auto-generated content"
     )
-    assert "Welcome" in data["content"], "Landing page should contain welcome text"
+    assert len(data["content"]) > 0, "Landing page should have non-empty content"
     logger.info(f"Landing page content: {len(data['content'])} bytes")