fix(contacts): persist all documented fields on create (fixes #716)

create_contact previously read only fn/email/tel from contact_data and
silently dropped org, organization, note, title, nickname, bday,
categories, url — and didn't accept phone as an alias for tel, so the
reporter's exact call lost every field except fn and email. Introduce
_build_contact_from_data, share it with update_contact's fallback, and
normalise str→list inputs so pythonvCard4 doesn't iterate bare strings
character-by-character for list-typed properties.

Closes #716

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

nextcloud_mcp_server/client/contacts.py

@@ -11,6 +11,108 @@ from .base import BaseNextcloudClient
 logger = logging.getLogger(__name__)
 
 
+# Keys that _build_contact_from_data consumes. Used to warn (not error) on unknown keys.
+_SUPPORTED_CONTACT_KEYS = frozenset(
+    {
+        "fn",
+        "email",
+        "tel",
+        "phone",
+        "org",
+        "organization",
+        "note",
+        "title",
+        "nickname",
+        "bday",
+        "categories",
+        "url",
+    }
+)
+
+
+def _wrap_contact_field(value) -> list[dict]:
+    """Normalize an email/tel/url input into pythonvCard4's list-of-dicts shape.
+
+    Accepts a plain string, a dict already in ``{value, type}`` form, or a list of
+    either. Empty strings are dropped. Always returns a list (possibly empty).
+    """
+    if value is None or value == "":
+        return []
+    items = value if isinstance(value, list) else [value]
+    out: list[dict] = []
+    for item in items:
+        if isinstance(item, dict) and item.get("value"):
+            types = item.get("type") or ["HOME"]
+            out.append({"value": item["value"], "type": list(types)})
+        elif isinstance(item, str) and item:
+            out.append({"value": item, "type": ["HOME"]})
+    return out
+
+
+def _build_contact_from_data(contact_data: dict, uid: str) -> Contact:
+    """Build a pythonvCard4 Contact from an MCP ``contact_data`` dict.
+
+    Maps every key documented on ``nc_contacts_create_contact`` onto the underlying
+    library, normalising shapes (list/str) to avoid pythonvCard4's char-by-char
+    iteration of bare strings — see issue #716.
+    """
+
+    # pythonvCard4 iterates bare strings character-by-character for list-typed fields
+    # (ORG, NICKNAME, CATEGORIES, URL), producing garbage like ``ORG:A;c;m;e``. Wrap
+    # single strings in a list to keep the vCard well-formed.
+    def _as_list(value):
+        if isinstance(value, list):
+            return value
+        if isinstance(value, str) and "," in value:
+            return [v.strip() for v in value.split(",") if v.strip()]
+        return [value]
+
+    kwargs: dict = {"fn": contact_data.get("fn"), "uid": uid}
+
+    emails = _wrap_contact_field(contact_data.get("email"))
+    if emails:
+        kwargs["email"] = emails
+
+    tels = _wrap_contact_field(contact_data.get("tel") or contact_data.get("phone"))
+    if tels:
+        kwargs["tel"] = tels
+
+    org_value = contact_data.get("org") or contact_data.get("organization")
+    if org_value:
+        kwargs["org"] = _as_list(org_value)
+
+    if contact_data.get("note"):
+        kwargs["note"] = contact_data["note"]
+
+    if contact_data.get("title"):
+        kwargs["title"] = contact_data["title"]
+
+    if contact_data.get("nickname"):
+        kwargs["nickname"] = _as_list(contact_data["nickname"])
+
+    if contact_data.get("categories"):
+        kwargs["categories"] = _as_list(contact_data["categories"])
+
+    if contact_data.get("url"):
+        kwargs["url"] = _as_list(contact_data["url"])
+
+    bday = contact_data.get("bday")
+    if bday:
+        if isinstance(bday, date):
+            kwargs["bday"] = bday
+        elif isinstance(bday, str):
+            try:
+                kwargs["bday"] = date.fromisoformat(bday)
+            except ValueError:
+                logger.warning("Ignoring non-ISO bday value: %r", bday)
+
+    unknown = set(contact_data) - _SUPPORTED_CONTACT_KEYS
+    if unknown:
+        logger.debug("Ignoring unknown contact_data keys: %s", sorted(unknown))
+
+    return Contact(**kwargs)  # type: ignore[arg-type]
+
+
 class ContactsClient(BaseNextcloudClient):
     """Client for NextCloud CardDAV contact operations."""
 
@@ -127,13 +229,7 @@ class ContactsClient(BaseNextcloudClient):
         carddav_path = self._get_carddav_base_path()
         url = f"{carddav_path}/{addressbook}/{uid}.vcf"
 
-        contact = Contact(fn=contact_data.get("fn"), uid=uid)  # type: ignore
+        vcard = _build_contact_from_data(contact_data, uid).to_vcard()
-        if "email" in contact_data:
-            contact.email = [{"value": contact_data["email"], "type": ["HOME"]}]
-        if "tel" in contact_data:
-            contact.tel = [{"value": contact_data["tel"], "type": ["HOME"]}]
-
-        vcard = contact.to_vcard()
 
         headers = {
             "Content-Type": "text/vcard; charset=utf-8",
@@ -177,12 +273,7 @@ class ContactsClient(BaseNextcloudClient):
             )
         else:
             # Fallback to creating new vCard if we couldn't get existing
-            contact = Contact(fn=contact_data.get("fn"), uid=uid)  # type: ignore
+            vcard_content = _build_contact_from_data(contact_data, uid).to_vcard()
-            if "email" in contact_data:
-                contact.email = [{"value": contact_data["email"], "type": ["HOME"]}]
-            if "tel" in contact_data:
-                contact.tel = [{"value": contact_data["tel"], "type": ["HOME"]}]
-            vcard_content = contact.to_vcard()
 
         headers = {
             "Content-Type": "text/vcard; charset=utf-8",

nextcloud_mcp_server/server/contacts.py

@@ -191,7 +191,23 @@ def configure_contacts_tools(mcp: FastMCP):
                 not the display name. Use nc_contacts_list_addressbooks to
                 find available URI slugs.
             uid: The unique ID for the contact.
-            contact_data: A dictionary with the contact's details, e.g. {"fn": "John Doe", "email": "john.doe@example.com"}.
+            contact_data: A dictionary with the contact's details. Supported keys:
+
+                - ``fn`` (str, required): Formatted full name.
+                - ``email`` (str or list of str/dicts): Email address(es).
+                - ``tel`` / ``phone`` (str or list): Phone number(s).
+                - ``org`` / ``organization`` (str or list of str): Organization.
+                  Lists become semicolon-separated ORG components per RFC 6350.
+                - ``title`` (str): Job title.
+                - ``note`` (str): Free-form note.
+                - ``nickname`` (str or list of str).
+                - ``bday`` (ISO date str ``"YYYY-MM-DD"`` or ``datetime.date``).
+                - ``categories`` (list of str, or comma-separated str).
+                - ``url`` (str or list of str).
+
+                Unknown keys are ignored. Example:
+                ``{"fn": "John Doe", "email": "john@example.com",
+                "organization": "Acme", "note": "Met at conference"}``.
         """
         client = await get_client(ctx)
         return await client.contacts.create_contact(

tests/client/contacts/test_contacts_operations.py

@@ -86,3 +86,47 @@ async def test_full_contact_workflow(
     contacts = await nc_client.contacts.list_contacts(addressbook=addressbook_name)
     contact_uids = [c["vcard_id"] for c in contacts]
     assert contact_uid not in contact_uids
+
+
+async def test_create_contact_persists_all_documented_fields(
+    nc_client: NextcloudClient, temporary_addressbook: str
+):
+    """Regression for issue #716: org/note/phone/organization must persist to the vCard.
+
+    Historically ``create_contact`` only handled fn/email/tel and silently dropped every
+    other key. Inspect the raw server-side vCard (not just the parsed list response) to
+    confirm each documented field round-trips.
+    """
+    addressbook_name = temporary_addressbook
+    contact_uid = f"test-full-{uuid.uuid4().hex[:8]}"
+    contact_data = {
+        "fn": "Full Field User",
+        "email": "full@example.com",
+        "phone": "555-0716",  # alias for tel
+        "organization": "Acme Corp",  # alias for org
+        "note": "Issue 716 regression",
+        "title": "Engineer",
+        "url": "https://example.com",
+    }
+
+    await nc_client.contacts.create_contact(
+        addressbook=addressbook_name,
+        uid=contact_uid,
+        contact_data=contact_data,
+    )
+    try:
+        raw_vcard, _etag = await nc_client.contacts._get_raw_vcard(
+            addressbook_name, contact_uid
+        )
+        assert "FN:Full Field User" in raw_vcard
+        assert "EMAIL" in raw_vcard and "full@example.com" in raw_vcard
+        assert "TEL" in raw_vcard and "555-0716" in raw_vcard
+        assert "ORG:Acme Corp" in raw_vcard
+        assert "NOTE:Issue 716 regression" in raw_vcard
+        assert "TITLE:Engineer" in raw_vcard
+        # Sabre rewrites bare URL: to URL;VALUE=URI: on PUT
+        assert "URL" in raw_vcard and "https://example.com" in raw_vcard
+    finally:
+        await nc_client.contacts.delete_contact(
+            addressbook=addressbook_name, uid=contact_uid
+        )

tests/server/test_contacts_mcp.py

@@ -24,6 +24,9 @@ async def test_mcp_contacts_workflow(
         "fn": f"MCP Contact {unique_suffix}",
         "email": f"mcp.contact.{unique_suffix}@example.com",
         "tel": "1234567890",
+        # Regression for issue #716 — these were silently dropped before
+        "organization": "MCP Test Corp",
+        "note": f"Created by test {unique_suffix}",
     }
 
     try:
@@ -51,9 +54,13 @@ async def test_mcp_contacts_workflow(
         )
         assert create_c_result.isError is False
 
-        # 4. Verify contact creation
+        # 4. Verify contact creation (and that all fields — #716 — actually persisted)
         contacts = await nc_client.contacts.list_contacts(addressbook=addressbook_name)
-        assert any(c["vcard_id"] == contact_uid for c in contacts)
+        created = next((c for c in contacts if c["vcard_id"] == contact_uid), None)
+        assert created is not None
+        raw_vcard = created.get("addressdata", "")
+        assert "ORG:MCP Test Corp" in raw_vcard
+        assert f"NOTE:Created by test {unique_suffix}" in raw_vcard
 
         # 5. Delete contact via MCP
         logger.info(f"Deleting contact {contact_uid} via MCP")