fix(deck): include archived cards in list tools for status=all/archived
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) <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Opus 4.8
parent
792c536802
commit
961449be30
+1
-1
@@ -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
|
||||
|
||||
@@ -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,
|
||||
|
||||
@@ -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"])
|
||||
|
||||
@@ -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)
|
||||
|
||||
Reference in New Issue
Block a user