diff --git a/docs/deck.md b/docs/deck.md index dfad7310..56c0978b 100644 --- a/docs/deck.md +++ b/docs/deck.md @@ -59,7 +59,7 @@ default** and support filtering so you fetch only what you need. | Parameter | Default | Effect | |-----------|---------|--------| | `detail` | `summary` | `summary` returns compact rows (id, title, stackId, labels as titles, assignee UIDs, due/done, counts, a short `descriptionPreview`); `full` returns the complete card objects (the pre-0.92 shape). | -| `status` | `open` | Filter before serialization: `open` (excludes archived **and** done), `done`, `archived`, or `all`. | +| `status` | `open` | Filter before serialization: `open`, `done`, `archived`, or `all`. The first three **partition** the board (no overlap) — a card that is both done and archived is reported only under `archived`. | | `label` | – | Only cards carrying a label with this exact title. | | `assigned_to` | – | Only cards assigned to this user UID. | | `description_max_length` | – | In `detail="full"`, truncate each description. | diff --git a/nextcloud_mcp_server/models/deck.py b/nextcloud_mcp_server/models/deck.py index 37f7261b..1d9890a1 100644 --- a/nextcloud_mcp_server/models/deck.py +++ b/nextcloud_mcp_server/models/deck.py @@ -145,24 +145,24 @@ class DeckCardSummary(BaseModel): title: str stackId: int archived: bool = False - duedate: Optional[datetime] = None - done: Optional[datetime] = None - labels: List[str] = Field( + duedate: datetime | None = None + done: datetime | None = None + labels: list[str] = Field( default_factory=list, description="Label titles assigned to the card" ) - assignedUsers: List[str] = Field( + assignedUsers: list[str] = Field( default_factory=list, description="UIDs of users assigned to the card" ) - attachmentCount: Optional[int] = Field( + attachmentCount: int | None = Field( default=None, description="Number of attachments on the card" ) - commentsUnread: Optional[int] = Field( + commentsUnread: int | None = Field( default=None, description="Number of unread comments (Deck exposes no total)" ) hasDescription: bool = Field( default=False, description="Whether the card has a non-empty description" ) - descriptionPreview: Optional[str] = Field( + descriptionPreview: str | None = Field( default=None, description="Truncated preview of the card description" ) @@ -176,7 +176,7 @@ class DeckStack(BaseModel): lastModified: Optional[int] = None # Cards may be projected to DeckCardSummary when a tool is called with # detail="summary" (the default for list tools). - cards: Optional[List[Union[DeckCard, DeckCardSummary]]] = None + cards: list[DeckCard | DeckCardSummary] | None = None etag: Optional[str] = Field(default=None, alias="ETag") @@ -279,9 +279,9 @@ class StackOverview(BaseModel): id: int = Field(description="Stack ID") title: str = Field(description="Stack title") - order: Optional[int] = Field(default=None, description="Stack sort order") + order: int | None = Field(default=None, description="Stack sort order") card_count: int = Field(description="Number of cards returned for this stack") - cards: List[DeckCardSummary] = Field( + cards: list[DeckCardSummary] = Field( default_factory=list, description="Compact card rows in this stack" ) @@ -295,10 +295,10 @@ class BoardOverviewResponse(BaseResponse): board_id: int = Field(description="Board ID") title: str = Field(description="Board title") - labels: List[str] = Field( + labels: list[str] = Field( default_factory=list, description="Board label titles (legend)" ) - stacks: List[StackOverview] = Field( + stacks: list[StackOverview] = Field( default_factory=list, description="Stacks with compact card rows" ) total_cards: int = Field(description="Total cards across all returned stacks") @@ -353,7 +353,7 @@ class CreateLabelResponse(BaseResponse): class ListCardsResponse(BaseResponse): """Response model for listing deck cards.""" - cards: list[Union[DeckCard, DeckCardSummary]] = Field( + cards: list[DeckCard | DeckCardSummary] = Field( description="List of deck cards (summaries unless detail='full')" ) total: int = Field(description="Total number of cards") @@ -379,7 +379,7 @@ class LabelOperationResponse(StatusResponse): class ListCardCommentsResponse(BaseResponse): """Response model for listing card comments.""" - results: list[Union[DeckComment, DeckCommentSummary]] = Field( + results: list[DeckComment | DeckCommentSummary] = Field( description="Card comments in this page (summaries unless detail='full')" ) count: int = Field( diff --git a/nextcloud_mcp_server/server/deck.py b/nextcloud_mcp_server/server/deck.py index e9bcf6ee..d90469ed 100644 --- a/nextcloud_mcp_server/server/deck.py +++ b/nextcloud_mcp_server/server/deck.py @@ -51,7 +51,7 @@ DetailLevel = Literal["summary", "full"] _DEFAULT_DESCRIPTION_PREVIEW = 140 -def _validate_description_max_length( +def _validate_positive_length( value: int | None, name: str = "description_max_length" ) -> None: """Tool-layer guard: reject zero/negative length thresholds. @@ -113,12 +113,16 @@ def _filter_cards( The upstream Deck API returns every card (including archived ones) inline, so this filtering reduces the tokens the caller sees but not network - bandwidth. ``status="open"`` excludes archived and explicitly-done cards. + bandwidth. + + ``open``/``done``/``archived`` partition the cards (no overlap): a card + that is both done and archived is reported only under ``archived``, since + archiving is the stronger "off the active board" state. """ if status == "open": cards = [c for c in cards if not c.archived and c.done is None] elif status == "done": - cards = [c for c in cards if c.done is not None] + cards = [c for c in cards if c.done is not None and not c.archived] elif status == "archived": cards = [c for c in cards if c.archived] # status == "all": no status filter @@ -473,8 +477,9 @@ def configure_deck_tools(mcp: FastMCP): via deck_get_cards. detail: "summary" (default) returns compact card rows; "full" returns the complete card objects (the old behavior). - status: Which cards to include — "open" (default; excludes - archived and done), "done", "archived", or "all". + status: Which cards to include — "open" (default), "done", + "archived", or "all". The first three partition the board + (a card that is both done and archived counts as "archived"). label: If set, only cards carrying a label with this exact title. assigned_to: If set, only cards assigned to this user UID. description_max_length: In detail="full", truncate each card's @@ -482,8 +487,8 @@ def configure_deck_tools(mcp: FastMCP): description_preview_length: In detail="summary", length of the description preview carried on each card (default 140). """ - _validate_description_max_length(description_max_length) - _validate_description_max_length( + _validate_positive_length(description_max_length) + _validate_positive_length( description_preview_length, "description_preview_length" ) client = await get_client(ctx) @@ -531,14 +536,15 @@ def configure_deck_tools(mcp: FastMCP): stack_id: The ID of the stack include_cards: Include cards in the stack (default True). detail: "summary" (default) or "full". - status: "open" (default), "done", "archived", or "all". + status: "open" (default), "done", "archived", or "all" + (non-overlapping; a done+archived card counts as "archived"). label: If set, only cards carrying a label with this exact title. assigned_to: If set, only cards assigned to this user UID. description_max_length: In detail="full", truncate descriptions. description_preview_length: In detail="summary", preview length. """ - _validate_description_max_length(description_max_length) - _validate_description_max_length( + _validate_positive_length(description_max_length) + _validate_positive_length( description_preview_length, "description_preview_length" ) client = await get_client(ctx) @@ -564,6 +570,8 @@ def configure_deck_tools(mcp: FastMCP): ctx: Context, board_id: int, detail: DetailLevel = "summary", + label: str | None = None, + assigned_to: str | None = None, description_max_length: int | None = None, description_preview_length: int = _DEFAULT_DESCRIPTION_PREVIEW, ) -> ListStacksResponse: @@ -576,30 +584,35 @@ def configure_deck_tools(mcp: FastMCP): Cards are always included on the returned stacks (an archived stack without its cards would have no audit value) and returned as compact - summaries by default. + summaries by default. There is no ``status`` filter — every card here + is archived by definition — but ``label``/``assigned_to`` narrow the + set just like the active-stack tools. Args: board_id: The ID of the board detail: "summary" (default) or "full". + label: If set, only cards carrying a label with this exact title. + assigned_to: If set, only cards assigned to this user UID. description_max_length: In detail="full", truncate descriptions. description_preview_length: In detail="summary", preview length. """ - _validate_description_max_length(description_max_length) - _validate_description_max_length( + _validate_positive_length(description_max_length) + _validate_positive_length( description_preview_length, "description_preview_length" ) client = await get_client(ctx) stacks = await client.deck.get_archived_stacks(board_id) # All cards in archived stacks are themselves archived; status="all" # keeps them (an "open"/"done" filter would drop the whole point). + # label/assigned_to still apply for targeted audits. stacks = [ _apply_stack_filters( stack, include_cards=True, detail=detail, status="all", - label=None, - assigned_to=None, + label=label, + assigned_to=assigned_to, description_max_length=description_max_length, description_preview_length=description_preview_length, ) @@ -637,15 +650,16 @@ def configure_deck_tools(mcp: FastMCP): stack_id: The ID of the stack detail: "summary" (default) returns compact card rows; "full" returns the complete card objects. - status: "open" (default; excludes archived and done), "done", - "archived", or "all". + status: "open" (default), "done", "archived", or "all". The first + three partition the board (a done+archived card counts as + "archived"). label: If set, only cards carrying a label with this exact title. assigned_to: If set, only cards assigned to this user UID. description_max_length: In detail="full", truncate descriptions. description_preview_length: In detail="summary", preview length. """ - _validate_description_max_length(description_max_length) - _validate_description_max_length( + _validate_positive_length(description_max_length) + _validate_positive_length( description_preview_length, "description_preview_length" ) client = await get_client(ctx) @@ -678,21 +692,23 @@ def configure_deck_tools(mcp: FastMCP): """Get a compact, whole-board snapshot in a single call. Returns the board title, its label legend, and every stack with its - cards projected to compact summary rows. This is the token-efficient - replacement for calling deck_get_board + deck_get_stacks when you just - need to see the state of the board — prefer it for "show me the - board" / "what's in progress" style requests on large boards. + cards projected to compact summary rows. Prefer it for "show me the + board" / "what's in progress" style requests on large boards — it is + the token-efficient way to view board *state*. It intentionally omits + the board-management fields (ACL, user list, full label objects) that + deck_get_board exposes; reach for deck_get_board when you need those. Args: board_id: The ID of the board - status: Which cards to include — "open" (default; excludes - archived and done), "done", "archived", or "all". + status: Which cards to include — "open" (default), "done", + "archived", or "all". The first three partition the board + (a card that is both done and archived counts as "archived"). label: If set, only cards carrying a label with this exact title. assigned_to: If set, only cards assigned to this user UID. description_preview_length: Length of the description preview carried on each card summary (default 140). """ - _validate_description_max_length( + _validate_positive_length( description_preview_length, "description_preview_length" ) client = await get_client(ctx) @@ -1324,7 +1340,7 @@ def configure_deck_tools(mcp: FastMCP): order: "newest" (default) or "oldest" — sort the page by creation time. """ - _validate_description_max_length(message_max_length, "message_max_length") + _validate_positive_length(message_max_length, "message_max_length") client = await get_client(ctx) comments = await client.deck.get_comments(card_id, limit=limit, offset=offset) shaped = _shape_comments( diff --git a/tests/unit/test_deck_server.py b/tests/unit/test_deck_server.py index 7fac6b30..4027dbcd 100644 --- a/tests/unit/test_deck_server.py +++ b/tests/unit/test_deck_server.py @@ -28,7 +28,7 @@ from nextcloud_mcp_server.server.deck import ( _summarize_card, _truncate_card_descriptions, _truncate_comment_message, - _validate_description_max_length, + _validate_positive_length, ) pytestmark = pytest.mark.unit @@ -182,30 +182,30 @@ def test_truncate_card_descriptions_shorter_than_limit_no_ellipsis(): assert cards[0].description == "hello" -# _validate_description_max_length ---------------------------------------- +# _validate_positive_length ---------------------------------------- -def test_validate_description_max_length_accepts_none(): +def test_validate_positive_length_accepts_none(): """None is the documented sentinel for "no truncation".""" - _validate_description_max_length(None) + _validate_positive_length(None) -def test_validate_description_max_length_accepts_positive(): +def test_validate_positive_length_accepts_positive(): """Positive values pass through silently.""" - _validate_description_max_length(1) - _validate_description_max_length(1000) + _validate_positive_length(1) + _validate_positive_length(1000) -def test_validate_description_max_length_rejects_zero(): +def test_validate_positive_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) + _validate_positive_length(0) -def test_validate_description_max_length_rejects_negative(): +def test_validate_positive_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) + _validate_positive_length(-10) # _apply_board_filters ------------------------------------------------------ @@ -299,8 +299,8 @@ def test_filter_cards_open_excludes_archived_and_done(): assert [c.id for c in result] == [1] -def test_filter_cards_done_keeps_only_done(): - """status="done" keeps only cards with a done timestamp.""" +def test_filter_cards_done_keeps_only_done_and_not_archived(): + """status="done" keeps done cards that are not archived.""" done_at = datetime(2024, 1, 1, tzinfo=timezone.utc) cards = [_make_card(1), _make_card(2, done=done_at)] result = _filter_cards(cards, status="done", label=None, assigned_to=None) @@ -314,6 +314,27 @@ def test_filter_cards_archived_keeps_only_archived(): assert [c.id for c in result] == [2] +def test_filter_cards_statuses_partition_the_board(): + """open/done/archived are non-overlapping; a done+archived card counts + only as "archived", not "done".""" + done_at = datetime(2024, 1, 1, tzinfo=timezone.utc) + open_card = _make_card(1) + done_card = _make_card(2, done=done_at) + done_and_archived = _make_card(3, done=done_at, archived=True) + cards = [open_card, done_card, done_and_archived] + + assert [ + c.id for c in _filter_cards(cards, status="open", label=None, assigned_to=None) + ] == [1] + assert [ + c.id for c in _filter_cards(cards, status="done", label=None, assigned_to=None) + ] == [2] + assert [ + c.id + for c in _filter_cards(cards, status="archived", label=None, assigned_to=None) + ] == [3] + + def test_filter_cards_all_keeps_everything(): """status="all" applies no status filter.""" cards = [_make_card(1), _make_card(2, archived=True)]