Skip to content

Conversation

@krissetto
Copy link
Contributor

@krissetto krissetto commented Feb 8, 2026

  • Inline text based files instead of using dedicated file api for now (can be improved to detect text files and send the standard plaintext content type instead, but this is at least standard and will work the same for all providers right now)
  • Don't strip placeholders in msg text, they can be useul when referencing multiple files
  • Fix double @@ when using the file picker for an @ attachment

Fixes these issues seen in the screenshots below:

  • text files not ending in .txt were being skipped;
  • .md files were being rejected by anthropic's file api
  • placeholders for the attachments were no longer showing in the TUI after sending the message
Screenshot 2026-02-08 at 13 17 49 Screenshot 2026-02-08 at 13 20 34

@krissetto krissetto requested a review from a team as a code owner February 8, 2026 15:01
- Inilne text based files instead of using dedicated file api for now
- Don't strip placeholders in msg text, they can be useul when referencing multiple files
- Fix double @@ when using the file picker for an @ attachment

Signed-off-by: Christopher Petito <chrisjpetito@gmail.com>
@krissetto krissetto force-pushed the improve-file-attach branch from ec524e9 to 4efe40d Compare February 8, 2026 15:07
Copy link
Contributor

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review Summary

I've reviewed the changes for better file attachment handling. The implementation looks solid overall - the shift to inlining text files directly instead of using provider-specific file APIs is a good approach for cross-provider compatibility.

Key Changes Verified

✅ Text files are now inlined with XML-like tags for context
✅ Binary files (images, PDFs) remain as file references
✅ Comprehensive test coverage added
✅ Proper error handling for file operations
✅ Size limits enforced (5MB for inline text files)

Minor Note

There's a theoretical TOCTOU (Time-of-Check-Time-of-Use) race condition where a file could grow between the size check and the actual read operation. However, this has minimal practical impact because:

  • The application is a local TUI/CLI tool operating on user-provided files
  • Error handling properly catches file access failures
  • The exploit window is microseconds and requires filesystem write access
  • Size checks prevent most DOS scenarios

The code is well-structured with good error handling and user warnings. Nice work!

@dgageot dgageot merged commit bdc5bcf into docker:main Feb 9, 2026
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants