diff --git a/ARCHITECTURE.md b/ARCHITECTURE.md index c6f959a8..a7b94730 100644 --- a/ARCHITECTURE.md +++ b/ARCHITECTURE.md @@ -49,7 +49,7 @@ The main server implementation using FastMCP framework. **Key Features:** -- 22 tools for comprehensive Zammad operations +- 21 tools for comprehensive Zammad operations - 4 resources with URI-based access pattern - 3 pre-configured prompts for common scenarios - Lifespan management for proper initialization diff --git a/README.md b/README.md index a5941890..107e997d 100644 --- a/README.md +++ b/README.md @@ -25,7 +25,6 @@ An MCP server that connects AI assistants to Zammad, providing tools for managin - **Attachment Support** - `zammad_get_article_attachments` - List attachments for a ticket article - `zammad_download_attachment` - Download attachment content (base64-encoded) - - `zammad_delete_attachment` - Delete attachments from ticket articles - **User & Organization Management** - `zammad_create_user` - Create a Zammad user @@ -428,15 +427,6 @@ Use zammad_add_article with attachments parameter: ] ``` -### Delete an Attachment - -```plaintext -Use zammad_delete_attachment with: -- ticket_id: 123 -- article_id: 456 -- attachment_id: 789 -``` - ## Development ### Setup diff --git a/mcp_zammad/client.py b/mcp_zammad/client.py index ce9da7d4..48a95f89 100644 --- a/mcp_zammad/client.py +++ b/mcp_zammad/client.py @@ -331,24 +331,6 @@ def add_article( return dict(self.api.ticket_article.create(article_data)) - def delete_attachment(self, ticket_id: int, article_id: int, attachment_id: int) -> bool: - """Delete an attachment from a ticket article. - - Args: - ticket_id: Ticket ID - article_id: Article ID - attachment_id: Attachment ID to delete - - Returns: - True if deletion succeeded - - Raises: - Exception if deletion fails - """ - result = self.api.ticket_article_attachment.destroy(attachment_id, article_id, ticket_id) - # destroy() returns True on success, may return dict on error - return bool(result) - def get_user(self, user_id: int) -> dict[str, Any]: """Get user information by ID.""" return dict(self.api.user.find(user_id)) diff --git a/mcp_zammad/models.py b/mcp_zammad/models.py index 29d0a4fb..0c276135 100644 --- a/mcp_zammad/models.py +++ b/mcp_zammad/models.py @@ -402,24 +402,6 @@ class DownloadAttachmentParams(StrictBaseModel): ) -class DeleteAttachmentParams(StrictBaseModel): - """Delete attachment request parameters.""" - - ticket_id: int = Field(gt=0, description="Ticket ID") - article_id: int = Field(gt=0, description="Article ID") - attachment_id: int = Field(gt=0, description="Attachment ID") - - -class DeleteAttachmentResult(StrictBaseModel): - """Result of attachment deletion operation.""" - - success: bool = Field(description="Whether the deletion succeeded") - ticket_id: int = Field(description="Ticket ID") - article_id: int = Field(description="Article ID") - attachment_id: int = Field(description="Attachment ID that was deleted") - message: str = Field(description="Human-readable result message") - - class TagOperationParams(StrictBaseModel): """Tag operation (add/remove) request parameters.""" diff --git a/mcp_zammad/server.py b/mcp_zammad/server.py index 4a5f5443..87bcfca0 100644 --- a/mcp_zammad/server.py +++ b/mcp_zammad/server.py @@ -26,8 +26,6 @@ ArticleCreate, Attachment, AttachmentDownloadError, - DeleteAttachmentParams, - DeleteAttachmentResult, DownloadAttachmentParams, GetArticleAttachmentsParams, GetOrganizationParams, @@ -59,27 +57,6 @@ ) -class AttachmentDeletionError(Exception): - """Raised when attachment deletion fails.""" - - def __init__(self, ticket_id: int, article_id: int, attachment_id: int, reason: str) -> None: - """Initialize attachment deletion error. - - Args: - ticket_id: Ticket ID - article_id: Article ID - attachment_id: Attachment ID that failed to delete - reason: Reason for failure - """ - self.ticket_id = ticket_id - self.article_id = article_id - self.attachment_id = attachment_id - self.reason = reason - super().__init__( - f"Failed to delete attachment {attachment_id} from article {article_id} in ticket {ticket_id}: {reason}" - ) - - # Protocol for items that can be dumped to dict (for type safety) class _Dumpable(Protocol): """Protocol for Pydantic models with id, name, and model_dump.""" @@ -1360,51 +1337,6 @@ def zammad_download_attachment(params: DownloadAttachmentParams) -> str: # Convert bytes to base64 string for transmission return base64.b64encode(attachment_data).decode("utf-8") - @self.mcp.tool(annotations=_destructive_write_annotations("Delete Attachment")) - def zammad_delete_attachment(params: DeleteAttachmentParams) -> DeleteAttachmentResult: - """Delete an attachment from a ticket article. - - Args: - params: DeleteAttachmentParams with ticket_id, article_id, attachment_id - - Returns: - DeleteAttachmentResult with success status and message - - Examples: - - Use when: Removing incorrect file uploads or outdated attachments - - Don't use when: Attachment IDs unknown (list attachments first) - - Note: - Requires Zammad delete permissions. Deletion is permanent. - """ - client = self.get_client() - - try: - success = client.delete_attachment( - ticket_id=params.ticket_id, - article_id=params.article_id, - attachment_id=params.attachment_id, - ) - except Exception as e: - raise AttachmentDeletionError( - ticket_id=params.ticket_id, - article_id=params.article_id, - attachment_id=params.attachment_id, - reason=str(e), - ) from e - - return DeleteAttachmentResult( - success=success, - ticket_id=params.ticket_id, - article_id=params.article_id, - attachment_id=params.attachment_id, - message=( - f"Successfully deleted attachment {params.attachment_id} from article {params.article_id} in ticket {params.ticket_id}" - if success - else f"Failed to delete attachment {params.attachment_id}" - ), - ) - @self.mcp.tool(annotations=_idempotent_write_annotations("Add Ticket Tag")) def zammad_add_ticket_tag(params: TagOperationParams) -> TagOperationResult: """Add a tag to a ticket (idempotent operation). diff --git a/tests/test_client.py b/tests/test_client.py index 31e8fa79..56184e2e 100644 --- a/tests/test_client.py +++ b/tests/test_client.py @@ -289,34 +289,3 @@ def test_add_article_without_time_unit_excludes_field(mock_api: MagicMock) -> No assert result["id"] == 789 call_args = mock_instance.ticket_article.create.call_args[0][0] assert "time_unit" not in call_args - - -@patch("mcp_zammad.client.ZammadAPI") -def test_delete_attachment_success(mock_api: MagicMock) -> None: - """Test successful attachment deletion.""" - mock_instance = mock_api.return_value - mock_instance.ticket_article_attachment.destroy.return_value = True - - with patch.dict( - os.environ, {"ZAMMAD_URL": "https://test.zammad.com/api/v1", "ZAMMAD_HTTP_TOKEN": "token"}, clear=True - ): - client = ZammadClient() - result = client.delete_attachment(ticket_id=123, article_id=456, attachment_id=789) - - assert result is True - mock_instance.ticket_article_attachment.destroy.assert_called_once_with(789, 456, 123) - - -@patch("mcp_zammad.client.ZammadAPI") -def test_delete_attachment_failure(mock_api: MagicMock) -> None: - """Test attachment deletion failure.""" - mock_instance = mock_api.return_value - mock_instance.ticket_article_attachment.destroy.return_value = False - - with patch.dict( - os.environ, {"ZAMMAD_URL": "https://test.zammad.com/api/v1", "ZAMMAD_HTTP_TOKEN": "token"}, clear=True - ): - client = ZammadClient() - result = client.delete_attachment(ticket_id=123, article_id=456, attachment_id=789) - - assert result is False diff --git a/tests/test_models.py b/tests/test_models.py index f9761bea..1b9ea654 100644 --- a/tests/test_models.py +++ b/tests/test_models.py @@ -6,8 +6,6 @@ from mcp_zammad.models import ( ArticleCreate, AttachmentUpload, - DeleteAttachmentParams, - DeleteAttachmentResult, GetTicketParams, ResponseFormat, TicketCreate, @@ -185,53 +183,3 @@ def test_article_without_attachments(self): assert article.ticket_id == 123 assert article.body == "Simple comment" assert article.attachments is None - - -class TestDeleteAttachmentParams: - """Tests for DeleteAttachmentParams model.""" - - def test_valid_params(self): - """Test creating valid delete attachment parameters.""" - params = DeleteAttachmentParams(ticket_id=123, article_id=456, attachment_id=789) - assert params.ticket_id == 123 - assert params.article_id == 456 - assert params.attachment_id == 789 - - def test_invalid_ticket_id(self): - """Test that ticket_id must be positive.""" - with pytest.raises(ValidationError, match="greater than 0"): - DeleteAttachmentParams(ticket_id=0, article_id=456, attachment_id=789) - - -class TestDeleteAttachmentResult: - """Tests for DeleteAttachmentResult model.""" - - def test_successful_deletion(self): - """Test creating successful deletion result.""" - result = DeleteAttachmentResult( - success=True, - ticket_id=123, - article_id=456, - attachment_id=789, - message="Successfully deleted attachment 789 from article 456 in ticket 123", - ) - assert result.success is True - assert result.ticket_id == 123 - assert result.article_id == 456 - assert result.attachment_id == 789 - assert "Successfully deleted" in result.message - - def test_failed_deletion(self): - """Test creating failed deletion result.""" - result = DeleteAttachmentResult( - success=False, - ticket_id=123, - article_id=456, - attachment_id=789, - message="Failed to delete attachment 789", - ) - assert result.success is False - assert result.ticket_id == 123 - assert result.article_id == 456 - assert result.attachment_id == 789 - assert "Failed" in result.message diff --git a/tests/test_server.py b/tests/test_server.py index e5e7e7be..0073ff38 100644 --- a/tests/test_server.py +++ b/tests/test_server.py @@ -20,7 +20,6 @@ ArticleType, Attachment, AttachmentUpload, - DeleteAttachmentParams, GetOrganizationParams, GetTicketParams, GetTicketStatsParams, @@ -45,7 +44,6 @@ ) from mcp_zammad.server import ( CHARACTER_LIMIT, - AttachmentDeletionError, ZammadMCPServer, _format_ticket_detail_markdown, main, @@ -2595,64 +2593,22 @@ def test_download_attachment_error(self) -> None: with pytest.raises(Exception, match="API Error"): server_inst.client.download_attachment(123, 456, 789) # type: ignore[union-attr] - def test_delete_attachment_tool_success(self, decorator_capturer) -> None: - """Test zammad_delete_attachment tool success.""" - server_inst = ZammadMCPServer() - server_inst.client = Mock() - - # Mock successful deletion - server_inst.client.delete_attachment.return_value = True # type: ignore[union-attr] - - # Setup tools using decorator_capturer fixture - test_tools, capture_tool = decorator_capturer(server_inst.mcp.tool) - server_inst.mcp.tool = capture_tool # type: ignore[method-assign, assignment] - server_inst.get_client = lambda: server_inst.client # type: ignore[method-assign, assignment, return-value] - server_inst._setup_tools() - - # Create params - params = DeleteAttachmentParams(ticket_id=123, article_id=456, attachment_id=789) + def test_no_attachment_deletion_tool_is_registered(self, decorator_capturer) -> None: + """Zammad exposes no attachment-deletion endpoint, so no tool may claim to. - # Call tool - result = test_tools["zammad_delete_attachment"](params) - - # Verify result structure - assert result.success is True - assert result.ticket_id == 123 - assert result.article_id == 456 - assert result.attachment_id == 789 - assert "Successfully deleted attachment 789" in result.message - assert "article 456" in result.message - assert "ticket 123" in result.message - - # Verify client called correctly - server_inst.client.delete_attachment.assert_called_once_with( # type: ignore[union-attr] - ticket_id=123, article_id=456, attachment_id=789 - ) - - def test_delete_attachment_tool_not_found(self, decorator_capturer) -> None: - """Test zammad_delete_attachment with non-existent attachment.""" + The REST API only routes GET /ticket_attachment/:ticket_id/:article_id/:id. + The closest supported operation is deleting the whole article via + DELETE /ticket_articles/:id. + """ server_inst = ZammadMCPServer() server_inst.client = Mock() - # Mock API error - server_inst.client.delete_attachment.side_effect = Exception("Attachment not found") # type: ignore[union-attr] - - # Setup tools using decorator_capturer fixture test_tools, capture_tool = decorator_capturer(server_inst.mcp.tool) server_inst.mcp.tool = capture_tool # type: ignore[method-assign, assignment] server_inst.get_client = lambda: server_inst.client # type: ignore[method-assign, assignment, return-value] server_inst._setup_tools() - # Create params - params = DeleteAttachmentParams(ticket_id=123, article_id=456, attachment_id=999) - - # Verify AttachmentDeletionError is raised - with pytest.raises(AttachmentDeletionError) as exc_info: - test_tools["zammad_delete_attachment"](params) - - # Verify error details - assert exc_info.value.attachment_id == 999 - assert "Attachment not found" in str(exc_info.value) + assert "zammad_delete_attachment" not in test_tools class TestJSONOutputAndTruncation: