feat(mail): support large email attachments#537
Merged
chanthuang merged 38 commits intomainfrom Apr 21, 2026
Merged
Conversation
github-actions
bot
added
domain/mail
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds end-to-end large-attachment support: classify attachments by estimated EML size, upload oversized files via media APIs, insert download-link HTML cards into draft HTML, store ordered tokens in a base64+JSON draft header, and preserve/remove cards and signatures via new draft/HTML helpers. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant Composer as Mail Command
participant Classifier as Attachment<br/>Classifier
participant Uploader as Media<br/>Upload API
participant HTMLBuilder as HTML Card<br/>Builder
participant Draft as Draft<br/>Snapshot
Client->>Composer: compose/send with attachments + body
Composer->>Classifier: classifyAttachments(files, emlBase)
Classifier-->>Composer: normal[], oversized[]
loop upload oversized files
Composer->>Uploader: upload file (medias/upload)
Uploader-->>Composer: {token, meta}
end
Composer->>HTMLBuilder: buildLargeAttachmentHTML(results)
HTMLBuilder-->>Composer: htmlCard
Composer->>Draft: InsertBeforeQuoteOrAppend(htmlCard)
Draft-->>Composer: updatedHTML
Composer->>Draft: set header `X-Lms-Large-Attachment-Ids` (base64 JSON)
Composer->>Composer: attach normal files to EML builder
Composer-->>Client: return/send result
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
🚀 PR Preview Install Guide🧰 CLI updatenpm i -g https://pkg.pr.new/larksuite/cli/@larksuite/cli@912c8a35d39542eaf667312d0b65e2d3a987ed4d🧩 Skill updatenpx skills add larksuite/cli#feat/mail-large-attachment -y -g |
chanthuang
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
shortcuts/mail/mail_draft_create.go (1)
162-200:⚠️ Potential issue | 🟠 MajorPlain-text + large attachment forces an HTML body onto the draft.
When
input.PlainTextis true,composedHTMLBodystays empty andbldonly has a text body. But if any attachment is oversized,processLargeAttachmentswill unconditionally callbld.HTMLBody(...)with the download-card HTML (viaInsertBeforeQuoteOrAppend("", largeHTML)→ justlargeHTML). Result: a draft the user asked to keep plain-text gets an HTML body containing only the large-attachment card, with no plain-text mention of the download link — so plain-text-only readers lose the link entirely and the--plain-textcontract is broken.The root cause is in
processLargeAttachments(shortcuts/mail/large_attachment.go); the same call site exists in the other four compose shortcuts (mail_send,mail_reply,mail_reply_all,mail_forward). Two reasonable fixes:
- Reject
--plain-text+ oversized attachments with a clear error; or- Append a plain-text block (e.g.,
Large files:\n - name (size) — <url>) viabld.TextBodyin addition to the HTML card so both bodies stay consistent.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@shortcuts/mail/mail_draft_create.go` around lines 162 - 200, The code allows input.PlainText to be true but processLargeAttachments can force an HTML body (via InsertBeforeQuoteOrAppend → bld.HTMLBody), breaking the plain-text contract; update the flow so plain-text drafts remain valid by either rejecting plain-text + oversized attachments or by ensuring a plain-text fallback is added: modify processLargeAttachments (in shortcuts/mail/large_attachment.go) to accept a plainText flag (or detect composedHTMLBody==""/input.PlainText) and when it would insert the download-card HTML (via InsertBeforeQuoteOrAppend / largeHTML) also return or apply a corresponding plain-text block and call bld.TextBody(...) (or return an explicit error) so the call site in mail_draft_create.go (where input.PlainText, composedHTMLBody and processLargeAttachments are used) preserves a proper text body instead of silently converting to HTML.
♻️ Duplicate comments (1)
shortcuts/mail/large_attachment.go (1)
412-416:⚠️ Potential issue | 🟠 MajorRoot cause of the plain-text HTML-body issue flagged at
mail_draft_create.go.
bld.HTMLBody(...)is called unconditionally here when any file is oversized, regardless of whether the caller built a plain-text-only draft. See the consolidated comment onshortcuts/mail/mail_draft_create.golines 162–200 for the full impact analysis and suggested fixes (error out on plain-text + oversized, or append a plain-text fallback listing).🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@shortcuts/mail/large_attachment.go` around lines 412 - 416, The code unconditionally calls bld.HTMLBody(...) with an oversized-file HTML block (built via buildLargeAttachmentHTML and inserted with draftpkg.InsertBeforeQuoteOrAppend) even when the caller intended a plain-text-only draft; change this to only set bld.HTMLBody when the original htmlBody is non-empty/HTML-mode (e.g., if htmlBody != "" or an “isHTML” flag), and for plain-text-only drafts do not call bld.HTMLBody but instead append a plain-text fallback listing of the oversized files to the text body (use bld.TextBody and create a simple plain-text representation from results) inserted before the quote or appended analogously; keep references to buildLargeAttachmentHTML, draftpkg.InsertBeforeQuoteOrAppend, bld.HTMLBody and bld.TextBody to locate and update the logic.
🧹 Nitpick comments (3)
shortcuts/mail/mail_draft_create_test.go (1)
77-104: Test now passes via the "no user identity" fallback, not the size-limit semantics it was written to verify.With
newRuntimeWithFromthe runtime has noConfig/UserOpenId, soprocessLargeAttachmentsalways returns via its identity-guard branch ("total attachment size ... exceeds the 25 MB EML limit; large attachment upload requires user identity"). Both substrings match regardless of whether the auto-resolved inline image is actually counted toward the EML budget — so this test no longer meaningfully verifies "auto-resolve counted in size limit" once identity is plumbed in. Consider either:
- tightening the assertion to require the exact "requires user identity" message, so regressions in the identity guard are caught, or
- extending the test with a runtime that has a (fake) user identity and a stub upload path to exercise the oversized classification itself.
Non-blocking; the current form is consistent with the ai_summary, but the test name now slightly overstates its coverage.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@shortcuts/mail/mail_draft_create_test.go` around lines 77 - 104, The test TestBuildRawEMLForDraftCreate_AutoResolveCountedInSizeLimit is passing due to the "no user identity" fallback (newRuntimeWithFrom yields no Config/UserOpenId) so processLargeAttachments returns the identity-guard message instead of exercising the actual size-counting logic; update the test either by tightening the assertion to explicitly expect the identity guard message (e.g., assert error contains "requires user identity" or the exact identity-related text from processLargeAttachments) or change the runtime passed into buildRawEMLForDraftCreate to include a fake UserOpenId/Config and a stubbed upload path so processLargeAttachments runs the oversized-classification branch that counts auto-resolved inline images — locate references in the test to newRuntimeWithFrom, buildRawEMLForDraftCreate, and processLargeAttachments to implement the chosen fix.shortcuts/mail/mail_forward.go (1)
185-205: Minor:origAttBytesis now dead code.
origAttBytesis accumulated (Line 204) but no longer read after the refactor —processLargeAttachmentsnow relies onorigAttEMLBytes(base64-estimated size) for headroom calculations. Consider removingorigAttBytesto keep the function lean.♻️ Proposed cleanup
var origAtts []downloadedAtt - var origAttBytes int64 type largeAttID struct { @@ origAtts = append(origAtts, downloadedAtt{content, contentType, att.Filename}) - origAttBytes += int64(len(content)) }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@shortcuts/mail/mail_forward.go` around lines 185 - 205, The variable origAttBytes is accumulated in the loop over sourceMsg.ForwardAttachments but is unused after the refactor (processLargeAttachments now uses origAttEMLBytes); remove the origAttBytes declaration and the line that increments it (origAttBytes += int64(len(content))) to eliminate dead code, leaving the rest of the loop (including largeAttID, largeAttIDs, and appending to origAtts) intact.shortcuts/mail/draft/large_attachment_parse.go (1)
98-137:extractItemMetafilename/size inference is positional and brittle.
texts[0]is treated as the filename andtexts[1]as the size display, but this depends on the exact template shape documented in the comment. IflargeAttItemTplever grows an icon<img alt="…">with a non-empty alt, or adds any other text node between the item root and the filename<div>, the projection silently reports the wrong filename and a size of0. Consider scoping extraction by structural role rather than text order — e.g. first read filename/size from a<div id="large-file-meta">wrapper (or dedicated classes), and only fall back to positional text collection.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@shortcuts/mail/draft/large_attachment_parse.go` around lines 98 - 137, The current extractItemMeta collects all non-anchor text nodes and assumes texts[0] is the filename and texts[1] the size, which is brittle; update extractItemMeta to first look for structured nodes (e.g., a wrapper element like a div with id/class such as "large-file-meta" or dedicated filename/size classes) and extract the filename and size from those nodes directly (use parseSizeDisplay on the size node's text); only if those structural selectors are not present, fall back to the existing positional collection (the token extraction via LargeAttachmentTokenAttr should remain unchanged) so you preserve backward compatibility and avoid mis-parsing when icons or extra text nodes are added.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@shortcuts/mail/draft/large_attachment_parse.go`:
- Around line 283-309: The current removeLargeFileItemFromHTML uses xhtml.Parse
and xhtml.Render which wrap fragment inputs into a full HTML document; change
removeLargeFileItemFromHTML to parse fragments instead (use xhtml.ParseFragment
with a body node: &xhtml.Node{Type: xhtml.ElementNode, DataAtom: atom.Body,
Data: "body"}), locate and remove the matching node from the returned fragment
slice, then serialize only those fragment nodes (not the full document) back to
string so original fragment structure is preserved; update
findLargeFileItemByToken/remove logic to work on the fragment nodes and add a
regression test named TestRemoveLargeAttachmentPreservesFragmentStructure that
passes a fragment like "<p>…</p><div>…</div>" through
removeLargeFileItemFromHTML and asserts the output remains a fragment (no
surrounding html/body).
In `@shortcuts/mail/mail_reply_all.go`:
- Around line 190-198: The code calls processLargeAttachments which
unconditionally injects an HTML card via bld.HTMLBody even when the draft is
plain-text (composedHTMLBody is empty), causing plain-text drafts to become
HTML; update the caller flow in mail_reply_all.go (and mirror the same change in
mail_reply.go, mail_send.go, mail_forward.go) to detect plain-text mode (e.g.,
composedHTMLBody == "" or the plain-text flag) before calling
processLargeAttachments and instead return a clear error like "oversized
attachments require HTML mode" (or skip HTML injection) so
processLargeAttachments is not allowed to convert plain-text drafts to HTML
unexpectedly.
In `@skills/lark-mail/references/lark-mail-send.md`:
- Line 172: Update the documentation to reflect the real attachment count limit
enforced by the code: change the "总附件数量上限 100 个" text to "总附件数量上限 250 个" in
lark-mail-send.md; note that the actual limit is defined by the
MaxAttachmentCount constant in shortcuts/mail/limits.go and enforced by
processLargeAttachments in shortcuts/mail/large_attachment.go, so keep the doc
consistent with those symbols.
---
Outside diff comments:
In `@shortcuts/mail/mail_draft_create.go`:
- Around line 162-200: The code allows input.PlainText to be true but
processLargeAttachments can force an HTML body (via InsertBeforeQuoteOrAppend →
bld.HTMLBody), breaking the plain-text contract; update the flow so plain-text
drafts remain valid by either rejecting plain-text + oversized attachments or by
ensuring a plain-text fallback is added: modify processLargeAttachments (in
shortcuts/mail/large_attachment.go) to accept a plainText flag (or detect
composedHTMLBody==""/input.PlainText) and when it would insert the download-card
HTML (via InsertBeforeQuoteOrAppend / largeHTML) also return or apply a
corresponding plain-text block and call bld.TextBody(...) (or return an explicit
error) so the call site in mail_draft_create.go (where input.PlainText,
composedHTMLBody and processLargeAttachments are used) preserves a proper text
body instead of silently converting to HTML.
---
Duplicate comments:
In `@shortcuts/mail/large_attachment.go`:
- Around line 412-416: The code unconditionally calls bld.HTMLBody(...) with an
oversized-file HTML block (built via buildLargeAttachmentHTML and inserted with
draftpkg.InsertBeforeQuoteOrAppend) even when the caller intended a
plain-text-only draft; change this to only set bld.HTMLBody when the original
htmlBody is non-empty/HTML-mode (e.g., if htmlBody != "" or an “isHTML” flag),
and for plain-text-only drafts do not call bld.HTMLBody but instead append a
plain-text fallback listing of the oversized files to the text body (use
bld.TextBody and create a simple plain-text representation from results)
inserted before the quote or appended analogously; keep references to
buildLargeAttachmentHTML, draftpkg.InsertBeforeQuoteOrAppend, bld.HTMLBody and
bld.TextBody to locate and update the logic.
---
Nitpick comments:
In `@shortcuts/mail/draft/large_attachment_parse.go`:
- Around line 98-137: The current extractItemMeta collects all non-anchor text
nodes and assumes texts[0] is the filename and texts[1] the size, which is
brittle; update extractItemMeta to first look for structured nodes (e.g., a
wrapper element like a div with id/class such as "large-file-meta" or dedicated
filename/size classes) and extract the filename and size from those nodes
directly (use parseSizeDisplay on the size node's text); only if those
structural selectors are not present, fall back to the existing positional
collection (the token extraction via LargeAttachmentTokenAttr should remain
unchanged) so you preserve backward compatibility and avoid mis-parsing when
icons or extra text nodes are added.
In `@shortcuts/mail/mail_draft_create_test.go`:
- Around line 77-104: The test
TestBuildRawEMLForDraftCreate_AutoResolveCountedInSizeLimit is passing due to
the "no user identity" fallback (newRuntimeWithFrom yields no Config/UserOpenId)
so processLargeAttachments returns the identity-guard message instead of
exercising the actual size-counting logic; update the test either by tightening
the assertion to explicitly expect the identity guard message (e.g., assert
error contains "requires user identity" or the exact identity-related text from
processLargeAttachments) or change the runtime passed into
buildRawEMLForDraftCreate to include a fake UserOpenId/Config and a stubbed
upload path so processLargeAttachments runs the oversized-classification branch
that counts auto-resolved inline images — locate references in the test to
newRuntimeWithFrom, buildRawEMLForDraftCreate, and processLargeAttachments to
implement the chosen fix.
In `@shortcuts/mail/mail_forward.go`:
- Around line 185-205: The variable origAttBytes is accumulated in the loop over
sourceMsg.ForwardAttachments but is unused after the refactor
(processLargeAttachments now uses origAttEMLBytes); remove the origAttBytes
declaration and the line that increments it (origAttBytes +=
int64(len(content))) to eliminate dead code, leaving the rest of the loop
(including largeAttID, largeAttIDs, and appending to origAtts) intact.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 059ddf86-4020-4a36-a61a-906b2af5d7bf
📒 Files selected for processing (27)
shortcuts/mail/draft/large_attachment_parse.goshortcuts/mail/draft/large_attachment_parse_test.goshortcuts/mail/draft/model.goshortcuts/mail/draft/patch.goshortcuts/mail/draft/patch_body_large_attachment_test.goshortcuts/mail/draft/patch_signature_test.goshortcuts/mail/draft/projection.goshortcuts/mail/emlbuilder/builder.goshortcuts/mail/emlbuilder/builder_test.goshortcuts/mail/helpers.goshortcuts/mail/helpers_test.goshortcuts/mail/large_attachment.goshortcuts/mail/large_attachment_test.goshortcuts/mail/mail_draft_create.goshortcuts/mail/mail_draft_create_test.goshortcuts/mail/mail_draft_edit.goshortcuts/mail/mail_forward.goshortcuts/mail/mail_reply.goshortcuts/mail/mail_reply_all.goshortcuts/mail/mail_send.goshortcuts/mail/signature_compose.goskills/lark-mail/references/lark-mail-draft-create.mdskills/lark-mail/references/lark-mail-draft-edit.mdskills/lark-mail/references/lark-mail-forward.mdskills/lark-mail/references/lark-mail-reply-all.mdskills/lark-mail/references/lark-mail-reply.mdskills/lark-mail/references/lark-mail-send.md
💤 Files with no reviewable changes (1)
- shortcuts/mail/helpers_test.go
There was a problem hiding this comment.
🧹 Nitpick comments (3)
shortcuts/mail/helpers.go (1)
2006-2020: Remove unusedfioandattachFlagparameters fromvalidateComposeInlineAndAttachments.The function body uses only
inlineFlag,plainText, andbody;fioandattachFlagare now unused and create misleading intent (appears to suggest attachment-level validation still occurs here). Simplify the signature to(inlineFlag string, plainText bool, body string)and update all 5 call sites:mail_forward.go:74,mail_send.go:72,mail_reply_all.go:67,mail_reply.go:66, andmail_draft_create.go:79.Attachment count limits (
MaxAttachmentCount) are enforced downstream inlarge_attachment.go:367–369duringprocessLargeAttachments, confirming the removed early validation is correctly deferred.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@shortcuts/mail/helpers.go` around lines 2006 - 2020, Remove the unused fio and attachFlag parameters from validateComposeInlineAndAttachments by changing its signature to validateComposeInlineAndAttachments(inlineFlag string, plainText bool, body string) and keep the existing body logic (including the parseInlineSpecs call); then update the five call sites in mail_forward.go, mail_send.go, mail_reply_all.go, mail_reply.go, and mail_draft_create.go to call the new three-argument function (pass inlineFlag, plainText, body) instead of the old five-argument form so attachment handling remains deferred to processLargeAttachments/large_attachment.go.shortcuts/mail/large_attachment_test.go (1)
98-113: Consider adding a token-escaping case.
buildLargeAttachmentPreviewURLpasses the token throughurl.QueryEscapeand the return value throughhtmlEscape(at the HTML layer). The current assertions only use alphanumeric tokens. A token containing&,=, or+would catch regressions in either escape, e.g.:{core.BrandFeishu, "a&b=c", "https://www.feishu.cn/mail/page/attachment?token=a%26b%3Dc"},Optional; not a correctness blocker.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@shortcuts/mail/large_attachment_test.go` around lines 98 - 113, Add a test case to TestBuildLargeAttachmentPreviewURL that verifies tokens with special characters are correctly escaped: update the table in the test to include an entry (for example using buildLargeAttachmentPreviewURL with core.BrandFeishu and a token like "a&b=c+1") and assert the expected URL contains the percent-encoded token (i.e., the token value should match url.QueryEscape's output); this targets the buildLargeAttachmentPreviewURL function and ensures both url.QueryEscape/html escaping regressions are caught.shortcuts/mail/draft/projection.go (1)
127-137: Minor: HTML part is parsed twice inProject.
findPart(snapshot.Body, snapshot.PrimaryHTMLPartID)+string(part.Body)is invoked at Line 78 and again here at Line 128. For large HTML bodies this performs a redundant byte→string conversion (plus thefindParttraversal). You could hoisthtmlBodyabove the first block and reuse it forsummarizeHTML/hasQuotedContent/ signature detection as well.♻️ Possible consolidation
+ var htmlBody string + if part := findPart(snapshot.Body, snapshot.PrimaryHTMLPartID); part != nil { + htmlBody = string(part.Body) + } - if part := findPart(snapshot.Body, snapshot.PrimaryHTMLPartID); part != nil { - html := string(part.Body) + if htmlBody != "" { + html := htmlBody proj.BodyHTMLSummary = summarizeHTML(html) proj.HasQuotedContent = hasQuotedContent(html) ... } ... - var htmlBody string - if part := findPart(snapshot.Body, snapshot.PrimaryHTMLPartID); part != nil { - htmlBody = string(part.Body) - for _, cid := range extractCIDRefs(htmlBody) { + if htmlBody != "" { + for _, cid := range extractCIDRefs(htmlBody) { ... } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@shortcuts/mail/draft/projection.go` around lines 127 - 137, The code is parsing the HTML part twice via findPart(snapshot.Body, snapshot.PrimaryHTMLPartID) and converting bytes→string again; to fix, compute htmlBody once (call findPart(...) and set htmlBody = string(part.Body) in a single hoisted variable) near where the snapshot is first processed and reuse that htmlBody for extractCIDRefs/proj.Warnings, projectLargeAttachments, summarizeHTML, hasQuotedContent and signature detection instead of re-calling findPart or reconverting the bytes; update references to findPart/snapshot.PrimaryHTMLPartID/htmlBody across functions (e.g., projectLargeAttachments and extractCIDRefs usage) so they use the shared htmlBody variable.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@shortcuts/mail/draft/projection.go`:
- Around line 127-137: The code is parsing the HTML part twice via
findPart(snapshot.Body, snapshot.PrimaryHTMLPartID) and converting bytes→string
again; to fix, compute htmlBody once (call findPart(...) and set htmlBody =
string(part.Body) in a single hoisted variable) near where the snapshot is first
processed and reuse that htmlBody for extractCIDRefs/proj.Warnings,
projectLargeAttachments, summarizeHTML, hasQuotedContent and signature detection
instead of re-calling findPart or reconverting the bytes; update references to
findPart/snapshot.PrimaryHTMLPartID/htmlBody across functions (e.g.,
projectLargeAttachments and extractCIDRefs usage) so they use the shared
htmlBody variable.
In `@shortcuts/mail/helpers.go`:
- Around line 2006-2020: Remove the unused fio and attachFlag parameters from
validateComposeInlineAndAttachments by changing its signature to
validateComposeInlineAndAttachments(inlineFlag string, plainText bool, body
string) and keep the existing body logic (including the parseInlineSpecs call);
then update the five call sites in mail_forward.go, mail_send.go,
mail_reply_all.go, mail_reply.go, and mail_draft_create.go to call the new
three-argument function (pass inlineFlag, plainText, body) instead of the old
five-argument form so attachment handling remains deferred to
processLargeAttachments/large_attachment.go.
In `@shortcuts/mail/large_attachment_test.go`:
- Around line 98-113: Add a test case to TestBuildLargeAttachmentPreviewURL that
verifies tokens with special characters are correctly escaped: update the table
in the test to include an entry (for example using
buildLargeAttachmentPreviewURL with core.BrandFeishu and a token like "a&b=c+1")
and assert the expected URL contains the percent-encoded token (i.e., the token
value should match url.QueryEscape's output); this targets the
buildLargeAttachmentPreviewURL function and ensures both url.QueryEscape/html
escaping regressions are caught.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: c4bfea97-9a6a-41cd-84c5-01197a9b19f2
📒 Files selected for processing (7)
shortcuts/mail/draft/large_attachment_parse.goshortcuts/mail/draft/large_attachment_parse_test.goshortcuts/mail/draft/projection.goshortcuts/mail/emlbuilder/builder.goshortcuts/mail/helpers.goshortcuts/mail/large_attachment.goshortcuts/mail/large_attachment_test.go
🚧 Files skipped from review as they are similar to previous changes (3)
- shortcuts/mail/draft/large_attachment_parse_test.go
- shortcuts/mail/large_attachment.go
- shortcuts/mail/draft/large_attachment_parse.go
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@shortcuts/mail/mail_forward.go`:
- Around line 216-222: The emlBase is undercounting the real EML size because it
uses int64(len(body)) instead of the actual composed body and it omits
source-message inline images; update the calculation so estimateEMLBaseSize uses
the length of the actually-composed body (use composedHTMLBody when HTML branch
is used, or call buildForwardedMessage(&orig, body) and use its length in the
plain-text branch) and include the byte total for source inline images (have
addInlineImagesToBuilder return or populate the inline paths and byte count and
add that byte count to origAttEMLBytes or a new extraBytes), ensure
allInlinePaths includes those inline image paths, then pass the corrected
emlBase into processLargeAttachments (symbols to locate: emlBase,
estimateEMLBaseSize, composedHTMLBody, buildForwardedMessage,
addInlineImagesToBuilder, allInlinePaths, origAttEMLBytes,
processLargeAttachments).
In `@shortcuts/mail/mail_reply.go`:
- Around line 179-181: emlBase understates the true EML size because it uses
int64(len(body)) and omits downloaded source-inline images added by
addInlineImagesToBuilder/bld.AddInline; update the flow so estimateEMLBaseSize
is seeded with the actual composed body length (use len(composedHTMLBody) or the
plain-text equivalent used for replies, not len(body)) and include the byte
count of source-inline images returned from addInlineImagesToBuilder (modify
addInlineImagesToBuilder to return totalBytes or otherwise report downloaded
byte size as extraBytes/srcInlineBytes); then add those bytes and/or the
returned inline file paths into allInlinePaths/extraBytes before calling
estimateEMLBaseSize or passing emlBase into processLargeAttachments so
classifyAttachments sees the true baseline.
In `@skills/lark-mail/references/lark-mail-send.md`:
- Line 71: Update the `--attach` parameter description to replace the ambiguous
phrase "超出部分自动上传为超大附件" with "超出的文件自动上传为超大附件" to make clear that whole files (not
parts) are uploaded, and append a brief constraint note that auto-uploaded large
attachments (download-card presentation) are incompatible with plain-text
messages—reflecting the rejection implemented in
shortcuts/mail/large_attachment.go (see the "plain-text messages cannot include
the download card" check). Ensure the `--attach <paths>` line mentions both the
single-file 3 GB limit and the incompatibility with plain-text messages.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 7235df45-5b6a-4469-9a62-c15f2aaf4bf9
📒 Files selected for processing (10)
shortcuts/mail/draft/large_attachment_parse.goshortcuts/mail/helpers.goshortcuts/mail/large_attachment.goshortcuts/mail/large_attachment_test.goshortcuts/mail/mail_draft_create.goshortcuts/mail/mail_forward.goshortcuts/mail/mail_reply.goshortcuts/mail/mail_reply_all.goshortcuts/mail/mail_send.goskills/lark-mail/references/lark-mail-send.md
✅ Files skipped from review due to trivial changes (3)
- shortcuts/mail/large_attachment_test.go
- shortcuts/mail/large_attachment.go
- shortcuts/mail/draft/large_attachment_parse.go
🚧 Files skipped from review as they are similar to previous changes (3)
- shortcuts/mail/mail_reply_all.go
- shortcuts/mail/mail_send.go
- shortcuts/mail/mail_draft_create.go
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
shortcuts/common/drive_media_upload.go (1)
55-78:⚠️ Potential issue | 🟡 MinorAdd test coverage for the
Readerupload path.The
Readerfield is used in production (shortcuts/mail/large_attachment.go:200), but no test exercises this code path. Add a test that constructsDriveMediaUploadAllConfigwithReaderset and verifies the multipart body contains the reader bytes, per the coding guideline that every behavior change must have an accompanying test.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@shortcuts/common/drive_media_upload.go` around lines 55 - 78, Add a unit test that exercises the UploadDriveMediaAll code path when DriveMediaUploadAllConfig.Reader is non-nil: create a mock RuntimeContext with a FileIO that is not used, construct DriveMediaUploadAllConfig with Reader set to an in-memory bytes.Reader and other required fields (FileName, ParentType, FileSize, etc.), call UploadDriveMediaAll, capture the created multipart/form-data payload (e.g., by injecting or mocking larkcore.NewFormdata or the HTTP client used by UploadDriveMediaAll), and assert that the multipart body contains the exact bytes from the provided Reader and correct form fields; focus on UploadDriveMediaAll, DriveMediaUploadAllConfig, and verifying the "file" part contains the reader bytes.
♻️ Duplicate comments (2)
shortcuts/mail/mail_forward.go (1)
203-208:⚠️ Potential issue | 🟡 Minor
emlBasestill underestimates the EML body and inline payloads.This still uses
len(body)instead of the actual composed body (composedHTMLBodyorbuildForwardedMessage(&orig, body)), and it does not account for source inline images added viaaddInlineImagesToBuilder. Near the 25MB limit, this can still classify an attachment as normal even though the final EML build exceeds the limit.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@shortcuts/mail/mail_forward.go` around lines 203 - 208, The EML size estimation uses len(body) and omits inline images added later, causing underestimation; update the estimation to use the actual composed body and include all inline image paths produced by addInlineImagesToBuilder: compute the composed HTML body (e.g., composedHTMLBody or via buildForwardedMessage(&orig, body)) and collect inline paths as allInlinePaths := append(inlineSpecFilePaths(inlineSpecs), autoResolvedPaths..., pathsReturnedByAddInlineImagesToBuilder...), then call estimateEMLBaseSize(runtime.FileIO(), int64(len(composedBody)), allInlinePaths, 0) so the EML base size reflects the final payload.shortcuts/mail/draft/large_attachment_parse.go (1)
430-433:⚠️ Potential issue | 🟠 MajorStrip the parser envelope atomically.
TrimSuffixcurrently runs even when the<html><head></head><body>prefix was not present. For a real full document with a non-empty<head>, removing a large-attachment card can return malformed HTML by stripping only</body></html>.🛠️ Proposed fix
func stripHTMLEnvelope(s string) string { - s = strings.TrimPrefix(s, "<html><head></head><body>") - s = strings.TrimSuffix(s, "</body></html>") + const prefix = "<html><head></head><body>" + const suffix = "</body></html>" + if strings.HasPrefix(s, prefix) && strings.HasSuffix(s, suffix) { + s = strings.TrimPrefix(s, prefix) + s = strings.TrimSuffix(s, suffix) + } return s }Verification: this demonstrates the one-sided suffix strip without touching repository files.
#!/bin/bash set -euo pipefail cat >/tmp/verify_strip_html_envelope.go <<'EOF' package main import ( "fmt" "strings" ) func stripHTMLEnvelope(s string) string { s = strings.TrimPrefix(s, "<html><head></head><body>") s = strings.TrimSuffix(s, "</body></html>") return s } func main() { in := `<html><head><style>.x{}</style></head><body><p>ok</p></body></html>` out := stripHTMLEnvelope(in) fmt.Println(out) if strings.Contains(out, "<body>") && !strings.Contains(out, "</body></html>") { panic("suffix stripped without prefix; output is malformed") } } EOF go run /tmp/verify_strip_html_envelope.go🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@shortcuts/mail/draft/large_attachment_parse.go` around lines 430 - 433, The stripHTMLEnvelope function currently trims the suffix even when the prefix wasn't present, which can produce malformed HTML; modify stripHTMLEnvelope to check for the full envelope atomically (e.g., test strings.HasPrefix(s, "<html><head></head><body>") && strings.HasSuffix(s, "</body></html>") ) and only remove both the prefix and suffix when both are present (otherwise return s unchanged), locating and updating the stripHTMLEnvelope function to implement this conditional removal.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@shortcuts/mail/mail_forward.go`:
- Around line 209-242: The classification loses the original ordering because
origAtts are appended to allFiles without an identity and code uses a shared
cursor (origIdx) when iterating classified.Normal and classified.Oversized; add
an OriginalIndex int field to the attachmentFile type, set OriginalIndex to the
index for each entry built from origAtts (and a sentinel like -1 for userFiles),
then change the embedding loops (the loop over classified.Normal and the similar
loop for classified.Oversized) to use f.OriginalIndex to fetch the correct
origAtts entry when f.Path == "" (instead of advancing origIdx), and keep using
AddAttachment for original entries and AddFileAttachment for file-path entries
so attachments are no longer swapped.
---
Outside diff comments:
In `@shortcuts/common/drive_media_upload.go`:
- Around line 55-78: Add a unit test that exercises the UploadDriveMediaAll code
path when DriveMediaUploadAllConfig.Reader is non-nil: create a mock
RuntimeContext with a FileIO that is not used, construct
DriveMediaUploadAllConfig with Reader set to an in-memory bytes.Reader and other
required fields (FileName, ParentType, FileSize, etc.), call
UploadDriveMediaAll, capture the created multipart/form-data payload (e.g., by
injecting or mocking larkcore.NewFormdata or the HTTP client used by
UploadDriveMediaAll), and assert that the multipart body contains the exact
bytes from the provided Reader and correct form fields; focus on
UploadDriveMediaAll, DriveMediaUploadAllConfig, and verifying the "file" part
contains the reader bytes.
---
Duplicate comments:
In `@shortcuts/mail/draft/large_attachment_parse.go`:
- Around line 430-433: The stripHTMLEnvelope function currently trims the suffix
even when the prefix wasn't present, which can produce malformed HTML; modify
stripHTMLEnvelope to check for the full envelope atomically (e.g., test
strings.HasPrefix(s, "<html><head></head><body>") && strings.HasSuffix(s,
"</body></html>") ) and only remove both the prefix and suffix when both are
present (otherwise return s unchanged), locating and updating the
stripHTMLEnvelope function to implement this conditional removal.
In `@shortcuts/mail/mail_forward.go`:
- Around line 203-208: The EML size estimation uses len(body) and omits inline
images added later, causing underestimation; update the estimation to use the
actual composed body and include all inline image paths produced by
addInlineImagesToBuilder: compute the composed HTML body (e.g., composedHTMLBody
or via buildForwardedMessage(&orig, body)) and collect inline paths as
allInlinePaths := append(inlineSpecFilePaths(inlineSpecs), autoResolvedPaths...,
pathsReturnedByAddInlineImagesToBuilder...), then call
estimateEMLBaseSize(runtime.FileIO(), int64(len(composedBody)), allInlinePaths,
0) so the EML base size reflects the final payload.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: fe1ff9df-5220-418a-aff9-f3fae8bf309d
📒 Files selected for processing (7)
shortcuts/common/drive_media_upload.goshortcuts/mail/draft/large_attachment_parse.goshortcuts/mail/draft/projection.goshortcuts/mail/helpers.goshortcuts/mail/helpers_test.goshortcuts/mail/large_attachment.goshortcuts/mail/mail_forward.go
✅ Files skipped from review due to trivial changes (1)
- shortcuts/mail/large_attachment.go
🚧 Files skipped from review as they are similar to previous changes (1)
- shortcuts/mail/draft/projection.go
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
shortcuts/mail/mail_forward.go (1)
157-188:⚠️ Potential issue | 🟡 MinorUse the actual forwarded body and source inline images in the EML estimate.
Line 218 still estimates from
len(body), but the built EML usescomposedHTMLBodyin HTML mode orbuildForwardedMessage(&orig, body)in plain-text mode, and source inline images added viaaddInlineImagesToBuilderare not included. Near 25 MB, this can misclassify an attachment as embeddable and fail during EML build/send.Also applies to: 217-218
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@shortcuts/mail/mail_forward.go` around lines 157 - 188, The current EML size estimation still uses len(body) and omits source inline images (srcCIDs), causing misclassification; update the estimator to use the actual content used for the EML: when HTML path is taken use len(composedHTMLBody) (or len(bodyWithSig) / len(composedHTMLBody) as appropriate after building composedHTMLBody in the block that calls buildForwardQuoteHTML, addInlineImagesToBuilder, draftpkg.ResolveLocalImagePaths, and addSignatureImagesToBuilder), and when plain-text path is taken use len(buildForwardedMessage(&orig, body)); also include all inline CIDs from srcCIDs, userCIDs (from inlineSpecs loop), and signatureCIDs(sigResult) in the inline/attachment size calculation so the estimation reflects images added via addInlineImagesToBuilder and addSignatureImagesToBuilder before calling validateInlineCIDs or EML build/send.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@shortcuts/mail/mail_forward.go`:
- Around line 237-240: The current attachment-count check uses totalCount :=
len(origAtts) + len(userFiles) but omits preserved large attachments
(largeAttIDs), allowing forwards with many large attachments to bypass
MaxAttachmentCount; update the check to include the number of preserved large
attachments (e.g., add len(largeAttIDs) into totalCount or otherwise account for
them before the if that returns output.ErrValidation), and keep the error
message using totalCount and MaxAttachmentCount so the validation fails locally
when the sum of origAtts, userFiles, and largeAttIDs exceeds MaxAttachmentCount.
- Around line 11-12: Replace the predictable temp file creation that uses
".lark-cli-fwd-"+filepath.Base(att.filename) with os.CreateTemp to generate a
unique temp file for each attachment; in the function that handles attachments
(references: att.filename, the upload path variable and the deferred cleanup
logic in mail_forward.go), write the attachment to the os.CreateTemp file, use
that returned temporary filename for the upload, and keep the existing deferred
removal but ensure it removes the exact os.CreateTemp path; apply the same
change to the other attachment-handling block that currently uses the
predictable name.
---
Duplicate comments:
In `@shortcuts/mail/mail_forward.go`:
- Around line 157-188: The current EML size estimation still uses len(body) and
omits source inline images (srcCIDs), causing misclassification; update the
estimator to use the actual content used for the EML: when HTML path is taken
use len(composedHTMLBody) (or len(bodyWithSig) / len(composedHTMLBody) as
appropriate after building composedHTMLBody in the block that calls
buildForwardQuoteHTML, addInlineImagesToBuilder,
draftpkg.ResolveLocalImagePaths, and addSignatureImagesToBuilder), and when
plain-text path is taken use len(buildForwardedMessage(&orig, body)); also
include all inline CIDs from srcCIDs, userCIDs (from inlineSpecs loop), and
signatureCIDs(sigResult) in the inline/attachment size calculation so the
estimation reflects images added via addInlineImagesToBuilder and
addSignatureImagesToBuilder before calling validateInlineCIDs or EML build/send.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 2a4a7639-1936-4faf-afe3-0b46fc6653ec
📒 Files selected for processing (2)
shortcuts/mail/large_attachment.goshortcuts/mail/mail_forward.go
✅ Files skipped from review due to trivial changes (1)
- shortcuts/mail/large_attachment.go
infeng
reviewed
Apr 20, 2026
infeng
reviewed
Apr 20, 2026
infeng
previously approved these changes
Apr 20, 2026
Collaborator
infeng
left a comment
infeng
left a comment
There was a problem hiding this comment.
Re-checked the latest head. All previously raised review items are addressed, local go test ./shortcuts/mail/... passes, and there are no unresolved review threads on the PR.
When attachments would cause the EML to exceed the 25MB limit, they are automatically uploaded to the mail attachment storage (medias/upload_all with parent_type="email") and a download-link card is injected into the HTML body, matching the desktop client's exportLargeFileArea style. Key changes: - Add classifyAttachments: EML-size-based splitting of normal vs oversized - Add uploadLargeAttachments: upload via medias API with email MountPoint - Add buildLargeAttachmentHTML: desktop-aligned card with CDN icons - Add processLargeAttachments: unified entry point for all compose shortcuts - Add LargeAttachmentHTML to emlbuilder.Builder for HTML block injection - Fix 7bit line folding: use RFC 5322 limit (998) instead of incorrect 76 - Integrate into +draft-create, +forward, +reply, +reply-all Known limitation: recipient access to large attachment links requires backend support to register tokens with the draft (see progress doc). Change-Id: If8d5938015cac8bc82de3ea3ff41022950f2571e Co-Authored-By: AI
- Remove checkAttachmentSizeLimit (replaced by processLargeAttachments) - Remove 25MB pre-check from validateComposeInlineAndAttachments so that large files reach Execute where they are uploaded as large attachments - Integrate processLargeAttachments into +send shortcut - Add 3GB single file limit aligned with desktop client - Clean up unused imports from helpers.go and helpers_test.go Change-Id: Ie590ad2b58263c075f48b338959b8f5b3f912f85 Co-Authored-By: AI
…emlbuilder - Add insertBeforeQuoteOrAppend: insert large attachment HTML before the quote block (lark-mail-quote) instead of appending to body end, matching desktop's exportLargeFileArea placement logic - Add preprocessLargeAttachmentsForDraftEdit: intercept add_attachment patch ops before draft.Apply, upload oversized files, inject HTML into snapshot's HTML body Part directly. No changes to draft sub-package. - Remove LargeAttachmentHTML field/setter/logic from emlbuilder — it was business logic (quote-aware insertion) that doesn't belong in a generic EML builder. processLargeAttachments now sets the full HTML body via bld.HTMLBody() after merging the large attachment card at the right position. - All compose shortcuts pass htmlBody to processLargeAttachments for quote-aware insertion (composedHTMLBody for reply/forward, body for others). Change-Id: If6e7ed7e77989ab9a8a41a93758f686d72ccf497 Co-Authored-By: AI
- Container ID: lark-mail-large-file-container → large-file-area (matching desktop's MAIL_LARGE_FILE_CONTAINER constant) - Item ID: lark-mail-large-file-item → large-file-item (matching desktop's MAIL_LARGE_FILE_ITEM constant) - Timestamp: truncate to 9 digits (matching TIMESTAMP_CUT_OUT_ID = 9) - Refactor HTML generation to use template constants for readability These IDs are used by the desktop client's BigAttachmentPlugin ([id^=large-file-area]) and the server's LargeFileRule to identify and remove the HTML block when rendering the attachment card UI. Change-Id: Ib5a77a1a3d60eeb3a05c585f2af0a5ddaacf887b Co-Authored-By: AI
Update --attach parameter descriptions across all compose shortcuts (+send, +reply, +reply-all, +forward, +draft-create, +draft-edit) to describe automatic large attachment handling when EML exceeds 25 MB. Change-Id: I8c30e390c127ea1119cb8c4b83ec636e41fbaf66 Co-Authored-By: AI
When both --signature-id and large attachments are used, the htmlBody passed to processLargeAttachments must include the already-injected signature. Previously mail_send and mail_draft_create passed the original body, causing processLargeAttachments to overwrite the signature-injected HTML body when inserting the large attachment card. Use composedHTMLBody variable (same pattern as reply/forward) to capture the full processed HTML including signature. Change-Id: I6be330776abca704b10cc3b8bfd5e20838e6e538 Co-Authored-By: AI
… preprocessing When all patch ops are add_attachment targeting oversized files, preprocessLargeAttachmentsForDraftEdit uploads them and removes the ops from the patch. The resulting empty patch caused draft.Apply to fail with "patch ops is required". Now skip Apply when no ops remain. Change-Id: I8067a54b5f849fa519e8344a7eb10c48f58e54b8 Co-Authored-By: AI
…attachment flow draft-edit's preprocessLargeAttachmentsForDraftEdit uploaded oversized files and injected HTML cards but never wrote the X-Lms-Large-Attachment-Ids header into the snapshot, so the mail server could not associate the attachments with the draft. Merge new token IDs with any existing ones already in the snapshot. Also extract the duplicated largeAttID struct and header name string into package-level declarations. Change-Id: Id256d948ec07e86296157436feefa3c2052af721 Co-Authored-By: AI
Parameterize title and download text in large attachment HTML templates. Chinese lang uses "来自Lark邮箱的超大附件"/"下载", others use "Large file from Lark Mail"/"Download", matching desktop's i18n keys Mail_Attachment_AttachmentFromFeishuMail and Mail_Attachment_Download. Change-Id: I2aada8d52af41ae77dd7001d24d14e333f12066e Co-Authored-By: AI
…ide nested quote insertBeforeQuoteOrAppend matched id="lark-mail-quote" which can appear deeply nested inside quoted content from previous replies in a thread. This caused the card to be placed inside the quote area instead of before it. Switch to matching the "history-quote-wrapper" class which is the outermost quote container generated by the CLI. Change-Id: I720b6d62d719613b411b7ed4b7820a1535bf14bd Co-Authored-By: AI
…l attachments Extend +draft-edit so that large attachments behave like normal attachments from the user's perspective: survive body edits, are listed in inspect output, and are removed via the same remove_attachment op. Code-wise: - remove_attachment target now accepts token (for large attachments) in addition to part_id / cid; priority part_id > cid > token. - setBody / setReplyBody auto-preserve the large attachment card in the HTML body, mirroring how normal attachments (MIME parts) survive body edits. Detection checks only the user-authored region of the value so cards inside an appended quote block (from the original quoted message) are not mistaken for user-supplied cards. - --inspect returns large_attachments_summary (token, filename, size) by parsing the X-Lms-Large-Attachment-Ids header and the HTML card DOM. - Well-known Lark HTML/header constants (LargeAttachmentIDsHeader, LargeFileContainerIDPrefix, LargeFileItemID, LargeAttachmentTokenAttr) moved to the draft package alongside QuoteWrapperClass; the mail package consumes them. - Shared helpers FindHTMLBodyPart and InsertBeforeQuoteOrAppend exported from the draft package; mail package switched to consume them, removing local duplicates. Skill reference (lark-mail-draft-edit.md) updated: three locator fields by attachment type, unified remove_attachment examples, set_body behavior. Change-Id: Ic064d1a8df0edf1cef6069cd44ec2a7534cd2182 Co-Authored-By: AI
When inserting a signature into a draft that already has a large attachment card, the signature was placed after the card, diverging from the compose-time layout where the order is [user][sig][card][quote]. Root cause: insertSignatureOp split only at the quote block, so the "user region" side inadvertently included the card. Centralize signature placement in draft.PlaceSignatureBeforeSystemTail, which splits at the earliest system-managed element (card or quote, whichever comes first). Both edit-time insertSignatureOp and compose-time injectSignatureIntoBody now share this single source of truth, removing the duplicated HTML splicing logic. Change-Id: I234bfebaaa31a32731ebbaa78c6596a72618b7c5 Co-Authored-By: AI
Previously set_body / set_reply_body replaced the entire HTML body, silently dropping the signature block. The "replace whole body" semantic treated signature as user-authored content, which is inconsistent with how attachments (normal + large) and quote blocks survive body edits — signature is a system-managed element managed via insert_signature / remove_signature ops. Unify the mental model: body-edit ops replace user-authored content only; signature, large attachment card, normal attachments, and (for set_reply_body) quote block are all auto-preserved. Users can override by including equivalents in value, or explicitly delete via dedicated ops (remove_signature, remove_attachment). - Add ExtractSignatureBlock helper (symmetric to RemoveSignatureHTML). - Rename autoPreserveLargeAttachmentCard to autoPreserveSystemManagedRegions; extract and inject both sig and card from old body, respecting user-supplied equivalents in value's user-authored region. - Update skill doc and patch template notes to reflect the new semantics consistently. Change-Id: I96660d2ff06a6c9cdf1b86793c2d89cf9cb09ffe Co-Authored-By: AI
The title "Large file from Lark Mail" / "来自Lark邮箱的超大附件" hard-coded "Lark" regardless of brand. The desktop client switches between "Feishu"/"飞书" and "Lark" based on the APP_DISPLAY_NAME i18n substitution. Add brandDisplayName(brand, lang) helper: - BrandLark → "Lark" - BrandFeishu → "飞书" (zh) / "Feishu" (en) Applied to title in buildLargeAttachmentHTML, aligning with the icon CDN and download URL, which already branch on brand. Change-Id: I06258b9982b6280a2230193d90a6a88884e10aa3 Co-Authored-By: AI
…ments The octet-stream override was only needed for the large attachment upload path (to prevent image/* from being treated as inline by the drive API). Normal attachments embedded in the EML should retain their original MIME type so recipients can preview/open them correctly. Change-Id: Ie40b7c362524a3b82255b58e9bcfd770eacfe911 Co-Authored-By: AI
The server strips HTML download cards from the EML body when storing drafts, so every draft read-back (regardless of creator) lacks them. Add ensureLargeAttachmentCards which runs before header normalization, compares server-format header tokens against existing HTML cards via data-mail-token, and rebuilds only the missing ones. This ensures external recipients see download links after draft-edit → send. Also exports ParseLargeAttachmentSummariesFromHeader and ParseLargeAttachmentItemsFromHTML from the draft package for cross-package use. Change-Id: I9cb0f47a9f4582909de24984d9a9f6e366521e62 Co-Authored-By: AI
Previously large attachments required an HTML body for the download card. Now plain-text emails (--plain-text or text/plain-only drafts) get download info appended as structured text (title + filename + size + URL), with i18n and brand awareness matching the HTML card. Changes: - Add buildLargeAttachmentPlainText and injectLargeAttachmentTextIntoSnapshot - Add FindTextBodyPart in draft/projection.go - Update processLargeAttachments to accept textBody parameter - Update ensureLargeAttachmentCards to handle text/plain body reconstruction - Update preprocessLargeAttachmentsForDraftEdit to allow text/plain drafts - Update all callers (send, draft-create, reply, reply-all, forward) Change-Id: I3b375e2ff34697eeb73a3768ace6d577d1bead3e Co-Authored-By: AI
…ill docs FindHTMLBodyPart and FindTextBodyPart now skip parts with Content-Disposition: attachment, preventing .txt/.html file attachments from being mistakenly treated as the email body. Also update all lark-mail skill reference docs to reflect that large attachments now work in both HTML (download card) and plain-text (download link text) modes. Change-Id: I1e6da4fd614217dff61304212304b5fd80c8246c Co-Authored-By: AI
…nt count on forward - Use SourceIndex instead of linear origIdx counter so classifyAttachments reordering does not cause content mismatch between normal/oversized loops - Use os.CreateTemp for temp files instead of predictable names in CWD - Include original large attachment count in totalCount limit check Change-Id: Ide5dce14b1efc672687800d77c3853f15dfc191b Co-Authored-By: AI
… estimation estimateEMLBaseSize was using len(body) (raw --body flag) instead of the actual composed body (which includes quotes, signatures, forward headers). Source inline images downloaded from the original message were also not counted. This could cause borderline attachments to be misclassified. - Use len(composedHTMLBody) + len(composedTextBody) for body size - Return total downloaded bytes from addInlineImagesToBuilder and pass as extraBytes to estimateEMLBaseSize - Fix applied to all compose shortcuts: send, draft-create, reply, reply-all, forward Change-Id: Ibe6c44e22d40ac51f0a4652d279e66bd92330723 Co-Authored-By: AI
…t edit When draft-edit had both set_body and add_attachment (oversized), the ensureLargeAttachmentCards and preprocessLargeAttachmentsForDraftEdit each created independent large-file-area containers. The subsequent set_body's autoPreserveSystemManagedRegions only captured the first container via SplitAtLargeAttachment, discarding the second one. Fix: injectLargeAttachmentHTMLIntoSnapshot now detects an existing large-file-area container and appends new items inside it instead of creating a new container, matching the desktop client's single-container behavior. Change-Id: I3d701683053842f1d7bdad34fc4b2ef26ede784e Co-Authored-By: AI
Reply and reply-all should not carry over the original email's large attachment HTML card into the quoted block. Extract the shared stripLargeAttachmentCard helper (also used by forward) that removes the card from orig.bodyRaw before quote construction. - Reply/reply-all: card is discarded (not re-inserted) - Forward: card is moved to body area before the quote (unchanged) Change-Id: I5399bb901c120206c7c045bed107f7d68be23bb1 Co-Authored-By: AI
When forwarding a message with deleted/expired attachments, the forward flow now automatically removes them instead of either blocking (normal attachments) or silently including dead references (large attachments). - Propagate failed_ids from fetchAttachmentURLs into composeSourceMessage - Skip failed attachments in the forward download loop with a warning - Remove corresponding large attachment HTML card items from the body - Extend itemContainsToken to match server-generated href?token= format Change-Id: I9c0096dcbe96f1d61caa0f6f0b2f8b738fdfa66b Co-Authored-By: AI
chanthuang
force-pushed
the
feat/mail-large-attachment
branch
from
e5250a4 to
4124382
Compare
infeng
reviewed
Apr 21, 2026
infeng
reviewed
Apr 21, 2026
…n classifier 1. Restore file existence and blocked-extension checks in validateComposeInlineAndAttachments so --dry-run surfaces local path errors before Execute. 2. Reserve 3KB per oversized file in classifyAttachments to account for the HTML card / plain-text block injected after classification. Change-Id: Ib48a75f86a50298413c1f9ab8226e583c0161a8c Co-Authored-By: AI
infeng
reviewed
Apr 21, 2026
The 3KB-per-oversized-file reserve in classifyAttachments addressed a boundary case that is practically impossible to trigger (requires Normal attachments to fill to within a few KB of 25MB). Remove it to keep the classifier simple. Change-Id: I5148f14ecca1a0dee677a1a2c60ec4efab160ea8 Co-Authored-By: AI
infeng
reviewed
Apr 21, 2026
Change-Id: Ib41aa22f94144f2d47b12675d444aa43cb333a88 Co-Authored-By: AI
infeng
reviewed
Apr 21, 2026
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #537 +/- ##
==========================================
+ Coverage 61.05% 61.29% +0.23%
==========================================
Files 400 402 +2
Lines 34130 35045 +915
==========================================
+ Hits 20838 21480 +642
- Misses 11368 11605 +237
- Partials 1924 1960 +36 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Collaborator
|
Re-checked the current head ( Residual risk remains the same: the borderline EML-size acceptance path discussed in the thread at |
Replace os.CreateTemp/os.WriteFile/os.Remove with in-memory Data field on attachmentFile, conforming to the project's forbidigo rule against temp files in shortcuts. Also remove dead uploadLargeAttachmentBytes. Change-Id: Ic26e4025eebfa1bac3948438ef185ff3e2f15abb Co-Authored-By: AI
…eTypeIcon Covers all branches: inline+plain-text conflict, inline+non-HTML body, missing file, blocked extension, valid pass-through, and all file type icon mappings. Change-Id: I8b81c1b34010a9ecb7153462a5524e3d7b171de2 Co-Authored-By: AI
infeng
previously approved these changes
Apr 21, 2026
…tions Add tests for snapshotEMLBaseSize, flattenSnapshotParts, estimateEMLBaseSize, normalizeLargeAttachmentHeader, processLargeAttachments error paths, preprocessLargeAttachmentsForDraftEdit early-return paths, inject edge cases, buildLargeAttachmentItems, statAttachmentFiles edge cases, and prettyDraftAddresses. Change-Id: Ie661e6ebea63512864d97e20135dd89cb9e9304e Co-Authored-By: AI
infeng
reviewed
Apr 21, 2026
infeng
approved these changes
Apr 21, 2026
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
2 participants
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
Adds support for sending emails with attachments that exceed the 25MB EML size limit — up to 3GB per file. Oversized attachments are automatically uploaded to mail attachment storage and rendered as a download-card in the email body, matching the desktop client's behavior.
What's new
Large attachments, automatically handled
--attachinmail +send/+draft-create/+reply/+reply-all/+forward. When the accumulated attachments would push the email past 25MB, the overflow files are uploaded as large attachments instead of embedded — no extra flag needed.Large attachment management in draft-edit
mail +draft-editnow treats large attachments as first-class siblings of normal attachments, with a consistent mental model:{ op: "add_attachment", path: "..." }— automatically routes to large-attachment upload when the file would overflow 25MB.{ op: "remove_attachment", target: { token: "..." } }— by file token, same op used for normal attachments (which usepart_id/cid).--inspectreturns a newlarge_attachments_summarylisting each large attachment's token, filename, and size.set_body/set_reply_body) automatically preserve signature block and attachment cards (normal + large), mirroring how normal attachment MIME parts survive body edits. Users delete signatures/attachments via the dedicated ops (remove_signature,remove_attachment), not by clearing the body.Reply/reply-all: strip large attachment card from quote
Forward: skip invalid/deleted attachments automatically and print warning info
RemoveLargeFileItemFromHTMLnow matches server-generated cards (which usehref?token=) in addition to CLI-generated cards (data-mail-tokenattribute).Localization and branding
Documentation
Skill references updated under
skills/lark-mail/references/for all affected compose shortcuts, andlark-mail-draft-edit.mddocuments the unified attachment model, the newlarge_attachments_summaryfield, and the body-edit preservation semantics.Test plan
mail +send/+reply/+reply-all/+forwardwith a mix of small and large attachmentsmail +draft-createwith signature + large attachmentmail +draft-edit --inspecton a draft with large attachments — verifylarge_attachments_summaryis populatedmail +draft-editwithadd_attachment(oversized) on existing draftsmail +draft-editwithremove_attachmentby tokenmail +draft-editwithset_body/set_reply_body— verify signature, large attachment card, and quote block (for set_reply_body) are preservedmail +reply/+reply-allon a message with large attachment cards — verify the card is stripped from the quotemail +forwardon a message with a deleted/expired large attachment — verify the attachment is skipped with a warning, HTML card removed, and the forward succeedsmail +forwardon a message with valid attachments — verify no regression, all attachments preserved