Skip to content

mail: support scheduled send shortcuts#511

Closed
Yi-Jiao-Chan wants to merge 1 commit intolarksuite:mainfrom
Yi-Jiao-Chan:harness/js81547mgrrawr0f7wvtks7g2h998
Closed

mail: support scheduled send shortcuts#511
Yi-Jiao-Chan wants to merge 1 commit intolarksuite:mainfrom
Yi-Jiao-Chan:harness/js81547mgrrawr0f7wvtks7g2h998

Conversation

@Yi-Jiao-Chan
Copy link
Copy Markdown

@Yi-Jiao-Chan Yi-Jiao-Chan commented Apr 16, 2026

Generated by the harness-coding skill.

  • Task ID: JS81547MGRRAWR0F7WVTKS7G2H998-3
  • Branch: harness/js81547mgrrawr0f7wvtks7g2h998
  • Target: main

Sprints

ID Title Status Commit
S1 Add scheduled send mail shortcuts passed f09f4dd

Source specs

  • /tmp/harness-agent/coding-dev/JS81547MGRRAWR0F7WVTKS7G2H998/JS81547MGRRAWR0F7WVTKS7G2H998-3/input/tech-design.md

This MR was created autonomously. Quality gates were enforced by the repo's own pre-commit hooks.

Summary by CodeRabbit

Release Notes

  • New Features

    • Added scheduled email sending capability via --send-time flag for reply, reply-all, send, and forward commands
    • Added ability to cancel scheduled sends and restore messages as drafts with new command
  • Documentation

    • Updated guides with scheduled sending usage and RFC3339 timestamp examples
    • Added reference documentation for scheduled send cancellation command

Add send-time validation and request wiring for mail send/reply/forward shortcuts, add cancel-scheduled-send, and sync mail docs/tests.

sprint: S1
@github-actions github-actions Bot added domain/mail PR touches the mail domain size/L Large or sensitive change across domains or core paths labels Apr 16, 2026
@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented Apr 16, 2026

📝 Walkthrough

Walkthrough

This pull request adds scheduled email sending functionality to the mail service. It introduces time-aware send methods (SendWithTime, CancelScheduledSend) in the draft service, adds --send-time flag support to five mail shortcuts (Send, Reply, ReplyAll, Forward), includes time validation and formatting utilities, creates a new cancel-scheduled-send shortcut, and updates related documentation.

Changes

Cohort / File(s) Summary
Draft Service API
shortcuts/mail/draft/service.go
Added SendWithTime() delegating from updated Send(), and new CancelScheduledSend() to cancel scheduled sends and restore drafts via dedicated endpoint.
Helper Utilities
shortcuts/mail/helpers.go, shortcuts/mail/send_time.go
Updated buildSendResult() signature to conditionally include scheduled_send_time and human-readable time fields; added parseAndValidateSendTime() and formatScheduledTimeHuman() for RFC3339 validation and user-facing time formatting.
Mail Shortcuts
shortcuts/mail/mail_send.go, shortcuts/mail/mail_reply.go, shortcuts/mail/mail_reply_all.go, shortcuts/mail/mail_forward.go
Extended each shortcut with --send-time flag and updated DryRun/Validate/Execute flows to parse, validate, and pass scheduled send times to SendWithTime() instead of Send().
Cancel Scheduled Send Shortcut
shortcuts/mail/mail_cancel_scheduled_send.go, shortcuts/mail/shortcuts.go
Added new exported shortcut MailCancelScheduledSend with required --message-id flag and optional --mailbox flag; registered in shortcut registry.
Test Coverage
shortcuts/mail/mail_scheduled_send_test.go, shortcuts/mail/send_time_test.go
Added test for time validation (RFC3339 parsing, 5-minute minimum lead time), test for cancel shortcut execution, and test verifying cancel shortcut is registered.
Documentation
skills/lark-mail/SKILL.md, skills/lark-mail/references/lark-mail-*.md
Updated skill overview and created/extended reference docs for Send, Reply, ReplyAll, Forward, and new CancelScheduledSend shortcut to document --send-time flag and cancel workflow.

Sequence Diagram(s)

