feat(mail): add +draft-send shortcut for batch draft sending#1017
Open
xukuncx wants to merge 1 commit into
Open
feat(mail): add +draft-send shortcut for batch draft sending#1017xukuncx wants to merge 1 commit into
xukuncx wants to merge 1 commit into
Conversation
|
|
📝 WalkthroughWalkthroughAdds a ChangesDraft-Send Shortcut Implementation
Sequence Diagram(s)sequenceDiagram
participant CLI
participant Runtime
participant MailAPI
participant LarkBackend
CLI->>Runtime: run +draft-send (draft IDs, mailbox, flags)
Runtime->>MailAPI: POST /user_mailboxes/:mb/drafts/:id/send (one per draft)
MailAPI->>LarkBackend: forward send request
LarkBackend-->>MailAPI: response {message_id, thread_id} or {detail, automation_send_disable}
MailAPI-->>Runtime: HTTP response
Runtime->>Runtime: isFatalSendErr / extractAutomationDisabledReason
Runtime-->>CLI: aggregated envelope (sent[], failed[]) or abort with ExitAPI
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 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)
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
![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 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_draft_send.go`:
- Around line 123-127: The loop over draftIDs trims id for validation but later
uses the original untrimmed id when building API paths; update the code so you
trim once and use the trimmed value for both validation and path construction
(e.g., assign strings.TrimSpace(id) to a local trimmedID and use trimmedID in
the validation check and when building the request path). Apply the same change
to the other loop/block referenced (lines 131-135) so all uses of draftIDs use
the trimmed value consistently.
🪄 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: 69681751-04ca-4e55-8af3-2a76eddeae86
📒 Files selected for processing (4)
internal/output/lark_errors.goshortcuts/mail/mail_draft_send.goshortcuts/mail/mail_draft_send_test.goshortcuts/mail/shortcuts.go
Comment on lines
+123
to
+127
| for _, id := range draftIDs { | ||
| if strings.TrimSpace(id) == "" { | ||
| return output.ErrValidation("--draft-id contains empty value") | ||
| } | ||
| } |
There was a problem hiding this comment.
Trim draft IDs before building API paths.
Line 124 validates TrimSpace(id) but the request path still uses untrimmed id, so whitespace-padded CSV values can slip through and hit incorrect endpoints.
Proposed fix
- for _, id := range draftIDs {
- if strings.TrimSpace(id) == "" {
+ normalizedDraftIDs := make([]string, 0, len(draftIDs))
+ for _, rawID := range draftIDs {
+ id := strings.TrimSpace(rawID)
+ if id == "" {
return output.ErrValidation("--draft-id contains empty value")
}
+ normalizedDraftIDs = append(normalizedDraftIDs, id)
}
- out := batchSendOutput{MailboxID: mailboxID, Total: len(draftIDs)}
+ out := batchSendOutput{MailboxID: mailboxID, Total: len(normalizedDraftIDs)}
stopOnErr := rt.Bool("stop-on-error")
- for _, id := range draftIDs {
+ for _, id := range normalizedDraftIDs {Also applies to: 131-135
🤖 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_draft_send.go` around lines 123 - 127, The loop over
draftIDs trims id for validation but later uses the original untrimmed id when
building API paths; update the code so you trim once and use the trimmed value
for both validation and path construction (e.g., assign strings.TrimSpace(id) to
a local trimmedID and use trimmedID in the validation check and when building
the request path). Apply the same change to the other loop/block referenced
(lines 131-135) so all uses of draftIDs use the trimmed value consistently.
Add `lark-cli mail +draft-send` shortcut that takes one or more existing
draft IDs and sends each via POST /drafts/:draft_id/send sequentially.
Per-draft failures are isolated and aggregated into a structured output;
fatal failures (auth, permission, network, mailbox quota) abort the
entire batch immediately while recoverable failures honor --stop-on-error.
Also extend internal/output with six mail-send-specific errno constants
(LarkErrMailboxNotFound=4013, LarkErrMailSendQuota{User,UserExt,TenantExt},
LarkErrMailQuota, LarkErrTenantStorageLimit) consumed by isFatalSendErr.
Risk is "high-risk-write" so the framework's --yes gate applies; the
shortcut declares only the minimal mail:user_mailbox.message:send scope
to avoid asking users for permissions it does not need.
There was a problem hiding this comment.
♻️ Duplicate comments (1)
shortcuts/mail/mail_draft_send.go (1)
123-127:⚠️ Potential issue | 🟠 Major | ⚡ Quick winNormalize
--draft-idvalues before using them in API paths.Line 124 trims only for emptiness validation, but Line 135 and Line 186 still use raw IDs. Inputs like
--draft-id "d1, d2"can produce malformed draft paths due to leading spaces.🔧 Proposed fix
- draftIDs := rt.StrSlice("draft-id") + rawDraftIDs := rt.StrSlice("draft-id") - if len(draftIDs) == 0 { + if len(rawDraftIDs) == 0 { return output.ErrValidation("--draft-id is required") } - if len(draftIDs) > MaxBatchSendDrafts { + if len(rawDraftIDs) > MaxBatchSendDrafts { return output.ErrValidation( "too many drafts: %d > %d (split into multiple batches)", - len(draftIDs), MaxBatchSendDrafts) + len(rawDraftIDs), MaxBatchSendDrafts) } - for _, id := range draftIDs { - if strings.TrimSpace(id) == "" { + draftIDs := make([]string, 0, len(rawDraftIDs)) + for _, rawID := range rawDraftIDs { + id := strings.TrimSpace(rawID) + if id == "" { return output.ErrValidation("--draft-id contains empty value") } + draftIDs = append(draftIDs, id) } @@ - draftIDs := rt.StrSlice("draft-id") + rawDraftIDs := rt.StrSlice("draft-id") @@ - for _, id := range draftIDs { + for _, rawID := range rawDraftIDs { + id := strings.TrimSpace(rawID) api = api.POST(mailboxPath(mailboxID, "drafts", id, "send")) }Also applies to: 131-135, 185-187
🤖 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_draft_send.go` around lines 123 - 127, The code trims draft IDs only for emptiness but later uses the original raw values (draftIDs) when building API paths, which allows leading/trailing spaces to create malformed paths; update the draftIDs slice by normalizing each entry (e.g., strings.TrimSpace) immediately after splitting/receiving it so that all subsequent uses (the loop using id, any path construction around where draftIDs are iterated/used) operate on the trimmed values; modify the spot where draftIDs is produced to overwrite each element with its trimmed version (or create a new normalized slice) and keep the existing empty-value validation against the trimmed entries.
🧹 Nitpick comments (1)
shortcuts/mail/mail_draft_send_test.go (1)
46-79: ⚡ Quick winPin the exact flag count in metadata contract assertions.
You currently verify required flags exist, but accidental extra user-declared flags would still pass. Add an exact count assertion to keep this test a strict public-surface guard.
Proposed patch
flagByName := map[string]common.Flag{} for _, fl := range MailDraftSend.Flags { flagByName[fl.Name] = fl } + if len(MailDraftSend.Flags) != 3 { + t.Fatalf("Flags count = %d, want 3", len(MailDraftSend.Flags)) + } mailbox, ok := flagByName["mailbox"]🤖 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_draft_send_test.go` around lines 46 - 79, The test currently checks presence and properties of individual flags but doesn't fail if extra flags are added; update the test to assert the exact flag count to lock the public surface by adding a single assertion that the number of flags on MailDraftSend (e.g., len(MailDraftSend.Flags) or len(flagByName)) equals 3 (the expected flags "mailbox", "draft-id", "stop-on-error"), so any accidental additional flags cause the test to fail.
🤖 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.
Duplicate comments:
In `@shortcuts/mail/mail_draft_send.go`:
- Around line 123-127: The code trims draft IDs only for emptiness but later
uses the original raw values (draftIDs) when building API paths, which allows
leading/trailing spaces to create malformed paths; update the draftIDs slice by
normalizing each entry (e.g., strings.TrimSpace) immediately after
splitting/receiving it so that all subsequent uses (the loop using id, any path
construction around where draftIDs are iterated/used) operate on the trimmed
values; modify the spot where draftIDs is produced to overwrite each element
with its trimmed version (or create a new normalized slice) and keep the
existing empty-value validation against the trimmed entries.
---
Nitpick comments:
In `@shortcuts/mail/mail_draft_send_test.go`:
- Around line 46-79: The test currently checks presence and properties of
individual flags but doesn't fail if extra flags are added; update the test to
assert the exact flag count to lock the public surface by adding a single
assertion that the number of flags on MailDraftSend (e.g.,
len(MailDraftSend.Flags) or len(flagByName)) equals 3 (the expected flags
"mailbox", "draft-id", "stop-on-error"), so any accidental additional flags
cause the test to fail.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: c0d8f7e3-ed2f-4ea5-adac-20f4de9a3976
📒 Files selected for processing (4)
internal/output/lark_errors.goshortcuts/mail/mail_draft_send.goshortcuts/mail/mail_draft_send_test.goshortcuts/mail/shortcuts.go
✅ Files skipped from review due to trivial changes (2)
- shortcuts/mail/shortcuts.go
- internal/output/lark_errors.go
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.
Generated by the harness-coding skill.
Sprints
mail +draft-sendshortcut + 6 mail send error codes + unit testsSummary
Adds
lark-cli mail +draft-send, a high-risk-write shortcut that takes one or more existing draft IDs and sends each viaPOST /drafts/:draft_id/sendsequentially. Per-draft failures are isolated and aggregated into a structured{sent[], failed[]}output; fatal failures (auth, permission, network, mailbox quota) abort the entire batch while recoverable failures honor--stop-on-error. Also extendsinternal/outputwith six new mail-send errno constants used by the new shortcut's fatal-error classifier.shortcuts/mail/mail_draft_send.go,shortcuts/mail/mail_draft_send_test.go,internal/output/lark_errors.goconstantsshortcuts/mail/shortcuts.go(registerMailDraftSend)mail:user_mailbox.message:sendonly (minimal-permission)user;Risk="high-risk-write"triggers framework--yesgateThis MR was created autonomously. Quality gates were enforced by the repo's own pre-commit hooks.
Summary by CodeRabbit
+draft-sendshortcut to batch-send existing drafts (max 50 per run), with aggregated JSON output showing sent and failed items, dry-run preview, confirmation gating, and--stop-on-errorsupport.