From 961449be3025f34bcf0d13d964a78df7db188688 Mon Sep 17 00:00:00 2001 From: Chris Coutinho Date: Sun, 7 Jun 2026 18:03:28 +0200 Subject: [PATCH] fix(deck): include archived cards in list tools for status=all/archived MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The active Deck listing endpoints (StackService::findAll / CardMapper::findAllForStacks and StackService::find / CardMapper::findAll) filter out archived cards at the SQL level — only the /stacks/archived endpoint returns them. The client-side status="all"/"archived" filters in deck_get_cards, deck_get_stacks, deck_get_stack and deck_get_board_overview therefore operated on a list the server had already stripped of archived cards, so they could never surface one. deck_get_card (by ID) bypasses the filter, which is why it appeared to work. Fixes #842. When status is "all" or "archived", also fetch /stacks/archived (client.deck.get_archived_stacks) and merge those cards back in per stack — concurrently with the active fetch where applicable. status="open"/"done" are unchanged and cost no extra call. Co-Authored-By: Claude Opus 4.8 (1M context) --- docker-compose.yml | 2 +- nextcloud_mcp_server/server/deck.py | 122 ++++++++++++++++++++++++++-- tests/server/test_deck_mcp.py | 120 +++++++++++++++++++++++++++ tests/unit/test_deck_server.py | 38 +++++++++ 4 files changed, 273 insertions(+), 9 deletions(-) diff --git a/docker-compose.yml b/docker-compose.yml index 2321a362..795c6ccb 100644 --- a/docker-compose.yml +++ b/docker-compose.yml @@ -36,7 +36,7 @@ services: # Mount OIDC development directory outside /var/www/html to avoid rsync conflicts # The post-installation hook will register /opt/apps as an additional app directory #- ./third_party:/opt/apps:ro - - ./third_party/astrolabe:/opt/apps/astrolabe:ro + #- ./third_party/astrolabe:/opt/apps/astrolabe:ro #- ./third_party/oidc:/opt/apps/oidc:ro environment: - NEXTCLOUD_TRUSTED_DOMAINS=app diff --git a/nextcloud_mcp_server/server/deck.py b/nextcloud_mcp_server/server/deck.py index d90469ed..998a0eab 100644 --- a/nextcloud_mcp_server/server/deck.py +++ b/nextcloud_mcp_server/server/deck.py @@ -215,6 +215,36 @@ def _apply_stack_filters( return stack +# Statuses whose result set can contain archived cards. The active Deck +# listing endpoints (get_stacks/get_stack) exclude archived cards at the SQL +# level — only the /stacks/archived endpoint returns them — so these statuses +# need a second fetch and merge. See issue #842. +_ARCHIVED_STATUSES: frozenset[str] = frozenset({"all", "archived"}) + + +async def _archived_cards_by_stack(client, board_id: int) -> dict[int, list[DeckCard]]: + """Map stack_id -> archived DeckCards for a board. + + The active stack/card listing endpoints filter out archived cards in SQL; + this hits ``/stacks/archived`` (the only endpoint that returns them) and + keys the cards by their stack so the list tools can merge them back in. + """ + archived_stacks = await client.deck.get_archived_stacks(board_id) + return { + stack.id: cast(list[DeckCard], stack.cards or []) for stack in archived_stacks + } + + +def _append_archived_cards(stack: DeckStack, extra: list[DeckCard]) -> None: + """Append archived cards onto a stack's existing card list, in place. + + Kept separate so the assignment stays correctly typed against + ``DeckStack.cards`` (``list[DeckCard | DeckCardSummary] | None``).""" + merged: list[DeckCard | DeckCardSummary] = list(stack.cards or []) + merged.extend(extra) + stack.cards = merged + + def _truncate_comment_message(message: str, message_max_length: int | None) -> str: """Truncate a comment strictly longer than the limit; appends "…".""" if message_max_length is not None and len(message) > message_max_length: @@ -480,6 +510,8 @@ def configure_deck_tools(mcp: FastMCP): 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"). + "archived"/"all" include archived cards, which the active + listing endpoint omits — this costs one extra API call. 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 @@ -492,7 +524,33 @@ def configure_deck_tools(mcp: FastMCP): description_preview_length, "description_preview_length" ) client = await get_client(ctx) - stacks = await client.deck.get_stacks(board_id) + + # Fetch active stacks and (when archived cards are in scope) the + # archived endpoint concurrently, then merge archived cards onto each + # stack by id before filtering. The active endpoint omits archived + # cards, so without this merge status="archived"/"all" would drop them. + stacks_holder: list[list[DeckStack]] = [] + archived_by_stack: dict[int, list[DeckCard]] = {} + merge_archived = include_cards and status in _ARCHIVED_STATUSES + + async def _get_active() -> None: + stacks_holder.append(await client.deck.get_stacks(board_id)) + + async def _get_archived() -> None: + archived_by_stack.update(await _archived_cards_by_stack(client, board_id)) + + async with anyio.create_task_group() as tg: + tg.start_soon(_get_active) + if merge_archived: + tg.start_soon(_get_archived) + + stacks = stacks_holder[0] + if merge_archived: + for stack in stacks: + extra = archived_by_stack.get(stack.id) + if extra: + _append_archived_cards(stack, extra) + stacks = [ _apply_stack_filters( stack, @@ -538,6 +596,8 @@ def configure_deck_tools(mcp: FastMCP): detail: "summary" (default) or "full". status: "open" (default), "done", "archived", or "all" (non-overlapping; a done+archived card counts as "archived"). + "archived"/"all" include archived cards, which the active + listing endpoint omits — this costs one extra API call. 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. @@ -549,6 +609,12 @@ def configure_deck_tools(mcp: FastMCP): ) client = await get_client(ctx) stack = await client.deck.get_stack(board_id, stack_id) + # Merge archived cards (omitted by the active endpoint) when in scope. + if include_cards and status in _ARCHIVED_STATUSES: + archived_by_stack = await _archived_cards_by_stack(client, board_id) + extra = archived_by_stack.get(stack_id) + if extra: + _append_archived_cards(stack, extra) return _apply_stack_filters( stack, include_cards=include_cards, @@ -578,9 +644,14 @@ def configure_deck_tools(mcp: FastMCP): """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. + This is the archived-only shortcut: it returns *only* archived cards + in a single call. The active list tools (deck_get_cards, + deck_get_stacks, deck_get_board_overview) also include archived cards + when called with status="archived"/"all"; use this tool when you want + archived cards exclusively and don't need the open ones. Typical use: + auditing completed work 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) and returned as compact @@ -652,7 +723,8 @@ def configure_deck_tools(mcp: FastMCP): returns the complete card objects. status: "open" (default), "done", "archived", or "all". The first three partition the board (a done+archived card counts as - "archived"). + "archived"). "archived"/"all" include archived cards, which the + active listing endpoint omits — this costs one extra API call. 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. @@ -663,9 +735,31 @@ def configure_deck_tools(mcp: FastMCP): description_preview_length, "description_preview_length" ) client = await get_client(ctx) - stack = await client.deck.get_stack(board_id, stack_id) + + # Archived cards are excluded by the active stack endpoint, so for + # statuses that can include them we also fetch /stacks/archived and + # merge. "open"/"done" need only the active stack (no extra call). + active_cards: list[DeckCard] = [] + archived_cards: list[DeckCard] = [] + need_active = status != "archived" + need_archived = status in _ARCHIVED_STATUSES + + async def _get_active() -> None: + stack = await client.deck.get_stack(board_id, stack_id) + active_cards.extend(cast(list[DeckCard], stack.cards or [])) + + async def _get_archived() -> None: + by_stack = await _archived_cards_by_stack(client, board_id) + archived_cards.extend(by_stack.get(stack_id, [])) + + async with anyio.create_task_group() as tg: + if need_active: + tg.start_soon(_get_active) + if need_archived: + tg.start_soon(_get_archived) + cards = _shape_cards( - cast(list[DeckCard], stack.cards or []), + active_cards + archived_cards, detail=detail, status=status, label=label, @@ -703,6 +797,8 @@ def configure_deck_tools(mcp: FastMCP): 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"). + "archived"/"all" include archived cards, which the active + listing endpoint omits — this costs one extra API call. 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 @@ -715,6 +811,8 @@ def configure_deck_tools(mcp: FastMCP): board_holder: list[DeckBoard] = [] stacks_holder: list[list[DeckStack]] = [] + archived_by_stack: dict[int, list[DeckCard]] = {} + merge_archived = status in _ARCHIVED_STATUSES async def _get_board() -> None: board_holder.append(await client.deck.get_board(board_id)) @@ -722,9 +820,14 @@ def configure_deck_tools(mcp: FastMCP): async def _get_stacks() -> None: stacks_holder.append(await client.deck.get_stacks(board_id)) + async def _get_archived() -> None: + archived_by_stack.update(await _archived_cards_by_stack(client, board_id)) + async with anyio.create_task_group() as tg: tg.start_soon(_get_board) tg.start_soon(_get_stacks) + if merge_archived: + tg.start_soon(_get_archived) board = board_holder[0] stacks = stacks_holder[0] @@ -732,10 +835,13 @@ def configure_deck_tools(mcp: FastMCP): stack_overviews: list[StackOverview] = [] total_cards = 0 for stack in stacks: + cards = cast(list[DeckCard], stack.cards or []) + if merge_archived: + cards = cards + archived_by_stack.get(stack.id, []) summaries = [ _summarize_card(c, description_preview_length) for c in _filter_cards( - cast(list[DeckCard], stack.cards or []), + cards, status=status, label=label, assigned_to=assigned_to, diff --git a/tests/server/test_deck_mcp.py b/tests/server/test_deck_mcp.py index 51b9fe21..00475083 100644 --- a/tests/server/test_deck_mcp.py +++ b/tests/server/test_deck_mcp.py @@ -421,3 +421,123 @@ async def test_deck_get_board_overview_mcp( assert stack["card_count"] == len(stack["cards"]) card_ids = [c["id"] for c in stack["cards"]] assert card_data["id"] in card_ids + + +# Archived-card visibility in the active list tools (issue #842) + + +async def _create_and_archive_card( + nc_mcp_client: ClientSession, board_id: int, stack_id: int +) -> int: + """Create a card in the given stack, archive it, and return its id. + + The card is cleaned up when the temporary stack is deleted by the + fixture teardown (deleting a stack removes its cards).""" + create_result = await nc_mcp_client.call_tool( + "deck_create_card", + { + "board_id": board_id, + "stack_id": stack_id, + "title": f"Archived Card {uuid.uuid4().hex[:8]}", + }, + ) + assert create_result.isError is False, f"create failed: {create_result.content}" + archived_id = json.loads(create_result.content[0].text)["id"] + + archive_result = await nc_mcp_client.call_tool( + "deck_archive_card", + {"board_id": board_id, "stack_id": stack_id, "card_id": archived_id}, + ) + assert archive_result.isError is False, f"archive failed: {archive_result.content}" + return archived_id + + +async def test_deck_get_cards_status_includes_archived_mcp( + nc_mcp_client: ClientSession, temporary_board_with_card: tuple +): + """deck_get_cards: archived cards appear under status="all"/"archived" and + are excluded under the default "open" — the regression behind issue #842.""" + board_data, stack_data, card_data = temporary_board_with_card + board_id = board_data["id"] + stack_id = stack_data["id"] + open_id = card_data["id"] + archived_id = await _create_and_archive_card(nc_mcp_client, board_id, stack_id) + + async def card_ids(status: str) -> list[int]: + result = await nc_mcp_client.call_tool( + "deck_get_cards", + {"board_id": board_id, "stack_id": stack_id, "status": status}, + ) + assert result.isError is False, f"deck_get_cards({status}) failed" + return [c["id"] for c in json.loads(result.content[0].text)["cards"]] + + open_ids = await card_ids("open") + assert open_id in open_ids + assert archived_id not in open_ids, "archived card must not show under 'open'" + + all_ids = await card_ids("all") + assert open_id in all_ids and archived_id in all_ids, ( + "status='all' must include both open and archived cards" + ) + + archived_only = await card_ids("archived") + assert archived_only == [archived_id], ( + f"status='archived' should return only the archived card, got {archived_only}" + ) + + +async def test_deck_get_stack_status_includes_archived_mcp( + nc_mcp_client: ClientSession, temporary_board_with_card: tuple +): + """deck_get_stack (single stack) honours archived cards for status + "all"/"archived" too, mirroring deck_get_cards (issue #842).""" + board_data, stack_data, card_data = temporary_board_with_card + board_id = board_data["id"] + stack_id = stack_data["id"] + archived_id = await _create_and_archive_card(nc_mcp_client, board_id, stack_id) + + async def stack_card_ids(status: str) -> list[int]: + result = await nc_mcp_client.call_tool( + "deck_get_stack", + {"board_id": board_id, "stack_id": stack_id, "status": status}, + ) + assert result.isError is False, f"deck_get_stack({status}) failed" + payload = json.loads(result.content[0].text) + return [c["id"] for c in (payload.get("cards") or [])] + + assert archived_id not in await stack_card_ids("open") + assert archived_id in await stack_card_ids("all") + assert await stack_card_ids("archived") == [archived_id] + + +async def test_deck_get_stacks_and_overview_include_archived_mcp( + nc_mcp_client: ClientSession, temporary_board_with_card: tuple +): + """deck_get_stacks and deck_get_board_overview surface archived cards when + status="all" (issue #842), keyed onto the correct stack.""" + board_data, stack_data, card_data = temporary_board_with_card + board_id = board_data["id"] + stack_id = stack_data["id"] + archived_id = await _create_and_archive_card(nc_mcp_client, board_id, stack_id) + + # deck_get_stacks(status="all") + stacks_result = await nc_mcp_client.call_tool( + "deck_get_stacks", {"board_id": board_id, "status": "all"} + ) + assert stacks_result.isError is False, "deck_get_stacks(all) failed" + stacks_payload = json.loads(stacks_result.content[0].text) + stack = next(s for s in stacks_payload["stacks"] if s["id"] == stack_id) + stack_card_ids = [c["id"] for c in (stack.get("cards") or [])] + assert card_data["id"] in stack_card_ids + assert archived_id in stack_card_ids, "archived card missing from deck_get_stacks" + + # deck_get_board_overview(status="all") + overview_result = await nc_mcp_client.call_tool( + "deck_get_board_overview", {"board_id": board_id, "status": "all"} + ) + assert overview_result.isError is False, "board overview failed" + overview_payload = json.loads(overview_result.content[0].text) + ov_stack = next(s for s in overview_payload["stacks"] if s["id"] == stack_id) + ov_card_ids = [c["id"] for c in ov_stack["cards"]] + assert archived_id in ov_card_ids, "archived card missing from board overview" + assert ov_stack["card_count"] == len(ov_stack["cards"]) diff --git a/tests/unit/test_deck_server.py b/tests/unit/test_deck_server.py index 4027dbcd..bd1d4ea4 100644 --- a/tests/unit/test_deck_server.py +++ b/tests/unit/test_deck_server.py @@ -17,8 +17,10 @@ from nextcloud_mcp_server.models.deck import ( ) from nextcloud_mcp_server.server.deck import ( _SHARE_TYPE_DECK, + _append_archived_cards, _apply_board_filters, _apply_stack_filters, + _archived_cards_by_stack, _extract_uid, _filter_cards, _resolve_note_attach_path, @@ -667,3 +669,39 @@ async def test_resolve_note_attach_path_handles_null_category(mocker): path = await _resolve_note_attach_path(client, note_id=7) assert path == "/Notes/Bare.md" + + +# Archived-card merge (issue #842) ----------------------------------------- + + +def test_append_archived_cards_merges_onto_existing(): + """Archived cards are appended after the stack's existing (open) cards.""" + stack = _make_stack(cards=[_make_card(1, archived=False)]) + _append_archived_cards(stack, [_make_card(2, archived=True)]) + assert stack.cards is not None + assert [c.id for c in stack.cards] == [1, 2] + + +def test_append_archived_cards_onto_none_cards(): + """A stack with cards=None gets a fresh list of the archived cards.""" + stack = _make_stack(cards=None) + _append_archived_cards(stack, [_make_card(2, archived=True)]) + assert stack.cards is not None + assert [c.id for c in stack.cards] == [2] + + +async def test_archived_cards_by_stack_maps_stack_id_to_cards(mocker): + """The helper keys archived cards by stack id, coercing None to [].""" + archived = [ + _make_stack(stack_id=3, cards=[_make_card(10, archived=True)]), + _make_stack(stack_id=4, cards=None), + ] + client = mocker.MagicMock() + client.deck.get_archived_stacks = mocker.AsyncMock(return_value=archived) + + result = await _archived_cards_by_stack(client, board_id=1) + + assert set(result) == {3, 4} + assert [c.id for c in result[3]] == [10] + assert result[4] == [] + client.deck.get_archived_stacks.assert_awaited_once_with(1)