fix(uploads): attach compiled binary for AI-generated docs, not source#5266
Conversation
AI-generated documents (pdf/docx/pptx/xlsx) created in Chat are stored as their generation source, with the rendered binary in a separate content-addressed artifact store. Read/preview paths swap in the binary, but attachment/upload/provider paths downloaded the raw source — so a generated PDF emailed via Gmail (and 30+ other tools) arrived as the generator script renamed .pdf. - Add shared resolveServableDocBytes resolver + downloadServableFileFromStorage wrapper; the file-serve route now delegates to the same resolver so the two paths resolve identically. - Migrate ~34 attachment/upload/parse tool routes + the LLM provider attachment path to the servable download; media-only tools and source-editing paths keep the raw download intentionally. - Surface a retryable 409 (shared docNotReadyResponse) when a doc artifact is still compiling instead of shipping source.
PR SummaryMedium Risk Overview Centralizes resolution in Migrates ~34 tool routes (email, cloud uploads, parse, A2A, Teams/Slack, etc.), LLM provider file uploads, and Adds Reviewed by Cursor Bugbot for commit a1a69cb. Configure here. |
Greptile SummaryThis PR fixes a bug where AI-generated documents (PDF/DOCX/PPTX/XLSX) created in Chat were sent as raw generation source (Python/JS script bytes) instead of the compiled binary artifact when attached via email, chat, or upload tools. The fix introduces a shared
Confidence Score: 5/5Safe to merge. The bug fix is well-scoped, the new resolver is the single source of truth for both the file-serve route and all attachment paths, and all previous feedback has been addressed. The core resolver logic is sound: magic-byte passthrough correctly handles real uploaded binaries for every affected format, DocCompileUserError is surfaced consistently as a 409 across all 34+ tool routes, and the post-resolve maxBytes re-check guards against compiled artifacts larger than their source. All previously flagged gaps (XLSX test coverage, empty content type fallback, Slack/Teams/UptimeRobot 409 handling) have been closed in follow-up commits. No files require special attention. The new shared files carry the most logic and are well-tested. Important Files Changed
Reviews (6): Last reviewed commit: "fix(sendgrid): reject attachments exceed..." | Re-trigger Greptile |
…sends The slack send-message and teams write_channel/write_chat routes call download helpers that can throw DocCompileUserError while a generated doc is still compiling. Map it to the shared docNotReadyResponse 409 (matching the other migrated tool routes) instead of a generic 500. The provider attachment path is internal LLM execution (no HTTP response), so it intentionally propagates the typed error.
|
The latest updates on your projects. Learn more about Vercel for GitHub. |
…lsx tests Address review findings: - uptimerobot create-psp/update-psp now map DocCompileUserError to the shared 409 (Greptile + Cursor flagged the gap alongside slack/teams). - downloadServableFileFromStorage returns the extension-derived MIME (getMimeTypeFromExtension) for non-doc files instead of an empty string when userFile.type is unset. - Add resolveServableDocBytes tests for the three xlsx branches (binary ZIP passthrough, not-ready throw under E2B+beta, no-workspaceId raw passthrough).
|
@greptile |
|
@cursor review |
Size limits were checked against userFile.size (source metadata) before resolution, but a generated doc resolves to a larger compiled binary — so a small-source doc could pass the pre-check yet exceed the service limit. Add a post-resolution check on the actual resolved bytes (mirroring docusign/vanta) across gmail send/draft/edit-draft, smtp, outlook send/draft, telegram, sftp, and teams; the cheap source pre-check stays as an early reject.
|
@greptile |
|
@cursor review |
There was a problem hiding this comment.
✅ Bugbot reviewed your changes and found no new issues!
Comment @cursor review or bugbot run to trigger another review on this PR
Reviewed by Cursor Bugbot for commit d1b3230. Configure here.
|
@greptile |
|
@cursor review |
The SFTP batch upload checked each resolved file against the 100MB cap individually, so multiple resolved attachments could each pass while their combined size exceeded the limit. Accumulate resolved bytes across the loop and reject once the running total exceeds the cap.
|
@cursor review |
…resolved bytes SendGrid had no attachment-size guard, so a generated doc resolving to a large compiled binary could be sent and fail opaquely at the API. Add a post-resolution total-size check (30MB, SendGrid's documented message limit) matching the gmail/smtp/outlook routes.
|
@greptile |
|
@cursor review |
There was a problem hiding this comment.
✅ Bugbot reviewed your changes and found no new issues!
Comment @cursor review or bugbot run to trigger another review on this PR
Reviewed by Cursor Bugbot for commit a1a69cb. Configure here.
Summary
.pdf(issue Mothership Chat PDF Attachment Sent via Gmail Is Corrupted #5260).resolveServableDocBytesresolver +downloadServableFileFromStoragewrapper; the file-serve route now delegates to the same resolver so the serve and attachment paths resolve identically.file/managecompress/decompress) intentionally keep the raw download.docNotReadyResponse) when a doc's artifact is still compiling, instead of shipping source bytes.Root cause / when introduced
%PDF/ZIP magic) were always unaffected.Type of Change
Testing
doc-servable.test.tscovering every resolver branch (passthrough / artifact-load / not-ready-throws / isolated-vm-compile / non-doc).agiloft+brexroute tests for the new return shape.tscclean,biomeclean,check:api-validationpasses.Checklist