From 8887aa241a1a28d232b7a3a7ea3b79a68916b73d Mon Sep 17 00:00:00 2001 From: Chris Coutinho Date: Fri, 20 Feb 2026 09:22:16 +0100 Subject: [PATCH] fix: wrap raw list returns in response models to produce single TextContent block MCP tools returning raw lists caused FastMCP's _convert_to_content() to create one TextContent block per element. Most MCP clients only read content[0], so they saw a single result instead of the full list. Wrapped 9 tool functions in proper response objects: - deck: deck_get_boards, deck_get_stacks, deck_get_cards, deck_get_labels - calendar: nc_calendar_list_events, nc_calendar_get_upcoming_events - contacts: nc_contacts_list_addressbooks, nc_contacts_list_contacts - tables: nc_tables_list_tables Closes #568 Co-Authored-By: Claude Opus 4.6 --- nextcloud_mcp_server/models/deck.py | 14 +++++ nextcloud_mcp_server/server/calendar.py | 54 +++++++++++++++---- nextcloud_mcp_server/server/contacts.py | 38 +++++++++++-- nextcloud_mcp_server/server/deck.py | 23 ++++---- nextcloud_mcp_server/server/tables.py | 7 ++- .../oauth/test_oauth_deck_permissions.py | 24 ++++++--- tests/server/test_mcp.py | 36 ++++++------- 7 files changed, 141 insertions(+), 55 deletions(-) diff --git a/nextcloud_mcp_server/models/deck.py b/nextcloud_mcp_server/models/deck.py index b636ddd..ca46562 100644 --- a/nextcloud_mcp_server/models/deck.py +++ b/nextcloud_mcp_server/models/deck.py @@ -261,6 +261,20 @@ class CreateLabelResponse(BaseResponse): color: str = Field(description="The created label color") +class ListCardsResponse(BaseResponse): + """Response model for listing deck cards.""" + + cards: List[DeckCard] = Field(description="List of deck cards") + total: int = Field(description="Total number of cards") + + +class ListLabelsResponse(BaseResponse): + """Response model for listing deck labels.""" + + labels: List[DeckLabel] = Field(description="List of deck labels") + total: int = Field(description="Total number of labels") + + class LabelOperationResponse(StatusResponse): """Response model for label operations like update/delete.""" diff --git a/nextcloud_mcp_server/server/calendar.py b/nextcloud_mcp_server/server/calendar.py index b54ae82..5974264 100644 --- a/nextcloud_mcp_server/server/calendar.py +++ b/nextcloud_mcp_server/server/calendar.py @@ -9,15 +9,39 @@ from nextcloud_mcp_server.auth import require_scopes from nextcloud_mcp_server.context import get_client from nextcloud_mcp_server.models.calendar import ( Calendar, + CalendarEventSummary, ListCalendarsResponse, + ListEventsResponse, ListTodosResponse, Todo, + UpcomingEventsResponse, ) from nextcloud_mcp_server.observability.metrics import instrument_tool logger = logging.getLogger(__name__) +def _event_dict_to_summary(event: dict) -> CalendarEventSummary: + """Convert a raw event dict from the calendar client to a CalendarEventSummary.""" + raw_categories = event.get("categories", []) + if isinstance(raw_categories, str): + categories = [c.strip() for c in raw_categories.split(",") if c.strip()] + else: + categories = raw_categories + + return CalendarEventSummary( + uid=event.get("uid", ""), + summary=event.get("title", ""), + start=event.get("start_datetime", ""), + end=event.get("end_datetime"), + all_day=event.get("all_day", False), + location=event.get("location") or None, + description=event.get("description") or None, + categories=categories, + status=event.get("status"), + ) + + def configure_calendar_tools(mcp: FastMCP): # Calendar tools @mcp.tool( @@ -204,7 +228,7 @@ def configure_calendar_tools(mcp: FastMCP): end_datetime=end_datetime, filters=filters if filters else None, ) - return events[:limit] + events = events[:limit] else: # Search in specific calendar events = await client.calendar.get_calendar_events( @@ -218,7 +242,14 @@ def configure_calendar_tools(mcp: FastMCP): if filters: events = client.calendar._apply_event_filters(events, filters) - return events + summaries = [_event_dict_to_summary(e) for e in events] + return ListEventsResponse( + events=summaries, + calendar_name=None if search_all_calendars else calendar_name, + start_date=start_date or None, + end_date=end_date or None, + total_found=len(summaries), + ) @mcp.tool( title="Get Calendar Event", @@ -420,7 +451,7 @@ def configure_calendar_tools(mcp: FastMCP): if calendar_name: # Get events from specific calendar - return await client.calendar.get_calendar_events( + events = await client.calendar.get_calendar_events( calendar_name=calendar_name, start_datetime=now, end_datetime=end_datetime, @@ -433,17 +464,13 @@ def configure_calendar_tools(mcp: FastMCP): for calendar in all_calendars: try: - events = await client.calendar.get_calendar_events( + cal_events = await client.calendar.get_calendar_events( calendar_name=calendar["name"], start_datetime=now, end_datetime=end_datetime, limit=limit, ) - # Add calendar info to each event - for event in events: - event["calendar_name"] = calendar["name"] - event["calendar_display_name"] = calendar["display_name"] - all_events.extend(events) + all_events.extend(cal_events) except Exception as e: logger.warning( f"Error getting events from calendar {calendar['name']}: {e}" @@ -452,7 +479,14 @@ def configure_calendar_tools(mcp: FastMCP): # Sort by start time and limit all_events.sort(key=lambda x: x.get("start_datetime", "")) - return all_events[:limit] + events = all_events[:limit] + + summaries = [_event_dict_to_summary(e) for e in events] + return UpcomingEventsResponse( + events=summaries, + days_ahead=days_ahead, + calendar_name=calendar_name or None, + ) @mcp.tool( title="Find Availability", diff --git a/nextcloud_mcp_server/server/contacts.py b/nextcloud_mcp_server/server/contacts.py index 9f5eafa..593c913 100644 --- a/nextcloud_mcp_server/server/contacts.py +++ b/nextcloud_mcp_server/server/contacts.py @@ -5,6 +5,12 @@ from mcp.types import ToolAnnotations from nextcloud_mcp_server.auth import require_scopes from nextcloud_mcp_server.context import get_client +from nextcloud_mcp_server.models.contacts import ( + AddressBook, + Contact, + ListAddressBooksResponse, + ListContactsResponse, +) from nextcloud_mcp_server.observability.metrics import instrument_tool logger = logging.getLogger(__name__) @@ -18,10 +24,21 @@ def configure_contacts_tools(mcp: FastMCP): ) @require_scopes("contacts:read") @instrument_tool - async def nc_contacts_list_addressbooks(ctx: Context): + async def nc_contacts_list_addressbooks(ctx: Context) -> ListAddressBooksResponse: """List all addressbooks for the user.""" client = await get_client(ctx) - return await client.contacts.list_addressbooks() + addressbooks_data = await client.contacts.list_addressbooks() + addressbooks = [ + AddressBook( + uri=ab["name"], + displayname=ab.get("display_name", ab["name"]), + ctag=ab.get("getctag"), + ) + for ab in addressbooks_data + ] + return ListAddressBooksResponse( + addressbooks=addressbooks, total_count=len(addressbooks) + ) @mcp.tool( title="List Contacts", @@ -29,10 +46,23 @@ def configure_contacts_tools(mcp: FastMCP): ) @require_scopes("contacts:read") @instrument_tool - async def nc_contacts_list_contacts(ctx: Context, *, addressbook: str): + async def nc_contacts_list_contacts( + ctx: Context, *, addressbook: str + ) -> ListContactsResponse: """List all contacts in the specified addressbook.""" client = await get_client(ctx) - return await client.contacts.list_contacts(addressbook=addressbook) + contacts_data = await client.contacts.list_contacts(addressbook=addressbook) + contacts = [ + Contact( + uid=c["vcard_id"], + fn=c.get("contact", {}).get("fullname", ""), + etag=c.get("getetag"), + ) + for c in contacts_data + ] + return ListContactsResponse( + contacts=contacts, addressbook=addressbook, total_count=len(contacts) + ) @mcp.tool( title="Create Address Book", diff --git a/nextcloud_mcp_server/server/deck.py b/nextcloud_mcp_server/server/deck.py index 93b5d64..ee6f499 100644 --- a/nextcloud_mcp_server/server/deck.py +++ b/nextcloud_mcp_server/server/deck.py @@ -17,6 +17,10 @@ from nextcloud_mcp_server.models.deck import ( DeckLabel, DeckStack, LabelOperationResponse, + ListBoardsResponse, + ListCardsResponse, + ListLabelsResponse, + ListStacksResponse, StackOperationResponse, ) from nextcloud_mcp_server.observability.metrics import instrument_tool @@ -124,11 +128,11 @@ def configure_deck_tools(mcp: FastMCP): ) @require_scopes("deck:read") @instrument_tool - async def deck_get_boards(ctx: Context) -> list[DeckBoard]: + async def deck_get_boards(ctx: Context) -> ListBoardsResponse: """Get all Nextcloud Deck boards""" client = await get_client(ctx) boards = await client.deck.get_boards() - return boards + return ListBoardsResponse(boards=boards, total=len(boards)) @mcp.tool( title="Get Deck Board", @@ -148,11 +152,11 @@ def configure_deck_tools(mcp: FastMCP): ) @require_scopes("deck:read") @instrument_tool - async def deck_get_stacks(ctx: Context, board_id: int) -> list[DeckStack]: + async def deck_get_stacks(ctx: Context, board_id: int) -> ListStacksResponse: """Get all stacks in a Nextcloud Deck board""" client = await get_client(ctx) stacks = await client.deck.get_stacks(board_id) - return stacks + return ListStacksResponse(stacks=stacks, total=len(stacks)) @mcp.tool( title="Get Deck Stack", @@ -174,13 +178,12 @@ def configure_deck_tools(mcp: FastMCP): @instrument_tool async def deck_get_cards( ctx: Context, board_id: int, stack_id: int - ) -> list[DeckCard]: + ) -> ListCardsResponse: """Get all cards in a Nextcloud Deck stack""" client = await get_client(ctx) stack = await client.deck.get_stack(board_id, stack_id) - if stack.cards: - return stack.cards - return [] + cards = stack.cards or [] + return ListCardsResponse(cards=cards, total=len(cards)) @mcp.tool( title="Get Deck Card", @@ -202,11 +205,11 @@ def configure_deck_tools(mcp: FastMCP): ) @require_scopes("deck:read") @instrument_tool - async def deck_get_labels(ctx: Context, board_id: int) -> list[DeckLabel]: + async def deck_get_labels(ctx: Context, board_id: int) -> ListLabelsResponse: """Get all labels in a Nextcloud Deck board""" client = await get_client(ctx) board = await client.deck.get_board(board_id) - return board.labels + return ListLabelsResponse(labels=board.labels, total=len(board.labels)) @mcp.tool( title="Get Deck Label", diff --git a/nextcloud_mcp_server/server/tables.py b/nextcloud_mcp_server/server/tables.py index 76e3c66..5a7f071 100644 --- a/nextcloud_mcp_server/server/tables.py +++ b/nextcloud_mcp_server/server/tables.py @@ -5,6 +5,7 @@ from mcp.types import ToolAnnotations from nextcloud_mcp_server.auth import require_scopes from nextcloud_mcp_server.context import get_client +from nextcloud_mcp_server.models.tables import ListTablesResponse, Table from nextcloud_mcp_server.observability.metrics import instrument_tool logger = logging.getLogger(__name__) @@ -18,10 +19,12 @@ def configure_tables_tools(mcp: FastMCP): ) @require_scopes("tables:read") @instrument_tool - async def nc_tables_list_tables(ctx: Context): + async def nc_tables_list_tables(ctx: Context) -> ListTablesResponse: """List all tables available to the user""" client = await get_client(ctx) - return await client.tables.list_tables() + tables_data = await client.tables.list_tables() + tables = [Table(**t) for t in tables_data] + return ListTablesResponse(tables=tables, total_count=len(tables)) @mcp.tool( title="Get Table Schema", diff --git a/tests/server/oauth/test_oauth_deck_permissions.py b/tests/server/oauth/test_oauth_deck_permissions.py index 1b12537..b5ade0e 100644 --- a/tests/server/oauth/test_oauth_deck_permissions.py +++ b/tests/server/oauth/test_oauth_deck_permissions.py @@ -78,8 +78,10 @@ async def test_deck_board_view_permissions( if not result.isError: response_data = json.loads(result.content[0].text) - # The response is directly a list of boards - if not isinstance(response_data, list): + # Response is a ListBoardsResponse with a "boards" field + if isinstance(response_data, dict) and "boards" in response_data: + response_data = response_data["boards"] + elif not isinstance(response_data, list): response_data = [response_data] if response_data else [] board_ids = [b["id"] for b in response_data] logger.info(f"Bob can see {len(response_data)} boards: {board_ids}") @@ -98,8 +100,10 @@ async def test_deck_board_view_permissions( if not result.isError: response_data = json.loads(result.content[0].text) - # The response is directly a list of boards - if not isinstance(response_data, list): + # Response is a ListBoardsResponse with a "boards" field + if isinstance(response_data, dict) and "boards" in response_data: + response_data = response_data["boards"] + elif not isinstance(response_data, list): response_data = [response_data] if response_data else [] board_ids = [b["id"] for b in response_data] logger.info(f"Diana can see {len(response_data)} boards") @@ -313,8 +317,10 @@ async def test_deck_user_isolation(nc_client, alice_mcp_client, bob_mcp_client): if not result.isError: response_data = json.loads(result.content[0].text) - # The response is directly a list of boards - if not isinstance(response_data, list): + # Response is a ListBoardsResponse with a "boards" field + if isinstance(response_data, dict) and "boards" in response_data: + response_data = response_data["boards"] + elif not isinstance(response_data, list): response_data = [response_data] if response_data else [] board_ids = [b["id"] for b in response_data] logger.info(f"Alice can see boards: {board_ids}") @@ -332,8 +338,10 @@ async def test_deck_user_isolation(nc_client, alice_mcp_client, bob_mcp_client): if not result.isError: response_data = json.loads(result.content[0].text) - # The response is directly a list of boards - if not isinstance(response_data, list): + # Response is a ListBoardsResponse with a "boards" field + if isinstance(response_data, dict) and "boards" in response_data: + response_data = response_data["boards"] + elif not isinstance(response_data, list): response_data = [response_data] if response_data else [] board_ids = [b["id"] for b in response_data] logger.info(f"Bob can see boards: {board_ids}") diff --git a/tests/server/test_mcp.py b/tests/server/test_mcp.py index 9bcef10..03e379b 100644 --- a/tests/server/test_mcp.py +++ b/tests/server/test_mcp.py @@ -683,17 +683,15 @@ async def test_mcp_calendar_workflow( f"MCP list events failed: {list_result.content}" ) - events_data = json.loads(list_result.content[0].text) + events_response = json.loads(list_result.content[0].text) # Debug output to understand what nc_calendar_list_events returns - logger.info(f"list_events result type: {type(events_data)}") - logger.info(f"list_events result content: {events_data}") - - # Handle single event returned as dict instead of list (same fix as calendars) - if isinstance(events_data, dict): - # Single event returned as dict instead of list - events_data = [events_data] + logger.info(f"list_events result type: {type(events_response)}") + logger.info(f"list_events result content: {events_response}") + # Response is now a ListEventsResponse with an "events" field + assert isinstance(events_response, dict), "Expected response dict" + events_data = events_response.get("events", []) assert isinstance(events_data, list), "Expected events list" # Our created event should be in the list @@ -706,7 +704,7 @@ async def test_mcp_calendar_workflow( assert found_event is not None, ( f"Created event {event_uid} not found in events list" ) - assert found_event["title"] == test_event_title + assert found_event["summary"] == test_event_title # 6. Test list events across all calendars logger.info("Testing nc_calendar_list_events across all calendars") @@ -727,13 +725,11 @@ async def test_mcp_calendar_workflow( f"MCP list all events failed: {all_list_result.content}" ) - all_events_data = json.loads(all_list_result.content[0].text) - - # Handle single event returned as dict instead of list (same fix as calendars) - if isinstance(all_events_data, dict): - # Single event returned as dict instead of list - all_events_data = [all_events_data] + all_events_response = json.loads(all_list_result.content[0].text) + # Response is now a ListEventsResponse with an "events" field + assert isinstance(all_events_response, dict), "Expected response dict" + all_events_data = all_events_response.get("events", []) assert isinstance(all_events_data, list), "Expected events list" # Our event should still be found when searching all calendars @@ -780,13 +776,11 @@ async def test_mcp_calendar_workflow( f"MCP upcoming events failed: {upcoming_result.content}" ) - upcoming_events = json.loads(upcoming_result.content[0].text) - - # Handle single event returned as dict instead of list (same fix as other tools) - if isinstance(upcoming_events, dict): - # Single event returned as dict instead of list - upcoming_events = [upcoming_events] + upcoming_response = json.loads(upcoming_result.content[0].text) + # Response is now an UpcomingEventsResponse with an "events" field + assert isinstance(upcoming_response, dict), "Expected response dict" + upcoming_events = upcoming_response.get("events", []) assert isinstance(upcoming_events, list), "Expected upcoming events list" # 10. Delete event via MCP