From 454f6912bc2b36beebea6ae92b516c49573a9405 Mon Sep 17 00:00:00 2001 From: Chris Coutinho Date: Wed, 29 Apr 2026 13:39:24 +0200 Subject: [PATCH 1/3] feat(deck): add card comment tools MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Expose four new MCP tools backed by existing DeckClient comment methods: - deck_get_card_comments — list with limit/offset pagination - deck_create_card_comment — top-level or threaded (via parent_id) - deck_update_card_comment — author-only on the server - deck_delete_card_comment — author-only, destructive, idempotent Adds ListCardCommentsResponse and CardCommentOperationResponse models, and extends the client unit tests to cover replies, deletion, pagination, and the request shape for updates. Co-Authored-By: Claude Opus 4.7 (1M context) --- nextcloud_mcp_server/models/deck.py | 17 +++++ nextcloud_mcp_server/server/deck.py | 99 +++++++++++++++++++++++++++++ tests/client/deck/test_deck_api.py | 90 ++++++++++++++++++++++++++ 3 files changed, 206 insertions(+) diff --git a/nextcloud_mcp_server/models/deck.py b/nextcloud_mcp_server/models/deck.py index 8c42bff8..6fa7357e 100644 --- a/nextcloud_mcp_server/models/deck.py +++ b/nextcloud_mcp_server/models/deck.py @@ -280,3 +280,20 @@ class LabelOperationResponse(StatusResponse): label_id: int = Field(description="ID of the affected label") board_id: int = Field(description="ID of the board containing the label") + + +# Comment Response Models + + +class ListCardCommentsResponse(BaseResponse): + """Response model for listing card comments.""" + + results: list[DeckComment] = Field(description="List of card comments") + total: int = Field(description="Number of comments returned") + + +class CardCommentOperationResponse(StatusResponse): + """Response model for card comment create/update/delete operations.""" + + card_id: int = Field(description="ID of the card the comment belongs to") + comment_id: int = Field(description="ID of the affected comment") diff --git a/nextcloud_mcp_server/server/deck.py b/nextcloud_mcp_server/server/deck.py index f9ae9dc3..aa3ef1c9 100644 --- a/nextcloud_mcp_server/server/deck.py +++ b/nextcloud_mcp_server/server/deck.py @@ -7,6 +7,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.deck import ( + CardCommentOperationResponse, CardOperationResponse, CreateBoardResponse, CreateCardResponse, @@ -14,10 +15,12 @@ from nextcloud_mcp_server.models.deck import ( CreateStackResponse, DeckBoard, DeckCard, + DeckComment, DeckLabel, DeckStack, LabelOperationResponse, ListBoardsResponse, + ListCardCommentsResponse, ListCardsResponse, ListLabelsResponse, ListStacksResponse, @@ -722,3 +725,99 @@ def configure_deck_tools(mcp: FastMCP): stack_id=stack_id, board_id=board_id, ) + + # Card Comment Tools + + @mcp.tool( + title="List Deck Card Comments", + annotations=ToolAnnotations(readOnlyHint=True, openWorldHint=True), + ) + @require_scopes("deck.read") + @instrument_tool + async def deck_get_card_comments( + ctx: Context, card_id: int, limit: int = 20, offset: int = 0 + ) -> ListCardCommentsResponse: + """List comments on a Nextcloud Deck card + + Args: + card_id: The ID of the card + limit: Maximum number of comments to return (default 20, max 200) + offset: Pagination offset (default 0) + """ + client = await get_client(ctx) + comments = await client.deck.get_comments(card_id, limit=limit, offset=offset) + return ListCardCommentsResponse(results=comments, total=len(comments)) + + @mcp.tool( + title="Create Deck Card Comment", + annotations=ToolAnnotations(idempotentHint=False, openWorldHint=True), + ) + @require_scopes("deck.write") + @instrument_tool + async def deck_create_card_comment( + ctx: Context, + card_id: int, + message: str, + parent_id: Optional[int] = None, + ) -> DeckComment: + """Create a comment on a Nextcloud Deck card + + Supports @-mentions (e.g. "@alice"). Pass parent_id to reply to an + existing comment on the same card. Message is limited to 1000 characters. + + Args: + card_id: The ID of the card to comment on + message: The comment text (max 1000 characters) + parent_id: Optional ID of a parent comment to reply to + """ + client = await get_client(ctx) + return await client.deck.create_comment(card_id, message, parent_id=parent_id) + + @mcp.tool( + title="Update Deck Card Comment", + annotations=ToolAnnotations(idempotentHint=False, openWorldHint=True), + ) + @require_scopes("deck.write") + @instrument_tool + async def deck_update_card_comment( + ctx: Context, card_id: int, comment_id: int, message: str + ) -> DeckComment: + """Update a Nextcloud Deck card comment + + Only the comment's author can update it; the server returns 403 otherwise. + + Args: + card_id: The ID of the card the comment belongs to + comment_id: The ID of the comment to update + message: The new comment text (max 1000 characters) + """ + client = await get_client(ctx) + return await client.deck.update_comment(card_id, comment_id, message) + + @mcp.tool( + title="Delete Deck Card Comment", + annotations=ToolAnnotations( + destructiveHint=True, idempotentHint=True, openWorldHint=True + ), + ) + @require_scopes("deck.write") + @instrument_tool + async def deck_delete_card_comment( + ctx: Context, card_id: int, comment_id: int + ) -> CardCommentOperationResponse: + """Delete a Nextcloud Deck card comment + + Only the comment's author can delete it; the server returns 403 otherwise. + + Args: + card_id: The ID of the card the comment belongs to + comment_id: The ID of the comment to delete + """ + client = await get_client(ctx) + await client.deck.delete_comment(card_id, comment_id) + return CardCommentOperationResponse( + success=True, + message="Comment deleted successfully", + card_id=card_id, + comment_id=comment_id, + ) diff --git a/tests/client/deck/test_deck_api.py b/tests/client/deck/test_deck_api.py index db333e21..c5bec571 100644 --- a/tests/client/deck/test_deck_api.py +++ b/tests/client/deck/test_deck_api.py @@ -475,6 +475,96 @@ async def test_deck_update_comment(mocker): assert comment.message == "Updated comment" mock_make_request.assert_called_once() + call_args = mock_make_request.call_args + assert call_args[0][0] == "PUT" + assert "/cards/789/comments/222" in call_args[0][1] + assert call_args[1]["json"] == {"message": "Updated comment"} + + +async def test_deck_create_comment_reply(mocker): + """Test that create_comment forwards parent_id when replying.""" + mock_response = create_mock_deck_comment_response( + comment_id=333, message="A reply", card_id=789 + ) + + mock_client = mocker.AsyncMock(spec=httpx.AsyncClient) + mock_make_request = mocker.patch.object( + DeckClient, "_make_request", return_value=mock_response + ) + + client = DeckClient(mock_client, "testuser") + comment = await client.create_comment(card_id=789, message="A reply", parent_id=222) + + assert isinstance(comment, DeckComment) + assert comment.id == 333 + + mock_make_request.assert_called_once() + call_args = mock_make_request.call_args + assert call_args[0][0] == "POST" + assert "/cards/789/comments" in call_args[0][1] + assert call_args[1]["json"] == {"message": "A reply", "parentId": 222} + + +async def test_deck_create_comment_omits_parent_id_when_none(mocker): + """Test that create_comment does not send parentId when not given.""" + mock_response = create_mock_deck_comment_response( + comment_id=444, message="Top-level", card_id=789 + ) + + mock_client = mocker.AsyncMock(spec=httpx.AsyncClient) + mock_make_request = mocker.patch.object( + DeckClient, "_make_request", return_value=mock_response + ) + + client = DeckClient(mock_client, "testuser") + await client.create_comment(card_id=789, message="Top-level") + + call_args = mock_make_request.call_args + assert call_args[1]["json"] == {"message": "Top-level"} + assert "parentId" not in call_args[1]["json"] + + +async def test_deck_delete_comment(mocker): + """Test that delete_comment makes the correct API call.""" + mock_response = create_mock_response( + status_code=200, + json_data={"ocs": {"meta": {"status": "ok"}, "data": []}}, + ) + + mock_client = mocker.AsyncMock(spec=httpx.AsyncClient) + mock_make_request = mocker.patch.object( + DeckClient, "_make_request", return_value=mock_response + ) + + client = DeckClient(mock_client, "testuser") + result = await client.delete_comment(card_id=789, comment_id=222) + + assert result is None + mock_make_request.assert_called_once() + call_args = mock_make_request.call_args + assert call_args[0][0] == "DELETE" + assert "/cards/789/comments/222" in call_args[0][1] + + +async def test_deck_get_comments_pagination(mocker): + """Test that get_comments forwards limit and offset as query params.""" + mock_response = create_mock_response( + status_code=200, + json_data={"ocs": {"meta": {"status": "ok"}, "data": []}}, + ) + + mock_client = mocker.AsyncMock(spec=httpx.AsyncClient) + mock_make_request = mocker.patch.object( + DeckClient, "_make_request", return_value=mock_response + ) + + client = DeckClient(mock_client, "testuser") + await client.get_comments(card_id=789, limit=50, offset=100) + + call_args = mock_make_request.call_args + assert call_args[0][0] == "GET" + assert "/cards/789/comments" in call_args[0][1] + assert call_args[1]["params"] == {"limit": 50, "offset": 100} # Config Test From 13abaf3db7362ab2719e0205e5b91cb7272397e5 Mon Sep 17 00:00:00 2001 From: Chris Coutinho Date: Wed, 29 Apr 2026 15:35:10 +0200 Subject: [PATCH 2/3] test(deck): add integration tests for card comment tools MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Cover full CRUD lifecycle (create → list → update → delete → verify gone) and the reply path where parent_id populates replyTo on the new comment. Tests run against the live mcp container via the existing nc_mcp_client fixture and reuse the temporary_board_with_card fixture for setup/cleanup. Co-Authored-By: Claude Opus 4.7 (1M context) --- tests/server/test_deck_mcp.py | 114 ++++++++++++++++++++++++++++++++++ 1 file changed, 114 insertions(+) diff --git a/tests/server/test_deck_mcp.py b/tests/server/test_deck_mcp.py index ab3734dd..acca2746 100644 --- a/tests/server/test_deck_mcp.py +++ b/tests/server/test_deck_mcp.py @@ -221,3 +221,117 @@ async def test_deck_workflow_integration_mcp( # 3. Verify board data matches via resource (already done in step 1) logger.info(f"Board data verification completed for board: {board_id}") logger.info("Board structure and data verified successfully") + + +# Card Comment Tests + + +async def test_deck_card_comment_crud_workflow_mcp( + nc_mcp_client: ClientSession, + nc_client: NextcloudClient, + temporary_board_with_card: tuple, +): + """Full CRUD lifecycle for card comments via MCP tools.""" + _, _, card_data = temporary_board_with_card + card_id = card_data["id"] + + # 1. Create a top-level comment via MCP + create_result = await nc_mcp_client.call_tool( + "deck_create_card_comment", + {"card_id": card_id, "message": "Initial comment"}, + ) + assert create_result.isError is False, ( + f"Comment creation failed: {create_result.content}" + ) + comment = json.loads(create_result.content[0].text) + comment_id = comment["id"] + assert comment["objectId"] == card_id + assert comment["message"] == "Initial comment" + assert comment["replyTo"] is None + logger.info(f"Created comment ID {comment_id} on card {card_id}") + + # 2. List comments via MCP — verify the new comment is present + list_result = await nc_mcp_client.call_tool( + "deck_get_card_comments", {"card_id": card_id} + ) + assert list_result.isError is False, f"List comments failed: {list_result.content}" + listed = json.loads(list_result.content[0].text) + assert listed["total"] >= 1 + listed_ids = [c["id"] for c in listed["results"]] + assert comment_id in listed_ids, "Created comment not in list" + + # 3. Cross-check via direct client + direct_comments = await nc_client.deck.get_comments(card_id) + direct_ids = [c.id for c in direct_comments] + assert comment_id in direct_ids, "Created comment not visible via direct client" + + # 4. Update the comment via MCP + update_result = await nc_mcp_client.call_tool( + "deck_update_card_comment", + { + "card_id": card_id, + "comment_id": comment_id, + "message": "Edited comment", + }, + ) + assert update_result.isError is False, ( + f"Comment update failed: {update_result.content}" + ) + updated = json.loads(update_result.content[0].text) + assert updated["id"] == comment_id + assert updated["message"] == "Edited comment" + + # 5. Delete the comment via MCP + delete_result = await nc_mcp_client.call_tool( + "deck_delete_card_comment", + {"card_id": card_id, "comment_id": comment_id}, + ) + assert delete_result.isError is False, ( + f"Comment delete failed: {delete_result.content}" + ) + delete_response = json.loads(delete_result.content[0].text) + assert delete_response["success"] is True + assert delete_response["card_id"] == card_id + assert delete_response["comment_id"] == comment_id + + # 6. Verify the comment is gone + final_list_result = await nc_mcp_client.call_tool( + "deck_get_card_comments", {"card_id": card_id} + ) + final_listed = json.loads(final_list_result.content[0].text) + final_ids = [c["id"] for c in final_listed["results"]] + assert comment_id not in final_ids, "Comment still present after delete" + + +async def test_deck_card_comment_reply_mcp( + nc_mcp_client: ClientSession, temporary_board_with_card: tuple +): + """Replying with parent_id populates replyTo on the new comment.""" + _, _, card_data = temporary_board_with_card + card_id = card_data["id"] + + # Create the parent comment + parent_result = await nc_mcp_client.call_tool( + "deck_create_card_comment", + {"card_id": card_id, "message": "Parent message"}, + ) + assert parent_result.isError is False + parent = json.loads(parent_result.content[0].text) + parent_id = parent["id"] + + # Create a reply + reply_result = await nc_mcp_client.call_tool( + "deck_create_card_comment", + { + "card_id": card_id, + "message": "Reply message", + "parent_id": parent_id, + }, + ) + assert reply_result.isError is False, f"Reply failed: {reply_result.content}" + reply = json.loads(reply_result.content[0].text) + + assert reply["message"] == "Reply message" + assert reply["replyTo"] is not None, "replyTo should be populated for replies" + assert reply["replyTo"]["id"] == parent_id + assert reply["replyTo"]["message"] == "Parent message" From 2129bd6fac2d20fa5e644d3798e8b273ecd602af Mon Sep 17 00:00:00 2001 From: Chris Coutinho Date: Wed, 29 Apr 2026 16:57:46 +0200 Subject: [PATCH 3/3] fix(deck): address review feedback on card comment tools MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Wrap raw DeckComment returns in CardCommentResponse(BaseResponse) for create/update so the success/timestamp envelope matches other deck tools (#737 review issue 2). - Rename ListCardCommentsResponse.total → count and clarify in the description that it's the page size, not a server-side total — the Deck list endpoint does not expose one (#737 review issue 3). - Validate the documented 1000-character limit on create/update with an inline length check + ValueError, matching the pattern in api/management.py (#737 review issue 4). - Use modern int | None union syntax for the new parent_id parameter (#737 review issue 1); rest of the file is left in the existing Optional[...] style. Also add an MCP-level test that the >1000 char message is rejected, and update the existing comment tests to unwrap the new comment field. Co-Authored-By: Claude Opus 4.7 (1M context) --- nextcloud_mcp_server/models/deck.py | 17 ++++++++++++++--- nextcloud_mcp_server/server/deck.py | 29 ++++++++++++++++++++++------- tests/server/test_deck_mcp.py | 28 +++++++++++++++++++++++----- 3 files changed, 59 insertions(+), 15 deletions(-) diff --git a/nextcloud_mcp_server/models/deck.py b/nextcloud_mcp_server/models/deck.py index 6fa7357e..46f3eb5d 100644 --- a/nextcloud_mcp_server/models/deck.py +++ b/nextcloud_mcp_server/models/deck.py @@ -288,12 +288,23 @@ class LabelOperationResponse(StatusResponse): class ListCardCommentsResponse(BaseResponse): """Response model for listing card comments.""" - results: list[DeckComment] = Field(description="List of card comments") - total: int = Field(description="Number of comments returned") + results: list[DeckComment] = Field(description="Card comments in this page") + count: int = Field( + description=( + "Number of comments returned in this page (page size, not the " + "server-side total — the Deck list endpoint does not expose a total)." + ) + ) + + +class CardCommentResponse(BaseResponse): + """Response model returned when a single card comment is created or updated.""" + + comment: DeckComment = Field(description="The created or updated card comment") class CardCommentOperationResponse(StatusResponse): - """Response model for card comment create/update/delete operations.""" + """Response model for card comment operations that don't return comment data (e.g. delete).""" card_id: int = Field(description="ID of the card the comment belongs to") comment_id: int = Field(description="ID of the affected comment") diff --git a/nextcloud_mcp_server/server/deck.py b/nextcloud_mcp_server/server/deck.py index aa3ef1c9..6b7a89ba 100644 --- a/nextcloud_mcp_server/server/deck.py +++ b/nextcloud_mcp_server/server/deck.py @@ -8,6 +8,7 @@ from nextcloud_mcp_server.auth import require_scopes from nextcloud_mcp_server.context import get_client from nextcloud_mcp_server.models.deck import ( CardCommentOperationResponse, + CardCommentResponse, CardOperationResponse, CreateBoardResponse, CreateCardResponse, @@ -15,7 +16,6 @@ from nextcloud_mcp_server.models.deck import ( CreateStackResponse, DeckBoard, DeckCard, - DeckComment, DeckLabel, DeckStack, LabelOperationResponse, @@ -728,6 +728,15 @@ def configure_deck_tools(mcp: FastMCP): # Card Comment Tools + _COMMENT_MAX_LENGTH = 1000 + + def _validate_comment_message(message: str) -> None: + if len(message) > _COMMENT_MAX_LENGTH: + raise ValueError( + f"Comment message too long: {len(message)} characters " + f"(max {_COMMENT_MAX_LENGTH})" + ) + @mcp.tool( title="List Deck Card Comments", annotations=ToolAnnotations(readOnlyHint=True, openWorldHint=True), @@ -746,7 +755,7 @@ def configure_deck_tools(mcp: FastMCP): """ client = await get_client(ctx) comments = await client.deck.get_comments(card_id, limit=limit, offset=offset) - return ListCardCommentsResponse(results=comments, total=len(comments)) + return ListCardCommentsResponse(results=comments, count=len(comments)) @mcp.tool( title="Create Deck Card Comment", @@ -758,8 +767,8 @@ def configure_deck_tools(mcp: FastMCP): ctx: Context, card_id: int, message: str, - parent_id: Optional[int] = None, - ) -> DeckComment: + parent_id: int | None = None, + ) -> CardCommentResponse: """Create a comment on a Nextcloud Deck card Supports @-mentions (e.g. "@alice"). Pass parent_id to reply to an @@ -770,8 +779,12 @@ def configure_deck_tools(mcp: FastMCP): message: The comment text (max 1000 characters) parent_id: Optional ID of a parent comment to reply to """ + _validate_comment_message(message) client = await get_client(ctx) - return await client.deck.create_comment(card_id, message, parent_id=parent_id) + comment = await client.deck.create_comment( + card_id, message, parent_id=parent_id + ) + return CardCommentResponse(comment=comment) @mcp.tool( title="Update Deck Card Comment", @@ -781,7 +794,7 @@ def configure_deck_tools(mcp: FastMCP): @instrument_tool async def deck_update_card_comment( ctx: Context, card_id: int, comment_id: int, message: str - ) -> DeckComment: + ) -> CardCommentResponse: """Update a Nextcloud Deck card comment Only the comment's author can update it; the server returns 403 otherwise. @@ -791,8 +804,10 @@ def configure_deck_tools(mcp: FastMCP): comment_id: The ID of the comment to update message: The new comment text (max 1000 characters) """ + _validate_comment_message(message) client = await get_client(ctx) - return await client.deck.update_comment(card_id, comment_id, message) + comment = await client.deck.update_comment(card_id, comment_id, message) + return CardCommentResponse(comment=comment) @mcp.tool( title="Delete Deck Card Comment", diff --git a/tests/server/test_deck_mcp.py b/tests/server/test_deck_mcp.py index acca2746..d3fa6a9e 100644 --- a/tests/server/test_deck_mcp.py +++ b/tests/server/test_deck_mcp.py @@ -243,7 +243,9 @@ async def test_deck_card_comment_crud_workflow_mcp( assert create_result.isError is False, ( f"Comment creation failed: {create_result.content}" ) - comment = json.loads(create_result.content[0].text) + create_response = json.loads(create_result.content[0].text) + assert create_response["success"] is True + comment = create_response["comment"] comment_id = comment["id"] assert comment["objectId"] == card_id assert comment["message"] == "Initial comment" @@ -256,7 +258,7 @@ async def test_deck_card_comment_crud_workflow_mcp( ) assert list_result.isError is False, f"List comments failed: {list_result.content}" listed = json.loads(list_result.content[0].text) - assert listed["total"] >= 1 + assert listed["count"] >= 1 listed_ids = [c["id"] for c in listed["results"]] assert comment_id in listed_ids, "Created comment not in list" @@ -277,7 +279,8 @@ async def test_deck_card_comment_crud_workflow_mcp( assert update_result.isError is False, ( f"Comment update failed: {update_result.content}" ) - updated = json.loads(update_result.content[0].text) + update_response = json.loads(update_result.content[0].text) + updated = update_response["comment"] assert updated["id"] == comment_id assert updated["message"] == "Edited comment" @@ -316,7 +319,7 @@ async def test_deck_card_comment_reply_mcp( {"card_id": card_id, "message": "Parent message"}, ) assert parent_result.isError is False - parent = json.loads(parent_result.content[0].text) + parent = json.loads(parent_result.content[0].text)["comment"] parent_id = parent["id"] # Create a reply @@ -329,9 +332,24 @@ async def test_deck_card_comment_reply_mcp( }, ) assert reply_result.isError is False, f"Reply failed: {reply_result.content}" - reply = json.loads(reply_result.content[0].text) + reply = json.loads(reply_result.content[0].text)["comment"] assert reply["message"] == "Reply message" assert reply["replyTo"] is not None, "replyTo should be populated for replies" assert reply["replyTo"]["id"] == parent_id assert reply["replyTo"]["message"] == "Parent message" + + +async def test_deck_card_comment_message_too_long_mcp( + nc_mcp_client: ClientSession, temporary_board_with_card: tuple +): + """Creating a comment longer than 1000 chars is rejected client-side.""" + _, _, card_data = temporary_board_with_card + card_id = card_data["id"] + + too_long = "x" * 1001 + result = await nc_mcp_client.call_tool( + "deck_create_card_comment", + {"card_id": card_id, "message": too_long}, + ) + assert result.isError is True, "Expected validation error for >1000 char message"