feat(mail): auto-append default signature in compose shortcuts#1282
feat(mail): auto-append default signature in compose shortcuts#1282xzcong0820 wants to merge 3 commits into
Conversation
📝 WalkthroughWalkthroughThis PR extends five mail compose shortcuts (draft-create, forward, reply, reply-all, send) with body-file support, no-signature control, and lint reporting. It modernizes error handling to use structured mail error types, adds default-signature selection helpers to the signature provider, and enhances signature image downloads with HTTPS enforcement and size limits. Skill documentation is updated to describe the new flags and HTML writing preconditions. ChangesMail compose shortcuts: body-file, no-signature, lint, and structured errors
Signature image download robustness and error handling
Skill documentation: preconditions, body-file, and no-signature
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
⚔️ Resolve merge conflicts
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 |
- Add --no-signature flag to 5 compose shortcuts (+send, +draft-create, +reply, +reply-all, +forward) - Add DefaultSendID/DefaultReplyID helpers in shortcuts/mail/signature/provider.go - Auto-fetch and append default signature when no explicit --signature-id is given - Gracefully degrade (log warning, continue sending) if signature fetch fails - Update skills reference docs to document the new --no-signature flag
There was a problem hiding this comment.
Actionable comments posted: 6
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_reply_all.go (1)
99-100:⚠️ Potential issue | 🟠 Major | ⚡ Quick win
--no-signatureoverride is blocked by earlier validation.On Line 99, validation still rejects
--plain-text+--signature-idbefore the Line 139-145 override can clearsignature-id. This breaks the new--no-signature + --signature-id“warn and continue” behavior.Suggested fix
- if err := validateSignatureWithPlainText(runtime.Bool("plain-text"), runtime.Str("signature-id")); err != nil { - return err - } + if !runtime.Bool("no-signature") { + if err := validateSignatureWithPlainText(runtime.Bool("plain-text"), runtime.Str("signature-id")); err != nil { + return err + } + }Also applies to: 138-145
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@shortcuts/mail/mail_reply_all.go` around lines 99 - 100, The current call to validateSignatureWithPlainText(runtime.Bool("plain-text"), runtime.Str("signature-id")) runs before the later override logic that clears signature-id for the --no-signature flow, causing --no-signature + --signature-id to be rejected; move the validation to after the block that handles and potentially clears signature-id (the override in the region around lines 139-145) or alter the validation to early-bail when runtime.Bool("no-signature") is true so that validateSignatureWithPlainText is not applied when --no-signature is intended to override signature-id.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@shortcuts/mail/signature/provider.go`:
- Around line 73-76: Update the docstring to match the actual behavior: state
that DefaultSendID (and the helper pickSignatureID) return an empty string when
no address matches or when no default is configured, and do not fall back to
usages[0]; remove the incorrect "Falls back to usages[0]" line and describe the
actual fallback semantics (return usages[0] only when configured, otherwise "").
Mention the functions DefaultSendID and pickSignatureID by name so reviewers can
find the updated comment.
In `@skills/lark-mail/references/lark-mail-draft-create.md`:
- Line 11: The markdown link in the precondition text currently points to
"references/lark-mail-html.md" (one directory too deep); update the link target
to the same-directory relative path "lark-mail-html.md" so the precondition
"Read 工具读取 lark-mail-html.md" becomes clickable—edit the markdown link in the
precondition line that references lark-mail-html.md to replace
"references/lark-mail-html.md" with "lark-mail-html.md".
In `@skills/lark-mail/references/lark-mail-forward.md`:
- Line 16: The markdown link "references/lark-mail-html.md" in the sentence
"**CRITICAL - 编辑邮件内容前 MUST 先用 Read 工具读取
[references/lark-mail-html.md](references/lark-mail-html.md),其中包含邮件书写规范**" is a
broken relative path; update the link target to "./lark-mail-html.md" so it
correctly points to lark-mail-html.md in the same references directory (replace
the reference string "references/lark-mail-html.md" with "./lark-mail-html.md").
In `@skills/lark-mail/references/lark-mail-reply-all.md`:
- Line 16: Replace the broken relative link text "references/lark-mail-html.md"
in the document with the correct relative path "./lark-mail-html.md" so the
reference to lark-mail-html.md resolves correctly; locate the occurrence of the
string "references/lark-mail-html.md" (shown in the diff) and update it to
"./lark-mail-html.md".
In `@skills/lark-mail/references/lark-mail-reply.md`:
- Line 20: Update the broken relative link in the markdown line that references
"references/lark-mail-html.md": replace that path with "./lark-mail-html.md" so
the link resolves correctly from the current directory (modify the link text
that currently points to references/lark-mail-html.md).
In `@skills/lark-mail/references/lark-mail-send.md`:
- Line 15: The relative link to the HTML reference is incorrect; update the link
target in the markdown text that currently points to
"references/lark-mail-html.md" so it correctly references the sibling file as
"./lark-mail-html.md" (look for the link text or filename "lark-mail-html.md" in
the content of skills/lark-mail/references/lark-mail-send.md and replace the
extra "references/" segment with "./").
---
Outside diff comments:
In `@shortcuts/mail/mail_reply_all.go`:
- Around line 99-100: The current call to
validateSignatureWithPlainText(runtime.Bool("plain-text"),
runtime.Str("signature-id")) runs before the later override logic that clears
signature-id for the --no-signature flow, causing --no-signature +
--signature-id to be rejected; move the validation to after the block that
handles and potentially clears signature-id (the override in the region around
lines 139-145) or alter the validation to early-bail when
runtime.Bool("no-signature") is true so that validateSignatureWithPlainText is
not applied when --no-signature is intended to override signature-id.
🪄 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: ee0838fb-d0c9-4fd7-b9d5-bcfbd37b3973
📒 Files selected for processing (12)
shortcuts/mail/mail_draft_create.goshortcuts/mail/mail_forward.goshortcuts/mail/mail_reply.goshortcuts/mail/mail_reply_all.goshortcuts/mail/mail_send.goshortcuts/mail/signature/provider.goshortcuts/mail/signature_compose.goskills/lark-mail/references/lark-mail-draft-create.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
| // DefaultSendID returns the send_mail_signature_id for the given addr. | ||
| // Falls back to usages[0] if no entry matches, but returns "" when | ||
| // no default is configured (id empty or "0"). | ||
| // Returns "" if usages is empty. |
There was a problem hiding this comment.
Docstring claims fallback to usages[0] but implementation returns "".
The docstring states "Falls back to usages[0] if no entry matches" but pickSignatureID returns "" when no address matches (line 106). The implementation appears correct based on caller expectations; the docstring should be updated.
📝 Suggested docstring fix
// DefaultSendID returns the send_mail_signature_id for the given addr.
-// Falls back to usages[0] if no entry matches, but returns "" when
-// no default is configured (id empty or "0").
+// Returns "" when no entry matches or no default is configured
+// (id empty or "0").
// Returns "" if usages is empty.📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| // DefaultSendID returns the send_mail_signature_id for the given addr. | |
| // Falls back to usages[0] if no entry matches, but returns "" when | |
| // no default is configured (id empty or "0"). | |
| // Returns "" if usages is empty. | |
| // DefaultSendID returns the send_mail_signature_id for the given addr. | |
| // Returns "" when no entry matches or no default is configured | |
| // (id empty or "0"). | |
| // Returns "" if usages is empty. |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@shortcuts/mail/signature/provider.go` around lines 73 - 76, Update the
docstring to match the actual behavior: state that DefaultSendID (and the helper
pickSignatureID) return an empty string when no address matches or when no
default is configured, and do not fall back to usages[0]; remove the incorrect
"Falls back to usages[0]" line and describe the actual fallback semantics
(return usages[0] only when configured, otherwise ""). Mention the functions
DefaultSendID and pickSignatureID by name so reviewers can find the updated
comment.
|
|
||
| 如需修改已有草稿,不要使用此命令,请使用 `lark-cli mail +draft-edit`。 | ||
|
|
||
| **CRITICAL - 编辑邮件内容前 MUST 先用 Read 工具读取 [references/lark-mail-html.md](references/lark-mail-html.md),其中包含邮件书写规范** |
There was a problem hiding this comment.
Fix broken relative link to lark-mail-html.md.
On Line 11, the link target uses references/lark-mail-html.md, which is nested one level too deep from this file’s location. Use a same-directory relative path so the precondition is clickable.
Suggested fix
-**CRITICAL - 编辑邮件内容前 MUST 先用 Read 工具读取 [references/lark-mail-html.md](references/lark-mail-html.md),其中包含邮件书写规范**
+**CRITICAL - 编辑邮件内容前 MUST 先用 Read 工具读取 [references/lark-mail-html.md](./lark-mail-html.md),其中包含邮件书写规范**📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| **CRITICAL - 编辑邮件内容前 MUST 先用 Read 工具读取 [references/lark-mail-html.md](references/lark-mail-html.md),其中包含邮件书写规范** | |
| **CRITICAL - 编辑邮件内容前 MUST 先用 Read 工具读取 [references/lark-mail-html.md](./lark-mail-html.md),其中包含邮件书写规范** |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@skills/lark-mail/references/lark-mail-draft-create.md` at line 11, The
markdown link in the precondition text currently points to
"references/lark-mail-html.md" (one directory too deep); update the link target
to the same-directory relative path "lark-mail-html.md" so the precondition
"Read 工具读取 lark-mail-html.md" becomes clickable—edit the markdown link in the
precondition line that references lark-mail-html.md to replace
"references/lark-mail-html.md" with "lark-mail-html.md".
|
|
||
| ## CRITICAL — 发送工作流(必须遵循) | ||
|
|
||
| **CRITICAL - 编辑邮件内容前 MUST 先用 Read 工具读取 [references/lark-mail-html.md](references/lark-mail-html.md),其中包含邮件书写规范** |
There was a problem hiding this comment.
Fix broken relative link to lark-mail-html.md.
On Line 16, references/lark-mail-html.md points to a nested path from this file location. Switch to ./lark-mail-html.md.
Suggested fix
-**CRITICAL - 编辑邮件内容前 MUST 先用 Read 工具读取 [references/lark-mail-html.md](references/lark-mail-html.md),其中包含邮件书写规范**
+**CRITICAL - 编辑邮件内容前 MUST 先用 Read 工具读取 [references/lark-mail-html.md](./lark-mail-html.md),其中包含邮件书写规范**📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| **CRITICAL - 编辑邮件内容前 MUST 先用 Read 工具读取 [references/lark-mail-html.md](references/lark-mail-html.md),其中包含邮件书写规范** | |
| **CRITICAL - 编辑邮件内容前 MUST 先用 Read 工具读取 [references/lark-mail-html.md](./lark-mail-html.md),其中包含邮件书写规范** |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@skills/lark-mail/references/lark-mail-forward.md` at line 16, The markdown
link "references/lark-mail-html.md" in the sentence "**CRITICAL - 编辑邮件内容前 MUST
先用 Read 工具读取
[references/lark-mail-html.md](references/lark-mail-html.md),其中包含邮件书写规范**" is a
broken relative path; update the link target to "./lark-mail-html.md" so it
correctly points to lark-mail-html.md in the same references directory (replace
the reference string "references/lark-mail-html.md" with "./lark-mail-html.md").
|
|
||
| ## CRITICAL — 发送工作流(必须遵循) | ||
|
|
||
| **CRITICAL - 编辑邮件内容前 MUST 先用 Read 工具读取 [references/lark-mail-html.md](references/lark-mail-html.md),其中包含邮件书写规范** |
There was a problem hiding this comment.
Fix broken relative link to lark-mail-html.md.
On Line 16, the link is currently references/lark-mail-html.md, which is incorrect relative to this file. Use ./lark-mail-html.md.
Suggested fix
-**CRITICAL - 编辑邮件内容前 MUST 先用 Read 工具读取 [references/lark-mail-html.md](references/lark-mail-html.md),其中包含邮件书写规范**
+**CRITICAL - 编辑邮件内容前 MUST 先用 Read 工具读取 [references/lark-mail-html.md](./lark-mail-html.md),其中包含邮件书写规范**📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| **CRITICAL - 编辑邮件内容前 MUST 先用 Read 工具读取 [references/lark-mail-html.md](references/lark-mail-html.md),其中包含邮件书写规范** | |
| **CRITICAL - 编辑邮件内容前 MUST 先用 Read 工具读取 [references/lark-mail-html.md](./lark-mail-html.md),其中包含邮件书写规范** |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@skills/lark-mail/references/lark-mail-reply-all.md` at line 16, Replace the
broken relative link text "references/lark-mail-html.md" in the document with
the correct relative path "./lark-mail-html.md" so the reference to
lark-mail-html.md resolves correctly; locate the occurrence of the string
"references/lark-mail-html.md" (shown in the diff) and update it to
"./lark-mail-html.md".
|
|
||
| ## CRITICAL — 发送工作流(必须遵循) | ||
|
|
||
| **CRITICAL - 编辑邮件内容前 MUST 先用 Read 工具读取 [references/lark-mail-html.md](references/lark-mail-html.md),其中包含邮件书写规范** |
There was a problem hiding this comment.
Fix broken relative link to lark-mail-html.md.
On Line 20, references/lark-mail-html.md resolves incorrectly from this directory. Please use ./lark-mail-html.md.
Suggested fix
-**CRITICAL - 编辑邮件内容前 MUST 先用 Read 工具读取 [references/lark-mail-html.md](references/lark-mail-html.md),其中包含邮件书写规范**
+**CRITICAL - 编辑邮件内容前 MUST 先用 Read 工具读取 [references/lark-mail-html.md](./lark-mail-html.md),其中包含邮件书写规范**📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| **CRITICAL - 编辑邮件内容前 MUST 先用 Read 工具读取 [references/lark-mail-html.md](references/lark-mail-html.md),其中包含邮件书写规范** | |
| **CRITICAL - 编辑邮件内容前 MUST 先用 Read 工具读取 [references/lark-mail-html.md](./lark-mail-html.md),其中包含邮件书写规范** |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@skills/lark-mail/references/lark-mail-reply.md` at line 20, Update the broken
relative link in the markdown line that references
"references/lark-mail-html.md": replace that path with "./lark-mail-html.md" so
the link resolves correctly from the current directory (modify the link text
that currently points to references/lark-mail-html.md).
|
|
||
| ## CRITICAL — 发送工作流(必须遵循) | ||
|
|
||
| **CRITICAL - 编辑邮件内容前 MUST 先用 Read 工具读取 [references/lark-mail-html.md](references/lark-mail-html.md),其中包含邮件书写规范** |
There was a problem hiding this comment.
Fix broken relative link to lark-mail-html.md.
On Line 15, the relative link includes an extra references/ segment for this file location. Update it to ./lark-mail-html.md.
Suggested fix
-**CRITICAL - 编辑邮件内容前 MUST 先用 Read 工具读取 [references/lark-mail-html.md](references/lark-mail-html.md),其中包含邮件书写规范**
+**CRITICAL - 编辑邮件内容前 MUST 先用 Read 工具读取 [references/lark-mail-html.md](./lark-mail-html.md),其中包含邮件书写规范**📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| **CRITICAL - 编辑邮件内容前 MUST 先用 Read 工具读取 [references/lark-mail-html.md](references/lark-mail-html.md),其中包含邮件书写规范** | |
| **CRITICAL - 编辑邮件内容前 MUST 先用 Read 工具读取 [references/lark-mail-html.md](./lark-mail-html.md),其中包含邮件书写规范** |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@skills/lark-mail/references/lark-mail-send.md` at line 15, The relative link
to the HTML reference is incorrect; update the link target in the markdown text
that currently points to "references/lark-mail-html.md" so it correctly
references the sibling file as "./lark-mail-html.md" (look for the link text or
filename "lark-mail-html.md" in the content of
skills/lark-mail/references/lark-mail-send.md and replace the extra
"references/" segment with "./").
The PR branch was missing errs/ (introduced in fe72e41) and shortcuts/mail/lint/ packages that are referenced by: - shortcuts/mail/mail_forward.go - shortcuts/mail/signature_compose.go - shortcuts/mail/signature/provider.go - shortcuts/mail/mail_draft_create.go Merging main fixes the missing-package compile errors and aligns the branch with the full upstream feature set including the HTML lint library, draft sending, and typed error envelopes. Merge conflicts in mail_draft_create.go and signature/provider.go were resolved by keeping the PR branch's --no-signature flag implementation and DefaultSendID/DefaultReplyID helper functions which are part of the PR's feature work. sprint: S2
🚀 PR Preview Install Guide🧰 CLI updatenpm i -g https://pkg.pr.new/larksuite/cli/@larksuite/cli@cfdfacf8c2b1f7a4692694acca03ff7d071c45e3🧩 Skill updatenpx skills add xzcong0820/larksuite-cli#harness/01KT8TVP9ZWD94Z6RFP17D29HN -y -g |
Codecov Report❌ Patch coverage is ❌ Your patch check has failed because the patch coverage (28.04%) is below the target coverage (60.00%). You can increase the patch coverage or adjust the target coverage. Additional details and impacted files@@ Coverage Diff @@
## main #1282 +/- ##
==========================================
+ Coverage 70.17% 70.27% +0.10%
==========================================
Files 671 672 +1
Lines 65329 65403 +74
==========================================
+ Hits 45843 45963 +120
+ Misses 15796 15772 -24
+ Partials 3690 3668 -22 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Summary
--no-signatureflag to 5 compose shortcuts (+send,+draft-create,+reply,+reply-all,+forward)DefaultSendID/DefaultReplyIDhelpers inshortcuts/mail/signature/provider.goto resolve default signature from mailbox settings--signature-idis given and--plain-textis not set--no-signatureflagBehavior
ListUserMailboxSignatures--signature-id <id>--no-signature--no-signature+--signature-id--plain-textChanges
shortcuts/mail/signature_compose.go: addnoSignatureFlagshortcuts/mail/signature/provider.go: addDefaultSendID,DefaultReplyID,pickSignatureIDshortcuts/mail/mail_send.go: add flag + auto-resolve (DefaultSendID)shortcuts/mail/mail_draft_create.go: add flag + auto-resolve (DefaultSendID)shortcuts/mail/mail_reply.go: add flag + auto-resolve (DefaultReplyID)shortcuts/mail/mail_reply_all.go: add flag + auto-resolve (DefaultReplyID)shortcuts/mail/mail_forward.go: add flag + auto-resolve (DefaultReplyID)skills/lark-mail/references/lark-mail-*.md: document--no-signatureflag🤖 Generated with Claude Code (harness-coding)
Summary by CodeRabbit
Release Notes
New Features
--body-fileflag to read email body from a file (mutually exclusive with--body).--no-signatureflag to disable automatic signature insertion.--show-lint-detailsflag to display email validation and formatting details.Documentation