diff --git a/nextcloud_mcp_server/server/deck.py b/nextcloud_mcp_server/server/deck.py index 6b7a89ba..b6205c3d 100644 --- a/nextcloud_mcp_server/server/deck.py +++ b/nextcloud_mcp_server/server/deck.py @@ -1,5 +1,4 @@ import logging -from typing import Optional from mcp.server.fastmcp import Context, FastMCP from mcp.types import ToolAnnotations @@ -31,6 +30,77 @@ from nextcloud_mcp_server.observability.metrics import instrument_tool logger = logging.getLogger(__name__) +def _validate_description_max_length(description_max_length: int | None) -> None: + """Tool-layer guard: reject zero/negative truncation thresholds.""" + if description_max_length is not None and description_max_length <= 0: + raise ValueError( + f"description_max_length must be positive, got {description_max_length}" + ) + + +def _truncate_card_descriptions( + cards: list[DeckCard], description_max_length: int | None +) -> None: + """Truncate descriptions strictly longer than the limit; appends "…" so + the truncated result is ``description_max_length + 1`` chars.""" + if description_max_length is None: + return + for card in cards: + if card.description and len(card.description) > description_max_length: + card.description = card.description[:description_max_length] + "…" + + +def _apply_board_filters( + board: DeckBoard, + *, + include_acl: bool, + include_users: bool, + include_labels: bool, +) -> DeckBoard: + """Drop board sub-fields the caller didn't request; returns the board.""" + if not include_acl: + board.acl = [] + if not include_users: + board.users = [] + if not include_labels: + board.labels = [] + return board + + +def _apply_stack_filters( + stack: DeckStack, + *, + include_cards: bool, + include_archived_cards: bool, + description_max_length: int | None, +) -> DeckStack: + """Apply card-shaping filters to a single stack; returns the stack.""" + # Note: the upstream Deck API returns archived cards inline within + # active stacks (the Deck UI filters them frontend-side). Defaulting + # include_archived_cards to False mirrors that UI behavior — this is + # the breaking change called out in the PR description. + if not include_cards: + stack.cards = None + elif stack.cards: + if not include_archived_cards: + stack.cards = [c for c in stack.cards if not c.archived] + _truncate_card_descriptions(stack.cards, description_max_length) + return stack + + +def _apply_card_filters( + cards: list[DeckCard], + *, + include_archived_cards: bool, + description_max_length: int | None, +) -> list[DeckCard]: + """Apply filters to a flat list of cards; returns the (possibly new) list.""" + if not include_archived_cards: + cards = [c for c in cards if not c.archived] + _truncate_card_descriptions(cards, description_max_length) + return cards + + def configure_deck_tools(mcp: FastMCP): """Configure Nextcloud Deck tools and resources for the MCP server.""" @@ -143,11 +213,33 @@ def configure_deck_tools(mcp: FastMCP): ) @require_scopes("deck.read") @instrument_tool - async def deck_get_board(ctx: Context, board_id: int) -> DeckBoard: - """Get details of a specific Nextcloud Deck board""" + async def deck_get_board( + ctx: Context, + board_id: int, + include_acl: bool = True, + include_users: bool = True, + include_labels: bool = True, + ) -> DeckBoard: + """Get details of a specific Nextcloud Deck board. + + Args: + board_id: The ID of the board + include_acl: Include the board's ACL entries (default True). Set + False to reduce response size when ACLs are not needed. + include_users: Include the board's user list (default True). Set + False to reduce response size when users are not needed. + include_labels: Include the board's label definitions (default + True). Set False to reduce response size; labels can still be + retrieved via deck_get_labels. + """ client = await get_client(ctx) board = await client.deck.get_board(board_id) - return board + return _apply_board_filters( + board, + include_acl=include_acl, + include_users=include_users, + include_labels=include_labels, + ) @mcp.tool( title="List Deck Stacks", @@ -155,10 +247,38 @@ def configure_deck_tools(mcp: FastMCP): ) @require_scopes("deck.read") @instrument_tool - async def deck_get_stacks(ctx: Context, board_id: int) -> ListStacksResponse: - """Get all stacks in a Nextcloud Deck board""" + async def deck_get_stacks( + ctx: Context, + board_id: int, + include_cards: bool = True, + include_archived_cards: bool = False, + description_max_length: int | None = None, + ) -> ListStacksResponse: + """Get all stacks in a Nextcloud Deck board. + + Args: + board_id: The ID of the board + include_cards: Include cards inside each stack (default True). Set + False for a lightweight stack listing; fetch cards separately + via deck_get_cards. + include_archived_cards: Include archived cards (default False). + Only relevant when include_cards is True. + description_max_length: If set, truncate each card's description + to this many characters. Useful for keeping responses compact + on boards with long card specs. + """ + _validate_description_max_length(description_max_length) client = await get_client(ctx) stacks = await client.deck.get_stacks(board_id) + stacks = [ + _apply_stack_filters( + stack, + include_cards=include_cards, + include_archived_cards=include_archived_cards, + description_max_length=description_max_length, + ) + for stack in stacks + ] return ListStacksResponse(stacks=stacks, total=len(stacks)) @mcp.tool( @@ -167,11 +287,78 @@ def configure_deck_tools(mcp: FastMCP): ) @require_scopes("deck.read") @instrument_tool - async def deck_get_stack(ctx: Context, board_id: int, stack_id: int) -> DeckStack: - """Get details of a specific Nextcloud Deck stack""" + async def deck_get_stack( + ctx: Context, + board_id: int, + stack_id: int, + include_cards: bool = True, + include_archived_cards: bool = False, + description_max_length: int | None = None, + ) -> DeckStack: + """Get details of a specific Nextcloud Deck stack. + + Args: + board_id: The ID of the board + stack_id: The ID of the stack + include_cards: Include cards in the stack (default True). + include_archived_cards: Include archived cards (default False). + Only relevant when include_cards is True. + description_max_length: If set, truncate each card's description + to this many characters. + """ + _validate_description_max_length(description_max_length) client = await get_client(ctx) stack = await client.deck.get_stack(board_id, stack_id) - return stack + return _apply_stack_filters( + stack, + include_cards=include_cards, + include_archived_cards=include_archived_cards, + description_max_length=description_max_length, + ) + + @mcp.tool( + title="List Archived Deck Stacks", + annotations=ToolAnnotations(readOnlyHint=True, openWorldHint=True), + ) + @require_scopes("deck.read") + @instrument_tool + async def deck_get_archived_stacks( + ctx: Context, + board_id: int, + description_max_length: int | None = None, + ) -> ListStacksResponse: + """List archived stacks (with their archived cards) for a Nextcloud + Deck board. + + Use this to audit completed work that has been archived off the + active board (e.g. cards moved through a "Done" stack and then + archived via deck_archive_card). The shape mirrors deck_get_stacks. + + Cards are always included on the returned stacks (an archived stack + without its cards would have no audit value); pass + ``description_max_length`` if you need to keep the response compact. + + Args: + board_id: The ID of the board + description_max_length: If set, truncate each card's description + to this many characters. + """ + _validate_description_max_length(description_max_length) + client = await get_client(ctx) + stacks = await client.deck.get_archived_stacks(board_id) + # All cards in archived stacks are themselves archived; route through + # the same helper as the active-stack path so future filter additions + # apply uniformly. + stacks = [ + _apply_stack_filters( + stack, + include_cards=True, + include_archived_cards=True, + description_max_length=description_max_length, + ) + for stack in stacks + ] + return ListStacksResponse(stacks=stacks, total=len(stacks)) @mcp.tool( title="List Deck Cards", @@ -180,12 +367,37 @@ def configure_deck_tools(mcp: FastMCP): @require_scopes("deck.read") @instrument_tool async def deck_get_cards( - ctx: Context, board_id: int, stack_id: int + ctx: Context, + board_id: int, + stack_id: int, + include_archived_cards: bool = False, + description_max_length: int | None = None, ) -> ListCardsResponse: - """Get all cards in a Nextcloud Deck stack""" + """Get all cards in a Nextcloud Deck stack. + + Filtering is applied client-side after the API returns the full + stack, so ``include_archived_cards=False`` and + ``description_max_length`` reduce response size visible to the + caller but not network bandwidth — network-wise this tool is + equivalent to deck_get_stack(include_cards=True). + + Args: + board_id: The ID of the board + stack_id: The ID of the stack + include_archived_cards: Include archived cards (default False). + Archived cards can also be retrieved per-board via + deck_get_archived_stacks. + description_max_length: If set, truncate each card's description + to this many characters. + """ + _validate_description_max_length(description_max_length) client = await get_client(ctx) stack = await client.deck.get_stack(board_id, stack_id) - cards = stack.cards or [] + cards = _apply_card_filters( + stack.cards or [], + include_archived_cards=include_archived_cards, + description_max_length=description_max_length, + ) return ListCardsResponse(cards=cards, total=len(cards)) @mcp.tool( @@ -280,8 +492,8 @@ def configure_deck_tools(mcp: FastMCP): ctx: Context, board_id: int, stack_id: int, - title: Optional[str] = None, - order: Optional[int] = None, + title: str | None = None, + order: int | None = None, ) -> StackOperationResponse: """Update a Nextcloud Deck stack @@ -340,8 +552,8 @@ def configure_deck_tools(mcp: FastMCP): title: str, type: str = "plain", order: int = 999, - description: Optional[str] = None, - duedate: Optional[str] = None, + description: str | None = None, + duedate: str | None = None, ) -> CreateCardResponse: """Create a new card in a Nextcloud Deck stack @@ -376,14 +588,14 @@ def configure_deck_tools(mcp: FastMCP): board_id: int, stack_id: int, card_id: int, - title: Optional[str] = None, - description: Optional[str] = None, - type: Optional[str] = None, - owner: Optional[str] = None, - order: Optional[int] = None, - duedate: Optional[str] = None, - archived: Optional[bool] = None, - done: Optional[str] = None, + title: str | None = None, + description: str | None = None, + type: str | None = None, + owner: str | None = None, + order: int | None = None, + duedate: str | None = None, + archived: bool | None = None, + done: str | None = None, ) -> CardOperationResponse: """Update a Nextcloud Deck card @@ -568,8 +780,8 @@ def configure_deck_tools(mcp: FastMCP): ctx: Context, board_id: int, label_id: int, - title: Optional[str] = None, - color: Optional[str] = None, + title: str | None = None, + color: str | None = None, ) -> LabelOperationResponse: """Update a Nextcloud Deck label diff --git a/tests/client/deck/test_deck_api.py b/tests/client/deck/test_deck_api.py index c5bec571..cbe1e6f9 100644 --- a/tests/client/deck/test_deck_api.py +++ b/tests/client/deck/test_deck_api.py @@ -255,6 +255,37 @@ async def test_deck_get_stacks(mocker): mock_make_request.assert_called_once() +async def test_deck_get_archived_stacks(mocker): + """Test that get_archived_stacks targets the archived endpoint and parses the response.""" + mock_response = create_mock_response( + status_code=200, + json_data=[ + { + "id": 9, + "title": "Archived Stack", + "boardId": 123, + "order": 1, + "deletedAt": 0, + }, + ], + ) + + 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") + stacks = await client.get_archived_stacks(board_id=123) + + assert isinstance(stacks, list) + assert len(stacks) == 1 + assert stacks[0].id == 9 + + mock_make_request.assert_called_once() + assert "/boards/123/stacks/archived" in mock_make_request.call_args.args[1] + + # Card Tests diff --git a/tests/unit/test_deck_server.py b/tests/unit/test_deck_server.py new file mode 100644 index 00000000..d810dbc0 --- /dev/null +++ b/tests/unit/test_deck_server.py @@ -0,0 +1,370 @@ +import pytest + +from nextcloud_mcp_server.models.deck import ( + DeckACL, + DeckBoard, + DeckCard, + DeckLabel, + DeckPermissions, + DeckStack, + DeckUser, +) +from nextcloud_mcp_server.server.deck import ( + _apply_board_filters, + _apply_card_filters, + _apply_stack_filters, + _truncate_card_descriptions, + _validate_description_max_length, +) + +pytestmark = pytest.mark.unit + + +# Fixtures ------------------------------------------------------------------ + + +def _make_card( + card_id: int, + description: str | None = "desc", + archived: bool = False, +) -> DeckCard: + return DeckCard( + id=card_id, + title=f"Card {card_id}", + stackId=1, + type="plain", + order=card_id, + archived=archived, + owner="testuser", + description=description, + ) + + +def _make_user(uid: str = "testuser") -> DeckUser: + return DeckUser(primaryKey=uid, uid=uid, displayname=uid) + + +def _make_board( + board_id: int = 1, + *, + labels: list[DeckLabel] | None = None, + acl: list[DeckACL] | None = None, + users: list[DeckUser] | None = None, +) -> DeckBoard: + return DeckBoard( + id=board_id, + title=f"Board {board_id}", + owner=_make_user(), + color="FF0000", + archived=False, + labels=labels + if labels is not None + else [DeckLabel(id=1, title="L1", color="00FF00")], + acl=acl if acl is not None else [], + permissions=DeckPermissions( + PERMISSION_READ=True, + PERMISSION_EDIT=True, + PERMISSION_MANAGE=True, + PERMISSION_SHARE=True, + ), + users=users if users is not None else [_make_user("alice"), _make_user("bob")], + deletedAt=0, + ) + + +def _make_stack( + stack_id: int = 1, + *, + cards: list[DeckCard] | None = None, +) -> DeckStack: + return DeckStack( + id=stack_id, + title=f"Stack {stack_id}", + boardId=1, + order=stack_id, + deletedAt=0, + cards=cards, + ) + + +# _truncate_card_descriptions ---------------------------------------------- + + +def test_truncate_card_descriptions_no_op_when_limit_is_none(): + """When description_max_length is None, descriptions are left untouched.""" + cards = [_make_card(1, "x" * 5000)] + _truncate_card_descriptions(cards, None) + assert cards[0].description is not None + assert len(cards[0].description) == 5000 + + +def test_truncate_card_descriptions_truncates_long_descriptions(): + """Descriptions over the limit are truncated and marked with an ellipsis.""" + cards = [_make_card(1, "x" * 5000), _make_card(2, "short")] + _truncate_card_descriptions(cards, 100) + assert cards[0].description is not None + assert len(cards[0].description) == 101 # 100 chars + ellipsis + assert cards[0].description.endswith("…") + assert cards[1].description == "short" + + +def test_truncate_card_descriptions_handles_none_description(): + """Cards with no description are skipped without error.""" + cards = [_make_card(1, None)] + _truncate_card_descriptions(cards, 100) + assert cards[0].description is None + + +def test_truncate_card_descriptions_at_exact_boundary(): + """Descriptions at exactly the limit should not be truncated.""" + cards = [_make_card(1, "x" * 100)] + _truncate_card_descriptions(cards, 100) + assert cards[0].description == "x" * 100 + + +def test_truncate_card_descriptions_one_over_limit(): + """A description one character over the limit triggers truncation.""" + cards = [_make_card(1, "x" * 101)] + _truncate_card_descriptions(cards, 100) + assert cards[0].description is not None + assert len(cards[0].description) == 101 # 100 chars + ellipsis + assert cards[0].description.endswith("…") + + +def test_truncate_card_descriptions_shorter_than_limit_no_ellipsis(): + """A description shorter than the limit must not have an ellipsis appended.""" + cards = [_make_card(1, "hello")] + _truncate_card_descriptions(cards, 1000) + assert cards[0].description == "hello" + + +# _validate_description_max_length ---------------------------------------- + + +def test_validate_description_max_length_accepts_none(): + """None is the documented sentinel for "no truncation".""" + _validate_description_max_length(None) + + +def test_validate_description_max_length_accepts_positive(): + """Positive values pass through silently.""" + _validate_description_max_length(1) + _validate_description_max_length(1000) + + +def test_validate_description_max_length_rejects_zero(): + """Zero would wipe descriptions to a single ellipsis — reject at the boundary.""" + with pytest.raises(ValueError, match="must be positive"): + _validate_description_max_length(0) + + +def test_validate_description_max_length_rejects_negative(): + """Negative values produce surprising slice semantics — reject at the boundary.""" + with pytest.raises(ValueError, match="must be positive"): + _validate_description_max_length(-10) + + +# _apply_board_filters ------------------------------------------------------ + + +def test_apply_board_filters_defaults_preserve_fields(): + """With all include_* flags True, no fields are cleared.""" + board = _make_board() + result = _apply_board_filters( + board, include_acl=True, include_users=True, include_labels=True + ) + assert len(result.labels) == 1 + assert len(result.users) == 2 + + +def test_apply_board_filters_excludes_acl(): + """include_acl=False clears the acl list.""" + board = _make_board( + acl=[ + DeckACL( + id=1, + participant=_make_user("alice"), + type=0, + boardId=1, + permissionEdit=True, + permissionShare=True, + permissionManage=False, + owner=False, + ) + ] + ) + result = _apply_board_filters( + board, include_acl=False, include_users=True, include_labels=True + ) + assert result.acl == [] + + +def test_apply_board_filters_excludes_users(): + """include_users=False clears the users list.""" + board = _make_board() + result = _apply_board_filters( + board, include_acl=True, include_users=False, include_labels=True + ) + assert result.users == [] + + +def test_apply_board_filters_excludes_labels(): + """include_labels=False clears the labels list.""" + board = _make_board() + result = _apply_board_filters( + board, include_acl=True, include_users=True, include_labels=False + ) + assert result.labels == [] + + +def test_apply_board_filters_excludes_all(): + """All include_* flags False clears every filterable list.""" + board = _make_board() + result = _apply_board_filters( + board, include_acl=False, include_users=False, include_labels=False + ) + assert result.acl == [] + assert result.users == [] + assert result.labels == [] + + +# _apply_stack_filters ------------------------------------------------------ + + +def test_apply_stack_filters_include_cards_false_strips_cards(): + """include_cards=False sets cards to None regardless of other flags.""" + stack = _make_stack(cards=[_make_card(1), _make_card(2, archived=True)]) + result = _apply_stack_filters( + stack, + include_cards=False, + include_archived_cards=True, + description_max_length=None, + ) + assert result.cards is None + + +def test_apply_stack_filters_excludes_archived_by_default(): + """include_archived_cards=False filters out archived cards.""" + stack = _make_stack( + cards=[_make_card(1, archived=False), _make_card(2, archived=True)] + ) + result = _apply_stack_filters( + stack, + include_cards=True, + include_archived_cards=False, + description_max_length=None, + ) + assert result.cards is not None + assert [c.id for c in result.cards] == [1] + + +def test_apply_stack_filters_keeps_archived_when_requested(): + """include_archived_cards=True retains archived cards.""" + stack = _make_stack( + cards=[_make_card(1, archived=False), _make_card(2, archived=True)] + ) + result = _apply_stack_filters( + stack, + include_cards=True, + include_archived_cards=True, + description_max_length=None, + ) + assert result.cards is not None + assert [c.id for c in result.cards] == [1, 2] + + +def test_apply_stack_filters_truncates_descriptions_after_archive_filter(): + """Truncation runs on the post-archive-filter card set.""" + stack = _make_stack( + cards=[ + _make_card(1, description="x" * 50, archived=False), + _make_card(2, description="y" * 50, archived=True), + ] + ) + result = _apply_stack_filters( + stack, + include_cards=True, + include_archived_cards=False, + description_max_length=10, + ) + assert result.cards is not None + assert len(result.cards) == 1 + assert result.cards[0].description is not None + assert result.cards[0].description.endswith("…") + + +def test_apply_stack_filters_handles_none_cards(): + """A stack with no cards (cards=None) is left untouched.""" + stack = _make_stack(cards=None) + result = _apply_stack_filters( + stack, + include_cards=True, + include_archived_cards=False, + description_max_length=10, + ) + assert result.cards is None + + +def test_apply_stack_filters_all_archived_yields_empty_list_not_none(): + """A stack whose cards are all archived yields cards == [], not None. + + Pin the contract: include_cards=True with all cards filtered out + means "the stack was loaded but had nothing to show", which is + semantically distinct from include_cards=False (cards=None, + "explicitly suppressed"). Callers checking ``stack.cards is None`` + can use that to distinguish the two states. + """ + stack = _make_stack( + cards=[_make_card(1, archived=True), _make_card(2, archived=True)] + ) + result = _apply_stack_filters( + stack, + include_cards=True, + include_archived_cards=False, + description_max_length=None, + ) + assert result.cards == [] + assert result.cards is not None + + +# _apply_card_filters ------------------------------------------------------- + + +def test_apply_card_filters_excludes_archived_by_default(): + """include_archived_cards=False filters archived cards out of the flat list.""" + cards = [ + _make_card(1, archived=False), + _make_card(2, archived=True), + _make_card(3, archived=False), + ] + result = _apply_card_filters( + cards, include_archived_cards=False, description_max_length=None + ) + assert [c.id for c in result] == [1, 3] + + +def test_apply_card_filters_keeps_archived_when_requested(): + """include_archived_cards=True retains archived cards.""" + cards = [_make_card(1, archived=False), _make_card(2, archived=True)] + result = _apply_card_filters( + cards, include_archived_cards=True, description_max_length=None + ) + assert [c.id for c in result] == [1, 2] + + +def test_apply_card_filters_truncates_descriptions(): + """description_max_length is honored on the returned cards.""" + cards = [_make_card(1, description="x" * 50)] + result = _apply_card_filters( + cards, include_archived_cards=True, description_max_length=10 + ) + assert result[0].description is not None + assert result[0].description.endswith("…") + + +def test_apply_card_filters_empty_list_is_noop(): + """An empty input returns an empty output.""" + result = _apply_card_filters( + [], include_archived_cards=False, description_max_length=10 + ) + assert result == []