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 <noreply@anthropic.com>
This commit is contained in:
@@ -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."""
|
||||
|
||||
|
||||
@@ -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",
|
||||
|
||||
@@ -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",
|
||||
|
||||
@@ -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",
|
||||
|
||||
@@ -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",
|
||||
|
||||
@@ -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}")
|
||||
|
||||
+15
-21
@@ -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
|
||||
|
||||
Reference in New Issue
Block a user