feat(mail): add email signature support#485
Merged
chanthuang merged 7 commits intomainfrom Apr 15, 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 email signature support: signature models and provider, template interpolation and image download, compose-time signature resolution/injection and CID handling, draft patch ops for inserting/removing signatures (with MIME inline-image management), CLI flags and a new Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant CLI as Compose Shortcut
participant SigProv as Signature Provider
participant Interp as Interpolator
participant Downloader as Image Downloader
participant Builder as EML Builder
participant Draft as Draft Engine
User->>CLI: invoke with --signature-id
CLI->>CLI: validate flags (plain-text incompatibility)
CLI->>SigProv: Get(signatureID)
SigProv-->>CLI: Signature metadata
CLI->>Interp: InterpolateTemplate(signature, lang, sender)
Interp-->>CLI: Rendered HTML
CLI->>Downloader: download inline images (CID, URL)
Downloader-->>CLI: image bytes + content-types
alt Build/send message
CLI->>Builder: inject signature HTML
CLI->>Builder: add inline images
Builder-->>User: built message ready
else Edit draft (patch)
CLI->>Draft: apply insert_signature / remove_signature op
Draft->>Draft: remove old signature, split at quote, insert/remove signature HTML
Draft->>Draft: add/remove MIME inline parts as needed
Draft-->>User: updated draft stored/returned
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
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@b31754a4f6c164df5dd68a1e2f66e88a77e2f799🧩 Skill updatenpx skills add larksuite/cli#feat/mail-signature -y -g |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (6)
shortcuts/mail/signature/interpolate.go (1)
143-157: Verify HTML entity unescaping order.The
unescapeHTMLEntitiesfunction replaces"before&. This ordering is correct to prevent double-unescaping (e.g.,"should become", not"). However, theescapeHTMLfunction escapes&first (line 144), which is also correct.The implementations are sound, but consider adding a brief comment explaining the ordering rationale for future maintainers.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@shortcuts/mail/signature/interpolate.go` around lines 143 - 157, Add short explanatory comments to the escapeHTML and unescapeHTMLEntities functions documenting the required ordering: in escapeHTML escape ampersand (&) first to avoid turning already-escaped entities into new entities, and in unescapeHTMLEntities unescape " before & so sequences like " become " rather than " (i.e., mention that the ordering prevents double-escaping/unescaping and preserve entity integrity). Reference the function names escapeHTML and unescapeHTMLEntities when adding these comments.shortcuts/mail/draft/projection.go (1)
179-199:FindMatchingCloseDivdoesn't handle<divinside strings/comments.The nesting tracker uses simple prefix matching (
strings.HasPrefix(html[i:], "<div")) which could be confused by<divappearing inside HTML attributes, comments, or script blocks. This is acceptable for generated signature HTML but could misbehave on user-authored content with edge cases like<a title="<div trick">.For the current use case (removing CLI-generated signatures), this is low risk since the signature HTML structure is controlled.
🤖 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 179 - 199, The FindMatchingCloseDiv function miscounts when "<div" appears inside quoted attributes, HTML comments or script/style content; update the loop in FindMatchingCloseDiv to skip over quoted strings (on encountering ' or " advance i to the matching unescaped quote), skip HTML comments by detecting "<!--" and advancing to the next "-->", and skip script/style blocks by detecting "<script" or "<style" start tags and advancing to the corresponding closing tag; also only treat an opening "<div" as a real tag if the character after "<div" is whitespace, '>', or '/', and treat "</div>" normally while applying the same skips so nesting (depth) is only updated for real tags. Ensure you keep using the same identifiers (html, startPos, i, depth, FindMatchingCloseDiv) when making changes.skills/lark-mail/SKILL.md (1)
277-277: Add signature keywords to the skill frontmatter too.The new
+signatureshortcut is listed here, but the top-levelmetadata.descriptionstill has no “邮箱签名 / 邮件签名 / signature” terms. That makes signature-specific requests harder for the skill router to match.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@skills/lark-mail/SKILL.md` at line 277, Update the skill frontmatter metadata to include signature-related keywords so the router can match signature requests: add terms like "邮箱签名", "邮件签名", and "signature" to the top-level metadata.description (and if present, metadata.keywords or tags) in SKILL.md so the new `+signature` shortcut (referenced as `+signature`/references/lark-mail-signature.md) is discoverable by the skill router.shortcuts/mail/signature/provider.go (1)
14-17: Scope the cache to a command/runtime, not the package.
processCacheis keyed only bymailboxIDand never reset, so later calls in the same process can observe stale signatures from an earlier runtime/session."me"is especially prone to this in tests. A per-command cache (or at least a test reset hook) would avoid hidden cross-call coupling here.Also applies to: 25-46
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@shortcuts/mail/signature/provider.go` around lines 14 - 17, processCache is a package-level map keyed by mailboxID that persists across command invocations and causes stale data (e.g., "me" in tests); remove the package-level var and make the cache per-execution: either add a cache field (map[string]*GetSignaturesResponse) to the command/runtime struct that executes signature lookups (e.g., the Executor/Command type that calls GetSignatures) and initialize it at the start of each run, or introduce a RuntimeCache instance created per Execute and pass it into functions that currently reference processCache; update all references to processCache accordingly (including the code around GetSignaturesResponse and lines 25-46) and, if a full refactor is impractical, add a test-only ResetProcessCache() function that clears the map before each test to avoid cross-call coupling.shortcuts/mail/mail_signature.go (1)
184-212: Consider hoisting the compiled regex to package level.The
imgReregex is compiled on every call tocontentPreview. For repeated invocations (e.g., listing many signatures), this adds unnecessary overhead.♻️ Proposed refactor
+// imgTagRe matches <img ...> tags for content preview. +var imgTagRe = regexp.MustCompile(`<img[^>]*>`) + func contentPreview(html string, maxLen int) string { // Replace <img ...> with [图片]. - imgRe := regexp.MustCompile(`<img[^>]*>`) - s := imgRe.ReplaceAllString(html, "[图片]") + s := imgTagRe.ReplaceAllString(html, "[图片]")🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@shortcuts/mail/mail_signature.go` around lines 184 - 212, contentPreview currently compiles the img regex on every call (imgRe := regexp.MustCompile(`<img[^>]*>`)); hoist that compilation to a package-level variable (e.g., var imgTagRe = regexp.MustCompile(`<img[^>]*>`)) and update contentPreview to use the package-level imgTagRe instead of creating a new regex each invocation to avoid repeated compilation overhead.shortcuts/mail/signature_compose.go (1)
122-158: HTTP request should propagate context for cancellation support.The function receives
runtimewhich has access to context, but creates the HTTP request without context. This prevents proper cancellation if the user interrupts the CLI operation during image download.♻️ Proposed fix to add context propagation
The function signature should accept a context parameter:
-func downloadSignatureImage(runtime *common.RuntimeContext, downloadURL, filename string) ([]byte, string, error) { +func downloadSignatureImage(ctx context.Context, runtime *common.RuntimeContext, downloadURL, filename string) ([]byte, string, error) { httpClient, err := runtime.Factory.HttpClient() if err != nil { return nil, "", fmt.Errorf("signature image download: %w", err) } - req, err := http.NewRequest(http.MethodGet, downloadURL, nil) + req, err := http.NewRequestWithContext(ctx, http.MethodGet, downloadURL, nil) if err != nil { return nil, "", fmt.Errorf("signature image download: %w", err) }Then update the call site in
resolveSignature(line 57):- data, ct, err := downloadSignatureImage(runtime, img.DownloadURL, img.ImageName) + data, ct, err := downloadSignatureImage(ctx, runtime, img.DownloadURL, img.ImageName)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@shortcuts/mail/signature_compose.go` around lines 122 - 158, The downloadSignatureImage function should accept and use a context to allow cancellation: change downloadSignatureImage to take ctx (context.Context) and create the request with http.NewRequestWithContext(ctx, ...) using the runtime's context (e.g. runtime.Context() or runtime.Ctx) instead of http.NewRequest; propagate the ctx through callers (notably resolveSignature) so they pass their context into downloadSignatureImage; keep all existing error handling and behavior intact but replace the http.NewRequest call and any direct request uses with the context-aware request and ensure runtime.Factory.HttpClient() usage remains unchanged.
🤖 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/patch.go`:
- Around line 1246-1260: The loop in removeMIMEPartByCID can dereference a nil
child (root.Children may contain nil), so before accessing child.ContentID check
if child == nil and skip/continue; also use the normalizedCID for recursive
calls (pass normalizedCID or same normalization) and ensure removal logic
remains correct when skipping nil entries so indices align (i.e., continue when
child is nil, otherwise compare strings.Trim(child.ContentID, "<>") with
normalizedCID and remove/return as before).
- Around line 1262-1281: The addInlinePartToSnapshot function currently drops
inline parts when snapshot.Body exists but isn't multipart; change it to ensure
an inline multipart/related container (reuse the existing
ensureInlineContainerRef pattern) before appending: if snapshot.Body is nil keep
the early return, but if snapshot.Body.IsMultipart() is false, wrap the existing
snapshot.Body as the first child of a new multipart/related container
(preserving its current headers/state) and set snapshot.Body to that container,
then append the new Part; alternatively, if you prefer failure mode, return an
error from addInlinePartToSnapshot when snapshot.Body is non-multipart instead
of silently discarding the part — reference addInlinePartToSnapshot,
snapshot.Body, ensureInlineContainerRef and Part when making the change.
In `@shortcuts/mail/mail_draft_edit.go`:
- Around line 364-370: The top-level supported_ops list is missing the new
"insert_signature" and "remove_signature" entries so callers that read
supported_ops won't see them; update the code that builds the top-level
supported_ops (the same place that currently populates supported_ops_by_group)
to also append entries for "insert_signature" and "remove_signature" (with the
same shapes/notes used under the "signature" group) so the flat supported_ops
and supported_ops_by_group remain consistent (look for symbols like
supported_ops, supported_ops_by_group and the signature group definition in
mail_draft_edit.go).
In `@shortcuts/mail/signature_html.go`:
- Around line 71-80: The splitAtSignature helper slices the signature
incorrectly because draftpkg.FindMatchingCloseDiv returns the start index of the
closing "</div>" (so sig currently omits the closing tag and after begins with
"</div>") and SignatureSpacingRe().FindStringIndex is using the first match in
beforeSig which can pick unrelated earlier spacers; update splitAtSignature to
set sigEnd = resultFromFindMatchingCloseDiv + len("</div>") (or otherwise
advance to the end of the closing div) so the signature slice includes the
closing tag, and change the spacing detection to find the last spacing match
within beforeSig (e.g., use FindAllStringIndex or equivalent and take the final
match) before assigning sigStart so only the immediate preceding spacing is
included.
In `@skills/lark-mail/references/lark-mail-send.md`:
- Line 73: Update the `--signature-id` table row in
skills/lark-mail/references/lark-mail-send.md for the `+send` command: remove
the misleading "引用块之前" phrase and change the description to state simply
"附加邮箱签名到正文末尾" (keep the rest of the note about running `mail +signature` to view
signatures and that it cannot be used with `--plain-text` unchanged); edit the
row referencing `--signature-id` under the `+send` documentation to reflect this
exact text change.
---
Nitpick comments:
In `@shortcuts/mail/draft/projection.go`:
- Around line 179-199: The FindMatchingCloseDiv function miscounts when "<div"
appears inside quoted attributes, HTML comments or script/style content; update
the loop in FindMatchingCloseDiv to skip over quoted strings (on encountering '
or " advance i to the matching unescaped quote), skip HTML comments by detecting
"<!--" and advancing to the next "-->", and skip script/style blocks by
detecting "<script" or "<style" start tags and advancing to the corresponding
closing tag; also only treat an opening "<div" as a real tag if the character
after "<div" is whitespace, '>', or '/', and treat "</div>" normally while
applying the same skips so nesting (depth) is only updated for real tags. Ensure
you keep using the same identifiers (html, startPos, i, depth,
FindMatchingCloseDiv) when making changes.
In `@shortcuts/mail/mail_signature.go`:
- Around line 184-212: contentPreview currently compiles the img regex on every
call (imgRe := regexp.MustCompile(`<img[^>]*>`)); hoist that compilation to a
package-level variable (e.g., var imgTagRe = regexp.MustCompile(`<img[^>]*>`))
and update contentPreview to use the package-level imgTagRe instead of creating
a new regex each invocation to avoid repeated compilation overhead.
In `@shortcuts/mail/signature_compose.go`:
- Around line 122-158: The downloadSignatureImage function should accept and use
a context to allow cancellation: change downloadSignatureImage to take ctx
(context.Context) and create the request with http.NewRequestWithContext(ctx,
...) using the runtime's context (e.g. runtime.Context() or runtime.Ctx) instead
of http.NewRequest; propagate the ctx through callers (notably resolveSignature)
so they pass their context into downloadSignatureImage; keep all existing error
handling and behavior intact but replace the http.NewRequest call and any direct
request uses with the context-aware request and ensure
runtime.Factory.HttpClient() usage remains unchanged.
In `@shortcuts/mail/signature/interpolate.go`:
- Around line 143-157: Add short explanatory comments to the escapeHTML and
unescapeHTMLEntities functions documenting the required ordering: in escapeHTML
escape ampersand (&) first to avoid turning already-escaped entities into new
entities, and in unescapeHTMLEntities unescape " before & so sequences
like &quot; become " rather than " (i.e., mention that the ordering
prevents double-escaping/unescaping and preserve entity integrity). Reference
the function names escapeHTML and unescapeHTMLEntities when adding these
comments.
In `@shortcuts/mail/signature/provider.go`:
- Around line 14-17: processCache is a package-level map keyed by mailboxID that
persists across command invocations and causes stale data (e.g., "me" in tests);
remove the package-level var and make the cache per-execution: either add a
cache field (map[string]*GetSignaturesResponse) to the command/runtime struct
that executes signature lookups (e.g., the Executor/Command type that calls
GetSignatures) and initialize it at the start of each run, or introduce a
RuntimeCache instance created per Execute and pass it into functions that
currently reference processCache; update all references to processCache
accordingly (including the code around GetSignaturesResponse and lines 25-46)
and, if a full refactor is impractical, add a test-only ResetProcessCache()
function that clears the map before each test to avoid cross-call coupling.
In `@skills/lark-mail/SKILL.md`:
- Line 277: Update the skill frontmatter metadata to include signature-related
keywords so the router can match signature requests: add terms like "邮箱签名",
"邮件签名", and "signature" to the top-level metadata.description (and if present,
metadata.keywords or tags) in SKILL.md so the new `+signature` shortcut
(referenced as `+signature`/references/lark-mail-signature.md) is discoverable
by the skill router.
🪄 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: e5fd4b65-e601-4ccf-830c-7330e3b4b7e3
📒 Files selected for processing (27)
shortcuts/mail/draft/model.goshortcuts/mail/draft/patch.goshortcuts/mail/draft/projection.goshortcuts/mail/draft/projection_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/mail_signature.goshortcuts/mail/shortcuts.goshortcuts/mail/signature/interpolate.goshortcuts/mail/signature/interpolate_test.goshortcuts/mail/signature/model.goshortcuts/mail/signature/provider.goshortcuts/mail/signature_compose.goshortcuts/mail/signature_html.goskills/lark-mail/SKILL.mdskills/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.mdskills/lark-mail/references/lark-mail-signature.md
chanthuang
force-pushed
the
feat/mail-signature
branch
from
481da2f to
6207679
Compare
There was a problem hiding this comment.
Actionable comments posted: 4
♻️ Duplicate comments (2)
shortcuts/mail/draft/patch.go (2)
1247-1259:⚠️ Potential issue | 🟡 MinorGuard nil MIME children before dereferencing
ContentID.
root.Childrenis nil-tolerant elsewhere in this file, but this loop readschild.ContentIDunconditionally. A nil child here will panic the whole patch path.🐛 Proposed fix
func removeMIMEPartByCID(root *Part, cid string) { if root == nil { return } normalizedCID := strings.Trim(cid, "<>") for i, child := range root.Children { + if child == nil { + continue + } childCID := strings.Trim(child.ContentID, "<>") if strings.EqualFold(childCID, normalizedCID) { root.Children = append(root.Children[:i], root.Children[i+1:]...) return } removeMIMEPartByCID(child, cid) } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@shortcuts/mail/draft/patch.go` around lines 1247 - 1259, In removeMIMEPartByCID, avoid dereferencing a nil child in the loop: check that each child is non-nil before reading child.ContentID and before recursing; if child is nil, continue to next iteration, otherwise normalize child.ContentID (as with normalizedCID) and perform the comparison/removal and recursive call using removeMIMEPartByCID(child, cid). Ensure the guard covers both the ContentID access and the recursive descent.
1263-1280:⚠️ Potential issue | 🟠 MajorAttach signature images through a
multipart/relatedcontainer.On a plain
text/htmldraft, this helper returns without attaching anything, soinsert_signatureleavescid:refs unresolved andApplyfails inpostProcessInlineImages. On multipart roots, appending straight tosnapshot.Body.Childrencan also put the image undermultipart/alternative/multipart/mixedinstead of the primary related container.🐛 Proposed fix
func addInlinePartToSnapshot(snapshot *DraftSnapshot, data []byte, contentType, filename, cid string) { part := &Part{ MediaType: contentType, ContentDisposition: "inline", ContentID: strings.Trim(cid, "<>"), Body: data, Dirty: true, } if filename != "" { part.MediaParams = map[string]string{"name": filename} } - // Find or create the multipart/related container. if snapshot.Body == nil { return } - if snapshot.Body.IsMultipart() { - snapshot.Body.Children = append(snapshot.Body.Children, part) - } + containerRef := primaryBodyRootRef(&snapshot.Body) + if containerRef == nil || *containerRef == nil { + return + } + container, err := ensureInlineContainerRef(containerRef) + if err != nil { + return + } + container.Children = append(container.Children, part) + container.Dirty = true }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@shortcuts/mail/draft/patch.go` around lines 1263 - 1280, addInlinePartToSnapshot currently returns on a plain text/html root and naively appends images to snapshot.Body.Children which can place them under the wrong multipart (or not at all), leaving cid: refs unresolved; update addInlinePartToSnapshot to always attach the image into a multipart/related container: if snapshot.Body is nil return; if snapshot.Body.IsMultipart() search its tree for an existing multipart/related container and append the new Part there; if none exists, create a new multipart/related Part, move the current root body (or the appropriate HTML child) into that related container as the first child and replace the original position with the new multipart/related container, then append the image Part; if snapshot.Body is not multipart (plain text/html), wrap the existing body and the new image Part into a new multipart/related root and assign it to snapshot.Body so cid refs resolve (refer to addInlinePartToSnapshot, snapshot.Body, Part, and postProcessInlineImages/insert_signature to locate code).
🧹 Nitpick comments (1)
shortcuts/mail/signature/provider.go (1)
35-43: Consider using type assertion or direct unmarshaling.The marshal-then-unmarshal roundtrip works but is slightly inefficient. If
runtime.CallAPIreturns a typed response or allows direct unmarshaling into a target type, that would be cleaner. However, this pattern is acceptable ifCallAPIonly returnsinterface{}.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@shortcuts/mail/signature/provider.go` around lines 35 - 43, The code currently does a marshal-then-unmarshal roundtrip on the value returned by runtime.CallAPI (variable data) to produce a GetSignaturesResponse; replace this with a direct conversion: if runtime.CallAPI can return a typed value, use a type assertion to convert data to GetSignaturesResponse (or *GetSignaturesResponse) and assign to resp, otherwise if data is a JSON byte slice or json.RawMessage pass it directly to json.Unmarshal into resp (or change CallAPI to accept a target and decode directly). Update the logic around variable data and the resp variable to avoid the intermediate json.Marshal step and keep the same error handling.
🤖 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/patch.go`:
- Around line 1195-1199: The current loop over sigCIDs uses a case-sensitive
substring check against html which can miss variants like "CID:Logo123" or
"cid:<Logo123>", causing live inline parts to be removed; instead, normalize and
parse the HTML CID references case-insensitively and compare against those
parsed refs before calling removeMIMEPartByCID. Replace the
strings.Contains(html, "cid:"+cid) check with a lookup against a set of
parsed/normalized CIDs extracted from html (strip optional angle brackets,
ignore case, and remove any "cid:" prefix) so the sigCIDs vs HTML comparison is
robust; reuse the existing HTML CID parsing utility in this package if one
exists, otherwise build a single case-insensitive parser and use it in the loop
that iterates sigCIDs before calling removeMIMEPartByCID(snapshot.Body, cid).
- Around line 1157-1171: When replacing a signature, avoid duplicating inline
MIME parts by deduplicating by CID: in the insert flow that currently calls
RemoveSignatureHTML, SplitAtQuote, SignatureSpacing, BuildSignatureHTML and then
loops over op.SignatureImages calling addInlinePartToSnapshot, first remove or
mark any existing inline parts associated with the old signature CIDs (or check
the snapshot for existing parts with the same img.CID) and either replace the
existing part data or skip adding a new part with the same Content-ID; update
the code around addInlinePartToSnapshot to perform a lookup by CID in the
snapshot and overwrite the part (or remove the old one) so
postProcessInlineImages won’t leave duplicate Content-ID parts after repeated
signature inserts.
In `@shortcuts/mail/draft/projection.go`:
- Around line 37-38: signatureIDRe is brittle because it assumes id appears
before class; update the regex used to extract the signature id (signatureIDRe)
so it matches the id attribute regardless of attribute order by using
attribute-order-independent lookahead(s) that assert the presence of the
signature wrapper class and capture id (e.g., require
class="...SignatureWrapperClass..." via a lookahead and a separate
lookahead/capture for id), and apply the same order-independent approach to the
related regex used around lines 58-63 (the other signature/class extraction
regex) so HasSignature and SignatureID remain consistent regardless of attribute
ordering.
In `@shortcuts/mail/mail_signature.go`:
- Around line 184-187: contentPreview currently replaces images with a
hard-coded Chinese marker "[图片]"; update contentPreview(html string, maxLen int)
to produce a localized or neutral placeholder instead: either add a locale
parameter (e.g., contentPreview(html string, maxLen int, lang string)) or call
the existing resolveLang helper inside contentPreview, then replace "[图片]" with
a locale-aware token (e.g., use resolveLang to map an "image" key to
translations or fall back to a neutral "[image]"), keeping the img regex (imgRe)
and replacement logic unchanged otherwise; ensure callers of contentPreview are
updated to pass the locale (or that resolveLang is available) and adjust any
tests expecting the Chinese token.
---
Duplicate comments:
In `@shortcuts/mail/draft/patch.go`:
- Around line 1247-1259: In removeMIMEPartByCID, avoid dereferencing a nil child
in the loop: check that each child is non-nil before reading child.ContentID and
before recursing; if child is nil, continue to next iteration, otherwise
normalize child.ContentID (as with normalizedCID) and perform the
comparison/removal and recursive call using removeMIMEPartByCID(child, cid).
Ensure the guard covers both the ContentID access and the recursive descent.
- Around line 1263-1280: addInlinePartToSnapshot currently returns on a plain
text/html root and naively appends images to snapshot.Body.Children which can
place them under the wrong multipart (or not at all), leaving cid: refs
unresolved; update addInlinePartToSnapshot to always attach the image into a
multipart/related container: if snapshot.Body is nil return; if
snapshot.Body.IsMultipart() search its tree for an existing multipart/related
container and append the new Part there; if none exists, create a new
multipart/related Part, move the current root body (or the appropriate HTML
child) into that related container as the first child and replace the original
position with the new multipart/related container, then append the image Part;
if snapshot.Body is not multipart (plain text/html), wrap the existing body and
the new image Part into a new multipart/related root and assign it to
snapshot.Body so cid refs resolve (refer to addInlinePartToSnapshot,
snapshot.Body, Part, and postProcessInlineImages/insert_signature to locate
code).
---
Nitpick comments:
In `@shortcuts/mail/signature/provider.go`:
- Around line 35-43: The code currently does a marshal-then-unmarshal roundtrip
on the value returned by runtime.CallAPI (variable data) to produce a
GetSignaturesResponse; replace this with a direct conversion: if runtime.CallAPI
can return a typed value, use a type assertion to convert data to
GetSignaturesResponse (or *GetSignaturesResponse) and assign to resp, otherwise
if data is a JSON byte slice or json.RawMessage pass it directly to
json.Unmarshal into resp (or change CallAPI to accept a target and decode
directly). Update the logic around variable data and the resp variable to avoid
the intermediate json.Marshal step and keep the same error handling.
🪄 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: 70d1a3e7-367a-4860-8ee6-84ba04b0e62c
📒 Files selected for processing (28)
shortcuts/mail/draft/model.goshortcuts/mail/draft/patch.goshortcuts/mail/draft/patch_signature_test.goshortcuts/mail/draft/projection.goshortcuts/mail/draft/projection_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/mail_signature.goshortcuts/mail/shortcuts.goshortcuts/mail/signature/interpolate.goshortcuts/mail/signature/interpolate_test.goshortcuts/mail/signature/model.goshortcuts/mail/signature/provider.goshortcuts/mail/signature_compose.goshortcuts/mail/signature_html.goskills/lark-mail/SKILL.mdskills/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.mdskills/lark-mail/references/lark-mail-signature.md
✅ Files skipped from review due to trivial changes (6)
- shortcuts/mail/mail_draft_create_test.go
- skills/lark-mail/references/lark-mail-send.md
- skills/lark-mail/references/lark-mail-draft-create.md
- shortcuts/mail/signature/interpolate_test.go
- skills/lark-mail/references/lark-mail-signature.md
- shortcuts/mail/signature/model.go
🚧 Files skipped from review as they are similar to previous changes (9)
- shortcuts/mail/draft/projection_test.go
- shortcuts/mail/shortcuts.go
- skills/lark-mail/references/lark-mail-reply.md
- skills/lark-mail/references/lark-mail-forward.md
- skills/lark-mail/references/lark-mail-draft-edit.md
- skills/lark-mail/SKILL.md
- shortcuts/mail/mail_reply_all.go
- shortcuts/mail/signature_html.go
- shortcuts/mail/signature_compose.go
There was a problem hiding this comment.
♻️ Duplicate comments (2)
shortcuts/mail/draft/patch.go (2)
1169-1171:⚠️ Potential issue | 🟡 MinorReplacing a signature can duplicate inline parts for the same CID.
When replacing a signature, if the new signature reuses one of the previous signature's image CIDs,
addInlinePartToSnapshotappends another MIME part without checking for existing parts with the same CID. After multipleinsert_signatureoperations, the message can contain duplicate inline parts with identicalContent-IDvalues.Consider deduplicating by CID before adding: check if a part with the same CID already exists and either replace its data or skip adding.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@shortcuts/mail/draft/patch.go` around lines 1169 - 1171, When adding signature images in the loop over op.SignatureImages, avoid blindly appending duplicate inline MIME parts: before calling addInlinePartToSnapshot(snapshot, img.Data, img.ContentType, img.FileName, img.CID) check snapshot for an existing inline part with the same CID (Content-ID) and either replace that part's data/metadata or skip adding; update the logic around the loop that iterates op.SignatureImages to deduplicate by CID (use img.CID as the unique key) so repeated insert_signature operations don’t produce multiple MIME parts with identical Content-ID values.
1196-1200:⚠️ Potential issue | 🟡 MinorCase-sensitive CID check may incorrectly remove live inline parts.
The
strings.Contains(html, "cid:"+cid)check is case-sensitive, but CID references in HTML can use different casings (e.g.,CID:Logo123). Other CID comparisons in this package usestrings.EqualFoldfor case-insensitivity. This mismatch could causeremove_signatureto delete inline parts that are still referenced in the remaining HTML.Consider using a case-insensitive check, for example:
if !strings.Contains(strings.ToLower(html), "cid:"+strings.ToLower(cid)) {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@shortcuts/mail/draft/patch.go` around lines 1196 - 1200, The loop over sigCIDs uses a case-sensitive check when deciding to call removeMIMEPartByCID, which can remove inline parts referenced with different CID casing; update the condition in the loop (the code that checks html and cid) to perform a case-insensitive match — e.g., compare strings.ToLower(html) against "cid:"+strings.ToLower(cid) or otherwise normalize both sides before calling removeMIMEPartByCID(snapshot.Body, cid) so inline parts referenced as CID:, cid:, etc. are preserved; relevant symbols: sigCIDs, html, snapshot.Body, removeMIMEPartByCID.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@shortcuts/mail/draft/patch.go`:
- Around line 1169-1171: When adding signature images in the loop over
op.SignatureImages, avoid blindly appending duplicate inline MIME parts: before
calling addInlinePartToSnapshot(snapshot, img.Data, img.ContentType,
img.FileName, img.CID) check snapshot for an existing inline part with the same
CID (Content-ID) and either replace that part's data/metadata or skip adding;
update the logic around the loop that iterates op.SignatureImages to deduplicate
by CID (use img.CID as the unique key) so repeated insert_signature operations
don’t produce multiple MIME parts with identical Content-ID values.
- Around line 1196-1200: The loop over sigCIDs uses a case-sensitive check when
deciding to call removeMIMEPartByCID, which can remove inline parts referenced
with different CID casing; update the condition in the loop (the code that
checks html and cid) to perform a case-insensitive match — e.g., compare
strings.ToLower(html) against "cid:"+strings.ToLower(cid) or otherwise normalize
both sides before calling removeMIMEPartByCID(snapshot.Body, cid) so inline
parts referenced as CID:, cid:, etc. are preserved; relevant symbols: sigCIDs,
html, snapshot.Body, removeMIMEPartByCID.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: c5a020f0-4cdd-41c5-9819-e27cca12ce56
📒 Files selected for processing (3)
shortcuts/mail/draft/patch.goshortcuts/mail/mail_draft_edit.goskills/lark-mail/references/lark-mail-send.md
🚧 Files skipped from review as they are similar to previous changes (1)
- skills/lark-mail/references/lark-mail-send.md
chanthuang
force-pushed
the
feat/mail-signature
branch
from
fe70f44 to
0c4dfe5
Compare
There was a problem hiding this comment.
♻️ Duplicate comments (2)
shortcuts/mail/draft/patch.go (2)
1302-1305:⚠️ Potential issue | 🟡 MinorUse parsed CID refs instead of substring matching.
strings.Containsis still too loose here: it misses bracketed refs likecid:<logo123>and can false-match prefixes likecid:logo1234when checkinglogo123. That can either remove a live inline part or keep an orphaned one.🛠️ Suggested fix
func containsCIDIgnoreCase(html, cid string) bool { - return strings.Contains(strings.ToLower(html), "cid:"+strings.ToLower(cid)) + needle := strings.ToLower(normalizeCID(cid)) + for _, ref := range extractCIDRefs(html) { + if strings.EqualFold(normalizeCID(ref), needle) { + return true + } + } + return false }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@shortcuts/mail/draft/patch.go` around lines 1302 - 1305, containsCIDIgnoreCase currently uses substring matching which wrongly misses bracketed refs like cid:<logo123> and false-matches prefixes; replace it with parsing of CID references: in containsCIDIgnoreCase iterate over regex matches for case-insensitive CID refs (match patterns like cid:optional-<...> or cid:token) and normalize the captured token (strip surrounding angle brackets and compare lowercased) to check equality against the provided cid. Update the function containsCIDIgnoreCase to return true only when a captured CID equals the input cid (case-insensitive) so it won't match prefixes or miss bracketed forms.
1276-1294:⚠️ Potential issue | 🟠 MajorReuse the normal inline-part builder/container path here.
This helper is creating a different MIME shape than
add_inline/local-image resolution: it skipsTransfer-Encoding,Content-Dispositionfilename params, header sync, and it appends straight tosnapshot.Bodyinstead of the primarymultipart/relatedcontainer. On drafts rooted atmultipart/mixed, signature images can surface as regular attachments or fail CID rendering.🧩 Suggested fix
func addInlinePartToSnapshot(snapshot *DraftSnapshot, data []byte, contentType, filename, cid string) { - part := &Part{ - MediaType: contentType, - ContentDisposition: "inline", - ContentID: strings.Trim(cid, "<>"), - Body: data, - Dirty: true, - } - if filename != "" { - part.MediaParams = map[string]string{"name": filename} - } - // Find or create the multipart/related container. if snapshot.Body == nil { return } - if snapshot.Body.IsMultipart() { - snapshot.Body.Children = append(snapshot.Body.Children, part) - } + contentType, mediaParams := normalizedDetectedMediaType(contentType) + part := &Part{ + MediaType: contentType, + MediaParams: mediaParams, + ContentDisposition: "inline", + ContentDispositionArg: map[string]string{}, + ContentID: normalizeCID(cid), + TransferEncoding: "base64", + Body: data, + Dirty: true, + } + if filename != "" { + part.MediaParams["name"] = filename + part.ContentDispositionArg["filename"] = filename + } + syncStructuredPartHeaders(part) + + containerRef := primaryBodyRootRef(&snapshot.Body) + if containerRef == nil || *containerRef == nil { + return + } + container, err := ensureInlineContainerRef(containerRef) + if err != nil { + return + } + container.Children = append(container.Children, part) + container.Dirty = true // Non-multipart body: inline part is not added. This is expected when // the draft has a simple text/html body without multipart/related wrapper. // The signature HTML still references the CID, but the image won't render. // In practice, compose shortcuts wrap the body in multipart/related when // inline images are present, so this path rarely triggers. }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@shortcuts/mail/draft/patch.go` around lines 1276 - 1294, addInlinePartToSnapshot currently builds a Part differently than the normal inline-image path (it omits Transfer-Encoding, Content-Disposition filename params, header sync and appends directly to snapshot.Body), which causes attachments/CID rendering issues; change it to reuse the canonical inline-part builder/container path used by the local-image resolution code: construct the inline part via the same helper used by the add_inline/local-image flow (preserving Transfer-Encoding, Content-Disposition name param, and header-sync behavior), ensure ContentID is normalized (strings.Trim(cid,"<>")), and append the resulting part into the primary multipart/related container (creating a multipart/related child if the draft root is multipart/mixed) instead of appending directly to snapshot.Body; keep DraftSnapshot, addInlinePartToSnapshot, Part, snapshot.Body and IsMultipart references to locate and update the code.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@shortcuts/mail/draft/patch.go`:
- Around line 1302-1305: containsCIDIgnoreCase currently uses substring matching
which wrongly misses bracketed refs like cid:<logo123> and false-matches
prefixes; replace it with parsing of CID references: in containsCIDIgnoreCase
iterate over regex matches for case-insensitive CID refs (match patterns like
cid:optional-<...> or cid:token) and normalize the captured token (strip
surrounding angle brackets and compare lowercased) to check equality against the
provided cid. Update the function containsCIDIgnoreCase to return true only when
a captured CID equals the input cid (case-insensitive) so it won't match
prefixes or miss bracketed forms.
- Around line 1276-1294: addInlinePartToSnapshot currently builds a Part
differently than the normal inline-image path (it omits Transfer-Encoding,
Content-Disposition filename params, header sync and appends directly to
snapshot.Body), which causes attachments/CID rendering issues; change it to
reuse the canonical inline-part builder/container path used by the local-image
resolution code: construct the inline part via the same helper used by the
add_inline/local-image flow (preserving Transfer-Encoding, Content-Disposition
name param, and header-sync behavior), ensure ContentID is normalized
(strings.Trim(cid,"<>")), and append the resulting part into the primary
multipart/related container (creating a multipart/related child if the draft
root is multipart/mixed) instead of appending directly to snapshot.Body; keep
DraftSnapshot, addInlinePartToSnapshot, Part, snapshot.Body and IsMultipart
references to locate and update the code.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 27fc32cc-f831-4b8d-b201-7eed5acc337d
📒 Files selected for processing (5)
shortcuts/mail/draft/patch.goshortcuts/mail/draft/projection.goshortcuts/mail/mail_draft_edit.goshortcuts/mail/mail_signature.goskills/lark-mail/references/lark-mail-send.md
✅ Files skipped from review due to trivial changes (1)
- skills/lark-mail/references/lark-mail-send.md
🚧 Files skipped from review as they are similar to previous changes (1)
- shortcuts/mail/draft/projection.go
chanthuang
force-pushed
the
feat/mail-signature
branch
from
0c4dfe5 to
fbb3ce9
Compare
…hortcut - Add signature data model, API provider, and template variable interpolation with tests (shortcuts/mail/signature/) - Export signature-related symbols from draft package (SignatureWrapperClass, BuildSignatureHTML, FindMatchingCloseDiv, SplitAtQuote, RemoveSignatureHTML, SignatureSpacing, SignatureImage) for use by compose shortcuts - Add +signature shortcut for listing and viewing email signatures - Add signature reference documentation Change-Id: I62525e7b475692ada9ec8590b6d0252cf5afcdbc Co-Authored-By: AI
- Add --signature-id flag to +draft-create, +send, +reply, +reply-all, +forward for inserting a signature into the email body - Add signature image download with SSRF protection (https enforcement, no token leak, context timeout, size limit) - Add signature HTML insertion with quote-aware placement - Update compose shortcut reference docs Change-Id: Ic5606bab7826a20364084898ad1714778e5a8bd0 Co-Authored-By: AI
- Add insert_signature and remove_signature patch operations with old-signature MIME cleanup and case-insensitive CID matching - Expose signature ops in supported_ops flat list - Update SKILL.md and draft-edit reference docs Change-Id: I74affbf555e32351520f610ef42195f399a265d9 Co-Authored-By: AI
Test insert_signature and remove_signature ops: - Insert into basic HTML body - Insert before quote block (reply/forward) - Replace existing signature - Error on plain-text-only draft - Remove existing signature - Error when no signature present Change-Id: Icd713552b130d6eb461ef1cabca61e82327f4f0b Co-Authored-By: AI
infeng
reviewed
Apr 15, 2026
chanthuang
force-pushed
the
feat/mail-signature
branch
from
fbb3ce9 to
7df7101
Compare
There was a problem hiding this comment.
Actionable comments posted: 8
♻️ Duplicate comments (1)
shortcuts/mail/signature_compose.go (1)
47-50:⚠️ Potential issue | 🟠 MajorResolve template variables against the actual
--fromsender.This still interpolates the signature from the first
sendable_addressesentry, so alias/shared-mailbox flows can sendFrom: alias@example.comwhile the injected signature shows the primary mailbox name/address. Thread the resolved sender address into this lookup and match that entry first.Also applies to: 102-120
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@shortcuts/mail/signature_compose.go` around lines 47 - 50, The signature is interpolated using the primary mailbox entry; update the sender resolution so the actual resolved `--from` address is used when filling template variables: thread the effective sender address from `runtime` into `resolveSenderInfo` (or add a lookup after calling it) so it prefers the `sendable_addresses` entry that matches that resolved sender, then pass the resulting senderName/senderEmail into `signature.InterpolateTemplate`; touch `resolveSenderInfo`, the call site where `lang := resolveLang(runtime)` is used, and the signature interpolation call to ensure the matched alias/shared-mailbox entry is selected first.
🤖 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/patch_signature_test.go`:
- Around line 24-30: The test file is not gofmt-formatted; run gofmt (or `gofmt
-w`) on shortcuts/mail/draft/patch_signature_test.go to fix spacing/indentation
issues around the Apply call and the composite literals (DraftCtx, Patch,
PatchOp) so the snippet using Apply(&DraftCtx{FIO: testFIO}, snapshot,
Patch{Ops: []PatchOp{{Op: "insert_signature", SignatureID: "sig-123",
RenderedSignatureHTML: "<div>-- My Signature</div>",}},}) is properly formatted;
ensure Apply, DraftCtx, Patch and PatchOp literals follow gofmt conventions and
then re-run golangci-lint.
In `@shortcuts/mail/draft/patch.go`:
- Around line 1302-1305: containsCIDIgnoreCase should not do substring matching;
instead parse exact CID tokens from the HTML and compare normalized values.
Replace the substring check in containsCIDIgnoreCase(html, cid) with logic that
calls extractCIDRefs(html), normalizes both the target cid and the extracted
refs (strip surrounding < >, trim whitespace, and lowercase), then return true
if the normalized target is in the set of normalized refs; keep the function
name and signature but change its internals to use extractCIDRefs for exact
matching.
In `@shortcuts/mail/mail_send.go`:
- Line 36: The copied signature-flag block (referenced by the
variable/identifier signatureFlag) is not gofmt-formatted in mail_send.go (and
the same pattern in the sibling reply/forward shortcut files), causing
golangci-lint failures; run gofmt (or goimports) on the file(s) and reformat the
signatureFlag struct/initializer blocks so their commas, braces and indentation
match gofmt output (normalize the blocks around the copied signature-flag
occurrences) and commit the reformatted files to clear the CI lint error.
In `@shortcuts/mail/mail_signature.go`:
- Around line 50-53: The preview path is calling signature.InterpolateTemplate
with empty sender name/email so previews differ from compose; update both
executeSignatureDetail and executeSignatureList to resolve the mailbox sender
context (look up the mailbox by mailboxID the same way compose does) and pass
the mailbox's sender name and sender email into signature.InterpolateTemplate
instead of empty strings so mailbox-derived placeholders render the same in
preview and when inserted into a draft.
- Around line 34-36: Dry Run path is missing the "settings" segment so it
doesn't match the real endpoint used by signature.ListAll(); update the Dry Run
API call that constructs the request (the chain starting with
common.NewDryRunAPI().Desc(...).GET(...)) to call mailboxPath(mailboxID,
"settings", "signatures") instead of mailboxPath(mailboxID, "signatures") so the
dry-run URL matches the actual endpoint.
In `@shortcuts/mail/signature_html.go`:
- Around line 18-20: signatureIDRe currently only matches when id appears before
class; make it attribute-order agnostic by using an alternation that captures id
whether it appears before or after the class attribute (reuse the same pattern
used in draft/projection.go), e.g. change the regexp assigned to signatureIDRe
to include two branches with id captured in either branch and update
extractSignatureID (and the other similar regex around lines handling the same
pattern) to check both capture groups for the id value so it returns the
non-empty capture.
In `@shortcuts/mail/signature/interpolate.go`:
- Around line 86-92: The default switch branch in interpolate.go currently
returns raw val on unknown meta.Type, which can emit unescaped HTML; update the
default in the switch inside the function that calls
interpolateText/interpolateImage (the switch on meta.Type) to return
escapeHTML(val) (or return the original span wrapper) instead of raw val so
unknown variable types are safely escaped; ensure you import/retain the existing
escapeHTML helper and update any callers of this switch accordingly.
In `@shortcuts/mail/signature/model.go`:
- Around line 50-58: The Signature struct block in model.go is not
gofmt-formatted; run gofmt (or goimports) over the file and reformat the struct
declaration for Signature (fields: ID, Name, SignatureType, SignatureDevice,
Content, Images, TemplateJSONKeys, UserFields) so tags and alignment match gofmt
output; save the file and re-run golangci-lint to confirm the formatting error
is resolved.
---
Duplicate comments:
In `@shortcuts/mail/signature_compose.go`:
- Around line 47-50: The signature is interpolated using the primary mailbox
entry; update the sender resolution so the actual resolved `--from` address is
used when filling template variables: thread the effective sender address from
`runtime` into `resolveSenderInfo` (or add a lookup after calling it) so it
prefers the `sendable_addresses` entry that matches that resolved sender, then
pass the resulting senderName/senderEmail into `signature.InterpolateTemplate`;
touch `resolveSenderInfo`, the call site where `lang := resolveLang(runtime)` is
used, and the signature interpolation call to ensure the matched
alias/shared-mailbox entry is selected first.
🪄 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: 9d1c3d71-07b1-484c-8926-2fd8bfb79f42
📒 Files selected for processing (28)
shortcuts/mail/draft/model.goshortcuts/mail/draft/patch.goshortcuts/mail/draft/patch_signature_test.goshortcuts/mail/draft/projection.goshortcuts/mail/draft/projection_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/mail_signature.goshortcuts/mail/shortcuts.goshortcuts/mail/signature/interpolate.goshortcuts/mail/signature/interpolate_test.goshortcuts/mail/signature/model.goshortcuts/mail/signature/provider.goshortcuts/mail/signature_compose.goshortcuts/mail/signature_html.goskills/lark-mail/SKILL.mdskills/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.mdskills/lark-mail/references/lark-mail-signature.md
✅ Files skipped from review due to trivial changes (5)
- shortcuts/mail/mail_draft_create_test.go
- skills/lark-mail/references/lark-mail-send.md
- shortcuts/mail/signature/interpolate_test.go
- skills/lark-mail/references/lark-mail-draft-edit.md
- skills/lark-mail/references/lark-mail-signature.md
🚧 Files skipped from review as they are similar to previous changes (8)
- shortcuts/mail/shortcuts.go
- skills/lark-mail/references/lark-mail-draft-create.md
- skills/lark-mail/references/lark-mail-reply-all.md
- shortcuts/mail/draft/projection_test.go
- shortcuts/mail/mail_draft_edit.go
- skills/lark-mail/SKILL.md
- skills/lark-mail/references/lark-mail-reply.md
- shortcuts/mail/draft/projection.go
- Remove --device flag and device field from docs (not exposed in CLI) - Fix signature interpolation to match --from alias address in send_as list, instead of always using the primary mailbox address - Update lark-mail-signature.md reference doc Change-Id: I65f41a029cd33b17785e2355a99d042063962d23 Co-Authored-By: AI
chanthuang
force-pushed
the
feat/mail-signature
branch
from
b378760 to
4131dc7
Compare
- Remove unused cidSrcRe, collectSignatureCIDs, isCIDReferencedInHTML from signature_html.go (CID logic lives in draft/patch.go) - Remove unused strings import - Run gofmt on all affected files Change-Id: Ie142744a7ab17acf440dc69a5a78cefb3ce6c341 Co-Authored-By: AI
chanthuang
force-pushed
the
feat/mail-signature
branch
from
4131dc7 to
82ae97f
Compare
infeng
reviewed
Apr 15, 2026
…ft-edit Moved signature resolution after draft fetch+parse so insert_signature reads the From header from the existing draft. This ensures alias and shared-mailbox senders get correct template variable values (B-NAME, B-ENTERPRISE-EMAIL) instead of falling back to the primary address. Change-Id: I917016b17176090124814f30e8e15c67f1604de0 Co-Authored-By: AI
infeng
approved these changes
Apr 15, 2026
Collaborator
infeng
left a comment
infeng
left a comment
There was a problem hiding this comment.
LGTM. No issues found on the latest revision.
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
+signatureshortcut for listing and viewing email signatures with default usage info--signature-idflag to all compose shortcuts (+draft-create,+send,+reply,+reply-all,+forward) for inserting a signature into the email body+draft-editTest plan
lark-cli mail +signaturelists available signatureslark-cli mail +draft-create --signature-id <id>creates draft with signature insertedlark-cli mail +reply --signature-id <id>inserts signature before quoted contentlark-cli mail +draft-edit --patch-file <file>withinsert_signature/remove_signatureopsSummary by CodeRabbit