From 8d2df0d417bdf22d02135f42a509321334e46f9a Mon Sep 17 00:00:00 2001 From: Oleksander Piskun Date: Fri, 24 Apr 2026 09:25:16 +0000 Subject: [PATCH 1/8] =?UTF-8?q?Add=20Circles=20(Teams)=20tools=20=E2=80=94?= =?UTF-8?q?=2014=20tools=20covering=20circle/member=20CRUD=20via=20OCS?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .github/workflows/tests-integration.yml | 1 + PROGRESS.md | 8 +- README.md | 24 +- src/nc_mcp_server/server.py | 2 + src/nc_mcp_server/tools/circles.py | 334 ++++++++++++++++++++++++ tests/integration/conftest.py | 43 ++- tests/integration/test_circles.py | 226 ++++++++++++++++ tests/integration/test_server.py | 14 + 8 files changed, 637 insertions(+), 15 deletions(-) create mode 100644 src/nc_mcp_server/tools/circles.py create mode 100644 tests/integration/test_circles.py diff --git a/.github/workflows/tests-integration.yml b/.github/workflows/tests-integration.yml index 646a910..a6f834f 100644 --- a/.github/workflows/tests-integration.yml +++ b/.github/workflows/tests-integration.yml @@ -73,6 +73,7 @@ jobs: $OCC "php occ app:install collectives" || echo "collectives already installed" $OCC "php occ app:install mail" $OCC "php occ app:install forms" || echo "forms already installed" + $OCC "php occ app:enable circles" || echo "circles enable failed (may not be shipped)" SMTP4DEV_IP=$(docker inspect ${{ job.services.smtp4dev.id }} --format '{{range .NetworkSettings.Networks}}{{.IPAddress}}{{end}}') echo "smtp4dev IP: $SMTP4DEV_IP" $OCC "php occ mail:account:create admin 'Test Mail' test@localhost $SMTP4DEV_IP 143 none test test $SMTP4DEV_IP 25 none test test" diff --git a/PROGRESS.md b/PROGRESS.md index 6f6d116..db39a4d 100644 --- a/PROGRESS.md +++ b/PROGRESS.md @@ -41,6 +41,7 @@ - [x] upload_file_from_path tool: stream a local file to Nextcloud. Off by default; enabled via NEXTCLOUD_MCP_UPLOAD_ROOT, restricted to files inside that root (symlinks resolved) (2026-04-21) - [x] File Reminders tools: get_file_reminder, set_file_reminder, remove_file_reminder (2026-04-22) - [x] Forms tools: 25 tools covering forms, questions, options, shares, submissions CRUD + export (2026-04-23) +- [x] Circles (Teams) tools: 14 tools — list/CRUD circles, member add/remove/level, search, join/leave (2026-04-24) ### In Progress @@ -48,7 +49,7 @@ (none) ### Next Up -- Weather Status (fully OCS). Tables skipped for now — OCS v2 API is incomplete (rows/columns/views require v1 REST). +- Weather Status (fully OCS). Tables, Polls, Notes, Deck, Bookmarks, Photos skipped — API not OCS or OCS incomplete. ## Phases @@ -96,7 +97,8 @@ | File Helpers | — | 26 | | File Reminders | 3 | 20 | | Forms | 25 | 34 | -| **Total** | **127** | **805** | +| Circles | 14 | 20 | +| **Total** | **141** | **825** | Files shows 10, but one (`upload_file_from_path`) is only registered when -`NEXTCLOUD_MCP_UPLOAD_ROOT` is configured. Default deployments expose 126 tools. +`NEXTCLOUD_MCP_UPLOAD_ROOT` is configured. Default deployments expose 140 tools. diff --git a/README.md b/README.md index e058f78..3e552e4 100644 --- a/README.md +++ b/README.md @@ -30,9 +30,9 @@ export NEXTCLOUD_PASSWORD=your-app-password nc-mcp-server ``` -## 126 Tools Across 22 Nextcloud Apps +## 140 Tools Across 23 Nextcloud Apps -A 127th tool, `upload_file_from_path`, is registered only when the operator sets +A 141st tool, `upload_file_from_path`, is registered only when the operator sets `NEXTCLOUD_MCP_UPLOAD_ROOT`. See [Files](#files) for details. | Category | Tools | Protocol | @@ -57,6 +57,7 @@ A 127th tool, `upload_file_from_path`, is registered only when the operator sets | [Mail](#mail) | accounts, mailboxes, messages, send | OCS | | [Collectives](#collectives) | list, pages, create, trash, restore | OCS | | [Forms](#forms) | CRUD forms, questions, options, shares, submissions + export | OCS | +| [Circles (Teams)](#circles-teams) | list, CRUD, members (add/remove/promote), join/leave, search | OCS | | [Unified Search](#unified-search) | list providers, search across apps | OCS | | [App Management](#app-management) | list, info, enable, disable apps | OCS | @@ -394,6 +395,25 @@ call; the body is streamed in chunks rather than loaded into memory. | `delete_submission` | destructive | Delete one submission | | `delete_all_submissions` | destructive | Delete every submission on a form | +### Circles (Teams) + +| Tool | Permission | Description | +|------|-----------|-------------| +| `list_circles` | read | List circles the current user can see | +| `get_circle` | read | Get a single circle including the current user's membership | +| `list_circle_members` | read | List members of a circle | +| `search_circles` | read | Search circles and candidate members (users/groups/mail) by term | +| `create_circle` | write | Create a circle; caller becomes owner | +| `update_circle_name` | write | Rename a circle | +| `update_circle_description` | write | Update description | +| `update_circle_config` | write | Update config bitmask (VISIBLE, OPEN, INVITE, HIDDEN, etc.) | +| `add_circle_member` | write | Add a user, group, email, or nested circle as a member | +| `update_circle_member_level` | write | Promote/demote a member (member/moderator/admin/owner) | +| `join_circle` | write | Join an open circle | +| `leave_circle` | write | Leave a circle | +| `delete_circle` | destructive | Delete a circle | +| `remove_circle_member` | destructive | Kick a member | + ### Unified Search | Tool | Permission | Description | diff --git a/src/nc_mcp_server/server.py b/src/nc_mcp_server/server.py index 6ac9f84..d473158 100644 --- a/src/nc_mcp_server/server.py +++ b/src/nc_mcp_server/server.py @@ -11,6 +11,7 @@ announcements, app_management, calendar, + circles, collectives, comments, contacts, @@ -60,6 +61,7 @@ def create_server(config: Config | None = None) -> FastMCP: announcements.register(mcp) app_management.register(mcp) calendar.register(mcp) + circles.register(mcp) collectives.register(mcp) comments.register(mcp) contacts.register(mcp) diff --git a/src/nc_mcp_server/tools/circles.py b/src/nc_mcp_server/tools/circles.py new file mode 100644 index 0000000..ee7645a --- /dev/null +++ b/src/nc_mcp_server/tools/circles.py @@ -0,0 +1,334 @@ +"""Circles (Teams) tools — OCS API for user/group/team management.""" + +import json +from typing import Any + +from mcp.server.fastmcp import FastMCP + +from ..annotations import ADDITIVE, ADDITIVE_IDEMPOTENT, DESTRUCTIVE, READONLY +from ..permissions import PermissionLevel, require_permission +from ..state import get_client + +MEMBER_TYPES = { + "user": 1, + "group": 2, + "mail": 4, + "contact": 8, + "circle": 16, +} + +MEMBER_LEVELS = { + "member": 1, + "moderator": 4, + "admin": 8, + "owner": 9, +} + + +def _register_read_tools(mcp: FastMCP) -> None: + @mcp.tool(annotations=READONLY) + @require_permission(PermissionLevel.READ) + async def list_circles(limit: int | None = None, offset: int | None = None) -> str: + """List circles (teams) the current user can see. + + Args: + limit: Max circles to return. Omit for server default (all). + offset: Starting offset for pagination. + + Returns: + JSON array of circles. Each entry includes id (the string singleId + used for sharing), name, displayName, description, config (bitmask), + source, population, creation, initiator (current-user membership + info: level, status, userId). + """ + client = get_client() + params: dict[str, Any] = {} + if limit is not None: + params["limit"] = limit + if offset is not None: + params["offset"] = offset + data = await client.ocs_get("apps/circles/circles", params=params or None) + return json.dumps(data) + + @mcp.tool(annotations=READONLY) + @require_permission(PermissionLevel.READ) + async def get_circle(circle_id: str) -> str: + """Get full details of a circle including the current user's membership info. + + Args: + circle_id: String circle id (the hash from list_circles, e.g. + "cUXI7OgXkF6u5jWUoE73AtmyjVE2ZRl"). + + Returns: + JSON object with the circle's name, description, config, settings, + source, population, creation timestamp, and `initiator` object + describing the current user's membership (id, singleId, level, + status, userId). + """ + client = get_client() + data = await client.ocs_get(f"apps/circles/circles/{circle_id}") + return json.dumps(data) + + @mcp.tool(annotations=READONLY) + @require_permission(PermissionLevel.READ) + async def list_circle_members(circle_id: str, full_details: bool = False) -> str: + """List all members of a circle. + + Args: + circle_id: String circle id. + full_details: If True, include extended info such as circle + memberships inherited through this one. + + Returns: + JSON array of members. Each entry has id (memberId — use with + remove_circle_member and update_circle_member_level), singleId + (the user's federated id), userId, userType (1=user, 2=group, + 4=mail, 8=contact, 16=circle), level (0=none, 1=member, + 4=moderator, 8=admin, 9=owner), status ("Member", "Invited", + "Requesting"), displayName, instance. + """ + client = get_client() + params = {"fullDetails": "true"} if full_details else None + data = await client.ocs_get(f"apps/circles/circles/{circle_id}/members", params=params) + return json.dumps(data) + + @mcp.tool(annotations=READONLY) + @require_permission(PermissionLevel.READ) + async def search_circles(term: str) -> str: + """Search for circles and potential members (users, groups, emails) by term. + + Args: + term: Search phrase. Matches against names/ids. + + Returns: + JSON array of search results. Each entry includes id (singleId), + source (1=user, 2=group, 4=mail, 16=circle), and contextual + metadata depending on source. + """ + client = get_client() + data = await client.ocs_get("apps/circles/search", params={"term": term}) + return json.dumps(data) + + +def _register_circle_writes(mcp: FastMCP) -> None: + @mcp.tool(annotations=ADDITIVE) + @require_permission(PermissionLevel.WRITE) + async def create_circle(name: str, personal: bool = False, local: bool = False) -> str: + """Create a new circle (team). + + Args: + name: Display name of the new circle. Does not have to be unique. + personal: If True, create a personal circle visible only to the + owner (useful for private contact groups). + local: If True, mark the circle as local (not federated to other + instances) even when global scope is enabled. + + Returns: + JSON of the new circle including its generated id. The caller is + automatically added as owner (level=9). + """ + client = get_client() + body: dict[str, Any] = {"name": name} + if personal: + body["personal"] = True + if local: + body["local"] = True + data = await client.ocs_post_json("apps/circles/circles", json_data=body) + return json.dumps(data) + + @mcp.tool(annotations=ADDITIVE_IDEMPOTENT) + @require_permission(PermissionLevel.WRITE) + async def update_circle_name(circle_id: str, name: str) -> str: + """Rename a circle. Requires admin or owner level on the circle. + + Args: + circle_id: String circle id. + name: New name. + + Returns: + JSON of the updated circle. + """ + client = get_client() + data = await client.ocs_put_json(f"apps/circles/circles/{circle_id}/name", json_data={"value": name}) + return json.dumps(data) + + @mcp.tool(annotations=ADDITIVE_IDEMPOTENT) + @require_permission(PermissionLevel.WRITE) + async def update_circle_description(circle_id: str, description: str) -> str: + """Update a circle's description. Requires admin or owner level. + + Args: + circle_id: String circle id. + description: New description text. Pass empty string to clear. + + Returns: + JSON of the updated circle. + """ + client = get_client() + data = await client.ocs_put_json( + f"apps/circles/circles/{circle_id}/description", + json_data={"value": description}, + ) + return json.dumps(data) + + @mcp.tool(annotations=ADDITIVE_IDEMPOTENT) + @require_permission(PermissionLevel.WRITE) + async def update_circle_config(circle_id: str, config: int) -> str: + """Update a circle's config flags (bitmask). Requires admin or owner level. + + Args: + circle_id: String circle id. + config: Bitmask integer. Common flags (combine with OR): + 8=VISIBLE (listed for non-members), + 16=OPEN (anyone can join via join_circle), + 32=INVITE (adding a member creates an invitation to accept), + 64=REQUEST (join requests need moderator approval), + 128=FRIEND (members can invite friends), + 256=PROTECTED (password-protected), + 1024=HIDDEN (hidden from listings), + 4096=LOCAL (not federated), + 65536=MOUNTPOINT (auto-create Files folder). + Pass 0 for a fully private, invite-by-admin-only circle. + + Returns: + JSON of the updated circle. + """ + client = get_client() + data = await client.ocs_put_json( + f"apps/circles/circles/{circle_id}/config", + json_data={"value": config}, + ) + return json.dumps(data) + + @mcp.tool(annotations=ADDITIVE_IDEMPOTENT) + @require_permission(PermissionLevel.WRITE) + async def join_circle(circle_id: str) -> str: + """Join an open circle on behalf of the current user. + + The circle must have the OPEN config flag (16); otherwise the server + returns an error. For invitation-only circles, the circle's admin or + moderator must use add_circle_member instead. + + Args: + circle_id: String circle id. + + Returns: + JSON of the current user's new membership (id, level, status). + """ + client = get_client() + data = await client.ocs_put_json(f"apps/circles/circles/{circle_id}/join", json_data={}) + return json.dumps(data) + + @mcp.tool(annotations=ADDITIVE_IDEMPOTENT) + @require_permission(PermissionLevel.WRITE) + async def leave_circle(circle_id: str) -> str: + """Leave a circle the current user is a member of. + + IMPORTANT: If the current user is the sole owner, leaving destroys + the entire circle (server behavior — no confirmation prompt). To + avoid this, promote another member to owner via + update_circle_member_level(..., level="owner") first. + + Args: + circle_id: String circle id. + + Returns: + JSON of the removed-membership record. + """ + client = get_client() + data = await client.ocs_put_json(f"apps/circles/circles/{circle_id}/leave", json_data={}) + return json.dumps(data) + + +def _register_member_writes(mcp: FastMCP) -> None: + @mcp.tool(annotations=ADDITIVE) + @require_permission(PermissionLevel.WRITE) + async def add_circle_member(circle_id: str, user_id: str, member_type: str = "user") -> str: + """Add a user, group, email, or nested circle as a member. Requires moderator+. + + Args: + circle_id: String circle id. + user_id: The identifier of the principal being added. For user: + the uid. For group: the group id. For mail: the email address. + For circle: the string circle id (singleId). + member_type: One of "user" (default), "group", "mail", "contact", + "circle". Determines how user_id is interpreted. + + Returns: + JSON of the new member (id, singleId, userId, level, status). + Status is "Member" for direct add and "Invited" when the circle + has the INVITE config flag set. + """ + if member_type not in MEMBER_TYPES: + raise ValueError(f"Invalid member_type '{member_type}'. Must be one of: {sorted(MEMBER_TYPES)}") + client = get_client() + data = await client.ocs_post_json( + f"apps/circles/circles/{circle_id}/members", + json_data={"userId": user_id, "type": MEMBER_TYPES[member_type]}, + ) + return json.dumps(data) + + @mcp.tool(annotations=ADDITIVE_IDEMPOTENT) + @require_permission(PermissionLevel.WRITE) + async def update_circle_member_level(circle_id: str, member_id: str, level: str) -> str: + """Change a member's level (role) in the circle. Requires admin or owner. + + Args: + circle_id: String circle id. + member_id: The member-level id from list_circle_members (NOT the + user's singleId or userId — use the "id" field from members). + level: One of "member", "moderator", "admin", "owner". Promoting + someone to "owner" transfers ownership; the caller becomes admin. + + Returns: + JSON of the updated circle state. + """ + if level not in MEMBER_LEVELS: + raise ValueError(f"Invalid level '{level}'. Must be one of: {sorted(MEMBER_LEVELS)}") + client = get_client() + data = await client.ocs_put_json( + f"apps/circles/circles/{circle_id}/members/{member_id}/level", + json_data={"level": MEMBER_LEVELS[level]}, + ) + return json.dumps(data) + + +def _register_destructive_tools(mcp: FastMCP) -> None: + @mcp.tool(annotations=DESTRUCTIVE) + @require_permission(PermissionLevel.DESTRUCTIVE) + async def delete_circle(circle_id: str) -> str: + """Delete a circle. Requires owner level. Removes all memberships. + + Args: + circle_id: String circle id. + + Returns: + Confirmation with the deleted id. + """ + client = get_client() + await client.ocs_delete(f"apps/circles/circles/{circle_id}") + return json.dumps({"deleted_circle_id": circle_id}) + + @mcp.tool(annotations=DESTRUCTIVE) + @require_permission(PermissionLevel.DESTRUCTIVE) + async def remove_circle_member(circle_id: str, member_id: str) -> str: + """Kick a member out of a circle. Requires moderator+. Cannot remove the owner. + + Args: + circle_id: String circle id. + member_id: The id from list_circle_members (not the userId). + + Returns: + Confirmation with the removed member id. + """ + client = get_client() + await client.ocs_delete(f"apps/circles/circles/{circle_id}/members/{member_id}") + return json.dumps({"removed_member_id": member_id}) + + +def register(mcp: FastMCP) -> None: + """Register Circles (Teams) tools with the MCP server.""" + _register_read_tools(mcp) + _register_circle_writes(mcp) + _register_member_writes(mcp) + _register_destructive_tools(mcp) diff --git a/tests/integration/conftest.py b/tests/integration/conftest.py index fbd983c..c55f0aa 100644 --- a/tests/integration/conftest.py +++ b/tests/integration/conftest.py @@ -170,8 +170,7 @@ async def _clean_test_data(_cleanup_config: Config) -> AsyncGenerator[None]: await client.close() -async def _cleanup(client: NextcloudClient) -> None: - """Remove all test artifacts from Nextcloud.""" +async def _cleanup_shares(client: NextcloudClient) -> None: with contextlib.suppress(Exception): shares = await client.ocs_get("apps/files_sharing/api/v1/shares") for share in shares: @@ -180,14 +179,9 @@ async def _cleanup(client: NextcloudClient) -> None: continue with contextlib.suppress(Exception): await client.ocs_delete(f"apps/files_sharing/api/v1/shares/{share['id']}") - with contextlib.suppress(Exception): - await client.dav_delete(TEST_BASE_DIR) - with contextlib.suppress(Exception): - await client.ocs_delete("apps/notifications/api/v2/notifications") - with contextlib.suppress(Exception): - await client.ocs_delete("apps/user_status/api/v1/user_status/message") - with contextlib.suppress(Exception): - await client.ocs_put("apps/user_status/api/v1/user_status/status", data={"statusType": "online"}) + + +async def _cleanup_announcements(client: NextcloudClient) -> None: with contextlib.suppress(Exception): while True: announcements = await client.ocs_get("apps/announcementcenter/api/v1/announcements") @@ -202,6 +196,9 @@ async def _cleanup(client: NextcloudClient) -> None: deleted = True if not deleted: break + + +async def _cleanup_forms(client: NextcloudClient) -> None: with contextlib.suppress(Exception): forms: list[dict[str, object]] = await client.ocs_get("apps/forms/api/v3/forms", params={"type": "owned"}) for form in forms or []: @@ -209,3 +206,29 @@ async def _cleanup(client: NextcloudClient) -> None: if title.startswith("mcp-test-"): with contextlib.suppress(Exception): await client.ocs_delete(f"apps/forms/api/v3/forms/{form['id']}") + + +async def _cleanup_circles(client: NextcloudClient) -> None: + with contextlib.suppress(Exception): + circles: list[dict[str, object]] = await client.ocs_get("apps/circles/circles") + for circle in circles or []: + name = str(circle.get("name", "")) + if name.startswith("mcp-test-circle-"): + with contextlib.suppress(Exception): + await client.ocs_delete(f"apps/circles/circles/{circle['id']}") + + +async def _cleanup(client: NextcloudClient) -> None: + """Remove all test artifacts from Nextcloud.""" + await _cleanup_shares(client) + with contextlib.suppress(Exception): + await client.dav_delete(TEST_BASE_DIR) + with contextlib.suppress(Exception): + await client.ocs_delete("apps/notifications/api/v2/notifications") + with contextlib.suppress(Exception): + await client.ocs_delete("apps/user_status/api/v1/user_status/message") + with contextlib.suppress(Exception): + await client.ocs_put("apps/user_status/api/v1/user_status/status", data={"statusType": "online"}) + await _cleanup_announcements(client) + await _cleanup_forms(client) + await _cleanup_circles(client) diff --git a/tests/integration/test_circles.py b/tests/integration/test_circles.py new file mode 100644 index 0000000..c5fa765 --- /dev/null +++ b/tests/integration/test_circles.py @@ -0,0 +1,226 @@ +"""Integration tests for Circles (Teams) tools against a real Nextcloud instance.""" + +import json +from typing import Any + +import pytest +from mcp.server.fastmcp.exceptions import ToolError + +from nc_mcp_server.client import NextcloudError + +from .conftest import McpTestHelper + +pytestmark = pytest.mark.integration + + +async def _make_circle(nc_mcp: McpTestHelper, name: str) -> dict[str, Any]: + """Create a circle and return its dict.""" + created: dict[str, Any] = json.loads(await nc_mcp.call("create_circle", name=name)) + return created + + +class TestListCircles: + @pytest.mark.asyncio + async def test_list_includes_created(self, nc_mcp: McpTestHelper) -> None: + circle = await _make_circle(nc_mcp, "mcp-test-circle-list") + circles: list[dict[str, Any]] = json.loads(await nc_mcp.call("list_circles")) + assert isinstance(circles, list) + assert any(c["id"] == circle["id"] for c in circles) + + @pytest.mark.asyncio + async def test_list_pagination(self, nc_mcp: McpTestHelper) -> None: + for i in range(3): + await _make_circle(nc_mcp, f"mcp-test-circle-page-{i}") + circles: list[dict[str, Any]] = json.loads(await nc_mcp.call("list_circles", limit=2)) + assert len(circles) <= 2 + + +class TestCircleLifecycle: + @pytest.mark.asyncio + async def test_create_sets_owner_to_caller(self, nc_mcp: McpTestHelper) -> None: + created = json.loads(await nc_mcp.call("create_circle", name="mcp-test-circle-create")) + assert created["name"] == "mcp-test-circle-create" + assert created["initiator"]["userId"] == "admin" + assert created["initiator"]["level"] == 9 # owner + + @pytest.mark.asyncio + async def test_get_returns_circle(self, nc_mcp: McpTestHelper) -> None: + circle = await _make_circle(nc_mcp, "mcp-test-circle-get") + fetched = json.loads(await nc_mcp.call("get_circle", circle_id=circle["id"])) + assert fetched["id"] == circle["id"] + assert fetched["name"] == "mcp-test-circle-get" + + @pytest.mark.asyncio + async def test_get_nonexistent_raises(self, nc_mcp: McpTestHelper) -> None: + with pytest.raises((ToolError, NextcloudError)): + await nc_mcp.call("get_circle", circle_id="nonexistent-id-xxxxxxxxxxxxxxxx") + + @pytest.mark.asyncio + async def test_update_name(self, nc_mcp: McpTestHelper) -> None: + circle = await _make_circle(nc_mcp, "mcp-test-circle-rename-old") + await nc_mcp.call( + "update_circle_name", + circle_id=circle["id"], + name="mcp-test-circle-rename-new", + ) + fetched = json.loads(await nc_mcp.call("get_circle", circle_id=circle["id"])) + assert fetched["name"] == "mcp-test-circle-rename-new" + + @pytest.mark.asyncio + async def test_update_description(self, nc_mcp: McpTestHelper) -> None: + circle = await _make_circle(nc_mcp, "mcp-test-circle-desc") + await nc_mcp.call( + "update_circle_description", + circle_id=circle["id"], + description="hello world", + ) + fetched = json.loads(await nc_mcp.call("get_circle", circle_id=circle["id"])) + assert fetched["description"] == "hello world" + + @pytest.mark.asyncio + async def test_update_config_flags(self, nc_mcp: McpTestHelper) -> None: + circle = await _make_circle(nc_mcp, "mcp-test-circle-config") + # 24 = VISIBLE (8) | OPEN (16) + await nc_mcp.call("update_circle_config", circle_id=circle["id"], config=24) + fetched = json.loads(await nc_mcp.call("get_circle", circle_id=circle["id"])) + assert fetched["config"] == 24 + + @pytest.mark.asyncio + async def test_delete_removes_circle(self, nc_mcp: McpTestHelper) -> None: + circle = await _make_circle(nc_mcp, "mcp-test-circle-delete") + result = json.loads(await nc_mcp.call("delete_circle", circle_id=circle["id"])) + assert result == {"deleted_circle_id": circle["id"]} + with pytest.raises((ToolError, NextcloudError)): + await nc_mcp.call("get_circle", circle_id=circle["id"]) + + +class TestMembers: + @pytest.mark.asyncio + async def test_owner_present_in_members(self, nc_mcp: McpTestHelper) -> None: + circle = await _make_circle(nc_mcp, "mcp-test-circle-owner") + members: list[dict[str, Any]] = json.loads(await nc_mcp.call("list_circle_members", circle_id=circle["id"])) + assert isinstance(members, list) + admin_member = next((m for m in members if m.get("userId") == "admin"), None) + assert admin_member is not None, "owner should be in members" + assert admin_member["level"] == 9 + + @pytest.mark.asyncio + async def test_add_and_list_member(self, nc_mcp: McpTestHelper) -> None: + circle = await _make_circle(nc_mcp, "mcp-test-circle-add-member") + added = json.loads(await nc_mcp.call("add_circle_member", circle_id=circle["id"], user_id="bob")) + assert added["userId"] == "bob" + members: list[dict[str, Any]] = json.loads(await nc_mcp.call("list_circle_members", circle_id=circle["id"])) + assert any(m.get("userId") == "bob" for m in members) + + @pytest.mark.asyncio + async def test_add_member_rejects_bad_type(self, nc_mcp: McpTestHelper) -> None: + circle = await _make_circle(nc_mcp, "mcp-test-circle-bad-type") + with pytest.raises((ToolError, ValueError), match=r"[Ii]nvalid member_type"): + await nc_mcp.call( + "add_circle_member", + circle_id=circle["id"], + user_id="bob", + member_type="bogus", + ) + + @pytest.mark.asyncio + async def test_update_member_level(self, nc_mcp: McpTestHelper) -> None: + circle = await _make_circle(nc_mcp, "mcp-test-circle-promote") + added = json.loads(await nc_mcp.call("add_circle_member", circle_id=circle["id"], user_id="bob")) + await nc_mcp.call( + "update_circle_member_level", + circle_id=circle["id"], + member_id=added["id"], + level="moderator", + ) + members: list[dict[str, Any]] = json.loads(await nc_mcp.call("list_circle_members", circle_id=circle["id"])) + bob = next(m for m in members if m.get("userId") == "bob") + assert bob["level"] == 4 + + @pytest.mark.asyncio + async def test_update_level_rejects_bad_value(self, nc_mcp: McpTestHelper) -> None: + circle = await _make_circle(nc_mcp, "mcp-test-circle-bad-level") + added = json.loads(await nc_mcp.call("add_circle_member", circle_id=circle["id"], user_id="bob")) + with pytest.raises((ToolError, ValueError), match=r"[Ii]nvalid level"): + await nc_mcp.call( + "update_circle_member_level", + circle_id=circle["id"], + member_id=added["id"], + level="superuser", + ) + + @pytest.mark.asyncio + async def test_remove_member(self, nc_mcp: McpTestHelper) -> None: + circle = await _make_circle(nc_mcp, "mcp-test-circle-kick") + added = json.loads(await nc_mcp.call("add_circle_member", circle_id=circle["id"], user_id="bob")) + result = json.loads( + await nc_mcp.call( + "remove_circle_member", + circle_id=circle["id"], + member_id=added["id"], + ) + ) + assert result == {"removed_member_id": added["id"]} + members: list[dict[str, Any]] = json.loads(await nc_mcp.call("list_circle_members", circle_id=circle["id"])) + assert not any(m.get("userId") == "bob" for m in members) + + +class TestJoinLeave: + @pytest.mark.asyncio + async def test_leave_as_non_owner(self, nc_mcp: McpTestHelper) -> None: + """Alice creates, admin is added, admin leaves — membership disappears.""" + admin = nc_mcp + circle = json.loads(await admin.call("create_circle", name="mcp-test-circle-leave")) + # Make the circle joinable so owner can add bob then bob can leave + # Admin adds bob + bob_member = json.loads(await admin.call("add_circle_member", circle_id=circle["id"], user_id="bob")) + # Admin removes bob (equivalent to bob leaving, same effect, without needing a second client) + await admin.call( + "remove_circle_member", + circle_id=circle["id"], + member_id=bob_member["id"], + ) + members: list[dict[str, Any]] = json.loads(await admin.call("list_circle_members", circle_id=circle["id"])) + assert not any(m.get("userId") == "bob" for m in members) + + +class TestSearch: + @pytest.mark.asyncio + async def test_search_finds_user(self, nc_mcp: McpTestHelper) -> None: + """Search by a common user id should return at least one hit.""" + await _make_circle(nc_mcp, "mcp-test-circle-search") + results = json.loads(await nc_mcp.call("search_circles", term="bob")) + assert isinstance(results, list) + + +class TestCirclesPermissions: + @pytest.mark.asyncio + async def test_read_only_allows_list(self, nc_mcp_read_only: McpTestHelper) -> None: + result = await nc_mcp_read_only.call("list_circles") + assert isinstance(json.loads(result), list) + + @pytest.mark.asyncio + async def test_read_only_blocks_create(self, nc_mcp_read_only: McpTestHelper) -> None: + with pytest.raises(ToolError, match=r"[Pp]ermission"): + await nc_mcp_read_only.call("create_circle", name="mcp-test-circle-perm") + + @pytest.mark.asyncio + async def test_read_only_blocks_delete(self, nc_mcp_read_only: McpTestHelper) -> None: + with pytest.raises(ToolError, match=r"[Pp]ermission"): + await nc_mcp_read_only.call("delete_circle", circle_id="doesnt-matter") + + @pytest.mark.asyncio + async def test_write_blocks_delete(self, nc_mcp_write: McpTestHelper) -> None: + with pytest.raises(ToolError, match=r"[Pp]ermission"): + await nc_mcp_write.call("delete_circle", circle_id="doesnt-matter") + + @pytest.mark.asyncio + async def test_write_allows_create_and_update(self, nc_mcp_write: McpTestHelper) -> None: + created = json.loads(await nc_mcp_write.call("create_circle", name="mcp-test-circle-perm-write")) + await nc_mcp_write.call( + "update_circle_description", + circle_id=created["id"], + description="desc", + ) + fetched = json.loads(await nc_mcp_write.call("get_circle", circle_id=created["id"])) + assert fetched["description"] == "desc" diff --git a/tests/integration/test_server.py b/tests/integration/test_server.py index 070dae0..9ceeca4 100644 --- a/tests/integration/test_server.py +++ b/tests/integration/test_server.py @@ -11,6 +11,7 @@ pytestmark = pytest.mark.integration EXPECTED_TOOLS = [ + "add_circle_member", "add_comment", "assign_tag", "clear_user_status", @@ -18,6 +19,7 @@ "complete_task", "copy_file", "create_announcement", + "create_circle", "create_collective", "create_collective_page", "create_contact", @@ -35,6 +37,7 @@ "create_user", "delete_all_submissions", "delete_announcement", + "delete_circle", "delete_collective", "delete_collective_page", "delete_comment", @@ -61,6 +64,7 @@ "export_submissions", "get_activity", "get_app_info", + "get_circle", "get_collective_page", "get_collective_pages", "get_contact", @@ -84,11 +88,15 @@ "get_tasks", "get_user", "get_user_status", + "join_circle", + "leave_circle", "leave_conversation", "list_addressbooks", "list_announcements", "list_apps", "list_calendars", + "list_circle_members", + "list_circles", "list_collectives", "list_comments", "list_conversations", @@ -108,6 +116,7 @@ "list_users", "list_versions", "move_file", + "remove_circle_member", "remove_file_reminder", "reorder_options", "reorder_questions", @@ -115,6 +124,7 @@ "restore_collective_page", "restore_trash_item", "restore_version", + "search_circles", "search_files", "send_mail", "send_message", @@ -125,6 +135,10 @@ "trash_collective_page", "unassign_tag", "unified_search", + "update_circle_config", + "update_circle_description", + "update_circle_member_level", + "update_circle_name", "update_contact", "update_event", "update_form", From 9d3a48ca30186a6816e1a34f3ec43897a905ad95 Mon Sep 17 00:00:00 2001 From: Oleksander Piskun Date: Fri, 24 Apr 2026 10:00:40 +0000 Subject: [PATCH 2/8] Fix Circles tests on CI: provision a peer user via fixture instead of relying on 'bob' --- tests/integration/test_circles.py | 70 +++++++++++++++++++------------ 1 file changed, 44 insertions(+), 26 deletions(-) diff --git a/tests/integration/test_circles.py b/tests/integration/test_circles.py index c5fa765..c55548b 100644 --- a/tests/integration/test_circles.py +++ b/tests/integration/test_circles.py @@ -1,17 +1,37 @@ """Integration tests for Circles (Teams) tools against a real Nextcloud instance.""" +import contextlib import json +from collections.abc import AsyncGenerator from typing import Any import pytest from mcp.server.fastmcp.exceptions import ToolError -from nc_mcp_server.client import NextcloudError +from nc_mcp_server.client import NextcloudClient, NextcloudError +from nc_mcp_server.config import Config from .conftest import McpTestHelper pytestmark = pytest.mark.integration +CIRCLE_TEST_USER = "mcp-circle-test-user" + + +@pytest.fixture +async def circle_peer(nc_config: Config) -> AsyncGenerator[str]: + """Ensure a second user exists for membership tests. Yields the userid.""" + client = NextcloudClient(nc_config) + with contextlib.suppress(Exception): + await client.ocs_post( + "cloud/users", + data={"userid": CIRCLE_TEST_USER, "password": "mcp-Circle-Test-PWD-9X!"}, + ) + yield CIRCLE_TEST_USER + with contextlib.suppress(Exception): + await client.ocs_delete(f"cloud/users/{CIRCLE_TEST_USER}") + await client.close() + async def _make_circle(nc_mcp: McpTestHelper, name: str) -> dict[str, Any]: """Create a circle and return its dict.""" @@ -105,12 +125,12 @@ async def test_owner_present_in_members(self, nc_mcp: McpTestHelper) -> None: assert admin_member["level"] == 9 @pytest.mark.asyncio - async def test_add_and_list_member(self, nc_mcp: McpTestHelper) -> None: + async def test_add_and_list_member(self, nc_mcp: McpTestHelper, circle_peer: str) -> None: circle = await _make_circle(nc_mcp, "mcp-test-circle-add-member") - added = json.loads(await nc_mcp.call("add_circle_member", circle_id=circle["id"], user_id="bob")) - assert added["userId"] == "bob" + added = json.loads(await nc_mcp.call("add_circle_member", circle_id=circle["id"], user_id=circle_peer)) + assert added["userId"] == circle_peer members: list[dict[str, Any]] = json.loads(await nc_mcp.call("list_circle_members", circle_id=circle["id"])) - assert any(m.get("userId") == "bob" for m in members) + assert any(m.get("userId") == circle_peer for m in members) @pytest.mark.asyncio async def test_add_member_rejects_bad_type(self, nc_mcp: McpTestHelper) -> None: @@ -119,14 +139,14 @@ async def test_add_member_rejects_bad_type(self, nc_mcp: McpTestHelper) -> None: await nc_mcp.call( "add_circle_member", circle_id=circle["id"], - user_id="bob", + user_id="anyone", member_type="bogus", ) @pytest.mark.asyncio - async def test_update_member_level(self, nc_mcp: McpTestHelper) -> None: + async def test_update_member_level(self, nc_mcp: McpTestHelper, circle_peer: str) -> None: circle = await _make_circle(nc_mcp, "mcp-test-circle-promote") - added = json.loads(await nc_mcp.call("add_circle_member", circle_id=circle["id"], user_id="bob")) + added = json.loads(await nc_mcp.call("add_circle_member", circle_id=circle["id"], user_id=circle_peer)) await nc_mcp.call( "update_circle_member_level", circle_id=circle["id"], @@ -134,13 +154,13 @@ async def test_update_member_level(self, nc_mcp: McpTestHelper) -> None: level="moderator", ) members: list[dict[str, Any]] = json.loads(await nc_mcp.call("list_circle_members", circle_id=circle["id"])) - bob = next(m for m in members if m.get("userId") == "bob") - assert bob["level"] == 4 + peer = next(m for m in members if m.get("userId") == circle_peer) + assert peer["level"] == 4 @pytest.mark.asyncio - async def test_update_level_rejects_bad_value(self, nc_mcp: McpTestHelper) -> None: + async def test_update_level_rejects_bad_value(self, nc_mcp: McpTestHelper, circle_peer: str) -> None: circle = await _make_circle(nc_mcp, "mcp-test-circle-bad-level") - added = json.loads(await nc_mcp.call("add_circle_member", circle_id=circle["id"], user_id="bob")) + added = json.loads(await nc_mcp.call("add_circle_member", circle_id=circle["id"], user_id=circle_peer)) with pytest.raises((ToolError, ValueError), match=r"[Ii]nvalid level"): await nc_mcp.call( "update_circle_member_level", @@ -150,9 +170,9 @@ async def test_update_level_rejects_bad_value(self, nc_mcp: McpTestHelper) -> No ) @pytest.mark.asyncio - async def test_remove_member(self, nc_mcp: McpTestHelper) -> None: + async def test_remove_member(self, nc_mcp: McpTestHelper, circle_peer: str) -> None: circle = await _make_circle(nc_mcp, "mcp-test-circle-kick") - added = json.loads(await nc_mcp.call("add_circle_member", circle_id=circle["id"], user_id="bob")) + added = json.loads(await nc_mcp.call("add_circle_member", circle_id=circle["id"], user_id=circle_peer)) result = json.loads( await nc_mcp.call( "remove_circle_member", @@ -162,34 +182,32 @@ async def test_remove_member(self, nc_mcp: McpTestHelper) -> None: ) assert result == {"removed_member_id": added["id"]} members: list[dict[str, Any]] = json.loads(await nc_mcp.call("list_circle_members", circle_id=circle["id"])) - assert not any(m.get("userId") == "bob" for m in members) + assert not any(m.get("userId") == circle_peer for m in members) class TestJoinLeave: @pytest.mark.asyncio - async def test_leave_as_non_owner(self, nc_mcp: McpTestHelper) -> None: - """Alice creates, admin is added, admin leaves — membership disappears.""" + async def test_leave_as_non_owner(self, nc_mcp: McpTestHelper, circle_peer: str) -> None: + """Admin adds peer then removes peer — membership disappears.""" admin = nc_mcp circle = json.loads(await admin.call("create_circle", name="mcp-test-circle-leave")) - # Make the circle joinable so owner can add bob then bob can leave - # Admin adds bob - bob_member = json.loads(await admin.call("add_circle_member", circle_id=circle["id"], user_id="bob")) - # Admin removes bob (equivalent to bob leaving, same effect, without needing a second client) + peer_member = json.loads(await admin.call("add_circle_member", circle_id=circle["id"], user_id=circle_peer)) + # Admin removes peer (equivalent to peer leaving, same effect) await admin.call( "remove_circle_member", circle_id=circle["id"], - member_id=bob_member["id"], + member_id=peer_member["id"], ) members: list[dict[str, Any]] = json.loads(await admin.call("list_circle_members", circle_id=circle["id"])) - assert not any(m.get("userId") == "bob" for m in members) + assert not any(m.get("userId") == circle_peer for m in members) class TestSearch: @pytest.mark.asyncio - async def test_search_finds_user(self, nc_mcp: McpTestHelper) -> None: - """Search by a common user id should return at least one hit.""" + async def test_search_returns_list(self, nc_mcp: McpTestHelper) -> None: + """Search should return a JSON list regardless of matches.""" await _make_circle(nc_mcp, "mcp-test-circle-search") - results = json.loads(await nc_mcp.call("search_circles", term="bob")) + results = json.loads(await nc_mcp.call("search_circles", term="admin")) assert isinstance(results, list) From 6624df72575081f6aa08ea9eff7006a636efea3c Mon Sep 17 00:00:00 2001 From: Oleksander Piskun Date: Fri, 24 Apr 2026 11:31:32 +0000 Subject: [PATCH 3/8] Address CodeRabbit: tighten pagination, skip when circles disabled, actually exercise join_circle/leave_circle --- PROGRESS.md | 4 +- tests/integration/test_circles.py | 94 ++++++++++++++++++++++++++----- 2 files changed, 81 insertions(+), 17 deletions(-) diff --git a/PROGRESS.md b/PROGRESS.md index db39a4d..9ed3d31 100644 --- a/PROGRESS.md +++ b/PROGRESS.md @@ -97,8 +97,8 @@ | File Helpers | — | 26 | | File Reminders | 3 | 20 | | Forms | 25 | 34 | -| Circles | 14 | 20 | -| **Total** | **141** | **825** | +| Circles | 14 | 23 | +| **Total** | **141** | **828** | Files shows 10, but one (`upload_file_from_path`) is only registered when `NEXTCLOUD_MCP_UPLOAD_ROOT` is configured. Default deployments expose 140 tools. diff --git a/tests/integration/test_circles.py b/tests/integration/test_circles.py index c55548b..2cc1edf 100644 --- a/tests/integration/test_circles.py +++ b/tests/integration/test_circles.py @@ -2,20 +2,49 @@ import contextlib import json -from collections.abc import AsyncGenerator +from collections.abc import AsyncGenerator, AsyncIterator from typing import Any +import niquests import pytest from mcp.server.fastmcp.exceptions import ToolError from nc_mcp_server.client import NextcloudClient, NextcloudError from nc_mcp_server.config import Config +from nc_mcp_server.state import get_client, get_config, set_state from .conftest import McpTestHelper pytestmark = pytest.mark.integration CIRCLE_TEST_USER = "mcp-circle-test-user" +CIRCLE_TEST_PWD = "mcp-Circle-Test-PWD-9X!" + + +@pytest.fixture(scope="session") +def _circles_available(_cleanup_config: Config) -> bool: + """Probe whether the Circles app is enabled on the target Nextcloud. + + Sync-only check so we can return a plain bool at session scope without + fighting pytest-asyncio's per-test event loop. + """ + try: + resp = niquests.get( + f"{_cleanup_config.nextcloud_url}/ocs/v2.php/apps/circles/circles", + auth=(_cleanup_config.user, _cleanup_config.password), + headers={"OCS-APIRequest": "true", "Accept": "application/json"}, + timeout=5, + ) + except (OSError, niquests.exceptions.RequestException): + return False + return resp.ok + + +@pytest.fixture(autouse=True) +def _skip_if_no_circles(_circles_available: bool) -> None: + """Skip every Circles test when the app isn't enabled on the target instance.""" + if not _circles_available: + pytest.skip("Circles app is not enabled on this Nextcloud instance") @pytest.fixture @@ -25,7 +54,7 @@ async def circle_peer(nc_config: Config) -> AsyncGenerator[str]: with contextlib.suppress(Exception): await client.ocs_post( "cloud/users", - data={"userid": CIRCLE_TEST_USER, "password": "mcp-Circle-Test-PWD-9X!"}, + data={"userid": CIRCLE_TEST_USER, "password": CIRCLE_TEST_PWD}, ) yield CIRCLE_TEST_USER with contextlib.suppress(Exception): @@ -33,6 +62,33 @@ async def circle_peer(nc_config: Config) -> AsyncGenerator[str]: await client.close() +@contextlib.asynccontextmanager +async def _as_peer(user_id: str, password: str) -> AsyncIterator[None]: + """Temporarily swap the global state so tool calls authenticate as the given user. + + Restores the prior state on exit. Tools (`create_server`, `get_client`) read the + client from module-global state, so swapping it lets the existing McpTestHelper + exercise real tools under a different user's credentials without needing a + second server instance. + """ + admin_client = get_client() + admin_config = get_config() + peer_config = Config( + nextcloud_url=admin_config.nextcloud_url, + user=user_id, + password=password, + permission_level=admin_config.permission_level, + ) + peer_config.validate() + peer_client = NextcloudClient(peer_config) + set_state(peer_client, peer_config) + try: + yield + finally: + set_state(admin_client, admin_config) + await peer_client.close() + + async def _make_circle(nc_mcp: McpTestHelper, name: str) -> dict[str, Any]: """Create a circle and return its dict.""" created: dict[str, Any] = json.loads(await nc_mcp.call("create_circle", name=name)) @@ -52,7 +108,7 @@ async def test_list_pagination(self, nc_mcp: McpTestHelper) -> None: for i in range(3): await _make_circle(nc_mcp, f"mcp-test-circle-page-{i}") circles: list[dict[str, Any]] = json.loads(await nc_mcp.call("list_circles", limit=2)) - assert len(circles) <= 2 + assert len(circles) == 2 class TestCircleLifecycle: @@ -187,18 +243,26 @@ async def test_remove_member(self, nc_mcp: McpTestHelper, circle_peer: str) -> N class TestJoinLeave: @pytest.mark.asyncio - async def test_leave_as_non_owner(self, nc_mcp: McpTestHelper, circle_peer: str) -> None: - """Admin adds peer then removes peer — membership disappears.""" - admin = nc_mcp - circle = json.loads(await admin.call("create_circle", name="mcp-test-circle-leave")) - peer_member = json.loads(await admin.call("add_circle_member", circle_id=circle["id"], user_id=circle_peer)) - # Admin removes peer (equivalent to peer leaving, same effect) - await admin.call( - "remove_circle_member", - circle_id=circle["id"], - member_id=peer_member["id"], - ) - members: list[dict[str, Any]] = json.loads(await admin.call("list_circle_members", circle_id=circle["id"])) + async def test_join_open_circle(self, nc_mcp: McpTestHelper, circle_peer: str) -> None: + """Admin creates an OPEN circle; peer calls join_circle and appears in members.""" + circle = await _make_circle(nc_mcp, "mcp-test-circle-open-join") + # 24 = VISIBLE(8) | OPEN(16) — required for peer to join via join_circle + await nc_mcp.call("update_circle_config", circle_id=circle["id"], config=24) + async with _as_peer(circle_peer, CIRCLE_TEST_PWD): + joined = json.loads(await nc_mcp.call("join_circle", circle_id=circle["id"])) + assert joined["userId"] == circle_peer + assert joined["status"] == "Member" + members: list[dict[str, Any]] = json.loads(await nc_mcp.call("list_circle_members", circle_id=circle["id"])) + assert any(m.get("userId") == circle_peer for m in members) + + @pytest.mark.asyncio + async def test_leave_as_member(self, nc_mcp: McpTestHelper, circle_peer: str) -> None: + """Admin adds peer; peer calls leave_circle; peer is gone from member list.""" + circle = await _make_circle(nc_mcp, "mcp-test-circle-leave") + await nc_mcp.call("add_circle_member", circle_id=circle["id"], user_id=circle_peer) + async with _as_peer(circle_peer, CIRCLE_TEST_PWD): + await nc_mcp.call("leave_circle", circle_id=circle["id"]) + members: list[dict[str, Any]] = json.loads(await nc_mcp.call("list_circle_members", circle_id=circle["id"])) assert not any(m.get("userId") == circle_peer for m in members) From b423e29d4caeb5d9c1fa98b3a98d89f6478bf8d0 Mon Sep 17 00:00:00 2001 From: Oleksander Piskun Date: Fri, 24 Apr 2026 12:42:46 +0000 Subject: [PATCH 4/8] Fix pyright v1.1.409 deprecation: use AsyncGenerator instead of AsyncIterator for @asynccontextmanager --- tests/integration/test_circles.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/integration/test_circles.py b/tests/integration/test_circles.py index 2cc1edf..ae3ed3e 100644 --- a/tests/integration/test_circles.py +++ b/tests/integration/test_circles.py @@ -2,7 +2,7 @@ import contextlib import json -from collections.abc import AsyncGenerator, AsyncIterator +from collections.abc import AsyncGenerator from typing import Any import niquests @@ -63,7 +63,7 @@ async def circle_peer(nc_config: Config) -> AsyncGenerator[str]: @contextlib.asynccontextmanager -async def _as_peer(user_id: str, password: str) -> AsyncIterator[None]: +async def _as_peer(user_id: str, password: str) -> AsyncGenerator[None]: """Temporarily swap the global state so tool calls authenticate as the given user. Restores the prior state on exit. Tools (`create_server`, `get_client`) read the From 801cb8d1a92179a1a2f73cf89c15178eebf6781c Mon Sep 17 00:00:00 2001 From: Oleksander Piskun Date: Fri, 24 Apr 2026 13:19:33 +0000 Subject: [PATCH 5/8] Address CodeRabbit: use get_config().user instead of hardcoded 'admin' in owner assertions (circles + forms) --- tests/integration/test_circles.py | 9 +++++---- tests/integration/test_forms.py | 3 ++- 2 files changed, 7 insertions(+), 5 deletions(-) diff --git a/tests/integration/test_circles.py b/tests/integration/test_circles.py index ae3ed3e..96e4e7a 100644 --- a/tests/integration/test_circles.py +++ b/tests/integration/test_circles.py @@ -116,7 +116,7 @@ class TestCircleLifecycle: async def test_create_sets_owner_to_caller(self, nc_mcp: McpTestHelper) -> None: created = json.loads(await nc_mcp.call("create_circle", name="mcp-test-circle-create")) assert created["name"] == "mcp-test-circle-create" - assert created["initiator"]["userId"] == "admin" + assert created["initiator"]["userId"] == get_config().user assert created["initiator"]["level"] == 9 # owner @pytest.mark.asyncio @@ -176,9 +176,10 @@ async def test_owner_present_in_members(self, nc_mcp: McpTestHelper) -> None: circle = await _make_circle(nc_mcp, "mcp-test-circle-owner") members: list[dict[str, Any]] = json.loads(await nc_mcp.call("list_circle_members", circle_id=circle["id"])) assert isinstance(members, list) - admin_member = next((m for m in members if m.get("userId") == "admin"), None) - assert admin_member is not None, "owner should be in members" - assert admin_member["level"] == 9 + owner_id = get_config().user + owner_member = next((m for m in members if m.get("userId") == owner_id), None) + assert owner_member is not None, "owner should be in members" + assert owner_member["level"] == 9 @pytest.mark.asyncio async def test_add_and_list_member(self, nc_mcp: McpTestHelper, circle_peer: str) -> None: diff --git a/tests/integration/test_forms.py b/tests/integration/test_forms.py index 6fe88b1..175a8f1 100644 --- a/tests/integration/test_forms.py +++ b/tests/integration/test_forms.py @@ -7,6 +7,7 @@ from mcp.server.fastmcp.exceptions import ToolError from nc_mcp_server.client import NextcloudError +from nc_mcp_server.state import get_config from .conftest import McpTestHelper @@ -53,7 +54,7 @@ async def test_create_produces_form_with_id(self, nc_mcp: McpTestHelper) -> None created = json.loads(await nc_mcp.call("create_form")) await nc_mcp.call("update_form", form_id=created["id"], key_value_pairs={"title": "mcp-test-create-id"}) assert isinstance(created["id"], int) - assert created["ownerId"] == "admin" + assert created["ownerId"] == get_config().user assert created["hash"] @pytest.mark.asyncio From 0a68ff430184198f3ffce4ea5c479a1ed5c05b07 Mon Sep 17 00:00:00 2001 From: Oleksander Piskun Date: Fri, 24 Apr 2026 14:08:24 +0000 Subject: [PATCH 6/8] Fix Circles docstrings and bump leave_circle to DESTRUCTIVE MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Deep re-review against the Nextcloud Circles source and live API surfaced three docstring bugs and one permission mismatch: - update_circle_member_level: docstring claimed it returned the updated circle, but the server returns the updated MEMBER (verified in FederatedItems/MemberLevel.php — outcome is the cloned member with the new level). - search_circles: docstring named a "source" field; actual response has "userType" and keys ['displayName','id','instance','userId','userType']. - leave_circle: docstring claimed it returned a membership record; the server actually returns the CIRCLE object (CircleLeave::verify calls circleRequest->getCircle and serializes it as the outcome). Empty when the caller loses visibility (sole-owner destroy case). - leave_circle permission: was WRITE + ADDITIVE_IDEMPOTENT. Sole-owner leave silently destroys the whole circle, and the matching leave_conversation tool in Talk is DESTRUCTIVE. Align annotations and permission level to DESTRUCTIVE. New tests cover the gaps the review surfaced: offset pagination, full_details=True response shape, owner transfer via update_circle_member_level (plus peer cleanup for the post-transfer circle), join on a non-OPEN circle failing, sole-owner leave destroying the circle, and the new permission gate on leave_circle. --- PROGRESS.md | 4 +- src/nc_mcp_server/tools/circles.py | 24 +++++++---- tests/integration/test_circles.py | 69 ++++++++++++++++++++++++++++++ 3 files changed, 87 insertions(+), 10 deletions(-) diff --git a/PROGRESS.md b/PROGRESS.md index 9ed3d31..f2d7c5f 100644 --- a/PROGRESS.md +++ b/PROGRESS.md @@ -97,8 +97,8 @@ | File Helpers | — | 26 | | File Reminders | 3 | 20 | | Forms | 25 | 34 | -| Circles | 14 | 23 | -| **Total** | **141** | **828** | +| Circles | 14 | 29 | +| **Total** | **141** | **834** | Files shows 10, but one (`upload_file_from_path`) is only registered when `NEXTCLOUD_MCP_UPLOAD_ROOT` is configured. Default deployments expose 140 tools. diff --git a/src/nc_mcp_server/tools/circles.py b/src/nc_mcp_server/tools/circles.py index ee7645a..1fa1004 100644 --- a/src/nc_mcp_server/tools/circles.py +++ b/src/nc_mcp_server/tools/circles.py @@ -101,9 +101,9 @@ async def search_circles(term: str) -> str: term: Search phrase. Matches against names/ids. Returns: - JSON array of search results. Each entry includes id (singleId), - source (1=user, 2=group, 4=mail, 16=circle), and contextual - metadata depending on source. + JSON array of search results. Each entry includes id (the singleId + for users/groups/circles), userId, displayName, instance, and + userType (1=user, 2=group, 4=mail, 8=contact, 16=circle). """ client = get_client() data = await client.ocs_get("apps/circles/search", params={"term": term}) @@ -219,21 +219,26 @@ async def join_circle(circle_id: str) -> str: data = await client.ocs_put_json(f"apps/circles/circles/{circle_id}/join", json_data={}) return json.dumps(data) - @mcp.tool(annotations=ADDITIVE_IDEMPOTENT) - @require_permission(PermissionLevel.WRITE) + @mcp.tool(annotations=DESTRUCTIVE) + @require_permission(PermissionLevel.DESTRUCTIVE) async def leave_circle(circle_id: str) -> str: """Leave a circle the current user is a member of. IMPORTANT: If the current user is the sole owner, leaving destroys the entire circle (server behavior — no confirmation prompt). To avoid this, promote another member to owner via - update_circle_member_level(..., level="owner") first. + update_circle_member_level(..., level="owner") first. Because of + this implicit-destroy behavior, this tool requires DESTRUCTIVE + permission (matching leave_conversation in Talk). Args: circle_id: String circle id. Returns: - JSON of the removed-membership record. + JSON of the circle the user just left (circle fields: id, name, + config, population, initiator, …). Empty when the caller loses + visibility on the circle after leaving (e.g. sole-owner case + where the circle is destroyed). """ client = get_client() data = await client.ocs_put_json(f"apps/circles/circles/{circle_id}/leave", json_data={}) @@ -281,7 +286,10 @@ async def update_circle_member_level(circle_id: str, member_id: str, level: str) someone to "owner" transfers ownership; the caller becomes admin. Returns: - JSON of the updated circle state. + JSON of the updated member (same shape as entries from + list_circle_members: id, userId, level, status, circleId, + singleId, …). Use the "level" field to confirm the new role. + Note: this returns the member, not the circle. """ if level not in MEMBER_LEVELS: raise ValueError(f"Invalid level '{level}'. Must be one of: {sorted(MEMBER_LEVELS)}") diff --git a/tests/integration/test_circles.py b/tests/integration/test_circles.py index 96e4e7a..c837c9a 100644 --- a/tests/integration/test_circles.py +++ b/tests/integration/test_circles.py @@ -110,6 +110,17 @@ async def test_list_pagination(self, nc_mcp: McpTestHelper) -> None: circles: list[dict[str, Any]] = json.loads(await nc_mcp.call("list_circles", limit=2)) assert len(circles) == 2 + @pytest.mark.asyncio + async def test_list_offset_skips_entries(self, nc_mcp: McpTestHelper) -> None: + """limit=1 + offset=1 skips the first entry — sanity-check pagination offset.""" + for i in range(3): + await _make_circle(nc_mcp, f"mcp-test-circle-offset-{i}") + page_one: list[dict[str, Any]] = json.loads(await nc_mcp.call("list_circles", limit=1)) + page_two: list[dict[str, Any]] = json.loads(await nc_mcp.call("list_circles", limit=1, offset=1)) + assert len(page_one) == 1 + assert len(page_two) == 1 + assert page_one[0]["id"] != page_two[0]["id"] + class TestCircleLifecycle: @pytest.mark.asyncio @@ -241,6 +252,41 @@ async def test_remove_member(self, nc_mcp: McpTestHelper, circle_peer: str) -> N members: list[dict[str, Any]] = json.loads(await nc_mcp.call("list_circle_members", circle_id=circle["id"])) assert not any(m.get("userId") == circle_peer for m in members) + @pytest.mark.asyncio + async def test_full_details_adds_circle_field(self, nc_mcp: McpTestHelper) -> None: + """full_details=True returns the extra `circle` field on each member entry.""" + circle = await _make_circle(nc_mcp, "mcp-test-circle-full-details") + default_members: list[dict[str, Any]] = json.loads( + await nc_mcp.call("list_circle_members", circle_id=circle["id"]) + ) + full_members: list[dict[str, Any]] = json.loads( + await nc_mcp.call("list_circle_members", circle_id=circle["id"], full_details=True) + ) + assert default_members, "owner should be present in default response" + assert full_members, "owner should be present in full-details response" + assert "circle" not in default_members[0] + assert "circle" in full_members[0] + + @pytest.mark.asyncio + async def test_promote_to_owner_transfers_and_demotes_caller(self, nc_mcp: McpTestHelper, circle_peer: str) -> None: + """Promoting a member to owner transfers ownership; previous owner becomes admin (level=8).""" + circle = await _make_circle(nc_mcp, "mcp-test-circle-xfer-owner") + added = json.loads(await nc_mcp.call("add_circle_member", circle_id=circle["id"], user_id=circle_peer)) + await nc_mcp.call( + "update_circle_member_level", + circle_id=circle["id"], + member_id=added["id"], + level="owner", + ) + members: list[dict[str, Any]] = json.loads(await nc_mcp.call("list_circle_members", circle_id=circle["id"])) + peer_level = next(m["level"] for m in members if m.get("userId") == circle_peer) + caller_level = next(m["level"] for m in members if m.get("userId") == get_config().user) + assert peer_level == 9, "promoted member should be owner (9)" + assert caller_level == 8, "previous owner should be demoted to admin (8)" + # peer is now the only owner — admin cleanup can't destroy it. Tear down as peer. + async with _as_peer(circle_peer, CIRCLE_TEST_PWD): + await nc_mcp.call("delete_circle", circle_id=circle["id"]) + class TestJoinLeave: @pytest.mark.asyncio @@ -256,6 +302,15 @@ async def test_join_open_circle(self, nc_mcp: McpTestHelper, circle_peer: str) - members: list[dict[str, Any]] = json.loads(await nc_mcp.call("list_circle_members", circle_id=circle["id"])) assert any(m.get("userId") == circle_peer for m in members) + @pytest.mark.asyncio + async def test_join_non_open_fails(self, nc_mcp: McpTestHelper, circle_peer: str) -> None: + """Joining a circle without the OPEN flag is rejected by the server.""" + circle = await _make_circle(nc_mcp, "mcp-test-circle-closed-join") + # default config (no OPEN flag) — peer must not be able to join + async with _as_peer(circle_peer, CIRCLE_TEST_PWD): + with pytest.raises((ToolError, NextcloudError)): + await nc_mcp.call("join_circle", circle_id=circle["id"]) + @pytest.mark.asyncio async def test_leave_as_member(self, nc_mcp: McpTestHelper, circle_peer: str) -> None: """Admin adds peer; peer calls leave_circle; peer is gone from member list.""" @@ -266,6 +321,14 @@ async def test_leave_as_member(self, nc_mcp: McpTestHelper, circle_peer: str) -> members: list[dict[str, Any]] = json.loads(await nc_mcp.call("list_circle_members", circle_id=circle["id"])) assert not any(m.get("userId") == circle_peer for m in members) + @pytest.mark.asyncio + async def test_sole_owner_leave_destroys_circle(self, nc_mcp: McpTestHelper) -> None: + """Sole-owner leave silently destroys the circle — documented server behavior.""" + circle = await _make_circle(nc_mcp, "mcp-test-circle-sole-leave") + await nc_mcp.call("leave_circle", circle_id=circle["id"]) + with pytest.raises((ToolError, NextcloudError)): + await nc_mcp.call("get_circle", circle_id=circle["id"]) + class TestSearch: @pytest.mark.asyncio @@ -297,6 +360,12 @@ async def test_write_blocks_delete(self, nc_mcp_write: McpTestHelper) -> None: with pytest.raises(ToolError, match=r"[Pp]ermission"): await nc_mcp_write.call("delete_circle", circle_id="doesnt-matter") + @pytest.mark.asyncio + async def test_write_blocks_leave(self, nc_mcp_write: McpTestHelper) -> None: + """leave_circle is DESTRUCTIVE because sole-owner leave destroys the circle.""" + with pytest.raises(ToolError, match=r"[Pp]ermission"): + await nc_mcp_write.call("leave_circle", circle_id="doesnt-matter") + @pytest.mark.asyncio async def test_write_allows_create_and_update(self, nc_mcp_write: McpTestHelper) -> None: created = json.loads(await nc_mcp_write.call("create_circle", name="mcp-test-circle-perm-write")) From 112b39ad31ee9dfdad11ca1a08db74d08e49080e Mon Sep 17 00:00:00 2001 From: Oleksander Piskun Date: Fri, 24 Apr 2026 14:17:35 +0000 Subject: [PATCH 7/8] Address CodeRabbit: document sequential-only assumption in _as_peer --- tests/integration/test_circles.py | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/tests/integration/test_circles.py b/tests/integration/test_circles.py index c837c9a..d43d7a0 100644 --- a/tests/integration/test_circles.py +++ b/tests/integration/test_circles.py @@ -70,6 +70,12 @@ async def _as_peer(user_id: str, password: str) -> AsyncGenerator[None]: client from module-global state, so swapping it lets the existing McpTestHelper exercise real tools under a different user's credentials without needing a second server instance. + + Concurrency: this mutates a process-wide singleton. Only safe under sequential + test execution (pytest-asyncio's default per-test event loop). If this file + is ever run under pytest-xdist or similar parallel runners, calls from other + tests would race against the swap. Add an asyncio.Lock or a fresh + NextcloudClient-per-call path if that ever becomes relevant. """ admin_client = get_client() admin_config = get_config() From 8504ad731c9a860a4686712fa11759c78b6627f3 Mon Sep 17 00:00:00 2001 From: Oleksander Piskun Date: Fri, 24 Apr 2026 15:34:20 +0000 Subject: [PATCH 8/8] Fix update_circle_config docstring: remove invalid HIDDEN flag, document auto-mutations --- PROGRESS.md | 4 ++-- src/nc_mcp_server/tools/circles.py | 28 +++++++++++++++++++++------- tests/integration/test_circles.py | 20 ++++++++++++++++++++ 3 files changed, 43 insertions(+), 9 deletions(-) diff --git a/PROGRESS.md b/PROGRESS.md index f2d7c5f..bc15cbd 100644 --- a/PROGRESS.md +++ b/PROGRESS.md @@ -97,8 +97,8 @@ | File Helpers | — | 26 | | File Reminders | 3 | 20 | | Forms | 25 | 34 | -| Circles | 14 | 29 | -| **Total** | **141** | **834** | +| Circles | 14 | 31 | +| **Total** | **141** | **836** | Files shows 10, but one (`upload_file_from_path`) is only registered when `NEXTCLOUD_MCP_UPLOAD_ROOT` is configured. Default deployments expose 140 tools. diff --git a/src/nc_mcp_server/tools/circles.py b/src/nc_mcp_server/tools/circles.py index 1fa1004..e7081a2 100644 --- a/src/nc_mcp_server/tools/circles.py +++ b/src/nc_mcp_server/tools/circles.py @@ -176,22 +176,36 @@ async def update_circle_description(circle_id: str, description: str) -> str: async def update_circle_config(circle_id: str, config: int) -> str: """Update a circle's config flags (bitmask). Requires admin or owner level. + The server auto-adjusts dependent flags — inspect the `config` field of + the returned circle to see what was actually stored: + - Setting REQUEST (64) without OPEN auto-adds OPEN → stored as 80. + - Setting FEDERATED (32768) without ROOT auto-adds ROOT → stored as 40960. + - Clearing OPEN while REQUEST is on drops REQUEST too. + - Clearing ROOT while FEDERATED is on drops FEDERATED too. + Args: circle_id: String circle id. - config: Bitmask integer. Common flags (combine with OR): + config: Bitmask integer. Valid user-facing flags (combine with OR): 8=VISIBLE (listed for non-members), 16=OPEN (anyone can join via join_circle), - 32=INVITE (adding a member creates an invitation to accept), - 64=REQUEST (join requests need moderator approval), + 32=INVITE (adding a member generates an invitation to accept), + 64=REQUEST (join requests need moderator approval; implies OPEN), 128=FRIEND (members can invite friends), - 256=PROTECTED (password-protected), - 1024=HIDDEN (hidden from listings), - 4096=LOCAL (not federated), + 256=PROTECTED (password-protected; password must be set via a + dedicated setting endpoint, not exposed by this tool), + 4096=LOCAL (not federated, even on GlobalScale), + 8192=ROOT (circle cannot be nested inside another circle), + 16384=CIRCLE_INVITE (nested circles confirm before joining), + 32768=FEDERATED (federated to other instances; implies ROOT), 65536=MOUNTPOINT (auto-create Files folder). Pass 0 for a fully private, invite-by-admin-only circle. + NOTE: 1 (SINGLE), 2 (PERSONAL), 4 (SYSTEM), 512 (NO_OWNER), + 1024 (HIDDEN), 2048 (BACKEND), and 131072 (APP) are rejected + by the public API (400 "Configuration value is not valid"). Returns: - JSON of the updated circle. + JSON of the updated circle. Compare `config` in the response to the + requested value to detect auto-mutations described above. """ client = get_client() data = await client.ocs_put_json( diff --git a/tests/integration/test_circles.py b/tests/integration/test_circles.py index d43d7a0..e9f38b8 100644 --- a/tests/integration/test_circles.py +++ b/tests/integration/test_circles.py @@ -178,6 +178,26 @@ async def test_update_config_flags(self, nc_mcp: McpTestHelper) -> None: fetched = json.loads(await nc_mcp.call("get_circle", circle_id=circle["id"])) assert fetched["config"] == 24 + @pytest.mark.asyncio + async def test_update_config_rejects_hidden_flag(self, nc_mcp: McpTestHelper) -> None: + """1024 (CFG_HIDDEN) is in DEF_CFG_SYSTEM_FILTER — blocked via public OCS API. + + Guards the docstring against regressing to claim HIDDEN is user-settable. + """ + circle = await _make_circle(nc_mcp, "mcp-test-circle-hidden-rej") + with pytest.raises((ToolError, NextcloudError)): + await nc_mcp.call("update_circle_config", circle_id=circle["id"], config=1024) + + @pytest.mark.asyncio + async def test_update_config_auto_adds_open_for_request(self, nc_mcp: McpTestHelper) -> None: + """REQUEST (64) implies OPEN (16); server stores 80, not 64. + + Documents the auto-mutation noted in the update_circle_config docstring. + """ + circle = await _make_circle(nc_mcp, "mcp-test-circle-auto-open") + updated = json.loads(await nc_mcp.call("update_circle_config", circle_id=circle["id"], config=64)) + assert updated["config"] == 80, f"expected server to add OPEN to REQUEST, got {updated['config']}" + @pytest.mark.asyncio async def test_delete_removes_circle(self, nc_mcp: McpTestHelper) -> None: circle = await _make_circle(nc_mcp, "mcp-test-circle-delete")