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?

[arndjan]

Hi all, I just had my PR #359 closed in favor of this one, so I wanted to share my approach here since it addresses the base64 concern raised above.

In my fork I use a filePath parameter instead of base64 — the MCP server reads the file from disk and streams it to the upload API. This avoids loading the entire file into the LLM context window, which was the main scalability concern with base64.

{
  comments: [{
    taskId: "123456",
    content: "Here's the quarterly report",
    filePath: "/path/to/quarterly-report.pdf"
  }]
}

The LLM only passes a file path string, not the file contents. The MCP server handles the actual file reading and upload. This works well in practice — I've been using it in production for weeks for automated invoice filing workflows.

This seems like a pragmatic middle ground while pre-signed URLs or Manifest-level upload/get steps are still being designed.

[scottlovegrove]

@arndjan I'm not sure the file path approach would work for the remote MCP server, would it?

[arndjan]

@scottlovegrove You're right — filePath won't work for the remote/hosted MCP server. But for stdio/self-hosted setups (which is what most Claude Code, Cursor, etc. users run), it's the pragmatic solution and works well in practice.

Maybe both approaches could coexist: filePath for local MCP servers, and pre-signed URLs (as @goncalossilva suggested) for remote. That way you don't block the local use case while working toward a proper remote solution.

[adrian-skybaker]

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.

"Find the booking emails with pdfs for my flights, car rental, parking and add them all as attachments under my trip in todoist"

exact use case today, the following discussion with the LLM lead me here, and next to setting it up with an API key to workaround.

[arndjan]

@adrian-skybaker Great use case — very similar to what I built this for.

My main workflow: I have Claude Code monitor my email for incoming invoices (PDF attachments), download them, file them in Dropbox, and attach them as a comment to a payment task in Todoist. The task gets a deadline (due date minus 2 days) and the file path in the notes. Fully automated weekly invoice filing.

Another one: attaching generated reports or exported documents to project tasks as documentation.

Both of these work well with the filePath approach in my fork (file stays on disk, MCP server streams it to the upload API, LLM only passes the path string). Happy to help if you want to set it up.