Support repeated mail compose flags - #2271
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:
📝 WalkthroughWalkthroughMail shortcuts now accept repeatable recipient and attachment flags. Inline images accept repeated JSON objects or arrays. Shared normalization runs during preview, validation, and execution. Template commands upload explicit inline attachments and reject them in plain-text mode. ChangesMail input normalization
Message shortcut integration
Template inline attachment flow
Mail parameter documentation
Estimated code review effort: 3 (Moderate) | ~30 minutes Sequence Diagram(s)sequenceDiagram
participant MailCommand
participant NormalizationHelpers
participant TemplatePayloadBuilder
participant Drive
participant TemplateAPI
MailCommand->>NormalizationHelpers: normalize flags and parse inline JSON
NormalizationHelpers-->>MailCommand: normalized recipients, attachments, and inline specifications
MailCommand->>TemplatePayloadBuilder: build template payload
TemplatePayloadBuilder->>Drive: upload inline files in input order
Drive-->>TemplatePayloadBuilder: uploaded file identifiers
TemplatePayloadBuilder->>TemplateAPI: submit payload with inline attachments
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✨ 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@78d5f9365fd6290a9c67501043da6ddb85c745a4🧩 Skill updatenpx skills add bubbmon233/cli#feat/e239120 -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: 6
🤖 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/address.go`:
- Around line 65-67: In the display-name encoding logic around encodeHeader,
replace the custom encoding path with mime.BEncoding.Encode("UTF-8", name) for
non-ASCII names. Preserve the existing behavior for names that need no encoding,
while ensuring long display names are split into RFC 2047-compliant
encoded-words.
In `@shortcuts/mail/helpers.go`:
- Around line 2382-2390: Update the inline JSON parsing flow around InlineSpec
to inspect the first non-whitespace byte of raw before unmarshalling; accept
only `{` for the object path or `[` for the array path, and return
mailValidationParamError("--inline", ...) for all other values, including null.
Preserve the existing unmarshalling and WithCause handling for valid object and
array inputs.
- Around line 2341-2348: Update normalizeInlineFlagValues after merging parsed
entries into all to validate that every CID is unique across the entire slice,
returning an error on duplicates before generating normalized JSON. Preserve the
existing parseInlineSpecs error handling and allow entries without a CID
according to current behavior.
In `@shortcuts/mail/mail_draft_create.go`:
- Around line 141-144: The template application flow must reject explicit inline
attachments when the selected template sets plain-text mode, rather than
silently omitting them. In shortcuts/mail/mail_draft_create.go lines 141-144,
after applyTemplate, return an error when input.Inline is non-empty and
input.PlainText is true; in shortcuts/mail/mail_send.go lines 144-147, apply the
equivalent check for inlineFlag and plainText. Add regression tests covering
both flows.
In `@shortcuts/mail/mail_template_update.go`:
- Around line 111-121: The inline/plain-text conflict is validated before
patch-file overrides are applied, so it must be checked against the final
template state. In shortcuts/mail/mail_template_update.go lines 111-121, remove
this conflict check from Validate; in lines 185-199, after
applyTemplatePatchFile, reject inline usage only when tpl.IsPlainTextMode is
true. Add regression tests covering both patch override directions: patching to
HTML permits inline, and patching to plain text rejects it.
In `@skills/lark-mail/references/lark-mail-template-update.md`:
- Line 74: Update the --inline option description in the template-update
documentation to state that inline attachments are rejected when the final
template uses plain-text mode, while preserving the existing ordering and
repeatable-argument behavior.
🪄 Autofix
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 Plus
Run ID: 8ce59ba9-d80a-4be0-b87e-c5b226b7a44c
📒 Files selected for processing (18)
shortcuts/mail/address.goshortcuts/mail/helpers.goshortcuts/mail/mail_draft_create.goshortcuts/mail/mail_forward.goshortcuts/mail/mail_repeatable_flags_test.goshortcuts/mail/mail_reply.goshortcuts/mail/mail_reply_all.goshortcuts/mail/mail_send.goshortcuts/mail/mail_template_create.goshortcuts/mail/mail_template_update.goshortcuts/mail/template_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.mdskills/lark-mail/references/lark-mail-template-create.mdskills/lark-mail/references/lark-mail-template-update.md
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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/mail_repeatable_flags_test.go`:
- Around line 98-106: Replace message-only assertions in
TestNormalizeRepeatedInlineFlagsRejectsDuplicateCID and the related tests in
shortcuts/mail/mail_repeatable_flags_test.go lines 98-106 and 108-117, and
shortcuts/mail/mail_template_shortcut_test.go lines 1154-1187, 1189-1220, and
1222-1258, with typed validation checks. Use errors.As to inspect
*errs.ValidationError, including Param set to --inline, and use errs.ProblemOf
only for problem-level metadata; assert the expected duplicate-CID,
non-object/non-array, send, draft-create, and final patched plain-text
validation categories and conflicts. Do not rely on err.Error() text alone, and
preserve the stated absence of wrapped causes for these local validation
branches.
In `@shortcuts/mail/mail_template_shortcut_test.go`:
- Around line 1285-1317: Strengthen the MailTemplateUpdate test around
runMountedMailShortcut by retaining the upload POST stub and asserting it
received a request, then inspect putStub.CapturedBody to verify the inline
attachment fields contain CID “hero” and the uploaded token “file_logo”. Keep
the existing HTML-mode assertion and ensure the test directly covers both upload
and attachment assembly.
🪄 Autofix
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 Plus
Run ID: fe6f4566-a5a8-4ef9-970b-04b5d2592554
📒 Files selected for processing (8)
shortcuts/mail/address.goshortcuts/mail/helpers.goshortcuts/mail/mail_draft_create.goshortcuts/mail/mail_repeatable_flags_test.goshortcuts/mail/mail_send.goshortcuts/mail/mail_template_shortcut_test.goshortcuts/mail/mail_template_update.goskills/lark-mail/references/lark-mail-template-update.md
🚧 Files skipped from review as they are similar to previous changes (6)
- shortcuts/mail/address.go
- shortcuts/mail/helpers.go
- shortcuts/mail/mail_draft_create.go
- skills/lark-mail/references/lark-mail-template-update.md
- shortcuts/mail/mail_template_update.go
- shortcuts/mail/mail_send.go
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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/mail_repeatable_flags_test.go`:
- Around line 123-134: Update assertInlineValidationError to assert that
ve.Category equals errs.CategoryValidation alongside the existing subtype and
parameter checks, preserving the helper’s typed metadata validation.
🪄 Autofix
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 Plus
Run ID: 68435373-6e90-463a-b069-0489b89d4572
📒 Files selected for processing (2)
shortcuts/mail/mail_repeatable_flags_test.goshortcuts/mail/mail_template_shortcut_test.go
🚧 Files skipped from review as they are similar to previous changes (1)
- shortcuts/mail/mail_template_shortcut_test.go
|
[P2 correctness] 非 ASCII display-name 会被内部归一化提前 MIME 编码 锚点： Line 2295 in 3c819de 这里不能用 建议归一化阶段只保护逗号边界和 ASCII quote，保留原始 Unicode name 给最终 EML 头编码/模板 JSON 渲染；补一条 |
|
[P2 correctness] 多逗号 display-name 会被拆成额外收件人 锚点： Line 2309 in 3c819de 这里只在当前片段无 建议从当前位置向后持续合并，直到形成完整 |
|
[P2 correctness] 锚点： Line 2355 in 3c819de
建议在 |
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
shortcuts/mail/mail_template_shortcut_test.go (1)
1154-1200: 📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick winAssert that plain-text compose omits explicit inline attachments.
Both tests only assert a successful draft ID. An implementation that still embeds
logo.pngas an inline MIME part with CIDherocan make the same POST request and pass.
shortcuts/mail/mail_template_shortcut_test.go#L1154-L1200: Retain the draft POST stub and inspect the raw EML. Assert that it has noContent-ID: <hero>inline part.shortcuts/mail/mail_template_shortcut_test.go#L1202-L1247: Retain the draft POST stub and assert the same absence for+draft-create.As per coding guidelines, “Every behavior change requires a nearby regression test that fails when the implementation is reverted; tests should assert fields, requests, typed errors, or side effects directly.”
🤖 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_template_shortcut_test.go` around lines 1154 - 1200, Strengthen TestMailSend_TemplatePlainTextIgnoresExplicitInline by inspecting the raw EML from the retained draft POST stub and asserting it contains no inline MIME part with Content-ID <hero>. Apply the same assertion to shortcuts/mail/mail_template_shortcut_test.go lines 1202-1247 for the +draft-create test; both sites require direct regression-test updates beyond checking only the draft ID.Source: Coding guidelines
🤖 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/mail_repeatable_flags_test.go`:
- Around line 100-115: Update normalizeInlineFlagValues and the parsed
inline-spec handling to canonicalize every CID by removing surrounding angle
brackets before marshaling, while preserving duplicate-CID compatibility. Extend
TestNormalizeRepeatedInlineFlagsAllowsDuplicateCIDForCompatibility to inspect
both normalized entries and assert their CID fields are the expected bare,
canonical value rather than only checking the entry count.
---
Outside diff comments:
In `@shortcuts/mail/mail_template_shortcut_test.go`:
- Around line 1154-1200: Strengthen
TestMailSend_TemplatePlainTextIgnoresExplicitInline by inspecting the raw EML
from the retained draft POST stub and asserting it contains no inline MIME part
with Content-ID <hero>. Apply the same assertion to
shortcuts/mail/mail_template_shortcut_test.go lines 1202-1247 for the
+draft-create test; both sites require direct regression-test updates beyond
checking only the draft ID.
🪄 Autofix
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 Plus
Run ID: e91de36b-2fd5-4d23-af58-76b6c82e4e56
📒 Files selected for processing (12)
shortcuts/mail/helpers.goshortcuts/mail/mail_draft_create.goshortcuts/mail/mail_repeatable_flags_test.goshortcuts/mail/mail_send.goshortcuts/mail/mail_template_shortcut_test.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.mdskills/lark-mail/references/lark-mail-template-create.mdskills/lark-mail/references/lark-mail-template-update.md
💤 Files with no reviewable changes (2)
- shortcuts/mail/mail_send.go
- shortcuts/mail/mail_draft_create.go
🚧 Files skipped from review as they are similar to previous changes (8)
- skills/lark-mail/references/lark-mail-template-create.md
- skills/lark-mail/references/lark-mail-draft-create.md
- skills/lark-mail/references/lark-mail-forward.md
- skills/lark-mail/references/lark-mail-reply-all.md
- skills/lark-mail/references/lark-mail-reply.md
- skills/lark-mail/references/lark-mail-template-update.md
- skills/lark-mail/references/lark-mail-send.md
- shortcuts/mail/helpers.go
| func TestNormalizeRepeatedInlineFlagsAllowsDuplicateCIDForCompatibility(t *testing.T) { | ||
| raw, err := normalizeInlineFlagValues([]string{ | ||
| `{"cid":"Logo","file_path":"./a.png"}`, | ||
| `[{"cid":"<logo>","file_path":"./b.png"}]`, | ||
| }) | ||
| if err != nil { | ||
| t.Fatalf("expected duplicate cid compatibility, got %v", err) | ||
| } | ||
| specs, err := parseInlineSpecs(raw) | ||
| if err != nil { | ||
| t.Fatalf("parseInlineSpecs(%q) error = %v", raw, err) | ||
| } | ||
| if len(specs) != 2 { | ||
| t.Fatalf("inline specs = %#v, want 2 entries", specs) | ||
| } | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Normalize parsed CIDs and assert canonical output.
normalizeInlineFlagValues marshals the parseInlineSpecs results. The supplied parser only checks that CID is non-empty. It leaves <logo> unchanged in normalized JSON.
Normalize each CID to its bare form before marshaling. Assert the expected normalized CID fields in this test. The entry-count assertion does not detect an inline attachment that uses <logo> while the HTML references cid:logo.
As per coding guidelines, “Every behavior change requires a nearby regression test that fails when the implementation is reverted; tests should assert fields, requests, typed errors, or side effects directly.”
🤖 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_repeatable_flags_test.go` around lines 100 - 115, Update
normalizeInlineFlagValues and the parsed inline-spec handling to canonicalize
every CID by removing surrounding angle brackets before marshaling, while
preserving duplicate-CID compatibility. Extend
TestNormalizeRepeatedInlineFlagsAllowsDuplicateCIDForCompatibility to inspect
both normalized entries and assert their CID fields are the expected bare,
canonical value rather than only checking the entry count.
Source: Coding guidelines
Change-Type: ci-fix
Change-Type: ci-fix
Change-Type: ci-fix
a2a1f61 to
78d5f93
Compare
1 participant
Adds support for passing list-style mail compose flags multiple times while keeping the existing comma-separated forms working.
Summary by CodeRabbit