feat: Add file attachment support to add-comment tool

[scottlovegrove]

Summary

Adds file attachment support to the add-comment tool using the new uploadFile() method from Todoist SDK v5.8.

Changes

• File Upload Integration: Uses two-step process: upload file first with uploadFile(), then attach to comment with addComment()
• API Support: Accepts fileData (base64-encoded), fileName (required when fileData provided), and fileType (optional MIME type)
• Backward Compatible: Existing usage without files continues to work unchanged
• Bulk Operations: Supports mixed scenarios where some comments have files and others don't
• Enhanced Output: Shows attachment count in user-friendly format: "Added 3 task comments (2 with an attachment)"
• Error Handling: Graceful handling of upload failures with clear error messages
• Comprehensive Tests: 12 test cases covering all attachment scenarios

API Usage

{
  comments: [{
    taskId: "123456",
    content: "Here's the quarterly report",
    fileData: "JVBERi0xLjQKMSAwIG9iago8PC9UeXBlL...", // base64
    fileName: "quarterly-report.pdf",
    fileType: "application/pdf"  // optional
  }]
}

Technical Details

• Files transmitted as base64 strings (standard for JSON APIs)
• Each comment supports one attachment (matching SDK design)
• SDK's uploadFile() method accepts Buffer input with required fileName
• Attachment metadata from upload result is passed to comment creation
• File uploads happen sequentially to avoid API overwhelming

Test Coverage

• Single comment with attachment
• Mixed comments (with and without attachments)
• Project vs task file uploads
• Upload error handling
• Optional fileType handling
• Schema validation

All tests pass (337/337) ✅

[goncalossilva]

There may be security considerations before we add this. @deorus when you have a chance, can you share your 2c here?

[gnapse]

There may be security considerations before we add this.

If there are security considerations, shouldn't they be addressed at the API/backend level? If the API allows this operation, then there cannot be any security consideration in enabling it for the MCP server, right? Or am I missing something?

[scottlovegrove]

shouldn't they be addressed at the API/backend level?

That's a fair point. And the endpoint is [poorly] documented in our documentation.

[goncalossilva]

@gnapse it's tricky. :)

But if we actively block this tool on the work we're doing, it's less of a concern.

[goncalossilva]

Reviewing the change itself, I have a question and a significant concern:

Question: How do we expect this to be used? File uploads is not something I've seen used by an agent or in a workflow, so I'd love to understand this better.

Concern: I find the base64 approach very problematic. This means the whole file will be loaded into memory. Worse, it's our MCP server's memory. This is not a good idea. 😬

How are other MCP servers handling file uploads?

[gnapse]

Concern: I find the base64 approach very problematic. This means the whole file will be loaded into memory. Worse, it's our MCP server's memory. This is not a good idea. 😬

I did think of this before, when reviewing this. It means the entire base64 string needs to be handled in the LLM context, and reproduced correctly by the LLM when invoking the tool. There must be some other way to do this.

[scottlovegrove]

Question: How do we expect this to be used? File uploads is not something I've seen used by an agent or in a workflow, so I'd love to understand this better.

My thinking was that an LLM could create a doc/pdf/etc, and be able to attach that as a comment to a task (say a report of some description).

Concern: I find the base64 approach very problematic. This means the whole file will be loaded into memory. Worse, it's our MCP server's memory. This is not a good idea. 😬

I can investigate alternative approaches, but if you have any immediate thoughts, I'm open to them.

[goncalossilva]

Sorry @scottlovegrove, if I had a suggestion, I'd have mentioned it!

I can only think of pre-signed URLs, because the way we handle uploads normally isn't ideal either and they're the solution for it. And it could help in this case too: we'd hand out a pre-signed URL to the LLM, and then it (or a tool, or its user) would just direct the file upload to it. But that's an incomplete thought, and not something we support just yet.

[goncalossilva]

Analyzing some of the best MCP servers out there that support this would be the way I'd go about it.

[amix]

Another approach could be to add two new steps to our Manifest: ⁠upload_file and ⁠get_file. This would let us fetch and store files directly without involving the MCP.

As noted by @goncalossilva, base64 encoding has some fundamental limitations here, and I'm worried about scaling issues down the line. If we introduce specialized steps, we can unlock significant optimizations.

[scottlovegrove]

Another approach could be to add two new steps to our Manifest: ⁠upload_file and ⁠get_file. This would let us fetch and store files directly without involving the MCP.

Would that need to actually go in the manifest though?