feat: add scheduled send support for all send shortcuts and cancel-scheduled-send#468
Closed
Yi-Jiao-Chan wants to merge 1 commit intolarksuite:mainfrom
Closed
Conversation
…heduled-send Add --send-time flag to +send, +reply, +reply-all, and +forward shortcuts to support scheduling emails for future delivery via RFC 3339 timestamps. Add +cancel-scheduled-send shortcut to cancel scheduled sends. Include time parsing/validation utilities and comprehensive tests. sprint: S2
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
There was a problem hiding this comment.
Your free trial has ended. If you'd like to continue receiving code reviews, you can add a payment method here.
github-actions
Bot
added
domain/mail
📝 WalkthroughWalkthroughThis PR adds scheduled email sending capability to mail shortcuts. It introduces a Changes
Sequence DiagramsequenceDiagram
participant CLI as CLI
participant Validate as Validate<br/>(Send-Time)
participant DryRun as DryRun
participant Execute as Execute
participant Draft as Draft<br/>Service
participant API as Mail API
CLI->>Validate: --send-time flag provided
Validate->>Validate: parseAndValidateSendTime()<br/>(RFC3339, 5min min lead)
alt Invalid or Too Soon
Validate-->>CLI: ErrValidation
else Valid
Validate-->>CLI: Validated time ✓
end
CLI->>DryRun: Preview operation
alt --confirm-send + --send-time
DryRun-->>CLI: "schedule send"<br/>(includes send_time in request)
else No --send-time
DryRun-->>CLI: "send" or "save draft"
end
CLI->>Execute: Confirm send operation
alt --confirm-send + --send-time
Execute->>Draft: SendWithBody()<br/>(send_time in payload)
Draft->>API: POST /drafts/{id}/send<br/>with {send_time}
API-->>Draft: {message_id, status}
Draft-->>Execute: message_id
Execute-->>CLI: status: "scheduled"<br/>send_time, message_id<br/>+cancel-scheduled-send tip
else --confirm-send only
Execute->>Draft: Send()
Draft->>API: POST /drafts/{id}/send
API-->>Draft: {message_id, status}
Draft-->>Execute: message_id
Execute-->>CLI: status: "sent"
else No --confirm-send
Execute-->>CLI: Draft saved<br/>(+ scheduling tip if --send-time set)
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (1 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 |
|
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 7
🧹 Nitpick comments (1)
shortcuts/mail/mail_scheduled_send_test.go (1)
46-83: Assert the/sendrequest body in the shared scheduled-send stubs.The confirm-send tests below currently pass as long as
/drafts/.../sendis called. They do not prove that the shortcut actually sent{"send_time": ...}, so a regression back todraftpkg.Send(...)would still go green. Please make these stubs validate the request payload, and ideally assert the echoedsend_time/cancel tip in stdout too.Also applies to: 108-165
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@shortcuts/mail/mail_scheduled_send_test.go` around lines 46 - 83, The current stubScheduledSendEndpoints stub for the POST "/user_mailboxes/me/drafts/draft_sched001/send" only checks that the endpoint is hit; update stubScheduledSendEndpoints to assert the incoming request body contains the expected send payload (e.g., presence and value/format of "send_time" and any cancel tip field) by adding request body validation/assertion to the httpmock.Stub for that URL (and mirror the same validation for the additional stubs around lines 108-165), and update the test harness to capture and assert the echoed send_time/cancel tip printed to stdout so the tests fail if the shortcut doesn't send the JSON payload (reference the function stubScheduledSendEndpoints and the POST stub for "/user_mailboxes/me/drafts/draft_sched001/send").
🤖 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 66-77: The result payload currently only returns "message_id" and
then prints tips that use the wrong command names/flags and the wrong
identifier; update the restore/cancel logic to capture the draft identifier
returned by the restore operation (e.g., draftID variable) and include
"draft_id": draftID in the result map, and change the tip strings to use the
correct commands and flags: "lark-cli mail +draft-edit --draft-id %s" and
"lark-cli mail user_mailbox.drafts send --draft-id %s --confirm-send" (replace
messageID with draftID); reference the result map construction and the
fmt.Fprintf call in mail_cancel_scheduled_send.go and ensure the flag name
matches mail_draft_edit.go's expected --draft-id and the command name matches
helpers.go's user_mailbox.drafts convention.
In `@shortcuts/mail/mail_forward.go`:
- Around line 228-265: The scheduling hint/result currently loses actionable
state and reports the raw --send-time; update the success and draft tips to
include the original forward parameters and use the API-normalized send time
from the response: when creating the draft-only tip (where hintSendDraft is
called) include the original recipients/body/subject (the values used to create
draft) in the suggested re-run CLI command and when scheduling succeeds (after
draftpkg.SendWithBody / draftpkg.Send) read the normalized timestamp from
resData (e.g. resData["send_time"] or equivalent returned field) and emit that
in outData["send_time"] instead of sendTimeStr, and also build the cancel/re-run
tip to include mailboxID, draftID and preserved recipients/body so the command
is actionable; adjust code around parseAndValidateSendTime,
draftpkg.SendWithBody, hintSendDraft and runtime.Out to pass and log these
fields.
In `@shortcuts/mail/mail_reply_all.go`:
- Around line 205-242: The tip/output for scheduled sends must include all
required rerun inputs and reflect the normalized send time; parse sendTimeStr
with parseAndValidateSendTime whenever you build tips (including the save-only
branch that calls hintSendDraft) so you can echo validatedTime (UTC-normalized)
not the raw flag, and when constructing the final outData/tip after
draftpkg.Send or draftpkg.SendWithBody include the actual message id
(resData["message_id"]) and either the draft id or body flag needed to rerun
(e.g. --message-id <message_id> --body '<body>' or --draft-id <draftID>), and
ensure the scheduled tip uses validatedTime; update code paths around
draftpkg.SendWithBody, draftpkg.Send, runtime.Out and hintSendDraft to use
validatedTime and resData values when building the tip.
In `@shortcuts/mail/mail_reply.go`:
- Around line 190-200: The tip shown when confirmSend is false and sendTimeStr
!= "" is incorrect because it tells the user to rerun "+reply" (which requires
--message-id/--body) instead of scheduling the draft that was just created;
update the branch that sets outData["tip"] (the block referencing sendTimeStr,
draftID, mailboxID and runtime.Out) to instead show a runnable command that
schedules the returned draft, e.g. use the existing drafts send command with the
draft_id and user_mailbox_id and send_time and include --confirm-send (format
the tip with mailboxID, draftID and sendTimeStr), and keep calling
hintSendDraft(runtime, mailboxID, draftID) unchanged.
- Around line 48-60: The dry-run currently attaches send_time to the draft
creation body (variable body) which makes the preview inaccurate; instead, do
not include send_time in the draft creation body and, when sendTimeStr != "" and
confirmSend is true, attach send_time to the dry-run POST for the send step (the
POST to mailboxPath(mailboxID, "drafts", "<draft_id>", "send")). Locate the
body/sendTimeStr logic and the api builder (variables body, sendTimeStr, api and
confirmSend in mail_reply.go) and change it so send_time is omitted from the
draft POST and added to the send POST's body (or Body call) for the send step
only. Ensure mailboxPath(..., "drafts") uses the draft-only body and
mailboxPath(..., "drafts", "<draft_id>", "send") carries the send_time when
confirmSend is set.
In `@shortcuts/mail/mail_send.go`:
- Around line 154-164: The tip for scheduling a saved draft is incorrect: update
the message and any hint to tell users to rerun the send command including the
draft_id (and mailbox id) so the saved draft is scheduled rather than creating a
new draft. Specifically, change the formatted tip in the !confirmSend branch
(where sendTimeStr, mailboxID, draftID are used) to show a command including
--draft-id "<draftID>" (and --user_mailbox_id "<mailboxID>") together with
--send-time and --confirm-send, and ensure hintSendDraft(runtime, mailboxID,
draftID) (or its scheduling-aware overload) is invoked/updated so the helper
also references the draft_id when sendTimeStr is present.
- Around line 49-66: The dry-run currently adds send_time to bodyPreview which
is used for POST /drafts, but the real flow sends the schedule in the send
request (draftpkg.SendWithBody -> POST /drafts/{draft_id}/send); modify the
construction so bodyPreview (used with POST mailboxPath(..., "drafts")) does not
include send_time, and when confirmSend is true create a separate sendBody that
includes send_time (or attach send_time to the Body for the POST
mailboxPath(mailboxID, "drafts", "<draft_id>", "send")) and chain that POST with
Body(sendBody) to mirror SendWithBody; update the api variable accordingly
(referencing bodyPreview, sendBody, api, mailboxPath, confirmSend).
---
Nitpick comments:
In `@shortcuts/mail/mail_scheduled_send_test.go`:
- Around line 46-83: The current stubScheduledSendEndpoints stub for the POST
"/user_mailboxes/me/drafts/draft_sched001/send" only checks that the endpoint is
hit; update stubScheduledSendEndpoints to assert the incoming request body
contains the expected send payload (e.g., presence and value/format of
"send_time" and any cancel tip field) by adding request body
validation/assertion to the httpmock.Stub for that URL (and mirror the same
validation for the additional stubs around lines 108-165), and update the test
harness to capture and assert the echoed send_time/cancel tip printed to stdout
so the tests fail if the shortcut doesn't send the JSON payload (reference the
function stubScheduledSendEndpoints and the POST stub for
"/user_mailboxes/me/drafts/draft_sched001/send").
🪄 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: f365aceb-0714-4d48-81f7-d5964eedfe4a
📒 Files selected for processing (11)
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_scheduled_send_test.goshortcuts/mail/mail_send.goshortcuts/mail/scheduled_send.goshortcuts/mail/scheduled_send_test.goshortcuts/mail/shortcuts.go
Comment on lines
+66
to
+77
| result := map[string]interface{}{ | ||
| "message_id": messageID, | ||
| "status": "cancelled", | ||
| "restored_as_draft": true, | ||
| } | ||
|
|
||
| runtime.Out(result, nil) | ||
| fmt.Fprintf(runtime.IO().ErrOut, | ||
| "tip: The scheduled send has been cancelled and the message restored as a draft.\n"+ | ||
| " Use lark-cli mail +draft-edit --id %s to edit\n"+ | ||
| " Use lark-cli mail user-mailbox-drafts send --draft-id %s --confirm-send to send immediately\n", | ||
| messageID, messageID) |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
rg -n -C3 '\+draft-edit|user_mailbox\.drafts send|user-mailbox-drafts send|--draft-id|--id' shortcuts/mailRepository: larksuite/cli
Length of output: 8584
Fix the follow-up commands: use correct command names, flags, and draft identifiers.
After cancel, the tip instructions have three issues:
- Command name mismatch: Uses
user-mailbox-drafts sendinstead ofuser_mailbox.drafts send(which all other draft shortcuts use, seehelpers.go:44) - Flag name error: Suggests
+draft-edit --idinstead of+draft-edit --draft-id(required bymail_draft_edit.go:46,77) - Wrong identifier: Both commands receive
messageIDbut should receive adraft_idreturned by the cancel operation
The result payload (lines 66-72) captures only message_id and does not extract the draft_id that the restore operation should return. Either the API response includes a draft ID that must be captured in the result, or the tips should not suggest commands that cannot be executed with the available data.
🤖 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 66 - 77, The
result payload currently only returns "message_id" and then prints tips that use
the wrong command names/flags and the wrong identifier; update the
restore/cancel logic to capture the draft identifier returned by the restore
operation (e.g., draftID variable) and include "draft_id": draftID in the result
map, and change the tip strings to use the correct commands and flags: "lark-cli
mail +draft-edit --draft-id %s" and "lark-cli mail user_mailbox.drafts send
--draft-id %s --confirm-send" (replace messageID with draftID); reference the
result map construction and the fmt.Fprintf call in
mail_cancel_scheduled_send.go and ensure the flag name matches
mail_draft_edit.go's expected --draft-id and the command name matches
helpers.go's user_mailbox.drafts convention.
Comment on lines
+228
to
+265
| outData := map[string]interface{}{ | ||
| "draft_id": draftID, | ||
| "tip": fmt.Sprintf(`draft saved. To send: lark-cli mail user_mailbox.drafts send --params '{"user_mailbox_id":"%s","draft_id":"%s"}'`, mailboxID, draftID), | ||
| }, nil) | ||
| } | ||
| if sendTimeStr != "" { | ||
| outData["tip"] = fmt.Sprintf( | ||
| `draft saved. To schedule send, add --confirm-send: lark-cli mail +forward --send-time %q --confirm-send`, sendTimeStr) | ||
| } | ||
| runtime.Out(outData, nil) | ||
| hintSendDraft(runtime, mailboxID, draftID) | ||
| return nil | ||
| } | ||
| resData, err := draftpkg.Send(runtime, mailboxID, draftID) | ||
| var resData map[string]interface{} | ||
| if sendTimeStr != "" { | ||
| validatedTime, parseErr := parseAndValidateSendTime(sendTimeStr) | ||
| if parseErr != nil { | ||
| return parseErr | ||
| } | ||
| sendBody := map[string]interface{}{"send_time": validatedTime} | ||
| resData, err = draftpkg.SendWithBody(runtime, mailboxID, draftID, sendBody) | ||
| } else { | ||
| resData, err = draftpkg.Send(runtime, mailboxID, draftID) | ||
| } | ||
| if err != nil { | ||
| return fmt.Errorf("failed to send forward (draft %s created but not sent): %w", draftID, err) | ||
| } | ||
| runtime.Out(map[string]interface{}{ | ||
| outData := map[string]interface{}{ | ||
| "message_id": resData["message_id"], | ||
| "thread_id": resData["thread_id"], | ||
| }, nil) | ||
| } | ||
| if sendTimeStr != "" { | ||
| outData["status"] = "scheduled" | ||
| outData["send_time"] = sendTimeStr | ||
| if msgID, ok := resData["message_id"].(string); ok && msgID != "" { | ||
| outData["tip"] = fmt.Sprintf("To cancel: lark-cli mail +cancel-scheduled-send --message-id %s", msgID) | ||
| } | ||
| } | ||
| runtime.Out(outData, nil) |
There was a problem hiding this comment.
The scheduling hint/result should preserve actionable state.
The draft-only tip here cannot be used to rerun the forward because it drops the original recipients/body context. Also, once scheduling succeeds, stdout should report the normalized timestamp actually sent to the API instead of the raw --send-time input.
🤖 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 228 - 265, The scheduling
hint/result currently loses actionable state and reports the raw --send-time;
update the success and draft tips to include the original forward parameters and
use the API-normalized send time from the response: when creating the draft-only
tip (where hintSendDraft is called) include the original recipients/body/subject
(the values used to create draft) in the suggested re-run CLI command and when
scheduling succeeds (after draftpkg.SendWithBody / draftpkg.Send) read the
normalized timestamp from resData (e.g. resData["send_time"] or equivalent
returned field) and emit that in outData["send_time"] instead of sendTimeStr,
and also build the cancel/re-run tip to include mailboxID, draftID and preserved
recipients/body so the command is actionable; adjust code around
parseAndValidateSendTime, draftpkg.SendWithBody, hintSendDraft and runtime.Out
to pass and log these fields.
Comment on lines
+205
to
+242
| outData := map[string]interface{}{ | ||
| "draft_id": draftID, | ||
| "tip": fmt.Sprintf(`draft saved. To send: lark-cli mail user_mailbox.drafts send --params '{"user_mailbox_id":"%s","draft_id":"%s"}'`, mailboxID, draftID), | ||
| }, nil) | ||
| } | ||
| if sendTimeStr != "" { | ||
| outData["tip"] = fmt.Sprintf( | ||
| `draft saved. To schedule send, add --confirm-send: lark-cli mail +reply-all --send-time %q --confirm-send`, sendTimeStr) | ||
| } | ||
| runtime.Out(outData, nil) | ||
| hintSendDraft(runtime, mailboxID, draftID) | ||
| return nil | ||
| } | ||
| resData, err := draftpkg.Send(runtime, mailboxID, draftID) | ||
| var resData map[string]interface{} | ||
| if sendTimeStr != "" { | ||
| validatedTime, parseErr := parseAndValidateSendTime(sendTimeStr) | ||
| if parseErr != nil { | ||
| return parseErr | ||
| } | ||
| sendBody := map[string]interface{}{"send_time": validatedTime} | ||
| resData, err = draftpkg.SendWithBody(runtime, mailboxID, draftID, sendBody) | ||
| } else { | ||
| resData, err = draftpkg.Send(runtime, mailboxID, draftID) | ||
| } | ||
| if err != nil { | ||
| return fmt.Errorf("failed to send reply-all (draft %s created but not sent): %w", draftID, err) | ||
| } | ||
| runtime.Out(map[string]interface{}{ | ||
| outData := map[string]interface{}{ | ||
| "message_id": resData["message_id"], | ||
| "thread_id": resData["thread_id"], | ||
| }, nil) | ||
| } | ||
| if sendTimeStr != "" { | ||
| outData["status"] = "scheduled" | ||
| outData["send_time"] = sendTimeStr | ||
| if msgID, ok := resData["message_id"].(string); ok && msgID != "" { | ||
| outData["tip"] = fmt.Sprintf("To cancel: lark-cli mail +cancel-scheduled-send --message-id %s", msgID) | ||
| } | ||
| } | ||
| runtime.Out(outData, nil) |
There was a problem hiding this comment.
The new scheduling tip/output drops information the user needs.
When --send-time is set without --confirm-send, the emitted tip cannot be rerun as-is because it omits required inputs like --message-id and --body. And on the scheduled path, you already compute validatedTime for the API call but still echo the raw flag value back to stdout, which is misleading for timezone-less input because that gets normalized to UTC before send.
🤖 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 205 - 242, The tip/output for
scheduled sends must include all required rerun inputs and reflect the
normalized send time; parse sendTimeStr with parseAndValidateSendTime whenever
you build tips (including the save-only branch that calls hintSendDraft) so you
can echo validatedTime (UTC-normalized) not the raw flag, and when constructing
the final outData/tip after draftpkg.Send or draftpkg.SendWithBody include the
actual message id (resData["message_id"]) and either the draft id or body flag
needed to rerun (e.g. --message-id <message_id> --body '<body>' or --draft-id
<draftID>), and ensure the scheduled tip uses validatedTime; update code paths
around draftpkg.SendWithBody, draftpkg.Send, runtime.Out and hintSendDraft to
use validatedTime and resData values when building the tip.
Comment on lines
+48
to
60
| body := map[string]interface{}{"raw": "<base64url-EML>"} | ||
| if sendTimeStr != "" { | ||
| body["send_time"] = sendTimeStr | ||
| } | ||
| api := common.NewDryRunAPI(). | ||
| Desc(desc). | ||
| GET(mailboxPath(mailboxID, "messages", messageId)). | ||
| GET(mailboxPath(mailboxID, "profile")). | ||
| POST(mailboxPath(mailboxID, "drafts")). | ||
| Body(map[string]interface{}{"raw": "<base64url-EML>"}) | ||
| Body(body) | ||
| if confirmSend { | ||
| api = api.POST(mailboxPath(mailboxID, "drafts", "<draft_id>", "send")) | ||
| } |
There was a problem hiding this comment.
Attach send_time to the dry-run /send step, not POST /drafts.
Execute only submits send_time via draftpkg.SendWithBody(...) on /drafts/{draft_id}/send, but this dry-run always shows it on the draft-creation body and never on the send call. That makes the preview inaccurate for both draft-only and --confirm-send flows.
Suggested adjustment
- body := map[string]interface{}{"raw": "<base64url-EML>"}
- if sendTimeStr != "" {
+ body := map[string]interface{}{"raw": "<base64url-EML>"}
+ if confirmSend && sendTimeStr != "" {
body["send_time"] = sendTimeStr
}
api := common.NewDryRunAPI().
Desc(desc).
GET(mailboxPath(mailboxID, "messages", messageId)).
GET(mailboxPath(mailboxID, "profile")).
POST(mailboxPath(mailboxID, "drafts")).
Body(body)
if confirmSend {
api = api.POST(mailboxPath(mailboxID, "drafts", "<draft_id>", "send"))
+ if sendTimeStr != "" {
+ api = api.Body(map[string]interface{}{"send_time": sendTimeStr})
+ }
}📝 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
| body := map[string]interface{}{"raw": "<base64url-EML>"} | |
| if sendTimeStr != "" { | |
| body["send_time"] = sendTimeStr | |
| } | |
| api := common.NewDryRunAPI(). | |
| Desc(desc). | |
| GET(mailboxPath(mailboxID, "messages", messageId)). | |
| GET(mailboxPath(mailboxID, "profile")). | |
| POST(mailboxPath(mailboxID, "drafts")). | |
| Body(map[string]interface{}{"raw": "<base64url-EML>"}) | |
| Body(body) | |
| if confirmSend { | |
| api = api.POST(mailboxPath(mailboxID, "drafts", "<draft_id>", "send")) | |
| } | |
| body := map[string]interface{}{"raw": "<base64url-EML>"} | |
| if confirmSend && sendTimeStr != "" { | |
| body["send_time"] = sendTimeStr | |
| } | |
| api := common.NewDryRunAPI(). | |
| Desc(desc). | |
| GET(mailboxPath(mailboxID, "messages", messageId)). | |
| GET(mailboxPath(mailboxID, "profile")). | |
| POST(mailboxPath(mailboxID, "drafts")). | |
| Body(body) | |
| if confirmSend { | |
| api = api.POST(mailboxPath(mailboxID, "drafts", "<draft_id>", "send")) | |
| if sendTimeStr != "" { | |
| api = api.Body(map[string]interface{}{"send_time": sendTimeStr}) | |
| } | |
| } |
🤖 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 48 - 60, The dry-run currently
attaches send_time to the draft creation body (variable body) which makes the
preview inaccurate; instead, do not include send_time in the draft creation body
and, when sendTimeStr != "" and confirmSend is true, attach send_time to the
dry-run POST for the send step (the POST to mailboxPath(mailboxID, "drafts",
"<draft_id>", "send")). Locate the body/sendTimeStr logic and the api builder
(variables body, sendTimeStr, api and confirmSend in mail_reply.go) and change
it so send_time is omitted from the draft POST and added to the send POST's body
(or Body call) for the send step only. Ensure mailboxPath(..., "drafts") uses
the draft-only body and mailboxPath(..., "drafts", "<draft_id>", "send") carries
the send_time when confirmSend is set.
Comment on lines
190
to
200
| if !confirmSend { | ||
| runtime.Out(map[string]interface{}{ | ||
| outData := map[string]interface{}{ | ||
| "draft_id": draftID, | ||
| "tip": fmt.Sprintf(`draft saved. To send: lark-cli mail user_mailbox.drafts send --params '{"user_mailbox_id":"%s","draft_id":"%s"}'`, mailboxID, draftID), | ||
| }, nil) | ||
| } | ||
| if sendTimeStr != "" { | ||
| outData["tip"] = fmt.Sprintf( | ||
| `draft saved. To schedule send, add --confirm-send: lark-cli mail +reply --send-time %q --confirm-send`, sendTimeStr) | ||
| } | ||
| runtime.Out(outData, nil) | ||
| hintSendDraft(runtime, mailboxID, draftID) |
There was a problem hiding this comment.
The draft-only scheduling tip is not runnable.
Line 197 suggests rerunning +reply with only --send-time and --confirm-send, but +reply still requires --message-id and --body, and rerunning it would create a new draft instead of scheduling the draft_id you just returned.
🤖 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 190 - 200, The tip shown when
confirmSend is false and sendTimeStr != "" is incorrect because it tells the
user to rerun "+reply" (which requires --message-id/--body) instead of
scheduling the draft that was just created; update the branch that sets
outData["tip"] (the block referencing sendTimeStr, draftID, mailboxID and
runtime.Out) to instead show a runnable command that schedules the returned
draft, e.g. use the existing drafts send command with the draft_id and
user_mailbox_id and send_time and include --confirm-send (format the tip with
mailboxID, draftID and sendTimeStr), and keep calling hintSendDraft(runtime,
mailboxID, draftID) unchanged.
Comment on lines
+49
to
66
| bodyPreview := map[string]interface{}{ | ||
| "raw": "<base64url-EML>", | ||
| "_preview": map[string]interface{}{ | ||
| "to": to, | ||
| "subject": subject, | ||
| }, | ||
| } | ||
| if sendTimeStr != "" { | ||
| bodyPreview["send_time"] = sendTimeStr | ||
| } | ||
| api := common.NewDryRunAPI(). | ||
| Desc(desc). | ||
| GET(mailboxPath(mailboxID, "profile")). | ||
| POST(mailboxPath(mailboxID, "drafts")). | ||
| Body(map[string]interface{}{ | ||
| "raw": "<base64url-EML>", | ||
| "_preview": map[string]interface{}{ | ||
| "to": to, | ||
| "subject": subject, | ||
| }, | ||
| }) | ||
| Body(bodyPreview) | ||
| if confirmSend { | ||
| api = api.POST(mailboxPath(mailboxID, "drafts", "<draft_id>", "send")) | ||
| } |
There was a problem hiding this comment.
Dry-run is previewing send_time on the wrong request.
At runtime the schedule is sent through draftpkg.SendWithBody(...) to /drafts/{draft_id}/send, but this preview adds send_time to POST /drafts whenever the flag is present. The dry-run should mirror the actual request sequence.
Suggested adjustment
- bodyPreview := map[string]interface{}{
+ bodyPreview := map[string]interface{}{
"raw": "<base64url-EML>",
"_preview": map[string]interface{}{
"to": to,
"subject": subject,
},
}
- if sendTimeStr != "" {
+ if confirmSend && sendTimeStr != "" {
bodyPreview["send_time"] = sendTimeStr
}
api := common.NewDryRunAPI().
Desc(desc).
GET(mailboxPath(mailboxID, "profile")).
POST(mailboxPath(mailboxID, "drafts")).
Body(bodyPreview)
if confirmSend {
api = api.POST(mailboxPath(mailboxID, "drafts", "<draft_id>", "send"))
+ if sendTimeStr != "" {
+ api = api.Body(map[string]interface{}{"send_time": sendTimeStr})
+ }
}🤖 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 49 - 66, The dry-run currently adds
send_time to bodyPreview which is used for POST /drafts, but the real flow sends
the schedule in the send request (draftpkg.SendWithBody -> POST
/drafts/{draft_id}/send); modify the construction so bodyPreview (used with POST
mailboxPath(..., "drafts")) does not include send_time, and when confirmSend is
true create a separate sendBody that includes send_time (or attach send_time to
the Body for the POST mailboxPath(mailboxID, "drafts", "<draft_id>", "send"))
and chain that POST with Body(sendBody) to mirror SendWithBody; update the api
variable accordingly (referencing bodyPreview, sendBody, api, mailboxPath,
confirmSend).
Comment on lines
154
to
164
| if !confirmSend { | ||
| runtime.Out(map[string]interface{}{ | ||
| outData := map[string]interface{}{ | ||
| "draft_id": draftID, | ||
| "tip": fmt.Sprintf(`draft saved. To send: lark-cli mail user_mailbox.drafts send --params '{"user_mailbox_id":"%s","draft_id":"%s"}'`, mailboxID, draftID), | ||
| }, nil) | ||
| } | ||
| if sendTimeStr != "" { | ||
| outData["tip"] = fmt.Sprintf( | ||
| `draft saved. To schedule send, add --confirm-send: lark-cli mail +send --send-time %q --confirm-send`, sendTimeStr) | ||
| } | ||
| runtime.Out(outData, nil) | ||
| hintSendDraft(runtime, mailboxID, draftID) |
There was a problem hiding this comment.
This “schedule later” tip will not work for the saved draft.
Line 161 tells users to rerun +send with only --send-time and --confirm-send, but that command is missing the required compose flags and would create a new draft instead of scheduling the draft_id returned on Line 156.
🤖 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 154 - 164, The tip for scheduling a
saved draft is incorrect: update the message and any hint to tell users to rerun
the send command including the draft_id (and mailbox id) so the saved draft is
scheduled rather than creating a new draft. Specifically, change the formatted
tip in the !confirmSend branch (where sendTimeStr, mailboxID, draftID are used)
to show a command including --draft-id "<draftID>" (and --user_mailbox_id
"<mailboxID>") together with --send-time and --confirm-send, and ensure
hintSendDraft(runtime, mailboxID, draftID) (or its scheduling-aware overload) is
invoked/updated so the helper also references the draft_id when sendTimeStr is
present.
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
3 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
Source specs
This MR was created autonomously. Quality gates were enforced by the repo's own pre-commit hooks.
Summary by CodeRabbit
Release Notes
New Features
--send-timeflag to schedule email delivery for future dates/times across send, reply, reply-all, and forward commands.Tests