Merge pull request #736 from cbcoutinho/fix/notes-v5-write-drift-730

fix(notes): defensively unwrap list-shaped Notes responses (refs #730)

nextcloud_mcp_server/client/notes.py

@@ -9,6 +9,42 @@ from .webdav import WebDAVClient
 logger = logging.getLogger(__name__)
 
 
+def _expect_note_object(payload: Any, *, operation: str) -> Dict[str, Any]:
+    """Coerce a Notes API single-note response into a dict.
+
+    Notes v5.0.0 has a catch-all route (``notes_api#fail``) that returns ``[]``
+    as JSON for unmatched paths, and a few edge cases where the response is a
+    list-wrapped object instead of a bare object — see issue #730. Without this
+    guard, callers hit a cryptic Pydantic ``"argument after ** must be a mapping,
+    not list"`` from ``Note(**payload)``.
+
+    Returns the dict unchanged. If the payload is a single-element list, returns
+    the inner dict. Anything else (empty list, list of multiple, non-dict) raises
+    a clear ``ValueError`` so the failure mode is obvious in logs.
+    """
+    if isinstance(payload, dict):
+        return payload
+    if isinstance(payload, list):
+        if len(payload) == 1 and isinstance(payload[0], dict):
+            logger.warning(
+                "Notes API returned a single-element list for %s; unwrapping. "
+                "This is a Notes app v5.0.0 quirk — see #730.",
+                operation,
+            )
+            return payload[0]
+        raise ValueError(
+            f"{operation}: Notes API returned a list-shaped payload "
+            f"({len(payload)} elements) where a single note object was expected. "
+            f"This typically means the request was routed to the catch-all "
+            f"notes_api#fail handler (e.g. unmatched URL or wrong API version). "
+            f"Verify the Notes app version and URL prefix (#732)."
+        )
+    raise ValueError(
+        f"{operation}: Notes API returned an unexpected payload type "
+        f"({type(payload).__name__}) where a single note object was expected."
+    )
+
+
 class NotesClient(BaseNextcloudClient):
     """Client for Nextcloud Notes app operations."""
 
@@ -79,7 +115,7 @@ class NotesClient(BaseNextcloudClient):
         response = await self._make_request(
             "GET", f"/apps/notes/api/v1/notes/{note_id}"
         )
-        return response.json()
+        return _expect_note_object(response.json(), operation="get_note")
 
     async def create_note(
         self,
@@ -99,7 +135,7 @@ class NotesClient(BaseNextcloudClient):
         response = await self._make_request(
             "POST", "/apps/notes/api/v1/notes", json=body
         )
-        return response.json()
+        return _expect_note_object(response.json(), operation="create_note")
 
     async def update(
         self,
@@ -146,7 +182,7 @@ class NotesClient(BaseNextcloudClient):
         logger.info(
             f"Update response for note {note_id}: Status {response.status_code}"
         )
-        updated_note = response.json()
+        updated_note = _expect_note_object(response.json(), operation="update_note")
 
         # Check for category change and cleanup old attachment directory if needed
         if (

tests/unit/client/test_notes.py

@@ -0,0 +1,65 @@
+"""Unit tests for the NotesClient response-shape guard.
+
+Notes app v5.0.0 has cases where the API returns a list-shaped JSON payload
+where the MCP server expects a single note object — see issue #730. The
+``_expect_note_object`` helper coerces these and surfaces clear errors instead
+of letting Pydantic raise the cryptic ``"argument after ** must be a mapping,
+not list"``.
+"""
+
+import pytest
+
+from nextcloud_mcp_server.client.notes import _expect_note_object
+
+pytestmark = pytest.mark.unit
+
+
+def test_dict_payload_passes_through():
+    """The healthy case: API returns a single note object — return it unchanged."""
+    payload = {"id": 1, "title": "Test", "content": "body", "etag": "abc"}
+    assert _expect_note_object(payload, operation="create_note") is payload
+
+
+def test_single_element_list_is_unwrapped(caplog):
+    """Notes v5.0.0 sometimes wraps a single note in a list; unwrap it and
+    log a warning so operators notice the upstream quirk.
+    """
+    inner = {"id": 1, "title": "Test", "etag": "abc"}
+    with caplog.at_level("WARNING"):
+        result = _expect_note_object([inner], operation="create_note")
+    assert result is inner
+    assert any("single-element list" in r.message for r in caplog.records)
+
+
+def test_empty_list_raises_clear_error():
+    """notes_api#fail returns ``[]`` for unmatched routes (#730). Surface a
+    diagnostic error rather than letting Pydantic complain about ``** mapping``.
+    """
+    with pytest.raises(ValueError) as exc:
+        _expect_note_object([], operation="create_note")
+    msg = str(exc.value)
+    assert "create_note" in msg
+    assert "list-shaped payload" in msg
+    assert "notes_api#fail" in msg
+
+
+def test_multi_element_list_raises_clear_error():
+    """Defensive: a list with multiple elements is also wrong shape; bail loudly."""
+    with pytest.raises(ValueError) as exc:
+        _expect_note_object([{"id": 1}, {"id": 2}], operation="update_note")
+    assert "update_note" in str(exc.value)
+
+
+def test_non_dict_non_list_raises_clear_error():
+    """Defensive: a string / int / None payload is also unexpected."""
+    with pytest.raises(ValueError) as exc:
+        _expect_note_object("not a note", operation="create_note")
+    assert "unexpected payload type" in str(exc.value)
+    assert "str" in str(exc.value)
+
+
+def test_list_of_non_dict_raises_clear_error():
+    """A list whose element isn't a dict still fails with a clear error."""
+    with pytest.raises(ValueError) as exc:
+        _expect_note_object(["not a note"], operation="get_note")
+    assert "list-shaped payload" in str(exc.value)