fix(mail): write X-LMS-Reply-Type header on +reply/+reply-all/+forward#1621
fix(mail): write X-LMS-Reply-Type header on +reply/+reply-all/+forward#1621xzcong0820 wants to merge 1 commit into
Conversation
larkcli reply/reply-all/forward built the reply relationship via X-LMS-Reply-To-Message-Id but never carried the reply type, so the backend could not distinguish REPLY vs FORWARD and never labeled the original message. Add emlbuilder.LMSReplyType (REPLY/FORWARD) and call it after LMSReplyToMessageID in the three mail shortcuts.
📝 WalkthroughWalkthroughThe mail EML builder now supports ChangesLMS reply type support
Sequence Diagram(s)sequenceDiagram
participant MailReply
participant MailReplyAll
participant MailForward
participant Builder
MailReply->>Builder: LMSReplyToMessageID(messageId) and LMSReplyType("REPLY")
MailReplyAll->>Builder: LMSReplyToMessageID(messageId) and LMSReplyType("REPLY")
MailForward->>Builder: LMSReplyToMessageID(messageId) and LMSReplyType("FORWARD")
Builder->>Builder: Build() emits X-LMS-Reply-Type when In-Reply-To exists
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 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 |
github-actions
Bot
added
domain/mail
🚀 PR Preview Install Guide🧰 CLI updatenpm i -g https://pkg.pr.new/larksuite/cli/@larksuite/cli@f9e8ac5b6ea85bc1115ca16f638cd8fde1de0473🧩 Skill updatenpx skills add xzcong0820/larksuite-cli#fix/reply-forward-label -y -g |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
shortcuts/mail/emlbuilder/builder.go (1)
740-747: 🎯 Functional Correctness | 🟡 MinorLMS reply headers are incorrectly gated on
inReplyToconditionThe
X-LMS-Reply-To-Message-IdandX-LMS-Reply-Typeheaders are only written whenb.inReplyTo != "", but these builder fields are set independently ofinReplyTo. In the call sites (mail_reply.go,mail_reply_all.go,mail_forward.go),inReplyTois populated only whennormalizeMessageID(orig.smtpMessageId) != "", whilelmsReplyToMessageIDandlmsReplyTypeare set whenevermessageIdis provided.If a message lacks an SMTP
Message-IDbut has the LarkmessageId(a realistic case for fetched Lark messages), the LMS header setters are called but the headers are silently dropped. This prevents the backend from labeling the original message asREPLIED/FORWARD, defeating the PR's purpose.Fix: Write the LMS headers unconditionally when their respective builder fields are non-empty, independent of the
inReplyTocheck:Current code (lines 740-747)
if b.inReplyTo != "" { writeHeader(&buf, "In-Reply-To", "<"+b.inReplyTo+">") if b.lmsReplyToMessageID != "" { writeHeader(&buf, "X-LMS-Reply-To-Message-Id", b.lmsReplyToMessageID) } if b.lmsReplyType != "" { writeHeader(&buf, "X-LMS-Reply-Type", b.lmsReplyType) }Proposed fix
if b.inReplyTo != "" { writeHeader(&buf, "In-Reply-To", "<"+b.inReplyTo+">") } if b.lmsReplyToMessageID != "" { writeHeader(&buf, "X-LMS-Reply-To-Message-Id", b.lmsReplyToMessageID) } if b.lmsReplyType != "" { writeHeader(&buf, "X-LMS-Reply-Type", b.lmsReplyType) }🤖 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/emlbuilder/builder.go` around lines 740 - 747, The LMS reply headers are incorrectly tied to the In-Reply-To branch in Builder.writeHeaders, so X-LMS-Reply-To-Message-Id and X-LMS-Reply-Type get dropped whenever b.inReplyTo is empty. Update the header-writing logic so b.lmsReplyToMessageID and b.lmsReplyType are emitted independently whenever those fields are non-empty, while keeping the In-Reply-To header behind its existing check. This change should preserve the reply-specific LMS metadata set by the mail_reply.go, mail_reply_all.go, and mail_forward.go call sites even when normalizeMessageID(orig.smtpMessageId) yields an empty value.
🤖 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.
Outside diff comments:
In `@shortcuts/mail/emlbuilder/builder.go`:
- Around line 740-747: The LMS reply headers are incorrectly tied to the
In-Reply-To branch in Builder.writeHeaders, so X-LMS-Reply-To-Message-Id and
X-LMS-Reply-Type get dropped whenever b.inReplyTo is empty. Update the
header-writing logic so b.lmsReplyToMessageID and b.lmsReplyType are emitted
independently whenever those fields are non-empty, while keeping the In-Reply-To
header behind its existing check. This change should preserve the reply-specific
LMS metadata set by the mail_reply.go, mail_reply_all.go, and mail_forward.go
call sites even when normalizeMessageID(orig.smtpMessageId) yields an empty
value.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: f108a1ee-8a42-46b7-bd00-69cc8948c0b0
📒 Files selected for processing (5)
shortcuts/mail/emlbuilder/builder.goshortcuts/mail/emlbuilder/builder_test.goshortcuts/mail/mail_forward.goshortcuts/mail/mail_reply.goshortcuts/mail/mail_reply_all.go
1 participant
larkcli +reply/+reply-all/+forward now emit an X-LMS-Reply-Type EML header (REPLY/FORWARD) alongside X-LMS-Reply-To-Message-Id, so the mail backend can label the original message with the REPLIED(-603)/FORWARD(-604) system label and the client shows the replied/forwarded arrow.
Summary by CodeRabbit
New Features
Bug Fixes
Tests