[harness:01KP342QM8XZZT86SCJJGGK50Y-4] Mail CLI 支持定时发送 — CLI 技术方案#452
[harness:01KP342QM8XZZT86SCJJGGK50Y-4] Mail CLI 支持定时发送 — CLI 技术方案#452
Conversation
…ancel-scheduled-send shortcut Add scheduled send support to the Mail CLI: - New shortcuts/mail/scheduled_send.go with parseAndValidateSendTime (RFC 3339 parsing, min 5-minute lead time, UTC default) and formatScheduledTimeHuman - New shortcuts/mail/scheduled_send_test.go with comprehensive time parsing tests - Modified +send, +reply, +reply-all, +forward shortcuts to accept --send-time flag with validation, scheduled send via SendWithBody, and status/hint output - New +cancel-scheduled-send shortcut with --message-id (required) and --user-mailbox-id (default: me) flags - New shortcuts/mail/mail_cancel_scheduled_send_test.go with success, error, missing-id, and custom-mailbox test cases - Added SendWithBody to draft/service.go for passing send_time in request body - Registered MailCancelScheduledSend in shortcuts.go
github-actions
bot
added
domain/mail
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
✅ Files skipped from review due to trivial changes (2)
📝 WalkthroughWalkthroughAdds scheduled send support across mail shortcuts: a new Draft API helper Changes
Sequence Diagram(s)sequenceDiagram
actor User
participant CLI as CLI
participant Draft as Draft Service
participant API as Mail API
User->>CLI: run +send / +reply ... --send-time "2026-04-15T14:30:00Z" --confirm-send
CLI->>CLI: parseAndValidateSendTime(send-time) (RFC3339 / tz-less -> UTC, ensure >= 5m)
CLI->>Draft: SendWithBody(runtime, mailboxID, draftID, { "send_time": "<unix_ts>" })
activate Draft
Draft->>API: POST /vX/.../drafts/{draftID}/send with JSON body
activate API
API-->>Draft: { message_id, thread_id, ... , status: "scheduled" }
deactivate API
Draft-->>CLI: response map
CLI-->>User: print output (message_id, thread_id, status/send_time tip)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ❌ 3❌ Failed checks (2 warnings, 1 inconclusive)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
Greptile SummaryThis PR adds scheduled email sending via a new
Confidence Score: 4/5Merge after verifying the cancel scope and the send_time JSON type against the Lark API spec — both could cause silent runtime failures. Two concerns remain: the cancel shortcut uses the shortcuts/mail/mail_cancel_scheduled_send.go (scope), shortcuts/mail/scheduled_send.go (send_time JSON type) Important Files Changed
Sequence DiagramsequenceDiagram
participant User
participant CLI
participant Validate
participant Execute
participant DraftAPI
participant SendAPI
participant CancelAPI
User->>CLI: lark-cli mail +send --send-time T --confirm-send
CLI->>Validate: parseAndValidateSendTime(T)
Validate-->>CLI: ok / validation error
CLI->>Execute: run
Execute->>DraftAPI: POST /drafts (create draft)
DraftAPI-->>Execute: draft_id
alt send-time provided
Execute->>Execute: parseAndValidateSendTime(T) → unix_ts string
Execute->>SendAPI: POST /drafts/{id}/send {send_time: unix_ts}
SendAPI-->>Execute: message_id
Execute-->>User: {status: scheduled, send_time: raw_input_str}
Note over Execute,User: tip: lark-cli mail +cancel-scheduled-send --message-id ...
else immediate send
Execute->>SendAPI: POST /drafts/{id}/send
SendAPI-->>Execute: message_id
Execute-->>User: {message_id, thread_id}
end
User->>CLI: lark-cli mail +cancel-scheduled-send --message-id M
CLI->>CancelAPI: POST /messages/{M}/cancel_scheduled_send
CancelAPI-->>CLI: ok
CLI-->>User: {status: cancelled, restored_as_draft: true}
Reviews (2): Last reviewed commit: "fix: return Unix timestamp instead of RF..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| if sendTimeStr != "" { | ||
| outData["status"] = "scheduled" | ||
| outData["send_time"] = sendTimeStr | ||
| fmt.Fprintf(runtime.IO().ErrOut, |
There was a problem hiding this comment.
outData["send_time"] is set to the raw sendTimeStr from the user, not the validatedTime returned by parseAndValidateSendTime. Because validatedTime is declared inside the inner if block, it goes out of scope before the output is built. For timezone-free inputs like "2026-04-14T09:00:00", the API receives "2026-04-14T09:00:00Z" (UTC-normalized), but the output field shows the ambiguous original string — users who omit a timezone offset may believe the email is scheduled in their local time when it is not.
The same pattern appears in mail_reply.go (line 216), mail_reply_all.go, and mail_forward.go.
Fix: hoist validatedTime to an outer scope so it can be used when building outData:
var sendBody map[string]interface{}
var validatedSendTime string
if sendTimeStr != "" {
var timeErr error
validatedSendTime, timeErr = parseAndValidateSendTime(sendTimeStr)
if timeErr != nil {
return timeErr
}
sendBody = map[string]interface{}{
"send_time": validatedSendTime,
}
}
// ...
if sendTimeStr != "" {
outData["status"] = "scheduled"
outData["send_time"] = validatedSendTime // normalized, not raw user input| func formatScheduledTimeHuman(sendTime string) string { | ||
| t, err := time.Parse(time.RFC3339, sendTime) | ||
| if err != nil { | ||
| return sendTime | ||
| } | ||
| dur := time.Until(t) | ||
| var relative string | ||
| switch { | ||
| case dur < time.Hour: | ||
| relative = fmt.Sprintf("in %d minutes", int(dur.Minutes())) | ||
| case dur < 24*time.Hour: | ||
| relative = fmt.Sprintf("in %d hours", int(dur.Hours())) | ||
| default: | ||
| relative = fmt.Sprintf("in %d days", int(dur.Hours()/24)) | ||
| } | ||
| return fmt.Sprintf("%s (%s, %s)", sendTime, t.Format("Mon"), relative) | ||
| } |
There was a problem hiding this comment.
formatScheduledTimeHuman is dead code
This function is defined and unit-tested but never called from any production code path. None of the four --send-time send commands invoke it; the output just echoes sendTimeStr directly. Either wire it into the output (e.g. as a human-readable send_time_display field) or remove it and the corresponding tests to keep the package clean.
| _, err := runtime.CallAPI("POST", path, nil, nil) | ||
| if err != nil { | ||
| return output.ErrWithHint(output.ExitAPI, "api_error", | ||
| fmt.Sprintf("Failed to cancel scheduled send for message %s", messageID), | ||
| "Ensure the message ID is correct and the email has not already been sent.", | ||
| ) | ||
| } |
There was a problem hiding this comment.
When runtime.CallAPI returns an error, the original err is silently dropped and replaced with a generic hint. Other shortcuts in this package wrap the underlying error with fmt.Errorf("...: %w", err), which preserves the diagnostic message for debugging. Consider wrapping err here so that the actual API error code/message is still surfaced:
| _, err := runtime.CallAPI("POST", path, nil, nil) | |
| if err != nil { | |
| return output.ErrWithHint(output.ExitAPI, "api_error", | |
| fmt.Sprintf("Failed to cancel scheduled send for message %s", messageID), | |
| "Ensure the message ID is correct and the email has not already been sent.", | |
| ) | |
| } | |
| _, err := runtime.CallAPI("POST", path, nil, nil) | |
| if err != nil { | |
| return output.ErrWithHint(output.ExitAPI, "api_error", | |
| fmt.Sprintf("Failed to cancel scheduled send for message %s: %v", messageID, err), | |
| "Ensure the message ID is correct and the email has not already been sent.", | |
| ) | |
| } |
🚀 PR Preview Install Guide🧰 CLI updatenpm i -g https://pkg.pr.new/larksuite/cli/@larksuite/cli@150b713497bcb5cc5ae4f9ff024806356a071fa6🧩 Skill updatenpx skills add larksuite/cli#harness/01KP342QM8XZZT86SCJJGGK50Y -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: 2
🧹 Nitpick comments (1)
shortcuts/mail/mail_cancel_scheduled_send.go (1)
33-56: ReusemailboxPathso dry-run and execute can’t drift.These two blocks duplicate the mailbox defaulting and escaped URL template. The package already centralizes segment escaping in
mailboxPath(shortcuts/mail/helpers.go), so a small helper here would keep the dry-run path and the real API call aligned.♻️ Consolidate the path construction
import ( "context" "fmt" - "net/url" "github.com/larksuite/cli/internal/output" "github.com/larksuite/cli/shortcuts/common" ) +func cancelScheduledSendPath(userMailboxID, messageID string) string { + if userMailboxID == "" { + userMailboxID = "me" + } + return mailboxPath(userMailboxID, "messages", messageID, "cancel_scheduled_send") +} + var MailCancelScheduledSend = common.Shortcut{ ... DryRun: func(ctx context.Context, runtime *common.RuntimeContext) *common.DryRunAPI { messageID := runtime.Str("message-id") - userMailboxID := runtime.Str("user-mailbox-id") - if userMailboxID == "" { - userMailboxID = "me" - } return common.NewDryRunAPI(). Desc("Cancel scheduled send — message will be restored as draft"). - POST(fmt.Sprintf("/open-apis/mail/v1/user_mailboxes/%s/messages/%s/cancel_scheduled_send", - url.PathEscape(userMailboxID), - url.PathEscape(messageID), - )) + POST(cancelScheduledSendPath(runtime.Str("user-mailbox-id"), messageID)) }, Execute: func(ctx context.Context, runtime *common.RuntimeContext) error { messageID := runtime.Str("message-id") userMailboxID := runtime.Str("user-mailbox-id") - if userMailboxID == "" { - userMailboxID = "me" - } - - path := fmt.Sprintf("/open-apis/mail/v1/user_mailboxes/%s/messages/%s/cancel_scheduled_send", - url.PathEscape(userMailboxID), - url.PathEscape(messageID), - ) + path := cancelScheduledSendPath(userMailboxID, messageID)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@shortcuts/mail/mail_cancel_scheduled_send.go` around lines 33 - 56, DryRun and Execute duplicate mailbox defaulting and escaped URL construction; replace the duplicated fmt.Sprintf/url.PathEscape logic by calling the shared helper mailboxPath to build the endpoint so both blocks stay in sync. In both DryRun and Execute (functions named DryRun and Execute), compute userMailboxID as before, then call mailboxPath(userMailboxID, "messages", messageID, "cancel_scheduled_send") (or the existing mailboxPath signature used elsewhere) and use its return value for the DryRun POST path and the Execute path variable.
🤖 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/mail_cancel_scheduled_send.go`:
- Around line 72-74: The current fmt.Fprintf call that writes the tip to
runtime.IO().ErrOut uses the wrong flag (--id) and omits mailbox context; update
the fmt.Fprintf invocation (the one that calls sanitizeForTerminal(messageID))
to use --draft-id and include the mailbox selector by passing
sanitizeForTerminal(userMailboxID) and adding the --mailbox (or --from) flag so
the hint reads like: use lark-cli mail +draft-edit --draft-id <messageID>
--mailbox <userMailboxID>.
In `@shortcuts/mail/scheduled_send_test.go`:
- Around line 36-64: Update the two tests
(TestParseAndValidateSendTime_ValidWithTimezone and
TestParseAndValidateSendTime_WithoutTimezoneDefaultsToUTC) to assert the
returned instant rather than just string shape: call
parseAndValidateSendTime(timeStr), parse the returned result with
time.Parse(time.RFC3339, result) into a time.Time (resultTime) and compare it to
the expected instant derived from your original future variable (for the
timezone test compare instants accounting for the FixedZone offset, e.g.
expected := future.In(loc).UTC(); for the no-timezone test expect UTC), using
time equality or a small-tolerance Unix timestamp comparison to avoid flakiness;
additionally assert that the no-timezone case has UTC offset (+00:00) by
checking resultTime.UTC() equals resultTime or by verifying
resultTime.Location() / Offset corresponds to UTC.
---
Nitpick comments:
In `@shortcuts/mail/mail_cancel_scheduled_send.go`:
- Around line 33-56: DryRun and Execute duplicate mailbox defaulting and escaped
URL construction; replace the duplicated fmt.Sprintf/url.PathEscape logic by
calling the shared helper mailboxPath to build the endpoint so both blocks stay
in sync. In both DryRun and Execute (functions named DryRun and Execute),
compute userMailboxID as before, then call mailboxPath(userMailboxID,
"messages", messageID, "cancel_scheduled_send") (or the existing mailboxPath
signature used elsewhere) and use its return value for the DryRun POST path and
the Execute path variable.
🪄 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: 628b8b0a-bcd3-4a6b-abf9-ead5eaf5aa8f
📒 Files selected for processing (10)
shortcuts/mail/draft/service.goshortcuts/mail/mail_cancel_scheduled_send.goshortcuts/mail/mail_cancel_scheduled_send_test.goshortcuts/mail/mail_forward.goshortcuts/mail/mail_reply.goshortcuts/mail/mail_reply_all.goshortcuts/mail/mail_send.goshortcuts/mail/scheduled_send.goshortcuts/mail/scheduled_send_test.goshortcuts/mail/shortcuts.go
| fmt.Fprintf(runtime.IO().ErrOut, | ||
| "tip: the message has been restored as a draft. Use lark-cli mail +draft-edit --id %s to edit.\n", | ||
| sanitizeForTerminal(messageID)) |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
draft_edit_file="$(rg -n --type=go 'Command:\s*"\+draft-edit"' | cut -d: -f1 | head -n1)"
if [ -z "${draft_edit_file}" ]; then
echo "Could not find the +draft-edit shortcut definition" >&2
exit 1
fi
echo "=== +draft-edit shortcut definition: ${draft_edit_file} ==="
sed -n '1,240p' "${draft_edit_file}"
echo
echo "=== mailbox-related flags/path usage in ${draft_edit_file} ==="
rg -n 'Name:\s*"user-mailbox-id"|mailboxPath\(|runtime\.Str\("user-mailbox-id"\)' "${draft_edit_file}"Repository: larksuite/cli
Length of output: 12017
🏁 Script executed:
cat -n shortcuts/mail/mail_cancel_scheduled_send.go | sed -n '50,90p'Repository: larksuite/cli
Length of output: 1093
🏁 Script executed:
rg -n "messageID|mailboxID|resolveComposeMailboxID" shortcuts/mail/mail_cancel_scheduled_send.go | head -20Repository: larksuite/cli
Length of output: 368
🏁 Script executed:
# Search for similar hint patterns in mail shortcuts that might include mailbox context
rg -n "Use lark-cli mail" shortcuts/mail/ -A 1 -B 1Repository: larksuite/cli
Length of output: 372
🏁 Script executed:
cat -n shortcuts/mail/mail_cancel_scheduled_send.go | sed -n '35,65p'Repository: larksuite/cli
Length of output: 1335
🏁 Script executed:
rg -n "MailCancelScheduledSend\s*=" shortcuts/mail/mail_cancel_scheduled_send.go -A 30 | head -50Repository: larksuite/cli
Length of output: 1378
🏁 Script executed:
rg -n 'Name:\s*"draft-id"|Name:\s*"mailbox"|Name:\s*"from"' shortcuts/mail/mail_draft_edit.go | head -10Repository: larksuite/cli
Length of output: 548
Include mailbox context in the draft-edit hint and use the correct flag name.
Line 73 has two issues. The hint uses --id, but the correct flag is --draft-id. More critically, the hint omits the mailbox selector. When a user cancels scheduled send from a custom mailbox (e.g., --user-mailbox-id custom@example.com), the restored draft is placed in that mailbox. The hint should include the mailbox context so the follow-up command targets the correct mailbox; otherwise, +draft-edit defaults to me and the user won't find the restored draft.
The hint should include the mailbox context (captured from userMailboxID) using --mailbox or --from:
Suggested fix
fmt.Fprintf(runtime.IO().ErrOut,
"tip: the message has been restored as a draft. Use lark-cli mail +draft-edit --draft-id %s --mailbox %s to edit.\n",
sanitizeForTerminal(messageID), sanitizeForTerminal(userMailboxID))🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@shortcuts/mail/mail_cancel_scheduled_send.go` around lines 72 - 74, The
current fmt.Fprintf call that writes the tip to runtime.IO().ErrOut uses the
wrong flag (--id) and omits mailbox context; update the fmt.Fprintf invocation
(the one that calls sanitizeForTerminal(messageID)) to use --draft-id and
include the mailbox selector by passing sanitizeForTerminal(userMailboxID) and
adding the --mailbox (or --from) flag so the hint reads like: use lark-cli mail
+draft-edit --draft-id <messageID> --mailbox <userMailboxID>.
…endTime
The backend API expects send_time as a Unix seconds timestamp (i64),
not an RFC 3339 string. Update parseAndValidateSendTime to return
fmt.Sprintf("%d", t.Unix()) and adjust unit tests accordingly.
sprint: S1
1 participant
Generated by the harness-coding skill.
Sprints
Source specs
This MR was created autonomously. Quality gates were enforced by the repo's own pre-commit hooks.
Summary by CodeRabbit
New Features
Behavior / Validation
Tests