fix: address PR review feedback (round 6)

- Fix assign_tag sending Content-Type header with no body
- Mark collectives_update_collective as idempotent (no ETag involved)
- Raise OCSError when 'data' key missing instead of silent fallback
- Tighten color validator to 3 or 6 hex chars only
- Add comment explaining null emoji semantics in set_page_emoji

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

nextcloud_mcp_server/client/collectives.py

@@ -44,7 +44,9 @@ class CollectivesClient(BaseNextcloudClient):
         if status_code >= 400:
             message = meta.get("message", "OCS error")
             raise OCSError(status_code, message)
-        return ocs.get("data", {})
+        if "data" not in ocs:
+            raise OCSError(500, "OCS response missing 'data' field")
+        return ocs["data"]
 
     # Collectives
 
@@ -189,6 +191,7 @@ class CollectivesClient(BaseNextcloudClient):
         self, collective_id: int, page_id: int, emoji: str | None
     ) -> dict[str, Any]:
         """Set or clear the emoji on a page."""
+        # Sending {"emoji": null} intentionally clears the emoji on the server
         json_data = {"emoji": emoji}
         response = await self._make_request(
             "PUT",
@@ -245,7 +248,7 @@ class CollectivesClient(BaseNextcloudClient):
         response = await self._make_request(
             "PUT",
             f"{API_BASE}/collectives/{collective_id}/pages/{page_id}/tags/{tag_id}",
-            headers=self._OCS_HEADERS_JSON,
+            headers=self._OCS_HEADERS,
         )
         self._unwrap_ocs(response.json())
 

nextcloud_mcp_server/models/collectives.py

@@ -60,7 +60,7 @@ class CollectiveTag(BaseModel):
     @field_validator("color")
     @classmethod
     def validate_hex_color(cls, v: str) -> str:
-        if not re.fullmatch(r"[0-9A-Fa-f]{3,8}", v):
+        if not re.fullmatch(r"[0-9A-Fa-f]{3}(?:[0-9A-Fa-f]{3})?", v):
             raise ValueError(f"Invalid hex color: {v!r}")
         return v
 

nextcloud_mcp_server/server/collectives.py

@@ -236,7 +236,7 @@ def configure_collectives_tools(mcp: FastMCP):
 
     @mcp.tool(
         title="Update Collective",
-        annotations=ToolAnnotations(idempotentHint=False, openWorldHint=True),
+        annotations=ToolAnnotations(idempotentHint=True, openWorldHint=True),
     )
     @require_scopes("collectives:write")
     @instrument_tool

tests/client/collectives/test_collectives_api.py

@@ -358,8 +358,8 @@ async def test_restore_page(mocker):
 # --- Error Handling ---
 
 
-async def test_ocs_missing_data_raises_key_error(mocker):
-    """Test that OCS envelope without 'data' key causes KeyError on field access."""
+async def test_ocs_missing_data_raises_ocs_error(mocker):
+    """Test that OCS envelope without 'data' key raises OCSError."""
     mock_response = create_mock_response(
         status_code=200,
         json_data={
@@ -371,9 +371,7 @@ async def test_ocs_missing_data_raises_key_error(mocker):
     mocker.patch.object(CollectivesClient, "_make_request", return_value=mock_response)
 
     client = CollectivesClient(mocker.AsyncMock(spec=httpx.AsyncClient), "testuser")
-    # _unwrap_ocs returns {} when "data" is absent; the caller then
-    # raises KeyError when accessing the expected key (e.g. "collectives")
-    with pytest.raises(KeyError):
+    with pytest.raises(OCSError, match="missing 'data' field"):
         await client.get_collectives()