feat(mail): add mail rules, send_as, and --mailbox flag for alias/shared mailbox sending#318
feat(mail): add mail rules, send_as, and --mailbox flag for alias/shared mailbox sending#318
Conversation
Add user_mailbox.rules resource (create/delete/list/reorder/update) to core concepts, API resources, and permission table. Update skill description with rule-related trigger keywords.
|
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:
📝 WalkthroughWalkthroughCompose sender/mailbox resolution was refactored and a new Changes
Sequence Diagram(s)sequenceDiagram
rect rgba(200,200,255,0.5)
participant CLI as Runtime (flags)
participant Res as SenderResolver
participant API as ProfileAPI
participant Draft as DraftBuilder
end
CLI->>Res: resolve sender (--from, --mailbox)
alt `--from` present
Res-->>CLI: return `--from`
else if `--mailbox` non-empty and != "me"
Res->>API: fetch mailbox profile
API-->>Res: mailbox primary email
Res-->>CLI: return mailbox email
else
Res->>API: fetch profile("me")
API-->>Res: primary email
Res-->>CLI: return profile email
end
CLI->>Draft: build EML with resolved sender
Draft->>Draft: construct From header and continue
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes 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 docstrings
🧪 Generate unit tests (beta)
Comment |
github-actions
Bot
added
domain/mail
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@skills/lark-mail/SKILL.md`:
- Around line 322-340: Remove the duplicate user_mailbox.rules block and keep
the more complete version: delete the earlier, brief user_mailbox.rules section
(the first listing of the four actions) so only the fuller user_mailbox.rules
(with "reorder — 对收信规则进行排序" and "update — 更新收信规则") remains; ensure surrounding
headers (e.g., the user_mailbox.settings section) are unchanged and formatting
is preserved.
🪄 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: ebe0c0d6-de52-4e7c-81c2-1eaeb4eafa82
📒 Files selected for processing (16)
shortcuts/mail/helpers.goshortcuts/mail/helpers_test.goshortcuts/mail/mail_draft_create.goshortcuts/mail/mail_draft_edit.goshortcuts/mail/mail_forward.goshortcuts/mail/mail_reply.goshortcuts/mail/mail_reply_all.goshortcuts/mail/mail_send.goskill-template/domains/mail.mdskills/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.md
Greptile SummaryThis PR adds a The two remaining P2 findings are: (1) Confidence Score: 5/5Safe to merge — all remaining findings are P2 style/cleanup items with no correctness impact. The previously flagged shortcuts/mail/mail_draft_create.go (dead
|
| Filename | Overview |
|---|---|
| shortcuts/mail/helpers.go | Refactors sender/mailbox resolution into resolveComposeSenderEmail and resolveComposeMailboxID; fetchSelfEmailSet now always fetches "me" profile and adds mailboxID/--from literals — introduces a double profile API call when used alongside resolveComposeSenderEmail in reply commands. |
| shortcuts/mail/helpers_test.go | New TestResolveComposeSenderEmail and updated TestResolveComposeMailboxID cover flag-based short-circuit paths; the "no flags" / "mailbox=me without from" paths that hit the API are correctly deferred to integration tests. |
| shortcuts/mail/mail_draft_create.go | Adds --mailbox flag and delegates sender resolution to resolveComposeSenderEmail; draftCreateInput.From field is now dead code since the function reads from runtime directly instead of input.From. |
| shortcuts/mail/mail_send.go | Removes inline sender resolution in favour of resolveComposeSenderEmail; adds --mailbox flag; logic is clean and correct. |
| shortcuts/mail/mail_reply_all.go | Correctly wires --mailbox through resolveComposeMailboxID/resolveComposeSenderEmail/fetchSelfEmailSet; isSelfSent detection and buildReplyAllRecipients exclusion logic handle shared-mailbox and alias scenarios correctly. |
Flowchart
%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[Compose Shortcut Execute] --> B{resolveComposeMailboxID}
B --> |--mailbox set| C[return mailbox value]
B --> |--from set| D[return from value]
B --> |neither| E[return 'me']
A --> F{resolveComposeSenderEmail}
F --> |--from set| G[return from value]
F --> |--mailbox set AND != 'me'| H[return mailbox value]
F --> |neither / mailbox==me| I[fetchMailboxPrimaryEmail 'me']
A --> J{fetchSelfEmailSet}
J --> K[fetchMailboxPrimaryEmail 'me']
J --> |mailboxID != 'me'| L[add mailboxID literal]
J --> |--from set| M[add --from literal]
C & D & E --> N[Draft created in mailboxID]
G & H & I --> O[From header in EML]
K & L & M --> P[Reply-all exclude set]
Reviews (5): Last reviewed commit: "fix(mail): provide RuntimeContext in dra..." | Re-trigger Greptile
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
chanthuang
left a comment
chanthuang
left a comment
There was a problem hiding this comment.
PR Review
整体方向正确：原有 --from 同时承担「API 路由」和「设置 From 头」两个职责，在别名/共享邮箱场景下无法分离。新增 --mailbox flag 将两者拆分为 resolveComposeMailboxID（路由）和 resolveComposeSenderEmail（发件人），是合理的设计。
但有 1 个需要合入前修复的 bug 和 2 个改进建议：
🔴 Bug：resolveComposeSenderEmail 在 --mailbox=me 时返回字面量 "me"
func resolveComposeSenderEmail(runtime *common.RuntimeContext) string {
if from := runtime.Str("from"); from != "" {
return from
}
if mb := runtime.Str("mailbox"); mb != "" {
return mb // ← --mailbox=me 时直接返回 "me"，不是有效邮箱地址
}
email, _ := fetchMailboxPrimaryEmail(runtime, "me")
return email
}用户执行 +send --mailbox me --to ... 时，"me" 会被写入 EML From 头。测试用例的期望值也是错的：
{"mailbox=me without from", "me", "", "me"}, // 期望 "me" 不对修复：mailbox 分支加 mb != "me" 条件：
if mb := runtime.Str("mailbox"); mb != "" && mb != "me" {
return mb
}🟡 改进 1：fetchSelfEmailSet 未包含别名地址，reply-all 时自己可能出现在 CC 中
场景：+reply-all --mailbox shared@box.com --from alias@box.com
selfEmails包含me主邮箱 +shared@box.com- 但 不包含
alias@box.com（实际发件人），如果原邮件 To/CC 中有此地址，自己会出现在 CC 列表中
建议：在 fetchSelfEmailSet 中增加 --from 的排除：
if from := runtime.Str("from"); from != "" {
set[strings.ToLower(from)] = true
}🟡 改进 2：SKILL.md 中 user_mailbox.rules 小节重复
user_mailbox.rules 出现了两次，第一次 reorder/update 描述为空，第二次有完整描述。应删除第一个不完整的副本。
Revert simplified API method descriptions back to their original detailed versions for better developer guidance, and remove duplicated user_mailbox.rules section. Co-Authored-By: AI
… --from alias in reply-all - resolveComposeSenderEmail now falls through to profile API when --mailbox=me, avoiding an invalid "From: me" EML header. - fetchSelfEmailSet includes the --from alias address so reply-all correctly excludes the sender's own alias from recipients. Co-Authored-By: AI
Combine main's inline image --body descriptions with this branch's --from/--mailbox flag documentation in all reference files. Co-Authored-By: AI
🚀 PR Preview Install Guide🧰 CLI updatenpm i -g https://pkg.pr.new/larksuite/cli/@larksuite/cli@d0e8934c4164f141d228efabaa0b6d2fb1fb1346🧩 Skill updatenpx skills add larksuite/cli#feat/mail-rule-profile -y -g |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
shortcuts/mail/helpers.go (1)
198-210: Same nil-safety concern applies here.For consistency with
resolveComposeSenderEmail, this function should also handle nilruntime.Cmdgracefully to prevent potential panics in edge cases.🛡️ Proposed nil-safety check
func resolveComposeMailboxID(runtime *common.RuntimeContext) string { + if runtime == nil || runtime.Cmd == nil { + return "me" + } if mb := runtime.Str("mailbox"); mb != "" { return mb } if from := runtime.Str("from"); from != "" { return from } return "me" }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@shortcuts/mail/helpers.go` around lines 198 - 210, resolveComposeMailboxID currently calls runtime.Str(...) without guarding against a nil runtime.Cmd, which can panic; update resolveComposeMailboxID to mirror resolveComposeSenderEmail's nil-safety by first checking that runtime != nil and runtime.Cmd != nil (or using the same helper/guard used in resolveComposeSenderEmail) before calling runtime.Str("mailbox") and runtime.Str("from"), returning "me" if the command context is absent; reference the resolveComposeMailboxID function and the runtime.Str calls to locate where to add the nil checks.shortcuts/mail/mail_draft_edit.go (1)
29-30: Flag semantics differ from other compose commands.In
mail_draft_edit.go,--fromis described as "Mailbox email address containing the draft" with a default of "me", while in other compose commands (mail_send.go,mail_forward.go,mail_reply_all.go),--fromis the "Sender email address for the From header" with no default.This inconsistency could confuse users. Consider aligning the semantics:
- Remove
Default: "me"from--fromin draft-edit- Update the description to match other commands (sender address, not mailbox selection)
♻️ Proposed fix for consistency
- {Name: "from", Default: "me", Desc: "Mailbox email address containing the draft (default: me). Prefer --mailbox for clarity; --from is kept for backward compatibility."}, + {Name: "from", Desc: "Sender email address for the From header. When using an alias (send_as) address, set this to the alias and use --mailbox for the owning mailbox. Defaults to the mailbox's primary address."},🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@shortcuts/mail/mail_draft_edit.go` around lines 29 - 30, The --from flag in mail_draft_edit.go currently has Default: "me" and is described as "Mailbox email address containing the draft", which is inconsistent with other compose commands (mail_send.go, mail_forward.go, mail_reply_all.go) where --from is the sender address for the From header; change the --from flag definition in mail_draft_edit.go to remove Default: "me" and update its Desc to indicate it is the sender email address for the From header (not the mailbox selector), and ensure the existing --mailbox flag continues to represent the mailbox owning the draft and takes precedence when both are supplied.
🤖 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/helpers.go`:
- Around line 263-272: resolveComposeSenderEmail can panic when runtime or
runtime.Cmd is nil because runtime.Str() accesses runtime.Cmd.Flags(); add a
defensive guard at the top of resolveComposeSenderEmail: return an empty string
(or another safe default) if runtime == nil or runtime.Cmd == nil, and only call
runtime.Str(...) and fetchMailboxPrimaryEmail(runtime, "me") when those checks
pass; update the function body around resolveComposeSenderEmail to perform this
nil-safety check before using runtime.Str or calling fetchMailboxPrimaryEmail.
---
Nitpick comments:
In `@shortcuts/mail/helpers.go`:
- Around line 198-210: resolveComposeMailboxID currently calls runtime.Str(...)
without guarding against a nil runtime.Cmd, which can panic; update
resolveComposeMailboxID to mirror resolveComposeSenderEmail's nil-safety by
first checking that runtime != nil and runtime.Cmd != nil (or using the same
helper/guard used in resolveComposeSenderEmail) before calling
runtime.Str("mailbox") and runtime.Str("from"), returning "me" if the command
context is absent; reference the resolveComposeMailboxID function and the
runtime.Str calls to locate where to add the nil checks.
In `@shortcuts/mail/mail_draft_edit.go`:
- Around line 29-30: The --from flag in mail_draft_edit.go currently has
Default: "me" and is described as "Mailbox email address containing the draft",
which is inconsistent with other compose commands (mail_send.go,
mail_forward.go, mail_reply_all.go) where --from is the sender address for the
From header; change the --from flag definition in mail_draft_edit.go to remove
Default: "me" and update its Desc to indicate it is the sender email address for
the From header (not the mailbox selector), and ensure the existing --mailbox
flag continues to represent the mailbox owning the draft and takes precedence
when both are supplied.
🪄 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: e9f080cb-a787-4b4e-9d10-f89a25e6bcdc
📒 Files selected for processing (14)
shortcuts/mail/helpers.goshortcuts/mail/helpers_test.goshortcuts/mail/mail_draft_create.goshortcuts/mail/mail_draft_edit.goshortcuts/mail/mail_forward.goshortcuts/mail/mail_reply.goshortcuts/mail/mail_reply_all.goshortcuts/mail/mail_send.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 skipped from review due to trivial changes (2)
- skills/lark-mail/references/lark-mail-reply-all.md
- shortcuts/mail/mail_reply.go
🚧 Files skipped from review as they are similar to previous changes (5)
- skills/lark-mail/references/lark-mail-draft-edit.md
- skills/lark-mail/references/lark-mail-reply.md
- skills/lark-mail/references/lark-mail-draft-create.md
- skills/lark-mail/references/lark-mail-send.md
- shortcuts/mail/mail_draft_create.go
Tests were passing nil runtime to buildRawEMLForDraftCreate, which worked before but panics now that the function calls resolveComposeSenderEmail(runtime). Provide a proper RuntimeContext with --from flag set. Co-Authored-By: AI
There was a problem hiding this comment.
🧹 Nitpick comments (1)
shortcuts/mail/mail_draft_create_test.go (1)
15-24: Fail fast on flag setup in the helper.
cmd.Flags().Set("from", from)is currently ignored. If the flag name changes later, tests may silently exercise the wrong sender path. Consider threadingt *testing.Tinto this helper and asserting the set succeeds.Suggested patch
-// newRuntimeWithFrom creates a minimal RuntimeContext with --from flag set. -func newRuntimeWithFrom(from string) *common.RuntimeContext { +// newRuntimeWithFrom creates a minimal RuntimeContext with --from flag set. +func newRuntimeWithFrom(t *testing.T, from string) *common.RuntimeContext { + t.Helper() cmd := &cobra.Command{Use: "test"} cmd.Flags().String("from", "", "") cmd.Flags().String("mailbox", "", "") if from != "" { - _ = cmd.Flags().Set("from", from) + if err := cmd.Flags().Set("from", from); err != nil { + t.Fatalf("set --from flag failed: %v", err) + } } return &common.RuntimeContext{Cmd: cmd} }-rawEML, err := buildRawEMLForDraftCreate(newRuntimeWithFrom("sender@example.com"), input) +rawEML, err := buildRawEMLForDraftCreate(newRuntimeWithFrom(t, "sender@example.com"), input)🤖 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 15 - 24, The helper newRuntimeWithFrom silently ignores failures when setting the "from" flag; update it to accept t *testing.T, call cmd.Flags().Set("from", from) only when from != "", and assert the call succeeded (e.g., require.NoError/if err != nil t.Fatalf) so tests fail fast if the flag name changes or set fails; reference the newRuntimeWithFrom function and the cmd.Flags().Set("from", from) invocation when making the change.
🤖 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/mail_draft_create_test.go`:
- Around line 15-24: The helper newRuntimeWithFrom silently ignores failures
when setting the "from" flag; update it to accept t *testing.T, call
cmd.Flags().Set("from", from) only when from != "", and assert the call
succeeded (e.g., require.NoError/if err != nil t.Fatalf) so tests fail fast if
the flag name changes or set fails; reference the newRuntimeWithFrom function
and the cmd.Flags().Set("from", from) invocation when making the change.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 81a83d17-e3b7-4cf0-9a63-fa1c4a13bd0d
📒 Files selected for processing (1)
shortcuts/mail/mail_draft_create_test.go
2 participants
Summary
--mailboxflag to all compose shortcuts (+send,+reply,+reply-all,+forward,+draft-create,+draft-edit) to support sending via shared mailboxes and aliases (send_as). The--mailboxselects the owning mailbox while--fromsets the sender address in the From header.resolveComposeSenderEmail()and updateresolveComposeMailboxID()to respect the new--mailboxpriority chain (--mailbox>--from>"me").user_mailbox.rulesresource with create, delete, list, reorder, and update methods.send_asandaccessible_mailboxesAPI documentation for querying available sender addresses and accessible mailboxes.Test plan
resolveComposeMailboxIDwith--mailboxflag priorityresolveComposeSenderEmailflag-based resolution--mailbox shared@example.com--mailbox me --from alias@example.com--mailboxis omittedSummary by CodeRabbit
New Features
Behavior
Documentation
Tests