From c0a974c498bfee4a68aaf5b0148489ca5a4f52f4 Mon Sep 17 00:00:00 2001 From: Chris Coutinho Date: Sun, 10 May 2026 23:17:20 +0200 Subject: [PATCH 1/3] feat(deck): add file/note attachment MCP tools MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Adds four tools that expose Deck card attachments via the MCP surface: deck_attach_file, deck_attach_note, deck_list_attachments, and deck_delete_attachment. The attach* variants share an existing Files entry (or Notes-app note) with the card via OCS shareType=12 — same mechanism the Deck UI's "Share from Files" picker uses, no file copy. This replaces the prior workaround of appending bulky activity content as Deck card comments: per-PR/per-event narrative now lives in NC Notes and surfaces on the tracking card as a clickable attachment that opens the original note in place. Implementation reuses existing client methods (SharingClient.create_share, DeckClient.get/delete_attachment, NotesClient.get_settings/get_note); no new client code. _SHARE_TYPE_DECK is centralised with a CI-guard test to prevent silent drift, and SharingClient.create_share's wire format is pinned to what the Deck Vue source sends. Co-Authored-By: Claude Opus 4.7 (1M context) --- nextcloud_mcp_server/models/deck.py | 39 ++++++ nextcloud_mcp_server/server/deck.py | 177 ++++++++++++++++++++++++++++ tests/client/test_sharing_client.py | 89 ++++++++++++++ tests/unit/test_deck_server.py | 40 +++++++ 4 files changed, 345 insertions(+) create mode 100644 tests/client/test_sharing_client.py diff --git a/nextcloud_mcp_server/models/deck.py b/nextcloud_mcp_server/models/deck.py index 46f3eb5d..e0ee76bd 100644 --- a/nextcloud_mcp_server/models/deck.py +++ b/nextcloud_mcp_server/models/deck.py @@ -144,6 +144,11 @@ class DeckAttachmentExtendedData(BaseModel): filesize: int mimetype: str info: Dict[str, str] + # Populated for type="file" (Files share) attachments via FilesAppService. + path: Optional[str] = None + fileid: Optional[int] = None + hasPreview: Optional[bool] = None + permissions: Optional[int] = None class DeckAttachment(BaseModel): @@ -308,3 +313,37 @@ class CardCommentOperationResponse(StatusResponse): card_id: int = Field(description="ID of the card the comment belongs to") comment_id: int = Field(description="ID of the affected comment") + + +# Attachment Response Models + + +class AttachFileResponse(BaseResponse): + """Response model for attaching an existing Nextcloud file to a Deck card. + + The attachment is created by sharing the file with the card via the standard + OCS Sharing API using ``shareType=12`` (``IShare::TYPE_DECK``). The returned + ``attachment_id`` is the share ID, which is also the Deck attachment ID. + """ + + attachment_id: int = Field( + description="ID of the created attachment (share ID)", + ) + card_id: int = Field(description="ID of the card the file is attached to") + path: str = Field(description="Path of the shared file in the user's Files") + + +class ListAttachmentsResponse(BaseResponse): + """Response model for listing card attachments.""" + + results: list["DeckAttachment"] = Field( + description="Attachments on the card (both type='file' and type='deck_file')" + ) + count: int = Field(description="Number of attachments returned") + + +class AttachmentOperationResponse(StatusResponse): + """Response model for attachment operations that don't return data (e.g. delete).""" + + card_id: int = Field(description="ID of the card the attachment belongs to") + attachment_id: int = Field(description="ID of the affected attachment") diff --git a/nextcloud_mcp_server/server/deck.py b/nextcloud_mcp_server/server/deck.py index b6205c3d..9a99f939 100644 --- a/nextcloud_mcp_server/server/deck.py +++ b/nextcloud_mcp_server/server/deck.py @@ -6,6 +6,8 @@ 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 ( + AttachFileResponse, + AttachmentOperationResponse, CardCommentOperationResponse, CardCommentResponse, CardOperationResponse, @@ -18,6 +20,7 @@ from nextcloud_mcp_server.models.deck import ( DeckLabel, DeckStack, LabelOperationResponse, + ListAttachmentsResponse, ListBoardsResponse, ListCardCommentsResponse, ListCardsResponse, @@ -101,6 +104,29 @@ def _apply_card_filters( return cards +# Card attachments — file shares ("Share from Files" picker in the Deck UI). +# +# Mechanism: a Deck card attachment of type="file" is just a Nextcloud share +# with shareType=12 (IShare::TYPE_DECK) and shareWith=. The Deck UI +# fires this exact request — see Deck app's +# src/components/card/AttachmentList.vue:223-238 and lib/Service/FilesAppService.php. +# The file is NOT copied; the share row binds the file's existing path to the card. +_SHARE_TYPE_DECK = 12 + + +def _resolve_note_path(notes_folder: str, category: str, title: str) -> str: + """Reconstruct a note's file path from Notes API metadata. + + Notes are stored as ``//.md`` in the + user's Files; ``<category>`` may be empty or nested (``"Foo/Bar"``). + """ + parts = [notes_folder.strip("/")] + if category: + parts.append(category.strip("/")) + parts.append(f"{title}.md") + return "/" + "/".join(p for p in parts if p) + + def configure_deck_tools(mcp: FastMCP): """Configure Nextcloud Deck tools and resources for the MCP server.""" @@ -1048,3 +1074,154 @@ def configure_deck_tools(mcp: FastMCP): card_id=card_id, comment_id=comment_id, ) + + @mcp.tool( + title="Attach File to Deck Card", + annotations=ToolAnnotations(idempotentHint=False, openWorldHint=True), + ) + @require_scopes("deck.write") + @instrument_tool + async def deck_attach_file( + ctx: Context, card_id: int, path: str + ) -> AttachFileResponse: + """Attach an existing Nextcloud file to a Deck card without copying. + + Creates a share of ``path`` with the card (``shareType=12``, + ``shareWith=<card_id>``). The file stays in its original location; + clicking the attachment in the Deck UI opens the file in place. + + Use this to link Notes (``/Notes/<title>.md``) or any Files entry to + a card as a discoverable attachment, replacing the older pattern of + long card comments. Calling twice with the same ``path`` creates two + distinct shares — caller is responsible for de-duping. + + Args: + card_id: The ID of the Deck card to attach to + path: Path to the file in the user's Nextcloud Files (must start + with "/", e.g. "/Notes/My Note.md") + """ + if not path.startswith("/"): + raise ValueError( + f"path must start with '/', got: {path!r} " + "(paths are relative to the user's Files root)" + ) + client = await get_client(ctx) + share = await client.sharing.create_share( + path=path, + share_with=str(card_id), + share_type=_SHARE_TYPE_DECK, + permissions=1, + ) + return AttachFileResponse( + attachment_id=int(share["id"]), + card_id=card_id, + path=path, + ) + + @mcp.tool( + title="Attach Note to Deck Card", + annotations=ToolAnnotations(idempotentHint=False, openWorldHint=True), + ) + @require_scopes("deck.write", "notes.read") + @instrument_tool + async def deck_attach_note( + ctx: Context, card_id: int, note_id: int + ) -> AttachFileResponse: + """Attach a Nextcloud Note to a Deck card without copying. + + Convenience wrapper: looks up the note's filesystem path from the + Notes app settings + note metadata, then shares the file with the + card (same mechanism as :func:`deck_attach_file`). The note remains + editable in the Notes app; the card just shows a clickable link to + it. + + Path is reconstructed as ``<notes_folder>/<category>/<title>.md``. + If the note's title contains characters that the Notes app sanitises + differently (rare), use :func:`deck_attach_file` with the explicit + path instead. + + Args: + card_id: The ID of the Deck card to attach to + note_id: The ID of the Note to attach + """ + client = await get_client(ctx) + settings = await client.notes.get_settings() + note = await client.notes.get_note(note_id) + notes_folder = settings.get("notes_path") or "Notes" + path = _resolve_note_path( + notes_folder=notes_folder, + category=note.get("category") or "", + title=note["title"], + ) + share = await client.sharing.create_share( + path=path, + share_with=str(card_id), + share_type=_SHARE_TYPE_DECK, + permissions=1, + ) + return AttachFileResponse( + attachment_id=int(share["id"]), + card_id=card_id, + path=path, + ) + + @mcp.tool( + title="List Deck Card Attachments", + annotations=ToolAnnotations(readOnlyHint=True, openWorldHint=True), + ) + @require_scopes("deck.read") + @instrument_tool + async def deck_list_attachments( + ctx: Context, board_id: int, stack_id: int, card_id: int + ) -> ListAttachmentsResponse: + """List attachments on a Nextcloud Deck card. + + Returns both shared-file attachments (``type="file"``, created via + :func:`deck_attach_file` / :func:`deck_attach_note`) and uploaded + binary attachments (``type="deck_file"``). + + Args: + board_id: The ID of the board + stack_id: The ID of the stack + card_id: The ID of the card + """ + client = await get_client(ctx) + attachments = await client.deck.get_attachments(board_id, stack_id, card_id) + return ListAttachmentsResponse(results=attachments, count=len(attachments)) + + @mcp.tool( + title="Delete Deck Card Attachment", + annotations=ToolAnnotations( + destructiveHint=True, idempotentHint=True, openWorldHint=True + ), + ) + @require_scopes("deck.write") + @instrument_tool + async def deck_delete_attachment( + ctx: Context, + board_id: int, + stack_id: int, + card_id: int, + attachment_id: int, + ) -> AttachmentOperationResponse: + """Delete an attachment from a Nextcloud Deck card. + + For ``type="file"`` attachments this removes the share linking the + file to the card; the underlying file in the user's Files is left + untouched. For ``type="deck_file"`` blobs the binary is deleted from + Deck's storage. + + Args: + board_id: The ID of the board + stack_id: The ID of the stack + card_id: The ID of the card + attachment_id: The ID of the attachment to delete + """ + client = await get_client(ctx) + await client.deck.delete_attachment(board_id, stack_id, card_id, attachment_id) + return AttachmentOperationResponse( + success=True, + message="Attachment deleted successfully", + card_id=card_id, + attachment_id=attachment_id, + ) diff --git a/tests/client/test_sharing_client.py b/tests/client/test_sharing_client.py new file mode 100644 index 00000000..1723b1c5 --- /dev/null +++ b/tests/client/test_sharing_client.py @@ -0,0 +1,89 @@ +"""Unit tests for SharingClient — wire-format checks for the OCS Sharing API. + +These verify the payload shape sent to Nextcloud, particularly for +``shareType=12`` (``IShare::TYPE_DECK``), which is what powers Deck card +file attachments. The Deck UI fires this exact request — see +``~/Software/deck/src/components/card/AttachmentList.vue:223-238``. +""" + +import pytest +from httpx import AsyncClient + +from nextcloud_mcp_server.client.sharing import SharingClient + + +@pytest.fixture +def sharing_client(mocker): + """SharingClient with a mocked underlying httpx client.""" + mock_http = mocker.AsyncMock(spec=AsyncClient) + return SharingClient(mock_http, "testuser") + + +def _ok_share_response(mocker, share_id: int = 4242, **extra): + """Build a fake OCS create-share success response.""" + response = mocker.Mock() + response.raise_for_status = mocker.Mock() + response.json.return_value = { + "ocs": { + "meta": {"statuscode": 200, "message": "OK"}, + "data": {"id": share_id, **extra}, + } + } + return response + + +@pytest.mark.unit +async def test_create_share_deck_type_payload(sharing_client, mocker): + """create_share(share_type=12) must POST exactly what the Deck UI does: + {path, shareType: 12, shareWith: "<cardId>"} to /ocs/v2.php/apps/files_sharing/api/v1/shares. + + Drift here would silently break Deck attachments — Nextcloud's + ShareAPIController routes shareType=12 to DeckShareProvider, which + creates the deck-card share row binding the file to the card. + """ + sharing_client._client.post.return_value = _ok_share_response(mocker, share_id=99) + + share = await sharing_client.create_share( + path="/Notes/My Note.md", + share_with="123", + share_type=12, + permissions=1, + ) + + assert share["id"] == 99 + sharing_client._client.post.assert_called_once() + call = sharing_client._client.post.call_args + assert call.args[0] == "/ocs/v2.php/apps/files_sharing/api/v1/shares" + assert call.kwargs["data"] == { + "path": "/Notes/My Note.md", + "shareType": 12, + "shareWith": "123", + "permissions": 1, + } + # Nextcloud demands this header on OCS endpoints; without it the request + # is rejected as a CSRF risk. + assert call.kwargs["headers"]["OCS-APIRequest"] == "true" + + +@pytest.mark.unit +async def test_create_share_raises_on_ocs_failure(sharing_client, mocker): + """OCS error responses (statuscode != 100/200) raise RuntimeError.""" + response = mocker.Mock() + response.raise_for_status = mocker.Mock() + response.json.return_value = { + "ocs": { + "meta": { + "statuscode": 404, + "message": "Wrong path, file/folder doesn't exist", + }, + "data": [], + } + } + sharing_client._client.post.return_value = response + + with pytest.raises(RuntimeError, match="Wrong path"): + await sharing_client.create_share( + path="/nope.md", + share_with="1", + share_type=12, + ) diff --git a/tests/unit/test_deck_server.py b/tests/unit/test_deck_server.py index d810dbc0..f7ace51c 100644 --- a/tests/unit/test_deck_server.py +++ b/tests/unit/test_deck_server.py @@ -10,9 +10,11 @@ from nextcloud_mcp_server.models.deck import ( DeckUser, ) from nextcloud_mcp_server.server.deck import ( + _SHARE_TYPE_DECK, _apply_board_filters, _apply_card_filters, _apply_stack_filters, + _resolve_note_path, _truncate_card_descriptions, _validate_description_max_length, ) @@ -368,3 +370,41 @@ def test_apply_card_filters_empty_list_is_noop(): [], include_archived_cards=False, description_max_length=10 ) assert result == [] + + +# _resolve_note_path ------------------------------------------------------- + + +def test_resolve_note_path_no_category(): + """Path is /<notes_folder>/<title>.md when no category.""" + assert _resolve_note_path("Notes", "", "My Note") == "/Notes/My Note.md" + + +def test_resolve_note_path_with_category(): + """Category is inserted as a sub-path.""" + assert _resolve_note_path("Notes", "Work", "Standup") == "/Notes/Work/Standup.md" + + +def test_resolve_note_path_with_nested_category(): + """Nested categories (Notes app supports `/`-separated) are preserved.""" + assert _resolve_note_path("Notes", "Work/Q4", "Plan") == "/Notes/Work/Q4/Plan.md" + + +def test_resolve_note_path_strips_redundant_slashes(): + """Leading/trailing slashes on inputs do not produce `//` in the result.""" + assert _resolve_note_path("/Notes/", "/Work/", "Title") == "/Notes/Work/Title.md" + + +def test_resolve_note_path_custom_notes_folder(): + """Honors a non-default notes_folder from Notes app settings.""" + assert ( + _resolve_note_path("Documents/Notes", "", "Idea") == "/Documents/Notes/Idea.md" + ) + + +# Share-type constant ------------------------------------------------------- + + +def test_share_type_deck_constant_matches_deck_app(): + """Deck UI uses shareType=12 (IShare::TYPE_DECK) — must not drift.""" + assert _SHARE_TYPE_DECK == 12 From 271904c227acf0c687185e0369669bcf4cb2b0a9 Mon Sep 17 00:00:00 2001 From: Chris Coutinho <chris@coutinho.io> Date: Sun, 10 May 2026 23:28:37 +0200 Subject: [PATCH 2/3] =?UTF-8?q?fix(deck):=20address=20review=20=E2=80=94?= =?UTF-8?q?=20notesPath=20key,=20scopes,=20modernize=20types?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit PR #781 review round 1: - 🔴 Fix notesPath key: the Notes API returns the folder under camelCase ``notesPath`` (see models/notes.py:43), but `deck_attach_note` was looking up snake_case ``notes_path`` and silently falling back to ``"Notes"``. Users with a non-default notes folder would have produced shares pointing at non-existent files (404 on click in Deck UI). - 🔴 Add wire-through unit test that would have caught the above: extract `_resolve_note_attach_path(client, note_id)` as a testable helper that encapsulates the camelCase-key lookup. Three new tests: custom notesPath honored, missing key falls back to default, null category handled. - 🟡 Modernize new fields on `DeckAttachmentExtendedData` to PEP 604 (`X | None`) per CLAUDE.md. - 🟡 Drop unnecessary string forward reference on `ListAttachmentsResponse.results` — DeckAttachment is defined earlier in the same module. - 🟢 Move `pytestmark = pytest.mark.unit` to module level in test_sharing_client.py to match the convention in test_deck_server.py. Per user request: `deck_attach_file` is now scoped `deck.write` + ``files.read`` (was just `deck.write`) so the generic file-share permission story is consistent — only `deck_attach_note` keeps `notes.read` since it specifically reads from the Notes app. Docstring updated to emphasise the tool is generic over the user's Files (PDFs/images/etc., not just markdown). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> --- nextcloud_mcp_server/models/deck.py | 10 ++--- nextcloud_mcp_server/server/deck.py | 43 ++++++++++++++------- tests/client/test_sharing_client.py | 4 +- tests/unit/test_deck_server.py | 60 +++++++++++++++++++++++++++++ 4 files changed, 96 insertions(+), 21 deletions(-) diff --git a/nextcloud_mcp_server/models/deck.py b/nextcloud_mcp_server/models/deck.py index e0ee76bd..1c29c401 100644 --- a/nextcloud_mcp_server/models/deck.py +++ b/nextcloud_mcp_server/models/deck.py @@ -145,10 +145,10 @@ class DeckAttachmentExtendedData(BaseModel): mimetype: str info: Dict[str, str] # Populated for type="file" (Files share) attachments via FilesAppService. - path: Optional[str] = None - fileid: Optional[int] = None - hasPreview: Optional[bool] = None - permissions: Optional[int] = None + path: str | None = None + fileid: int | None = None + hasPreview: bool | None = None + permissions: int | None = None class DeckAttachment(BaseModel): @@ -336,7 +336,7 @@ class AttachFileResponse(BaseResponse): class ListAttachmentsResponse(BaseResponse): """Response model for listing card attachments.""" - results: list["DeckAttachment"] = Field( + results: list[DeckAttachment] = Field( description="Attachments on the card (both type='file' and type='deck_file')" ) count: int = Field(description="Number of attachments returned") diff --git a/nextcloud_mcp_server/server/deck.py b/nextcloud_mcp_server/server/deck.py index 9a99f939..ec8344c5 100644 --- a/nextcloud_mcp_server/server/deck.py +++ b/nextcloud_mcp_server/server/deck.py @@ -127,6 +127,26 @@ def _resolve_note_path(notes_folder: str, category: str, title: str) -> str: return "/" + "/".join(p for p in parts if p) +async def _resolve_note_attach_path(client, note_id: int) -> str: + """Resolve a Notes-app note ID to its filesystem path for sharing. + + Hits the Notes API twice (settings + note metadata) and reconstructs + the path. Encapsulates the camelCase key (``notesPath``, see + ``models/notes.py:43``) so a typo there can't silently route to the + default ``"Notes"`` folder for users who've configured a non-default + notes location — that bug is exactly what this helper exists to make + testable. + """ + settings = await client.notes.get_settings() + note = await client.notes.get_note(note_id) + notes_folder = settings.get("notesPath") or "Notes" + return _resolve_note_path( + notes_folder=notes_folder, + category=note.get("category") or "", + title=note["title"], + ) + + def configure_deck_tools(mcp: FastMCP): """Configure Nextcloud Deck tools and resources for the MCP server.""" @@ -1079,7 +1099,7 @@ def configure_deck_tools(mcp: FastMCP): title="Attach File to Deck Card", annotations=ToolAnnotations(idempotentHint=False, openWorldHint=True), ) - @require_scopes("deck.write") + @require_scopes("deck.write", "files.read") @instrument_tool async def deck_attach_file( ctx: Context, card_id: int, path: str @@ -1090,15 +1110,17 @@ def configure_deck_tools(mcp: FastMCP): ``shareWith=<card_id>``). The file stays in its original location; clicking the attachment in the Deck UI opens the file in place. - Use this to link Notes (``/Notes/<title>.md``) or any Files entry to - a card as a discoverable attachment, replacing the older pattern of - long card comments. Calling twice with the same ``path`` creates two - distinct shares — caller is responsible for de-duping. + Generic over the user's Files: works for any file the caller can + read — markdown notes, PDFs, images, spreadsheets, etc. Use + :func:`deck_attach_note` if you have a Notes-app note ID and want + the path resolved automatically. Calling twice with the same + ``path`` creates two distinct shares — caller is responsible for + de-duping. Args: card_id: The ID of the Deck card to attach to path: Path to the file in the user's Nextcloud Files (must start - with "/", e.g. "/Notes/My Note.md") + with "/", e.g. "/Documents/spec.pdf" or "/Notes/My Note.md") """ if not path.startswith("/"): raise ValueError( @@ -1145,14 +1167,7 @@ def configure_deck_tools(mcp: FastMCP): note_id: The ID of the Note to attach """ client = await get_client(ctx) - settings = await client.notes.get_settings() - note = await client.notes.get_note(note_id) - notes_folder = settings.get("notes_path") or "Notes" - path = _resolve_note_path( - notes_folder=notes_folder, - category=note.get("category") or "", - title=note["title"], - ) + path = await _resolve_note_attach_path(client, note_id) share = await client.sharing.create_share( path=path, share_with=str(card_id), diff --git a/tests/client/test_sharing_client.py b/tests/client/test_sharing_client.py index 1723b1c5..f4e40d14 100644 --- a/tests/client/test_sharing_client.py +++ b/tests/client/test_sharing_client.py @@ -11,6 +11,8 @@ from httpx import AsyncClient from nextcloud_mcp_server.client.sharing import SharingClient +pytestmark = pytest.mark.unit + @pytest.fixture def sharing_client(mocker): @@ -32,7 +34,6 @@ def _ok_share_response(mocker, share_id: int = 4242, **extra): return response -@pytest.mark.unit async def test_create_share_deck_type_payload(sharing_client, mocker): """create_share(share_type=12) must POST exactly what the Deck UI does: {path, shareType: 12, shareWith: "<cardId>"} to /ocs/v2.php/apps/files_sharing/api/v1/shares. @@ -65,7 +66,6 @@ async def test_create_share_deck_type_payload(sharing_client, mocker): assert call.kwargs["headers"]["OCS-APIRequest"] == "true" -@pytest.mark.unit async def test_create_share_raises_on_ocs_failure(sharing_client, mocker): """OCS error responses (statuscode != 100/200) raise RuntimeError.""" response = mocker.Mock() diff --git a/tests/unit/test_deck_server.py b/tests/unit/test_deck_server.py index f7ace51c..ff0ab859 100644 --- a/tests/unit/test_deck_server.py +++ b/tests/unit/test_deck_server.py @@ -14,6 +14,7 @@ from nextcloud_mcp_server.server.deck import ( _apply_board_filters, _apply_card_filters, _apply_stack_filters, + _resolve_note_attach_path, _resolve_note_path, _truncate_card_descriptions, _validate_description_max_length, @@ -408,3 +409,62 @@ def test_resolve_note_path_custom_notes_folder(): def test_share_type_deck_constant_matches_deck_app(): """Deck UI uses shareType=12 (IShare::TYPE_DECK) — must not drift.""" assert _SHARE_TYPE_DECK == 12 + + +# _resolve_note_attach_path (camelCase notesPath guard) --------------------- + + +async def test_resolve_note_attach_path_honors_camelcase_notes_path(mocker): + """Custom notes folders configured in the Notes app must be honored. + + Regression: the Notes API returns the folder under ``notesPath`` (camelCase, + see ``models/notes.py:43``). An earlier draft of this code looked up + ``notes_path`` (snake_case) and silently fell back to the default ``"Notes"``, + producing 404s for users with a non-default folder. This test pins the + correct key so that bug can't reappear. + """ + client = mocker.AsyncMock() + client.notes.get_settings.return_value = {"notesPath": "Documents/MyNotes"} + client.notes.get_note.return_value = { + "id": 42, + "title": "Q4 Plan", + "category": "Work", + } + + path = await _resolve_note_attach_path(client, note_id=42) + + assert path == "/Documents/MyNotes/Work/Q4 Plan.md" + client.notes.get_settings.assert_awaited_once() + client.notes.get_note.assert_awaited_once_with(42) + + +async def test_resolve_note_attach_path_falls_back_to_default_when_setting_missing( + mocker, +): + """Missing/empty ``notesPath`` falls back to the documented default.""" + client = mocker.AsyncMock() + client.notes.get_settings.return_value = {} + client.notes.get_note.return_value = { + "id": 1, + "title": "Idea", + "category": "", + } + + path = await _resolve_note_attach_path(client, note_id=1) + + assert path == "/Notes/Idea.md" + + +async def test_resolve_note_attach_path_handles_null_category(mocker): + """A note with ``category=None`` (rather than ``""``) must not crash.""" + client = mocker.AsyncMock() + client.notes.get_settings.return_value = {"notesPath": "Notes"} + client.notes.get_note.return_value = { + "id": 7, + "title": "Bare", + "category": None, + } + + path = await _resolve_note_attach_path(client, note_id=7) + + assert path == "/Notes/Bare.md" From 9072559d26d1bcbdf78456fb637d246d897676da Mon Sep 17 00:00:00 2001 From: Chris Coutinho <chris@coutinho.io> Date: Mon, 11 May 2026 00:22:38 +0200 Subject: [PATCH 3/3] chore: Address reviewers feedback --- nextcloud_mcp_server/server/deck.py | 20 +++++++++++++++++--- 1 file changed, 17 insertions(+), 3 deletions(-) diff --git a/nextcloud_mcp_server/server/deck.py b/nextcloud_mcp_server/server/deck.py index ec8344c5..296a3f09 100644 --- a/nextcloud_mcp_server/server/deck.py +++ b/nextcloud_mcp_server/server/deck.py @@ -1,5 +1,6 @@ import logging +import anyio from mcp.server.fastmcp import Context, FastMCP from mcp.types import ToolAnnotations @@ -137,8 +138,21 @@ async def _resolve_note_attach_path(client, note_id: int) -> str: notes location — that bug is exactly what this helper exists to make testable. """ - settings = await client.notes.get_settings() - note = await client.notes.get_note(note_id) + async with anyio.create_task_group() as tg: + settings_holder: list[dict] = [] + note_holder: list[dict] = [] + + async def _get_settings() -> None: + settings_holder.append(await client.notes.get_settings()) + + async def _get_note() -> None: + note_holder.append(await client.notes.get_note(note_id)) + + tg.start_soon(_get_settings) + tg.start_soon(_get_note) + + settings = settings_holder[0] + note = note_holder[0] notes_folder = settings.get("notesPath") or "Notes" return _resolve_note_path( notes_folder=notes_folder, @@ -1144,7 +1158,7 @@ def configure_deck_tools(mcp: FastMCP): title="Attach Note to Deck Card", annotations=ToolAnnotations(idempotentHint=False, openWorldHint=True), ) - @require_scopes("deck.write", "notes.read") + @require_scopes("deck.write", "files.read", "notes.read") @instrument_tool async def deck_attach_note( ctx: Context, card_id: int, note_id: int