fix!: remove zammad_delete_attachment, Zammad has no such endpoint

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

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

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))

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."""
 

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).

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

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

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 @@
         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: