fix: add trash/delete collective tools and address review feedback (round 4)
Add collectives_trash_collective and collectives_delete_collective MCP tools with proper destructiveHint annotations. Refactor integration test fixture to use MCP tools for cleanup instead of direct httpx/OCS calls. Optimize _get_ocs_headers() to class-level constant. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Opus 4.6
parent
f3caad122d
commit
95edd9ba8e
@@ -24,13 +24,15 @@ class CollectivesClient(BaseNextcloudClient):
|
||||
|
||||
app_name = "collectives"
|
||||
|
||||
_OCS_HEADERS: dict[str, str] = {
|
||||
"OCS-APIRequest": "true",
|
||||
"Content-Type": "application/json",
|
||||
"Accept": "application/json",
|
||||
}
|
||||
|
||||
def _get_ocs_headers(self) -> dict[str, str]:
|
||||
"""Get standard headers required for OCS API calls."""
|
||||
return {
|
||||
"OCS-APIRequest": "true",
|
||||
"Content-Type": "application/json",
|
||||
"Accept": "application/json",
|
||||
}
|
||||
return self._OCS_HEADERS
|
||||
|
||||
def _unwrap_ocs(self, response_json: dict[str, Any]) -> Any:
|
||||
"""Unwrap OCS envelope, validating the status before returning data."""
|
||||
@@ -90,6 +92,26 @@ class CollectivesClient(BaseNextcloudClient):
|
||||
data = self._unwrap_ocs(response.json())
|
||||
return data["collective"]
|
||||
|
||||
async def trash_collective(self, collective_id: int) -> None:
|
||||
"""Move a collective to trash (soft delete)."""
|
||||
await self._make_request(
|
||||
"DELETE",
|
||||
f"{API_BASE}/collectives/{collective_id}",
|
||||
headers=self._get_ocs_headers(),
|
||||
)
|
||||
|
||||
async def delete_collective(self, collective_id: int) -> None:
|
||||
"""Permanently delete a collective (must be trashed first).
|
||||
|
||||
This is irreversible. The collective must be in the trash before
|
||||
calling this method.
|
||||
"""
|
||||
await self._make_request(
|
||||
"DELETE",
|
||||
f"{API_BASE}/collectives/trash/{collective_id}",
|
||||
headers=self._get_ocs_headers(),
|
||||
)
|
||||
|
||||
# Pages
|
||||
|
||||
async def get_pages(self, collective_id: int) -> list[dict[str, Any]]:
|
||||
|
||||
@@ -265,6 +265,66 @@ def configure_collectives_tools(mcp: FastMCP):
|
||||
message=f"Collective updated (emoji: {collective.emoji})",
|
||||
)
|
||||
|
||||
@mcp.tool(
|
||||
title="Trash Collective",
|
||||
annotations=ToolAnnotations(
|
||||
destructiveHint=True, idempotentHint=False, openWorldHint=True
|
||||
),
|
||||
)
|
||||
@require_scopes("collectives:write")
|
||||
@instrument_tool
|
||||
async def collectives_trash_collective(
|
||||
ctx: Context, collective_id: int
|
||||
) -> CollectiveOperationResponse:
|
||||
"""Move a Nextcloud Collective to trash (soft delete).
|
||||
|
||||
The collective can be restored or permanently deleted afterwards.
|
||||
|
||||
Args:
|
||||
collective_id: ID of the collective to trash
|
||||
"""
|
||||
client = await get_client(ctx)
|
||||
try:
|
||||
await client.collectives.trash_collective(collective_id)
|
||||
except (OCSError, HTTPStatusError) as e:
|
||||
raise _handle_collectives_error(e) from e
|
||||
return CollectiveOperationResponse(
|
||||
collective_id=collective_id,
|
||||
status_code=200,
|
||||
message="Collective moved to trash",
|
||||
)
|
||||
|
||||
@mcp.tool(
|
||||
title="Delete Collective",
|
||||
annotations=ToolAnnotations(
|
||||
destructiveHint=True, idempotentHint=True, openWorldHint=True
|
||||
),
|
||||
)
|
||||
@require_scopes("collectives:write")
|
||||
@instrument_tool
|
||||
async def collectives_delete_collective(
|
||||
ctx: Context, collective_id: int
|
||||
) -> CollectiveOperationResponse:
|
||||
"""Permanently delete a Nextcloud Collective.
|
||||
|
||||
WARNING: This is irreversible. The collective must be in the trash
|
||||
first (use collectives_trash_collective). All pages and content
|
||||
will be permanently destroyed.
|
||||
|
||||
Args:
|
||||
collective_id: ID of the trashed collective to permanently delete
|
||||
"""
|
||||
client = await get_client(ctx)
|
||||
try:
|
||||
await client.collectives.delete_collective(collective_id)
|
||||
except (OCSError, HTTPStatusError) as e:
|
||||
raise _handle_collectives_error(e) from e
|
||||
return CollectiveOperationResponse(
|
||||
collective_id=collective_id,
|
||||
status_code=200,
|
||||
message="Collective permanently deleted",
|
||||
)
|
||||
|
||||
@mcp.tool(
|
||||
title="Create Collective Page",
|
||||
annotations=ToolAnnotations(idempotentHint=False, openWorldHint=True),
|
||||
|
||||
@@ -135,6 +135,7 @@ dev = [
|
||||
"ruff>=0.11.13",
|
||||
"reportlab>=4.0.0",
|
||||
"ty>=0.0.1a25",
|
||||
"pytest-otel>=2.0.1",
|
||||
]
|
||||
|
||||
[project.scripts]
|
||||
|
||||
@@ -115,6 +115,37 @@ async def test_create_collective(mocker):
|
||||
assert call_args[1]["json"]["emoji"] == "📚"
|
||||
|
||||
|
||||
async def test_trash_collective(mocker):
|
||||
"""Test trashing a collective sends DELETE to correct endpoint."""
|
||||
mock_response = create_mock_response(status_code=200, json_data={})
|
||||
mock_request = mocker.patch.object(
|
||||
CollectivesClient, "_make_request", return_value=mock_response
|
||||
)
|
||||
|
||||
client = CollectivesClient(mocker.AsyncMock(spec=httpx.AsyncClient), "testuser")
|
||||
await client.trash_collective(collective_id=5)
|
||||
|
||||
call_args = mock_request.call_args
|
||||
assert call_args[0][0] == "DELETE"
|
||||
assert "/collectives/5" in call_args[0][1]
|
||||
assert "/trash" not in call_args[0][1]
|
||||
|
||||
|
||||
async def test_delete_collective(mocker):
|
||||
"""Test permanently deleting a collective sends DELETE to trash endpoint."""
|
||||
mock_response = create_mock_response(status_code=200, json_data={})
|
||||
mock_request = mocker.patch.object(
|
||||
CollectivesClient, "_make_request", return_value=mock_response
|
||||
)
|
||||
|
||||
client = CollectivesClient(mocker.AsyncMock(spec=httpx.AsyncClient), "testuser")
|
||||
await client.delete_collective(collective_id=5)
|
||||
|
||||
call_args = mock_request.call_args
|
||||
assert call_args[0][0] == "DELETE"
|
||||
assert "/collectives/trash/5" in call_args[0][1]
|
||||
|
||||
|
||||
# --- Pages ---
|
||||
|
||||
|
||||
|
||||
@@ -2,25 +2,14 @@
|
||||
|
||||
import json
|
||||
import logging
|
||||
import os
|
||||
import uuid
|
||||
|
||||
import httpx
|
||||
import pytest
|
||||
from mcp import ClientSession
|
||||
|
||||
logger = logging.getLogger(__name__)
|
||||
pytestmark = pytest.mark.integration
|
||||
|
||||
# Nextcloud credentials from environment (matches .envrc / docker-compose.yml defaults)
|
||||
_NC_BASE = os.environ.get("NEXTCLOUD_HOST", "http://localhost:8080")
|
||||
_NC_USER = os.environ.get("NEXTCLOUD_USERNAME", "admin")
|
||||
_NC_PASS = os.environ.get("NEXTCLOUD_PASSWORD", "admin")
|
||||
_OCS_HEADERS = {
|
||||
"OCS-APIRequest": "true",
|
||||
"Accept": "application/json",
|
||||
}
|
||||
|
||||
|
||||
# --- Fixtures ---
|
||||
|
||||
@@ -56,20 +45,16 @@ async def temporary_collective(nc_mcp_client: ClientSession):
|
||||
"landing_page_id": landing_page_id,
|
||||
}
|
||||
|
||||
# Cleanup: trash and permanently delete the collective via direct OCS API
|
||||
# Cleanup: trash and permanently delete the collective via MCP tools
|
||||
try:
|
||||
async with httpx.AsyncClient(
|
||||
base_url=_NC_BASE, auth=(_NC_USER, _NC_PASS)
|
||||
) as client:
|
||||
api = "/ocs/v2.php/apps/collectives/api/v1.0"
|
||||
await client.delete(
|
||||
f"{api}/collectives/{collective_id}",
|
||||
headers=_OCS_HEADERS,
|
||||
)
|
||||
await client.delete(
|
||||
f"{api}/collectives/trash/{collective_id}",
|
||||
headers=_OCS_HEADERS,
|
||||
)
|
||||
await nc_mcp_client.call_tool(
|
||||
"collectives_trash_collective",
|
||||
{"collective_id": collective_id},
|
||||
)
|
||||
await nc_mcp_client.call_tool(
|
||||
"collectives_delete_collective",
|
||||
{"collective_id": collective_id},
|
||||
)
|
||||
logger.info(f"Cleaned up collective: {collective_id}")
|
||||
except Exception as e:
|
||||
logger.warning(f"Cleanup of collective {collective_id} failed: {e}")
|
||||
@@ -87,6 +72,8 @@ async def test_collectives_tools_available(nc_mcp_client: ClientSession):
|
||||
"collectives_get_collectives",
|
||||
"collectives_create_collective",
|
||||
"collectives_update_collective",
|
||||
"collectives_trash_collective",
|
||||
"collectives_delete_collective",
|
||||
"collectives_get_pages",
|
||||
"collectives_get_page",
|
||||
"collectives_create_page",
|
||||
|
||||
@@ -2131,6 +2131,7 @@ dev = [
|
||||
{ name = "pytest" },
|
||||
{ name = "pytest-cov" },
|
||||
{ name = "pytest-mock" },
|
||||
{ name = "pytest-otel" },
|
||||
{ name = "pytest-playwright-asyncio" },
|
||||
{ name = "pytest-timeout" },
|
||||
{ name = "reportlab" },
|
||||
@@ -2182,6 +2183,7 @@ dev = [
|
||||
{ name = "pytest", specifier = ">=8.3.5" },
|
||||
{ name = "pytest-cov", specifier = ">=6.1.1" },
|
||||
{ name = "pytest-mock", specifier = ">=3.15.1" },
|
||||
{ name = "pytest-otel", specifier = ">=2.0.1" },
|
||||
{ name = "pytest-playwright-asyncio", specifier = ">=0.7.1" },
|
||||
{ name = "pytest-timeout", specifier = ">=2.3.1" },
|
||||
{ name = "reportlab", specifier = ">=4.0.0" },
|
||||
@@ -2348,6 +2350,19 @@ wheels = [
|
||||
{ url = "https://files.pythonhosted.org/packages/05/85/d831a9bc0a9e0e1a304ff3d12c1489a5fbc9bf6690a15dcbdae372bbca45/opentelemetry_api-1.39.0-py3-none-any.whl", hash = "sha256:3c3b3ca5c5687b1b5b37e5c5027ff68eacea8675241b29f13110a8ffbb8f0459", size = 66357, upload-time = "2025-12-03T13:19:33.043Z" },
|
||||
]
|
||||
|
||||
[[package]]
|
||||
name = "opentelemetry-exporter-otlp"
|
||||
version = "1.39.0"
|
||||
source = { registry = "https://pypi.org/simple" }
|
||||
dependencies = [
|
||||
{ name = "opentelemetry-exporter-otlp-proto-grpc" },
|
||||
{ name = "opentelemetry-exporter-otlp-proto-http" },
|
||||
]
|
||||
sdist = { url = "https://files.pythonhosted.org/packages/13/be/0e9d889f47e55cadc4041e5b53d4e0cc688f9a74811134fb0ba7cbee6905/opentelemetry_exporter_otlp-1.39.0.tar.gz", hash = "sha256:b405da0287b895fe4e2450dedb2a5b072debba1dfcfed5bdb3d1d183d8daa296", size = 6146, upload-time = "2025-12-03T13:19:58.381Z" }
|
||||
wheels = [
|
||||
{ url = "https://files.pythonhosted.org/packages/fc/35/212d2cae4fa9a2c02e74438612268b640ab577b8ccb04590371eb4e0f542/opentelemetry_exporter_otlp-1.39.0-py3-none-any.whl", hash = "sha256:fe155d6968d581b325574ad6dc267c8de299397b18d11feeda2206d0a47928a9", size = 7017, upload-time = "2025-12-03T13:19:35.686Z" },
|
||||
]
|
||||
|
||||
[[package]]
|
||||
name = "opentelemetry-exporter-otlp-proto-common"
|
||||
version = "1.39.0"
|
||||
@@ -2378,6 +2393,24 @@ wheels = [
|
||||
{ url = "https://files.pythonhosted.org/packages/56/e8/d420b94ffddfd8cff85bb4aa5d98da26ce7935dc3cf3eca6b83cd39ab436/opentelemetry_exporter_otlp_proto_grpc-1.39.0-py3-none-any.whl", hash = "sha256:758641278050de9bb895738f35ff8840e4a47685b7e6ef4a201fe83196ba7a05", size = 19765, upload-time = "2025-12-03T13:19:38.143Z" },
|
||||
]
|
||||
|
||||
[[package]]
|
||||
name = "opentelemetry-exporter-otlp-proto-http"
|
||||
version = "1.39.0"
|
||||
source = { registry = "https://pypi.org/simple" }
|
||||
dependencies = [
|
||||
{ name = "googleapis-common-protos" },
|
||||
{ name = "opentelemetry-api" },
|
||||
{ name = "opentelemetry-exporter-otlp-proto-common" },
|
||||
{ name = "opentelemetry-proto" },
|
||||
{ name = "opentelemetry-sdk" },
|
||||
{ name = "requests" },
|
||||
{ name = "typing-extensions" },
|
||||
]
|
||||
sdist = { url = "https://files.pythonhosted.org/packages/81/dc/1e9bf3f6a28e29eba516bc0266e052996d02bc7e92675f3cd38169607609/opentelemetry_exporter_otlp_proto_http-1.39.0.tar.gz", hash = "sha256:28d78fc0eb82d5a71ae552263d5012fa3ebad18dfd189bf8d8095ba0e65ee1ed", size = 17287, upload-time = "2025-12-03T13:20:01.134Z" }
|
||||
wheels = [
|
||||
{ url = "https://files.pythonhosted.org/packages/bc/46/e4a102e17205bb05a50dbf24ef0e92b66b648cd67db9a68865af06a242fd/opentelemetry_exporter_otlp_proto_http-1.39.0-py3-none-any.whl", hash = "sha256:5789cb1375a8b82653328c0ce13a054d285f774099faf9d068032a49de4c7862", size = 19639, upload-time = "2025-12-03T13:19:39.536Z" },
|
||||
]
|
||||
|
||||
[[package]]
|
||||
name = "opentelemetry-instrumentation"
|
||||
version = "0.60b0"
|
||||
@@ -3296,6 +3329,21 @@ wheels = [
|
||||
{ url = "https://files.pythonhosted.org/packages/5a/cc/06253936f4a7fa2e0f48dfe6d851d9c56df896a9ab09ac019d70b760619c/pytest_mock-3.15.1-py3-none-any.whl", hash = "sha256:0a25e2eb88fe5168d535041d09a4529a188176ae608a6d249ee65abc0949630d", size = 10095, upload-time = "2025-09-16T16:37:25.734Z" },
|
||||
]
|
||||
|
||||
[[package]]
|
||||
name = "pytest-otel"
|
||||
version = "2.0.1"
|
||||
source = { registry = "https://pypi.org/simple" }
|
||||
dependencies = [
|
||||
{ name = "opentelemetry-api" },
|
||||
{ name = "opentelemetry-exporter-otlp" },
|
||||
{ name = "opentelemetry-sdk" },
|
||||
{ name = "pytest" },
|
||||
]
|
||||
sdist = { url = "https://files.pythonhosted.org/packages/ca/5e/771f8dbdf55ae57603d89bb26e046c13e8bee3492c74cb3afb6044166e9a/pytest_otel-2.0.1.tar.gz", hash = "sha256:3d529dc34105862cca39fd1258d00dd3d17f3b7d92ebf0953d55326bb017af3d", size = 17880, upload-time = "2025-12-08T14:56:40.393Z" }
|
||||
wheels = [
|
||||
{ url = "https://files.pythonhosted.org/packages/16/fa/0cfe23bac571f68b4f427ad7ae313ddd8de881ec2ff49dcea196a2a96592/pytest_otel-2.0.1-py2.py3-none-any.whl", hash = "sha256:501f36f02f55578ca34c3ccbe55e81cc1e14bb130da13f7cf0b6480d54a9e5db", size = 14530, upload-time = "2025-12-08T14:56:41.731Z" },
|
||||
]
|
||||
|
||||
[[package]]
|
||||
name = "pytest-playwright-asyncio"
|
||||
version = "0.7.2"
|
||||
|
||||
Reference in New Issue
Block a user