MCP: EmbeddedResource attachments, export_attachment tool, DuckDB type fixes, MIME date fix#55
Merged
MCP: EmbeddedResource attachments, export_attachment tool, DuckDB type fixes, MIME date fix#55
Conversation
…se64 Return attachment content as a native MCP EmbeddedResource with BlobResourceContents instead of embedding base64 in a custom JSON payload. MCP clients handle EmbeddedResource blobs natively, removing the need for clients to parse custom JSON and manually decode base64. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
The post-commit hook was gitignored and missing from the tracked .githooks directory, so it wasn't firing in worktrees. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Address code review findings: - Use json.Marshal for metadata instead of fmt.Sprintf with %q, which can produce invalid JSON for non-UTF8 or control bytes in filenames. - URL-encode the filename in the embedded resource URI to handle spaces and special characters. - Add tests for empty MIME type defaulting to application/octet-stream. - Add tests for filenames with spaces and unicode characters. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Add explicit CASTs in COALESCE expressions to prevent "Cannot mix values of VARCHAR and INTEGER_LITERAL" binder errors. Parquet column types inferred from SQLite may not match the literal fallback values (e.g., size_estimate stored as VARCHAR instead of BIGINT). Explicit CASTs ensure type consistency regardless of Parquet schema inference. Fixes SearchFast, ListMessages, Aggregate, and GetTotalStats queries. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
…olumns Creates Parquet files where conversation_id and size_estimate are stored as VARCHAR (not BIGINT), reproducing the DuckDB binder error "Cannot mix values of VARCHAR and INTEGER_LITERAL in COALESCE operator". Tests ListMessages, SearchFast, SearchFastCount, Aggregate, and GetTotalStats. Verified the test fails without the CAST fix and passes with it. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Cast all Parquet table columns to their expected types in parquetCTEs() using DuckDB's REPLACE syntax, fixing type mismatches that caused "Cannot compare values of type BIGINT and VARCHAR in IN/ANY/ALL clause" in ListMessages with sender/recipient filters. This also makes the per-column CASTs in COALESCE expressions redundant, so they are removed. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
time.Parse with named timezone abbreviations (MST, EST, PST) resolves them against the local system timezone, producing different results on different machines. On a CST system, parsing "15:04:05 EST" would resolve EST to -5h, convert to local time (14:04:05 CST), and then t.Hour() would return 14 instead of the input's 15. Fix by using time.ParseInLocation(..., time.UTC) which forces unknown named timezones to offset 0, preserving the input time values as UTC. Numeric offsets remain absolute and unaffected. This eliminates the need for the hasNumericOffset/toUTC helper functions entirely. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Claude Desktop can only display image attachments inline. For PDFs and other file types, add an export_attachment tool that writes the file to the local filesystem (defaults to ~/Downloads) and returns the path. Handles filename sanitization and collision avoidance (_1, _2, etc). Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Contributor
- Use TRY_CAST for has_attachments (BOOLEAN) and att.size (BIGINT) in parquetCTEs to handle non-castable VARCHAR values gracefully instead of failing at runtime. Test fixture updated to store both as VARCHAR. - Fix TOCTOU race in export_attachment: replace stat-then-write with O_CREATE|O_EXCL via createExclusive() to atomically create files without symlink race conditions. - Add tests for filename sanitization edge cases (empty, dot, path traversal, special characters) and parseDate named timezone coverage (EST, CST explicitly tested for platform independence). Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
O_EXCL returns EISDIR (not EEXIST) when a directory exists at the path. The new pathConflict helper catches both cases so createExclusive falls through to suffixed names instead of returning an error. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Contributor
|
Hmmm, I see. Golang has Key Differences from time.ParseThe main distinction from the standard time.Parse is how a lack of explicit timezone information in the input string is handled.
When the input string does contain timezone information (like a numeric offset or abbreviation), ParseInLocation uses the provided loc to resolve and match that information, whereas time.Parse tries to match it against the system's Local location. |
Owner
Author
|
I'm still actively working on this branch — there was a test failing after the previous PR was merged so bear with me |
Owner
Author
|
I'm reverting b23936c and will get this cleaned up |
This reverts commit b23936c.
time.Parse resolves named timezone abbreviations (EST, PST, etc.) against the local system timezone, producing different results on different machines. Fix by using time.ParseInLocation(..., time.UTC) which forces named timezones to offset 0, making parsing deterministic. This builds on the hasNumericOffset/toUTC helpers from PR #50, which correctly separate numeric-offset and named-timezone conversion logic. ParseInLocation ensures toUTC receives consistent input regardless of the host system's timezone. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
MCP get_attachment improvement:
EmbeddedResourcewithBlobResourceContentsinstead of embedding base64 in a custom JSON text payload. MCP clients (e.g. Claude Desktop) handle embedded resources natively without manual JSON parsing and base64 decoding.json.Marshalfor metadata JSON instead offmt.Sprintfwith%qto ensure valid JSON for all filename byte sequences.New
export_attachmentMCP tool:export_attachmenttool saves an attachment to the local filesystem (defaults to~/Downloads) and returns the file path._1,_2, etc.O_CREATE|O_EXCLto prevent TOCTOU races.EISDIR(directory occupying target name) by falling through to suffixed names.DuckDB type mismatch fixes:
SELECT * REPLACE (CAST(...))syntax. This fixes two classes of type mismatch errors caused by Parquet schema inference from SQLite storing numeric columns as VARCHAR:Cannot mix values of VARCHAR and INTEGER_LITERAL in COALESCE operator— in SearchFast, ListMessages, Aggregate, and GetTotalStats queries.Cannot compare values of type BIGINT and VARCHAR in IN/ANY/ALL clause— in ListMessages when sender/recipient filters trigger thefiltered_msgsCTE with JOIN conditions across differently-typed columns.TRY_CASTforhas_attachmentsand attachmentsizecolumns for resilient type conversion.MIME date parsing fix:
time.Parseresolves named timezone abbreviations (EST, MST, PST) against the local system timezone, producing different results on different machines.time.ParseInLocation(..., time.UTC)which forces named timezones to offset 0, making parsing deterministic. This builds on thehasNumericOffset/toUTChelpers from PR Bug mime date parse #50 which correctly separate numeric-offset and named-timezone conversion logic.Housekeeping:
post-commithook in.githooks/so it works across worktrees.$HOMEinstead of hardcoded path for portability.Test plan
go test ./...)export_attachmentin Claude Desktop saves PDF to ~/Downloads🤖 Generated with Claude Code