sequenceDiagram
    participant User
    participant Shortcut as mail_send<br/>Shortcut
    participant Service as Draft Service<br/>SendWithTime
    participant API as Mail API

    User->>Shortcut: Invoke with --confirm-send<br/>--send-time RFC3339
    Shortcut->>Shortcut: Parse & Validate<br/>send-time
    Shortcut->>Service: SendWithTime(mailboxID,<br/>draftID, sendTime)
    Service->>API: POST send<br/>body: {"send_time": sendTime}
    API-->>Service: Response with<br/>scheduled_send_time
    Service-->>Shortcut: Return sendTime fields
    Shortcut->>Shortcut: buildSendResult with<br/>sendTime parameter
    Shortcut-->>User: Output with scheduled_send_time<br/>& scheduled_send_time_human
Loading

Estimated Code Review Effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly Related PRs

Suggested Labels

domain/mail, size/M

Suggested Reviewers

  • chanthuang

Poem

🐰 A rabbit hops through email dreams,
With timed sends and cancel schemes,
Five shortcuts dance, one shortcut born,
To schedule sends from night to morn!
RFC3339 in the morning light,
Emails scheduled just right!

🚥 Pre-merge checks | ✅ 1 | ❌ 2

❌ Failed checks (2 warnings)

Check name Status Explanation Resolution
Description check ⚠️ Warning The PR description is missing critical sections required by the template including Summary, Changes list, and Test Plan with verification checkboxes. Add Summary, detailed Changes list, and Test Plan sections following the repository template structure before merging.
Docstring Coverage ⚠️ Warning Docstring coverage is 46.15% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (1 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly and concisely summarizes the main change: adding support for scheduled send functionality across mail shortcuts.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 8

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
skills/lark-mail/references/lark-mail-forward.md (1)

18-25: ⚠️ Potential issue | 🟠 Major

The scheduled-send docs now contradict the required draft-review workflow.

Step 1 says “without --confirm-send”, Line 24 then tells users to add --confirm-send --send-time, and Line 33 still says direct --confirm-send is forbidden. That guidance also points people at the wrong flow: scheduling should happen by sending the saved draft with send_time, not by skipping the draft-confirmation step. Also, Line 55’s 2026-04-14... example is already invalid as of April 16, 2026.

Also applies to: 28-33, 53-55, 75-75

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@skills/lark-mail/references/lark-mail-forward.md` around lines 18 - 25, The
docs contradict the required draft-review workflow: update Step 1 to explicitly
say create a forward draft without --confirm-send (do NOT suggest adding
--confirm-send there), and change the scheduling guidance to instruct users to
schedule by sending the saved draft using the send command with --send-time
(i.e., save draft → send draft with --send-time), remove or correct any lines
that tell users to use --confirm-send directly to schedule (references to
"--confirm-send --send-time" and the note forbidding direct --confirm-send
should be aligned to the draft→send flow), and update the example RFC3339
timestamp (the 2026-04-14 example) to a valid future date; touch the Step 1
text, the paragraph that mentions "--confirm-send --send-time", the sentence
forbidding direct --confirm-send, and the timestamp example.
🧹 Nitpick comments (1)
shortcuts/mail/mail_send.go (1)

169-176: Consider edge case: --send-time without --confirm-send.

When --send-time is provided without --confirm-send, the tip suggests re-running with both flags. However, the --send-time value is included verbatim in the tip message. If the user follows this tip later, the original time may have passed or be less than 5 minutes away.

This is acceptable behavior since the user would receive a clear validation error, but consider whether a more dynamic tip (e.g., suggesting they provide a new future time) would improve UX.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@shortcuts/mail/mail_send.go` around lines 169 - 176, The tip currently embeds
the original sendTime verbatim when --send-time is provided without
--confirm-send; change the tip generation in mail_send.go (the sendTime/tip
block that calls runtime.Out) so it does not blindly echo an old sendTime.
Instead, when strings.TrimSpace(sendTime) != "" and confirmSend is false, set
tip to recommend rerunning with --confirm-send and a new future --send-time (or
explicitly state "provide a new future time"), or optionally validate sendTime
against time.Now() and only echo it if it's at least e.g. 5 minutes in the
future; then pass that revised tip into runtime.Out with "draft_id" as before.
🤖 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 49-56: The merge currently seeds out with message_id and
mailbox_id then copies resp into out which can overwrite those IDs; change the
merge order so you first copy all keys from resp into out (or initialize out =
resp copy) and only after merging explicitly set out["message_id"] = messageID,
out["mailbox_id"] = mailboxID and out["status"] = "canceled" to ensure the
shortcut-owned IDs (from variables messageID, mailboxID) and status override any
values returned by draftpkg.CancelScheduledSend/resp.

In `@shortcuts/mail/mail_forward.go`:
- Around line 249-256: Move the send-time parsing/validation to the start of
Execute: call parseAndValidateSendTime(sendTime) once (when
strings.TrimSpace(sendTime) != "") and store the normalized value in a variable
(e.g., validatedSendTime) before any draft creation, then pass that same
validatedSendTime into CreateWithRaw and later into draftpkg.SendWithTime;
remove any later duplicate parse/validate calls so a failing validation occurs
before persisting the draft (referencing parseAndValidateSendTime,
validatedSendTime, CreateWithRaw, draftpkg.SendWithTime, and Execute).
- Around line 237-241: The tip for scheduled sends incorrectly tells users to
rerun +forward; instead update the branch that builds tip (variables: tip,
mailboxID, draftID, sendTime in mail_forward.go) so when sendTime is non-empty
it prints a command that calls user_mailbox.drafts send against the saved draft
including the send_time in the --params JSON (e.g. use user_mailbox_id, draft_id
and send_time fields) rather than instructing to rerun +forward; keep the same
overall format as the existing instant-send hint but include send_time.

In `@shortcuts/mail/mail_reply_all.go`:
- Around line 215-218: The tip message (variable tip) incorrectly tells the user
to rerun the command to create and schedule a new draft when sendTime is set;
instead update the branch that checks sendTime to instruct the user to call the
existing drafts send API on the saved draft (use mailboxID and draftID) with
send_time set to sendTime so the command targets the returned draft_id (i.e.,
construct the tip using mailboxID, draftID and sendTime rather than suggesting
rerunning +reply-all); modify the code around tip, sendTime, mailboxID and
draftID in mail_reply_all.go accordingly.
- Around line 226-233: Normalize and validate sendTime before any network call
that creates the draft (i.e., before calling CreateWithRaw) by calling
parseAndValidateSendTime(sendTime) once, storing the result in a variable
(validatedSendTime), and then reuse that same validatedSendTime when calling
draftpkg.SendWithTime(runtime, mailboxID, draftID, validatedSendTime); remove
the second parsing/validation after draft creation so CreateWithRaw and
SendWithTime operate on the same pre-validated value.

In `@shortcuts/mail/mail_reply.go`:
- Around line 200-204: The current tip shown when !confirmSend instructs the
user to rerun +reply, which creates a new draft; instead update the message to
show how to schedule the just-created draft directly using the existing send
endpoint (include mailboxID and draftID and pass send_time if sendTime is set).
Modify the logic that sets tip in mail_reply.go (variables confirmSend, tip,
mailboxID, draftID, sendTime) so the default tip is like: use lark-cli mail
user_mailbox.drafts send --params
'{"user_mailbox_id":"<mailboxID>","draft_id":"<draftID>","send_time":"<sendTime>"}'
when sendTime is present, otherwise omit send_time, ensuring the message
references the draft_id produced rather than suggesting a rerun of +reply.
- Around line 212-219: Move parsing/validation of the --send-time flag out of
the late send path and do it once during Execute so the normalized time is
reused for tip generation and when calling draftpkg.SendWithTime; specifically,
call parseAndValidateSendTime(sendTime) in Execute (store result in a
validatedSendTime variable), pass that normalized string into any tip-generation
functions, and supply it to draftpkg.SendWithTime(runtime, mailboxID, draftID,
validatedSendTime); remove the second validation block around
draftpkg.SendWithTime so you don't create a draft and later fail revalidating.

In `@shortcuts/mail/send_time.go`:
- Around line 28-30: Replace the hardcoded past example timestamp in the RFC3339
validation error for the --send-time flag with a non-expiring example (e.g., a
placeholder like 2026-12-31T09:00:00+00:00 or an explicit
"YYYY-MM-DDTHH:MM:SS±HH:MM") so the hint never points to a past date; update the
message produced by the output.ErrValidation call in send_time.go that currently
contains "--send-time must be RFC3339, for example 2026-04-14T09:00:00+08:00..."
to use the new far-future or placeholder example while keeping the rest of the
text unchanged.

---

Outside diff comments:
In `@skills/lark-mail/references/lark-mail-forward.md`:
- Around line 18-25: The docs contradict the required draft-review workflow:
update Step 1 to explicitly say create a forward draft without --confirm-send
(do NOT suggest adding --confirm-send there), and change the scheduling guidance
to instruct users to schedule by sending the saved draft using the send command
with --send-time (i.e., save draft → send draft with --send-time), remove or
correct any lines that tell users to use --confirm-send directly to schedule
(references to "--confirm-send --send-time" and the note forbidding direct
--confirm-send should be aligned to the draft→send flow), and update the example
RFC3339 timestamp (the 2026-04-14 example) to a valid future date; touch the
Step 1 text, the paragraph that mentions "--confirm-send --send-time", the
sentence forbidding direct --confirm-send, and the timestamp example.

---

Nitpick comments:
In `@shortcuts/mail/mail_send.go`:
- Around line 169-176: The tip currently embeds the original sendTime verbatim
when --send-time is provided without --confirm-send; change the tip generation
in mail_send.go (the sendTime/tip block that calls runtime.Out) so it does not
blindly echo an old sendTime. Instead, when strings.TrimSpace(sendTime) != ""
and confirmSend is false, set tip to recommend rerunning with --confirm-send and
a new future --send-time (or explicitly state "provide a new future time"), or
optionally validate sendTime against time.Now() and only echo it if it's at
least e.g. 5 minutes in the future; then pass that revised tip into runtime.Out
with "draft_id" as before.
🪄 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: f9f678a4-7c53-4a36-9695-34379a0b74cc

📥 Commits

Reviewing files that changed from the base of the PR and between 1608f95 and f09f4dd.

📒 Files selected for processing (17)
  • shortcuts/mail/draft/service.go
  • shortcuts/mail/helpers.go
  • shortcuts/mail/mail_cancel_scheduled_send.go
  • shortcuts/mail/mail_forward.go
  • shortcuts/mail/mail_reply.go
  • shortcuts/mail/mail_reply_all.go
  • shortcuts/mail/mail_scheduled_send_test.go
  • shortcuts/mail/mail_send.go
  • shortcuts/mail/send_time.go
  • shortcuts/mail/send_time_test.go
  • shortcuts/mail/shortcuts.go
  • skills/lark-mail/SKILL.md
  • skills/lark-mail/references/lark-mail-cancel-scheduled-send.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-send.md

Comment on lines +49 to +56
out := map[string]interface{}{
"message_id": messageID,
"mailbox_id": mailboxID,
}
for k, v := range resp {
out[k] = v
}
out["status"] = "canceled"
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Preserve the shortcut-owned IDs after merging the API payload.

draftpkg.CancelScheduledSend(...) returns the raw response map, so this loop can overwrite the message_id / mailbox_id you just seeded if the API ever returns those keys. That makes the CLI output contract unstable for callers. Merge resp first, then stamp message_id, mailbox_id, and status last.

Suggested fix
 		out := map[string]interface{}{
-			"message_id": messageID,
-			"mailbox_id": mailboxID,
 		}
 		for k, v := range resp {
 			out[k] = v
 		}
+		out["message_id"] = messageID
+		out["mailbox_id"] = mailboxID
 		out["status"] = "canceled"
 		runtime.Out(out, nil)
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
out := map[string]interface{}{
"message_id": messageID,
"mailbox_id": mailboxID,
}
for k, v := range resp {
out[k] = v
}
out["status"] = "canceled"
out := map[string]interface{}{}
for k, v := range resp {
out[k] = v
}
out["message_id"] = messageID
out["mailbox_id"] = mailboxID
out["status"] = "canceled"
🤖 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 49 - 56, The merge
currently seeds out with message_id and mailbox_id then copies resp into out
which can overwrite those IDs; change the merge order so you first copy all keys
from resp into out (or initialize out = resp copy) and only after merging
explicitly set out["message_id"] = messageID, out["mailbox_id"] = mailboxID and
out["status"] = "canceled" to ensure the shortcut-owned IDs (from variables
messageID, mailboxID) and status override any values returned by
draftpkg.CancelScheduledSend/resp.

Comment on lines 237 to +241
if !confirmSend {
tip := fmt.Sprintf(`draft saved. To send: lark-cli mail user_mailbox.drafts send --params '{"user_mailbox_id":"%s","draft_id":"%s"}'`, mailboxID, draftID)
if strings.TrimSpace(sendTime) != "" {
tip = fmt.Sprintf("draft saved. To schedule send, rerun with --confirm-send and --send-time %q", sendTime)
}
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Don’t tell users to rerun +forward to schedule this message.

That path creates a fresh draft instead of scheduling the one you just saved, so the returned draft_id becomes a dead end. The follow-up instruction should call user_mailbox.drafts send with send_time against the saved draft.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@shortcuts/mail/mail_forward.go` around lines 237 - 241, The tip for scheduled
sends incorrectly tells users to rerun +forward; instead update the branch that
builds tip (variables: tip, mailboxID, draftID, sendTime in mail_forward.go) so
when sendTime is non-empty it prints a command that calls user_mailbox.drafts
send against the saved draft including the send_time in the --params JSON (e.g.
use user_mailbox_id, draft_id and send_time fields) rather than instructing to
rerun +forward; keep the same overall format as the existing instant-send hint
but include send_time.

Comment on lines +249 to +256
var validatedSendTime string
if strings.TrimSpace(sendTime) != "" {
validatedSendTime, err = parseAndValidateSendTime(sendTime)
if err != nil {
return err
}
}
resData, err := draftpkg.SendWithTime(runtime, mailboxID, draftID, validatedSendTime)
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Move scheduled-send validation ahead of draft creation.

Because the second validation happens after CreateWithRaw, a request close to the 5-minute floor can fail only after the draft is already persisted. Parse once near the top of Execute and reuse the normalized value all the way through SendWithTime.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@shortcuts/mail/mail_forward.go` around lines 249 - 256, Move the send-time
parsing/validation to the start of Execute: call
parseAndValidateSendTime(sendTime) once (when strings.TrimSpace(sendTime) != "")
and store the normalized value in a variable (e.g., validatedSendTime) before
any draft creation, then pass that same validatedSendTime into CreateWithRaw and
later into draftpkg.SendWithTime; remove any later duplicate parse/validate
calls so a failing validation occurs before persisting the draft (referencing
parseAndValidateSendTime, validatedSendTime, CreateWithRaw,
draftpkg.SendWithTime, and Execute).

Comment on lines +215 to +218
tip := fmt.Sprintf(`draft saved. To send: lark-cli mail user_mailbox.drafts send --params '{"user_mailbox_id":"%s","draft_id":"%s"}'`, mailboxID, draftID)
if strings.TrimSpace(sendTime) != "" {
tip = fmt.Sprintf("draft saved. To schedule send, rerun with --confirm-send and --send-time %q", sendTime)
}
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Point the user at scheduling the saved draft, not rerunning +reply-all.

Following this tip creates another draft and schedules that one instead of the draft_id you just returned. The existing draft-send API already supports send_time, so the guidance should act on the saved draft directly.

Suggested fix
-			tip := fmt.Sprintf(`draft saved. To send: lark-cli mail user_mailbox.drafts send --params '{"user_mailbox_id":"%s","draft_id":"%s"}'`, mailboxID, draftID)
-			if strings.TrimSpace(sendTime) != "" {
-				tip = fmt.Sprintf("draft saved. To schedule send, rerun with --confirm-send and --send-time %q", sendTime)
-			}
+			tip := fmt.Sprintf(`draft saved. To send: lark-cli mail user_mailbox.drafts send --params '{"user_mailbox_id":"%s","draft_id":"%s"}'`, mailboxID, draftID)
+			if strings.TrimSpace(sendTime) != "" {
+				tip = fmt.Sprintf(
+					`draft saved. To schedule this draft: lark-cli mail user_mailbox.drafts send --params '{"user_mailbox_id":"%s","draft_id":"%s"}' --data '{"send_time":"%s"}'`,
+					mailboxID, draftID, sendTime,
+				)
+			}
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
tip := fmt.Sprintf(`draft saved. To send: lark-cli mail user_mailbox.drafts send --params '{"user_mailbox_id":"%s","draft_id":"%s"}'`, mailboxID, draftID)
if strings.TrimSpace(sendTime) != "" {
tip = fmt.Sprintf("draft saved. To schedule send, rerun with --confirm-send and --send-time %q", sendTime)
}
tip := fmt.Sprintf(`draft saved. To send: lark-cli mail user_mailbox.drafts send --params '{"user_mailbox_id":"%s","draft_id":"%s"}'`, mailboxID, draftID)
if strings.TrimSpace(sendTime) != "" {
tip = fmt.Sprintf(
`draft saved. To schedule this draft: lark-cli mail user_mailbox.drafts send --params '{"user_mailbox_id":"%s","draft_id":"%s"}' --data '{"send_time":"%s"}'`,
mailboxID, draftID, sendTime,
)
}
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@shortcuts/mail/mail_reply_all.go` around lines 215 - 218, The tip message
(variable tip) incorrectly tells the user to rerun the command to create and
schedule a new draft when sendTime is set; instead update the branch that checks
sendTime to instruct the user to call the existing drafts send API on the saved
draft (use mailboxID and draftID) with send_time set to sendTime so the command
targets the returned draft_id (i.e., construct the tip using mailboxID, draftID
and sendTime rather than suggesting rerunning +reply-all); modify the code
around tip, sendTime, mailboxID and draftID in mail_reply_all.go accordingly.

Comment on lines +226 to +233
var validatedSendTime string
if strings.TrimSpace(sendTime) != "" {
validatedSendTime, err = parseAndValidateSendTime(sendTime)
if err != nil {
return err
}
}
resData, err := draftpkg.SendWithTime(runtime, mailboxID, draftID, validatedSendTime)
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Avoid revalidating --send-time after CreateWithRaw.

This second parse can flip a near-boundary request from valid to invalid after the draft already exists, producing a partial-success failure mode. Normalize sendTime before any network work that creates the draft and reuse it here.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@shortcuts/mail/mail_reply_all.go` around lines 226 - 233, Normalize and
validate sendTime before any network call that creates the draft (i.e., before
calling CreateWithRaw) by calling parseAndValidateSendTime(sendTime) once,
storing the result in a variable (validatedSendTime), and then reuse that same
validatedSendTime when calling draftpkg.SendWithTime(runtime, mailboxID,
draftID, validatedSendTime); remove the second parsing/validation after draft
creation so CreateWithRaw and SendWithTime operate on the same pre-validated
value.

Comment on lines 200 to +204
if !confirmSend {
tip := fmt.Sprintf(`draft saved. To send: lark-cli mail user_mailbox.drafts send --params '{"user_mailbox_id":"%s","draft_id":"%s"}'`, mailboxID, draftID)
if strings.TrimSpace(sendTime) != "" {
tip = fmt.Sprintf("draft saved. To schedule send, rerun with --confirm-send and --send-time %q", sendTime)
}
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Schedule the saved draft instead of telling users to rerun +reply.

This tip points to a second +reply --confirm-send --send-time ... invocation, which creates a new draft and leaves the just-created one behind. Since user_mailbox.drafts send already accepts send_time, the next-step guidance should schedule draft_id directly.

Suggested fix
-			tip := fmt.Sprintf(`draft saved. To send: lark-cli mail user_mailbox.drafts send --params '{"user_mailbox_id":"%s","draft_id":"%s"}'`, mailboxID, draftID)
-			if strings.TrimSpace(sendTime) != "" {
-				tip = fmt.Sprintf("draft saved. To schedule send, rerun with --confirm-send and --send-time %q", sendTime)
-			}
+			tip := fmt.Sprintf(`draft saved. To send: lark-cli mail user_mailbox.drafts send --params '{"user_mailbox_id":"%s","draft_id":"%s"}'`, mailboxID, draftID)
+			if strings.TrimSpace(sendTime) != "" {
+				tip = fmt.Sprintf(
+					`draft saved. To schedule this draft: lark-cli mail user_mailbox.drafts send --params '{"user_mailbox_id":"%s","draft_id":"%s"}' --data '{"send_time":"%s"}'`,
+					mailboxID, draftID, sendTime,
+				)
+			}
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@shortcuts/mail/mail_reply.go` around lines 200 - 204, The current tip shown
when !confirmSend instructs the user to rerun +reply, which creates a new draft;
instead update the message to show how to schedule the just-created draft
directly using the existing send endpoint (include mailboxID and draftID and
pass send_time if sendTime is set). Modify the logic that sets tip in
mail_reply.go (variables confirmSend, tip, mailboxID, draftID, sendTime) so the
default tip is like: use lark-cli mail user_mailbox.drafts send --params
'{"user_mailbox_id":"<mailboxID>","draft_id":"<draftID>","send_time":"<sendTime>"}'
when sendTime is present, otherwise omit send_time, ensuring the message
references the draft_id produced rather than suggesting a rerun of +reply.

Comment on lines +212 to +219
var validatedSendTime string
if strings.TrimSpace(sendTime) != "" {
validatedSendTime, err = parseAndValidateSendTime(sendTime)
if err != nil {
return err
}
}
resData, err := draftpkg.SendWithTime(runtime, mailboxID, draftID, validatedSendTime)
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Parse --send-time before creating the draft.

Revalidating here is too late: near the 5-minute boundary, the command can create the draft and then fail on the second validation pass, leaving an unsent draft behind. Parse once earlier in Execute and reuse the normalized value through tip generation and SendWithTime.

Suggested fix
+		var validatedSendTime string
+		if strings.TrimSpace(sendTime) != "" {
+			validatedSendTime, err = parseAndValidateSendTime(sendTime)
+			if err != nil {
+				return err
+			}
+		}
+
 		draftID, err := draftpkg.CreateWithRaw(runtime, mailboxID, rawEML)
 		if err != nil {
 			return fmt.Errorf("failed to create draft: %w", err)
 		}
@@
-		var validatedSendTime string
-		if strings.TrimSpace(sendTime) != "" {
-			validatedSendTime, err = parseAndValidateSendTime(sendTime)
-			if err != nil {
-				return err
-			}
-		}
 		resData, err := draftpkg.SendWithTime(runtime, mailboxID, draftID, validatedSendTime)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@shortcuts/mail/mail_reply.go` around lines 212 - 219, Move parsing/validation
of the --send-time flag out of the late send path and do it once during Execute
so the normalized time is reused for tip generation and when calling
draftpkg.SendWithTime; specifically, call parseAndValidateSendTime(sendTime) in
Execute (store result in a validatedSendTime variable), pass that normalized
string into any tip-generation functions, and supply it to
draftpkg.SendWithTime(runtime, mailboxID, draftID, validatedSendTime); remove
the second validation block around draftpkg.SendWithTime so you don't create a
draft and later fail revalidating.

Comment on lines +28 to +30
return "", output.ErrValidation(
"--send-time must be RFC3339, for example 2026-04-14T09:00:00+08:00; if you omit the timezone, UTC is assumed",
)
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Use a non-expiring --send-time example.

As of April 16, 2026, the example 2026-04-14T09:00:00+08:00 is already in the past, so following this hint guarantees the next validation error. Please switch to a placeholder or a far-future example.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@shortcuts/mail/send_time.go` around lines 28 - 30, Replace the hardcoded past
example timestamp in the RFC3339 validation error for the --send-time flag with
a non-expiring example (e.g., a placeholder like 2026-12-31T09:00:00+00:00 or an
explicit "YYYY-MM-DDTHH:MM:SS±HH:MM") so the hint never points to a past date;
update the message produced by the output.ErrValidation call in send_time.go
that currently contains "--send-time must be RFC3339, for example
2026-04-14T09:00:00+08:00..." to use the new far-future or placeholder example
while keeping the rest of the text unchanged.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

domain/mail PR touches the mail domain size/L Large or sensitive change across domains or core paths

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants