Skip to content

zammad_delete_attachment always fails: Resource.destroy() takes 2 positional arguments but 4 were given #320

Description

@jovamateus26

Describe the bug

zammad_delete_attachment fails on every call, regardless of the ids passed:

Error calling tool 'zammad_delete_attachment': Failed to delete attachment 1 from
article 8 in ticket 4: Resource.destroy() takes 2 positional arguments but 4 were given

Root cause (two layers)

  1. Wrong arity. client.py#L348 calls self.api.ticket_article_attachment.destroy(attachment_id, article_id, ticket_id). TicketArticleAttachment only overrides download(); its destroy() is inherited from Resource, which takes a single id. Hence the TypeError.

  2. The endpoint does not exist. Fixing the arity would not make the tool work. Zammad routes exactly one path for attachments (config/routes/ticket.rb):

    match api_path + '/ticket_attachment/:ticket_id/:article_id/:id', to: 'ticket_articles#attachment', via: :get

    :via => :get only — there is no DELETE counterpart, and TicketArticleAttachment.path_attribute is ticket_attachment, so even a correct single-id call would hit an unrouted URL. The closest supported operation is deleting the whole article: DELETE /api/v1/ticket_articles/:id (ticket_articles#destroy).

Checked against Zammad 6.5 sources in a running container, not just the online docs.

Why CI did not catch it

tests/test_client.py::test_delete_attachment_success sets mock_instance.ticket_article_attachment.destroy.return_value = True on a plain MagicMock, which happily accepts a 3-argument call the real class cannot. A spec'd/autospec'd mock would have failed at that line.

To Reproduce

  1. Add an article with an attachment via zammad_add_article
  2. Confirm it exists with zammad_get_article_attachments (works fine)
  3. Call zammad_delete_attachment with the same ticket_id / article_id / attachment_id
  4. See the TypeError above

Expected behavior

Either the tool deletes the attachment, or it does not ship — since Zammad offers no such endpoint, I think the second is the honest option.

Environment

  • mcp-zammad at 8873b2e (main), run via uvx --from git+https://github.com/basher83/zammad-mcp.git mcp-zammad
  • zammad-py 3.2.1
  • Zammad 6.5 (docker-compose)
  • Python 3.12, Windows 11

Additional context

I have a PR that removes the tool and its model/client/error surface, replacing the mocked tests with a guard that asserts the tool is not registered. Happy to switch it to "keep the tool but raise a clear, documented NotSupportedError pointing at article deletion" if you would rather not drop a tool from the public surface — just say which you prefer.

Metadata

Metadata

Assignees

No one assigned

    Labels

    area:ci-cdContinuous integration and deployment pipelinesarea:docsKnowledge management and documentationarea:infraInfrastructure, networking, and automationarea:pythonPython development and toolingtype:bugSomething is not working correctly

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions