From 94c5b98b654a07be55989a263d102dbb91d46592 Mon Sep 17 00:00:00 2001 From: Malo Bourgon Date: Tue, 23 Jun 2026 11:51:04 -0700 Subject: [PATCH 1/3] refactor(gmail): extract shared compose builders and signature options Split the message-build half of gmail reply/reply-all/forward into pure builders (buildReplyComposeMessage/buildForwardComposeMessage) that take an already-acquired *gmail.Service, plus service-free resolveReplyInputs/resolveForwardInputs that resolve body/note exactly once and validate. The Run methods now orchestrate resolve -> dry-run -> acquire -> build -> finalize, so the acquire gate and finalize step are no longer baked into the builders. Extract composeSignatureOptions (the shared signature flags + signatureRequested/validateSignatureOptions/resolveComposeSignature methods) and embed it into GmailSendCmd and GmailReplyOptions, removing the throwaway GmailSendCmd the reply path constructed just to borrow those methods. Extract a GmailForwardOptions embed from GmailForwardCmd. No behavior change: CLI flags are byte-identical, and validation order, dry-run output, error wrapping, and finalize are unchanged; the existing reply/reply-all/forward/send/signature tests pass with assertions unchanged. This sets up reuse by upcoming drafts reply/reply-all/forward commands, which will share resolve+build and finalize with Drafts.Create under the non-send service gate. Co-Authored-By: Claude Opus 4.8 (1M context) --- internal/cmd/gmail_body_file_newlines_test.go | 6 +- internal/cmd/gmail_forward.go | 124 ++++++++--- internal/cmd/gmail_reply_commands.go | 210 +++++++++++------- internal/cmd/gmail_send.go | 38 ++-- internal/cmd/gmail_send_signature.go | 14 +- internal/cmd/gmail_send_signature_test.go | 47 ++-- 6 files changed, 277 insertions(+), 162 deletions(-) diff --git a/internal/cmd/gmail_body_file_newlines_test.go b/internal/cmd/gmail_body_file_newlines_test.go index 45cdc7bae..b4c00328a 100644 --- a/internal/cmd/gmail_body_file_newlines_test.go +++ b/internal/cmd/gmail_body_file_newlines_test.go @@ -147,8 +147,10 @@ func TestGmailForward_NoteFilePreservesTrailingNewlines(t *testing.T) { cmd := &GmailForwardCmd{ MessageID: "msg1", - To: "x@example.com", - NoteFile: notePath, + GmailForwardOptions: GmailForwardOptions{ + To: "x@example.com", + NoteFile: notePath, + }, } req := runDryRunRequest(t, cmd.Run) assertLen(t, req, "note_len", len(bodyFileWithTrailingNewlines)) diff --git a/internal/cmd/gmail_forward.go b/internal/cmd/gmail_forward.go index 8020e9315..1bf978668 100644 --- a/internal/cmd/gmail_forward.go +++ b/internal/cmd/gmail_forward.go @@ -8,12 +8,18 @@ import ( "strings" "time" + "google.golang.org/api/gmail/v1" + "github.com/openclaw/gogcli/internal/gmailcontent" "github.com/openclaw/gogcli/internal/ui" ) type GmailForwardCmd struct { - MessageID string `arg:"" name:"messageId" help:"Gmail message ID to forward"` + MessageID string `arg:"" name:"messageId" help:"Gmail message ID to forward"` + GmailForwardOptions `embed:""` +} + +type GmailForwardOptions struct { To string `name:"to" help:"Recipients (comma-separated; required)" required:""` Cc string `name:"cc" help:"CC recipients (comma-separated)"` Bcc string `name:"bcc" help:"BCC recipients (comma-separated)"` @@ -23,31 +29,37 @@ type GmailForwardCmd struct { SkipAttachments bool `name:"skip-attachments" help:"Do not include original attachments"` } +// forwardComposeInputs holds the validated, service-free inputs for a forward +// compose. The note is resolved exactly once here because '-' reads stdin, +// which cannot be read twice. +type forwardComposeInputs struct { + messageID string + note string + toRecipients []string +} + +// forwardComposeMessage carries the built forward message plus the metadata the +// caller needs to record results. +type forwardComposeMessage struct { + message *gmail.Message + fromHeader string +} + func (c *GmailForwardCmd) Run(ctx context.Context, flags *RootFlags) error { u := ui.FromContext(ctx) - messageID := normalizeGmailMessageID(c.MessageID) - if messageID == "" { - return usage("required: messageId") - } - - note, err := resolveBodyInput(ctx, c.Note, c.NoteFile) + inputs, err := c.resolveForwardInputs(ctx, c.MessageID) if err != nil { return err } - toRecipients := splitCSV(c.To) - if len(toRecipients) == 0 { - return usage("required: --to") - } - if dryRunErr := dryRunExit(ctx, flags, "gmail.forward", map[string]any{ - "message_id": messageID, - "to": toRecipients, + "message_id": inputs.messageID, + "to": inputs.toRecipients, "cc": splitCSV(c.Cc), "bcc": splitCSV(c.Bcc), "from": strings.TrimSpace(c.From), - "note_len": len(note), + "note_len": len(inputs.note), "skip_attachments": c.SkipAttachments, }); dryRunErr != nil { return dryRunErr @@ -58,15 +70,63 @@ func (c *GmailForwardCmd) Run(ctx context.Context, flags *RootFlags) error { return err } - from, err := resolveComposeSender(ctx, svc, account, c.From) + built, err := c.buildForwardComposeMessage(ctx, svc, account, inputs) if err != nil { return err } + sent, err := svc.Users.Messages.Send("me", built.message).Context(ctx).Do() + if err != nil { + return fmt.Errorf("send forward: %w", err) + } + + return writeGmailMessageResults(ctx, u, []gmailMessageResult{{ + From: built.fromHeader, + MessageID: sent.Id, + ThreadID: sent.ThreadId, + }}) +} + +// resolveForwardInputs normalizes the message ID, resolves the note input, and +// runs all validation that does not require a Gmail service. It reads the note +// exactly once so '-' (stdin) is consumed a single time. +func (c *GmailForwardOptions) resolveForwardInputs(ctx context.Context, messageID string) (forwardComposeInputs, error) { + messageID = normalizeGmailMessageID(messageID) + if messageID == "" { + return forwardComposeInputs{}, usage("required: messageId") + } + + note, err := resolveBodyInput(ctx, c.Note, c.NoteFile) + if err != nil { + return forwardComposeInputs{}, err + } + + toRecipients := splitCSV(c.To) + if len(toRecipients) == 0 { + return forwardComposeInputs{}, usage("required: --to") + } + + return forwardComposeInputs{ + messageID: messageID, + note: note, + toRecipients: toRecipients, + }, nil +} + +// buildForwardComposeMessage assembles the forwarded message from already-validated +// inputs and an already-acquired service. It resolves the sender, fetches the +// original message, and returns the message without sending so the caller +// controls how it is dispatched. +func (c *GmailForwardOptions) buildForwardComposeMessage(ctx context.Context, svc *gmail.Service, account string, inputs forwardComposeInputs) (forwardComposeMessage, error) { + from, err := resolveComposeSender(ctx, svc, account, c.From) + if err != nil { + return forwardComposeMessage{}, err + } + // Fetch the original message in full format (headers + body + attachment metadata). - origMsg, err := svc.Users.Messages.Get("me", messageID).Format(gmailFormatFull).Context(ctx).Do() + origMsg, err := svc.Users.Messages.Get("me", inputs.messageID).Format(gmailFormatFull).Context(ctx).Do() if err != nil { - return fmt.Errorf("fetch original message: %w", err) + return forwardComposeMessage{}, fmt.Errorf("fetch original message: %w", err) } origFrom := headerValue(origMsg.Payload, "From") @@ -84,23 +144,23 @@ func (c *GmailForwardCmd) Run(ctx context.Context, flags *RootFlags) error { // configured/local source as the reply quote and the outgoing Date header. loc, err := mailDateLocation(ctx, stderrWriter(ctx)) if err != nil { - return err + return forwardComposeMessage{}, err } // Build forwarded body (plain text). - fwdPlain := formatForwardedMessage(note, origFrom, origDate, origSubject, origTo, origCc, origPlain, loc) + fwdPlain := formatForwardedMessage(inputs.note, origFrom, origDate, origSubject, origTo, origCc, origPlain, loc) // Build forwarded body (HTML) if original had HTML. var fwdHTML string if origHTML != "" { - fwdHTML = formatForwardedMessageHTML(note, origFrom, origDate, origSubject, origTo, origCc, origHTML, loc) + fwdHTML = formatForwardedMessageHTML(inputs.note, origFrom, origDate, origSubject, origTo, origCc, origHTML, loc) } // Preserve CID-backed inline resources required by the forwarded HTML and, // unless disabled, ordinary attachments. - attachments, err := preserveForwardMessageParts(ctx, svc, messageID, origMsg.Payload, origHTML, !c.SkipAttachments) + attachments, err := preserveForwardMessageParts(ctx, svc, inputs.messageID, origMsg.Payload, origHTML, !c.SkipAttachments) if err != nil { - return fmt.Errorf("preserve forwarded message parts: %w", err) + return forwardComposeMessage{}, fmt.Errorf("preserve forwarded message parts: %w", err) } ccRecipients := splitCSV(c.Cc) @@ -113,24 +173,18 @@ func (c *GmailForwardCmd) Run(ctx context.Context, flags *RootFlags) error { BodyHTML: fwdHTML, Attachments: attachments, }, sendBatch{ - To: toRecipients, + To: inputs.toRecipients, Cc: ccRecipients, Bcc: bccRecipients, }, false) if err != nil { - return fmt.Errorf("build message: %w", err) + return forwardComposeMessage{}, fmt.Errorf("build message: %w", err) } - sent, err := svc.Users.Messages.Send("me", msg).Context(ctx).Do() - if err != nil { - return fmt.Errorf("send forward: %w", err) - } - - return writeGmailMessageResults(ctx, u, []gmailMessageResult{{ - From: from.header, - MessageID: sent.Id, - ThreadID: sent.ThreadId, - }}) + return forwardComposeMessage{ + message: msg, + fromHeader: from.header, + }, nil } type forwardedHeader struct { diff --git a/internal/cmd/gmail_reply_commands.go b/internal/cmd/gmail_reply_commands.go index 47540dd21..641d1c473 100644 --- a/internal/cmd/gmail_reply_commands.go +++ b/internal/cmd/gmail_reply_commands.go @@ -6,6 +6,8 @@ import ( "os" "strings" + "google.golang.org/api/gmail/v1" + "github.com/openclaw/gogcli/internal/mailmime" "github.com/openclaw/gogcli/internal/ui" ) @@ -21,22 +23,20 @@ type GmailReplyAllCmd struct { } type GmailReplyOptions struct { - To []string `name:"to" sep:"none" help:"Add or move recipients to To (repeatable)"` - Cc []string `name:"cc" sep:"none" help:"Add or move recipients to Cc (repeatable)"` - Bcc []string `name:"bcc" sep:"none" help:"Add or move recipients to Bcc (repeatable)"` - Remove []string `name:"remove" sep:"none" help:"Remove recipients from all fields (repeatable)"` - Subject string `name:"subject" help:"Override reply subject (a changed subject starts a new Gmail thread)"` - Body string `name:"body" help:"Body (plain text; required unless --body-html is set)"` - BodyFile string `name:"body-file" help:"Body file path (plain text; '-' for stdin)"` - BodyHTML string `name:"body-html" help:"Body (HTML; optional)"` - BodyHTMLFile string `name:"body-html-file" help:"HTML body file path ('-' for stdin)"` - NoQuote bool `name:"no-quote" help:"Do not include the original message below the reply"` - Attach []string `name:"attach" sep:"none" help:"Attachment file path (repeatable)"` - From string `name:"from" help:"Send from this email address (must be a verified send-as alias)"` - AutoFromAddressedAlias bool `name:"auto-from-addressed-alias" help:"When --from is omitted, reply from the verified send-as alias addressed by the original message" env:"GOG_GMAIL_AUTO_FROM_ADDRESSED_ALIAS"` - Signature bool `name:"signature" help:"Append the Gmail signature from the active send-as address"` - SignatureFrom string `name:"signature-from" help:"Append the Gmail signature from this send-as email address"` - SignatureFile string `name:"signature-file" help:"Append a local signature file (plain text or HTML)"` + To []string `name:"to" sep:"none" help:"Add or move recipients to To (repeatable)"` + Cc []string `name:"cc" sep:"none" help:"Add or move recipients to Cc (repeatable)"` + Bcc []string `name:"bcc" sep:"none" help:"Add or move recipients to Bcc (repeatable)"` + Remove []string `name:"remove" sep:"none" help:"Remove recipients from all fields (repeatable)"` + Subject string `name:"subject" help:"Override reply subject (a changed subject starts a new Gmail thread)"` + Body string `name:"body" help:"Body (plain text; required unless --body-html is set)"` + BodyFile string `name:"body-file" help:"Body file path (plain text; '-' for stdin)"` + BodyHTML string `name:"body-html" help:"Body (HTML; optional)"` + BodyHTMLFile string `name:"body-html-file" help:"HTML body file path ('-' for stdin)"` + NoQuote bool `name:"no-quote" help:"Do not include the original message below the reply"` + Attach []string `name:"attach" sep:"none" help:"Attachment file path (repeatable)"` + From string `name:"from" help:"Send from this email address (must be a verified send-as alias)"` + AutoFromAddressedAlias bool `name:"auto-from-addressed-alias" help:"When --from is omitted, reply from the verified send-as alias addressed by the original message" env:"GOG_GMAIL_AUTO_FROM_ADDRESSED_ALIAS"` + composeSignatureOptions `embed:""` } func (c *GmailReplyCmd) Run(ctx context.Context, flags *RootFlags) error { @@ -47,49 +47,35 @@ func (c *GmailReplyAllCmd) Run(ctx context.Context, flags *RootFlags) error { return c.Options.run(ctx, flags, c.MessageID, true) } -func (c *GmailReplyOptions) run(ctx context.Context, flags *RootFlags, messageID string, replyAll bool) error { - u := ui.FromContext(ctx) - messageID = normalizeGmailMessageID(messageID) - if messageID == "" { - return usage("required: messageId") - } +// replyComposeInputs holds the validated, service-free inputs for a reply +// compose. Body/HTML inputs are resolved exactly once here because '-' reads +// stdin, which cannot be read twice. +type replyComposeInputs struct { + messageID string + body string + htmlBody string + attachPaths []string +} - body, htmlBody, err := resolveComposeBodyInputs(ctx, c.Body, c.BodyFile, c.BodyHTML, c.BodyHTMLFile) - if err != nil { - return err - } - if strings.TrimSpace(body) == "" && strings.TrimSpace(htmlBody) == "" { - return usage("required: --body, --body-file, --body-html, or --body-html-file") - } - if validationErr := mailmime.ValidateHeaderValue(c.Subject); validationErr != nil { - return usagef("invalid --subject: %v", validationErr) - } - if validationErr := mailmime.ValidateHeaderValue(c.From); validationErr != nil { - return usagef("invalid --from: %v", validationErr) - } - if _, parseErr := parseExplicitRecipientFields(c.To, c.Cc, c.Bcc); parseErr != nil { - return parseErr - } - if _, parseErr := parseMailboxValues("--remove", c.Remove); parseErr != nil { - return parseErr - } +// replyComposeMessage carries the built reply message plus the metadata the +// caller needs to record results. +type replyComposeMessage struct { + message *gmail.Message + fromHeader string + to []string + attachmentMetadata []mailmime.AttachmentMetadata +} - signatureCmd := GmailSendCmd{ - Signature: c.Signature, - SignatureFrom: c.SignatureFrom, - SignatureFile: c.SignatureFile, - } - if signatureErr := signatureCmd.validateSignatureOptions(); signatureErr != nil { - return signatureErr - } +func (c *GmailReplyOptions) run(ctx context.Context, flags *RootFlags, messageID string, replyAll bool) error { + u := ui.FromContext(ctx) - attachPaths, err := expandComposeAttachmentPaths(c.Attach) + inputs, err := c.resolveReplyInputs(ctx, messageID) if err != nil { return err } if dryRunErr := dryRunExit(ctx, flags, "gmail."+replyModeName(replyAll), map[string]any{ - "message_id": messageID, + "message_id": inputs.messageID, "to_add": c.To, "cc_add": c.Cc, "bcc_add": c.Bcc, @@ -98,9 +84,9 @@ func (c *GmailReplyOptions) run(ctx context.Context, flags *RootFlags, messageID "quote": !c.NoQuote, "from": strings.TrimSpace(c.From), "auto_from_addressed_alias": c.AutoFromAddressedAlias, - "body_len": len(body), - "body_html_len": len(htmlBody), - "attachments": attachPaths, + "body_len": len(inputs.body), + "body_html_len": len(inputs.htmlBody), + "attachments": inputs.attachPaths, "signature": c.Signature, "signature_from": strings.TrimSpace(c.SignatureFrom), "signature_file": strings.TrimSpace(c.SignatureFile), @@ -113,14 +99,87 @@ func (c *GmailReplyOptions) run(ctx context.Context, flags *RootFlags, messageID return err } + built, err := c.buildReplyComposeMessage(ctx, svc, account, inputs, replyAll) + if err != nil { + return err + } + + sent, err := svc.Users.Messages.Send("me", built.message).Context(ctx).Do() + if err != nil { + return fmt.Errorf("send reply: %w", err) + } + + return writeGmailMessageResults(ctx, u, []gmailMessageResult{{ + From: built.fromHeader, + To: strings.Join(built.to, ", "), + MessageID: sent.Id, + ThreadID: sent.ThreadId, + Attachments: built.attachmentMetadata, + }}) +} + +// resolveReplyInputs normalizes the message ID, resolves body/HTML inputs, and +// runs all validation that does not require a Gmail service. It reads body +// inputs exactly once so '-' (stdin) is consumed a single time. +func (c *GmailReplyOptions) resolveReplyInputs(ctx context.Context, messageID string) (replyComposeInputs, error) { + messageID = normalizeGmailMessageID(messageID) + if messageID == "" { + return replyComposeInputs{}, usage("required: messageId") + } + + body, htmlBody, err := resolveComposeBodyInputs(ctx, c.Body, c.BodyFile, c.BodyHTML, c.BodyHTMLFile) + if err != nil { + return replyComposeInputs{}, err + } + if strings.TrimSpace(body) == "" && strings.TrimSpace(htmlBody) == "" { + return replyComposeInputs{}, usage("required: --body, --body-file, --body-html, or --body-html-file") + } + if validationErr := mailmime.ValidateHeaderValue(c.Subject); validationErr != nil { + return replyComposeInputs{}, usagef("invalid --subject: %v", validationErr) + } + if validationErr := mailmime.ValidateHeaderValue(c.From); validationErr != nil { + return replyComposeInputs{}, usagef("invalid --from: %v", validationErr) + } + if _, parseErr := parseExplicitRecipientFields(c.To, c.Cc, c.Bcc); parseErr != nil { + return replyComposeInputs{}, parseErr + } + if _, parseErr := parseMailboxValues("--remove", c.Remove); parseErr != nil { + return replyComposeInputs{}, parseErr + } + + if signatureErr := c.validateSignatureOptions(); signatureErr != nil { + return replyComposeInputs{}, signatureErr + } + + attachPaths, err := expandComposeAttachmentPaths(c.Attach) + if err != nil { + return replyComposeInputs{}, err + } + + return replyComposeInputs{ + messageID: messageID, + body: body, + htmlBody: htmlBody, + attachPaths: attachPaths, + }, nil +} + +// buildReplyComposeMessage assembles the outgoing reply from already-validated +// inputs and an already-acquired service. It resolves the sender and signature, +// builds the reply recipients and body, and returns the message without sending +// so the caller controls how it is dispatched. +func (c *GmailReplyOptions) buildReplyComposeMessage(ctx context.Context, svc *gmail.Service, account string, inputs replyComposeInputs, replyAll bool) (replyComposeMessage, error) { + u := ui.FromContext(ctx) + body, htmlBody := inputs.body, inputs.htmlBody + sendAs, sendAsErr := listSendAs(ctx, svc) from, err := resolveComposeFrom(ctx, svc, account, c.From, sendAs, sendAsErr) if err != nil { - return err + return replyComposeMessage{}, err } - info, err := fetchReplyInfo(ctx, svc, messageID, "", !c.NoQuote) + info, err := fetchReplyInfo(ctx, svc, inputs.messageID, "", !c.NoQuote) if err != nil { - return err + return replyComposeMessage{}, err } // When requested, reply as the verified alias the original was addressed to. Do this // before signature resolution so the signature matches the identity actually sending. @@ -131,10 +190,10 @@ func (c *GmailReplyOptions) run(ctx context.Context, flags *RootFlags, messageID } } } - if signatureCmd.signatureRequested() { - signature, source, sigErr := signatureCmd.resolveComposeSignature(ctx, svc, from.sendingEmail) + if c.signatureRequested() { + signature, source, sigErr := c.resolveComposeSignature(ctx, svc, from.sendingEmail) if sigErr != nil { - return sigErr + return replyComposeMessage{}, sigErr } if signature.empty() { u.Err().Linef("Warning: no signature configured for %s", source) @@ -144,7 +203,7 @@ func (c *GmailReplyOptions) run(ctx context.Context, flags *RootFlags, messageID } body, htmlBody, err = applyReplyQuote(ctx, !c.NoQuote, info, body, htmlBody) if err != nil { - return err + return replyComposeMessage{}, err } recipients, err := buildReplyRecipients( info, @@ -156,7 +215,7 @@ func (c *GmailReplyOptions) run(ctx context.Context, flags *RootFlags, messageID c.Remove, ) if err != nil { - return err + return replyComposeMessage{}, err } defaultSubject := autoReplySubject("", info.Subject) @@ -170,13 +229,14 @@ func (c *GmailReplyOptions) run(ctx context.Context, flags *RootFlags, messageID info.ThreadID = "" } - userAttachments, attachmentMetadata, err := mailmime.PrepareAttachments(attachmentsFromPaths(attachPaths), os.ReadFile) + userAttachments, attachmentMetadata, err := mailmime.PrepareAttachments(attachmentsFromPaths(inputs.attachPaths), os.ReadFile) if err != nil { - return err + return replyComposeMessage{}, err } attachments := append([]mailmime.Attachment{}, userAttachments...) attachments = append(attachments, info.InlineResources...) + toRecipients := formatMailboxes(recipients.To) msg, err := buildGmailMessage(ctx, sendMessageOptions{ FromAddr: from.header, Subject: subject, @@ -185,24 +245,18 @@ func (c *GmailReplyOptions) run(ctx context.Context, flags *RootFlags, messageID ReplyInfo: info, Attachments: attachments, }, sendBatch{ - To: formatMailboxes(recipients.To), + To: toRecipients, Cc: formatMailboxes(recipients.Cc), Bcc: formatMailboxes(recipients.Bcc), }, true) if err != nil { - return fmt.Errorf("build reply: %w", err) - } - - sent, err := svc.Users.Messages.Send("me", msg).Context(ctx).Do() - if err != nil { - return fmt.Errorf("send reply: %w", err) + return replyComposeMessage{}, fmt.Errorf("build reply: %w", err) } - return writeGmailMessageResults(ctx, u, []gmailMessageResult{{ - From: from.header, - To: strings.Join(formatMailboxes(recipients.To), ", "), - MessageID: sent.Id, - ThreadID: sent.ThreadId, - Attachments: attachmentMetadata, - }}) + return replyComposeMessage{ + message: msg, + fromHeader: from.header, + to: toRecipients, + attachmentMetadata: attachmentMetadata, + }, nil } diff --git a/internal/cmd/gmail_send.go b/internal/cmd/gmail_send.go index b8e83642a..e50ee44ab 100644 --- a/internal/cmd/gmail_send.go +++ b/internal/cmd/gmail_send.go @@ -14,26 +14,24 @@ import ( ) type GmailSendCmd struct { - To string `name:"to" help:"Recipients (comma-separated; required unless --reply-all is used)"` - Cc string `name:"cc" help:"CC recipients (comma-separated)"` - Bcc string `name:"bcc" help:"BCC recipients (comma-separated)"` - Subject string `name:"subject" help:"Subject (required unless replying; inherited with Re: for replies)"` - Body string `name:"body" help:"Body (plain text; required unless --body-html is set)"` - BodyFile string `name:"body-file" help:"Body file path (plain text; '-' for stdin)"` - BodyHTML string `name:"body-html" help:"Body (HTML; optional)"` - BodyHTMLFile string `name:"body-html-file" help:"HTML body file path ('-' for stdin)"` - ReplyToMessageID string `name:"reply-to-message-id" aliases:"in-reply-to" help:"Reply to Gmail message ID (sets In-Reply-To/References and thread)"` - ThreadID string `name:"thread-id" help:"Reply within a Gmail thread (uses latest message for headers)"` - ReplyAll bool `name:"reply-all" help:"Auto-populate recipients from original message (requires --reply-to-message-id or --thread-id)"` - ReplyTo string `name:"reply-to" help:"Reply-To header address"` - Attach []string `name:"attach" help:"Attachment file path (repeatable)"` - From string `name:"from" help:"Send from this email address (must be a verified send-as alias)"` - Signature bool `name:"signature" help:"Append the Gmail signature from the active send-as address"` - SignatureFrom string `name:"signature-from" help:"Append the Gmail signature from this send-as email address"` - SignatureFile string `name:"signature-file" help:"Append a local signature file (plain text or HTML)"` - Track bool `name:"track" help:"Enable open tracking (requires tracking setup)"` - TrackSplit bool `name:"track-split" help:"Send tracked messages separately per recipient"` - Quote bool `name:"quote" help:"Include quoted original message in reply (requires --reply-to-message-id or --thread-id)"` + To string `name:"to" help:"Recipients (comma-separated; required unless --reply-all is used)"` + Cc string `name:"cc" help:"CC recipients (comma-separated)"` + Bcc string `name:"bcc" help:"BCC recipients (comma-separated)"` + Subject string `name:"subject" help:"Subject (required unless replying; inherited with Re: for replies)"` + Body string `name:"body" help:"Body (plain text; required unless --body-html is set)"` + BodyFile string `name:"body-file" help:"Body file path (plain text; '-' for stdin)"` + BodyHTML string `name:"body-html" help:"Body (HTML; optional)"` + BodyHTMLFile string `name:"body-html-file" help:"HTML body file path ('-' for stdin)"` + ReplyToMessageID string `name:"reply-to-message-id" aliases:"in-reply-to" help:"Reply to Gmail message ID (sets In-Reply-To/References and thread)"` + ThreadID string `name:"thread-id" help:"Reply within a Gmail thread (uses latest message for headers)"` + ReplyAll bool `name:"reply-all" help:"Auto-populate recipients from original message (requires --reply-to-message-id or --thread-id)"` + ReplyTo string `name:"reply-to" help:"Reply-To header address"` + Attach []string `name:"attach" help:"Attachment file path (repeatable)"` + From string `name:"from" help:"Send from this email address (must be a verified send-as alias)"` + composeSignatureOptions `embed:""` + Track bool `name:"track" help:"Enable open tracking (requires tracking setup)"` + TrackSplit bool `name:"track-split" help:"Send tracked messages separately per recipient"` + Quote bool `name:"quote" help:"Include quoted original message in reply (requires --reply-to-message-id or --thread-id)"` } type sendBatch struct { diff --git a/internal/cmd/gmail_send_signature.go b/internal/cmd/gmail_send_signature.go index f1ed6f7f6..bef4e053f 100644 --- a/internal/cmd/gmail_send_signature.go +++ b/internal/cmd/gmail_send_signature.go @@ -25,18 +25,26 @@ func (s composeSignature) empty() bool { return strings.TrimSpace(s.Plain) == "" && strings.TrimSpace(s.HTML) == "" } -func (c *GmailSendCmd) signatureRequested() bool { +// composeSignatureOptions holds the shared signature flags used by the send and +// reply compose commands. +type composeSignatureOptions struct { + Signature bool `name:"signature" help:"Append the Gmail signature from the active send-as address"` + SignatureFrom string `name:"signature-from" help:"Append the Gmail signature from this send-as email address"` + SignatureFile string `name:"signature-file" help:"Append a local signature file (plain text or HTML)"` +} + +func (c *composeSignatureOptions) signatureRequested() bool { return c.Signature || strings.TrimSpace(c.SignatureFrom) != "" || strings.TrimSpace(c.SignatureFile) != "" } -func (c *GmailSendCmd) validateSignatureOptions() error { +func (c *composeSignatureOptions) validateSignatureOptions() error { if strings.TrimSpace(c.SignatureFile) != "" && (c.Signature || strings.TrimSpace(c.SignatureFrom) != "") { return usage("use only one of --signature/--signature-from or --signature-file") } return nil } -func (c *GmailSendCmd) resolveComposeSignature(ctx context.Context, svc *gmail.Service, sendingEmail string) (composeSignature, string, error) { +func (c *composeSignatureOptions) resolveComposeSignature(ctx context.Context, svc *gmail.Service, sendingEmail string) (composeSignature, string, error) { if path := strings.TrimSpace(c.SignatureFile); path != "" { signature, err := readComposeSignatureFile(path) return signature, path, err diff --git a/internal/cmd/gmail_send_signature_test.go b/internal/cmd/gmail_send_signature_test.go index a4553e76b..160d7e07a 100644 --- a/internal/cmd/gmail_send_signature_test.go +++ b/internal/cmd/gmail_send_signature_test.go @@ -27,11 +27,11 @@ func TestGmailSendCmd_Run_WithSendAsSignature(t *testing.T) { http.NotFound(w, r) } }, &GmailSendCmd{ - To: "recipient@example.com", - Subject: "Hello", - Body: "Body", - BodyHTML: "

Body

", - Signature: true, + To: "recipient@example.com", + Subject: "Hello", + Body: "Body", + BodyHTML: "

Body

", + composeSignatureOptions: composeSignatureOptions{Signature: true}, }) if !strings.Contains(raw, "Body\r\n\r\n--\r\nKind regards\r\nPrimary User") { @@ -59,11 +59,11 @@ func TestGmailSendCmd_Run_SignatureFromAlias(t *testing.T) { http.NotFound(w, r) } }, &GmailSendCmd{ - To: "recipient@example.com", - Subject: "Hello", - Body: "Body", - From: "alias@example.com", - SignatureFrom: "alias@example.com", + To: "recipient@example.com", + Subject: "Hello", + Body: "Body", + From: "alias@example.com", + composeSignatureOptions: composeSignatureOptions{SignatureFrom: "alias@example.com"}, }) if !strings.Contains(raw, `From: "Alias" `) { @@ -89,11 +89,11 @@ func TestGmailSendCmd_Run_WithSignatureFile(t *testing.T) { http.NotFound(w, r) } }, &GmailSendCmd{ - To: "recipient@example.com", - Subject: "Hello", - Body: "Body", - BodyHTML: "

Body

", - SignatureFile: path, + To: "recipient@example.com", + Subject: "Hello", + Body: "Body", + BodyHTML: "

Body

", + composeSignatureOptions: composeSignatureOptions{SignatureFile: path}, }) if !strings.Contains(raw, "Body\r\n\r\n--\r\nLocal Sig\r\nhttps://example.com") { @@ -147,10 +147,10 @@ func TestGmailSendCmd_Run_EmptySignatureWarnsAndSends(t *testing.T) { var stderr strings.Builder ctx := withGmailTestService(newGmailSendSignatureTestContext(t, io.Discard, &stderr), svc) err := (&GmailSendCmd{ - To: "recipient@example.com", - Subject: "Hello", - Body: "Body", - Signature: true, + To: "recipient@example.com", + Subject: "Hello", + Body: "Body", + composeSignatureOptions: composeSignatureOptions{Signature: true}, }).Run(ctx, &RootFlags{Account: "a@b.com"}) if err != nil { t.Fatalf("Run: %v", err) @@ -166,11 +166,10 @@ func TestGmailSendCmd_Run_EmptySignatureWarnsAndSends(t *testing.T) { func TestGmailSendCmd_Run_SignatureOptionConflict(t *testing.T) { err := (&GmailSendCmd{ - To: "recipient@example.com", - Subject: "Hello", - Body: "Body", - Signature: true, - SignatureFile: "sig.txt", + To: "recipient@example.com", + Subject: "Hello", + Body: "Body", + composeSignatureOptions: composeSignatureOptions{Signature: true, SignatureFile: "sig.txt"}, }).Run(context.Background(), &RootFlags{Account: "a@b.com"}) if err == nil || !strings.Contains(err.Error(), "use only one of") { t.Fatalf("expected signature option conflict, got %v", err) From 285f6fb1d078fd82e2d0f3ddaea12ff93cbf221d Mon Sep 17 00:00:00 2001 From: Malo Bourgon Date: Mon, 29 Jun 2026 11:19:11 -0400 Subject: [PATCH 2/3] feat(gmail): add drafts reply/reply-all/forward Add `gmail drafts reply`, `drafts reply-all`, and `drafts forward`, giving the drafts surface full flag/ergonomic parity with the send-side reply/reply-all/forward commands. They embed the same options structs and reuse the shared resolve/build helpers, differing only at finalize: they save a draft (Drafts.Create) instead of sending. Drafts use the non-send service gate, so they work under --gmail-no-send, matching gmail drafts create. `drafts forward` allows an addressless draft (no --to), matching drafts create and Gmail's UI. The recipient requirement is resolved explicitly per call, which also aligns gmail forward's required---to handling with send/reply (runtime check + explanatory help) while keeping the MIME missing-To backstop on the send path. Co-Authored-By: Claude Opus 4.8 (1M context) --- docs/commands.generated.md | 5 +- docs/commands/README.md | 5 +- docs/commands/gog-gmail-drafts-forward.md | 53 ++ docs/commands/gog-gmail-drafts-reply-all.md | 62 ++ docs/commands/gog-gmail-drafts-reply.md | 62 ++ docs/commands/gog-gmail-drafts.md | 3 + docs/commands/gog-gmail-forward.md | 4 +- docs/gmail-workflows.md | 28 +- docs/spec.md | 3 + internal/cmd/gmail_drafts.go | 15 +- internal/cmd/gmail_drafts_compose.go | 117 +++ internal/cmd/gmail_drafts_compose_test.go | 821 ++++++++++++++++++++ internal/cmd/gmail_forward.go | 76 +- internal/cmd/gmail_reply_commands.go | 55 +- internal/cmd/safety_profile_test.go | 9 +- safety-profiles/agent-safe.yaml | 3 + safety-profiles/readonly.yaml | 3 + 17 files changed, 1269 insertions(+), 55 deletions(-) create mode 100644 docs/commands/gog-gmail-drafts-forward.md create mode 100644 docs/commands/gog-gmail-drafts-reply-all.md create mode 100644 docs/commands/gog-gmail-drafts-reply.md create mode 100644 internal/cmd/gmail_drafts_compose.go create mode 100644 internal/cmd/gmail_drafts_compose_test.go diff --git a/docs/commands.generated.md b/docs/commands.generated.md index 59a53d530..e1435c166 100644 --- a/docs/commands.generated.md +++ b/docs/commands.generated.md @@ -403,11 +403,14 @@ Generated from `gog schema --json`. - [`gog gmail (mail,email) drafts (draft) `](commands/gog-gmail-drafts.md) - Draft operations - [`gog gmail (mail,email) drafts (draft) create (add,new) [flags]`](commands/gog-gmail-drafts-create.md) - Create a draft - [`gog gmail (mail,email) drafts (draft) delete (rm,del,remove) `](commands/gog-gmail-drafts-delete.md) - Permanently delete a draft (not recoverable; drafts are not moved to Trash) + - [`gog gmail (mail,email) drafts (draft) forward (fwd) [flags]`](commands/gog-gmail-drafts-forward.md) - Save a forward as a draft - [`gog gmail (mail,email) drafts (draft) get (info,show) [flags]`](commands/gog-gmail-drafts-get.md) - Get draft details - [`gog gmail (mail,email) drafts (draft) list (ls) [flags]`](commands/gog-gmail-drafts-list.md) - List drafts + - [`gog gmail (mail,email) drafts (draft) reply [flags]`](commands/gog-gmail-drafts-reply.md) - Save a reply as a draft + - [`gog gmail (mail,email) drafts (draft) reply-all (replyall) [flags]`](commands/gog-gmail-drafts-reply-all.md) - Save a reply-all as a draft - [`gog gmail (mail,email) drafts (draft) send (post) `](commands/gog-gmail-drafts-send.md) - Send a draft - [`gog gmail (mail,email) drafts (draft) update (edit,set) [flags]`](commands/gog-gmail-drafts-update.md) - Update a draft - - [`gog gmail (mail,email) forward (fwd) --to=STRING [flags]`](commands/gog-gmail-forward.md) - Forward a message to new recipients + - [`gog gmail (mail,email) forward (fwd) [flags]`](commands/gog-gmail-forward.md) - Forward a message to new recipients - [`gog gmail (mail,email) get (info,show) [flags]`](commands/gog-gmail-get.md) - Get a message (full|metadata|raw) - [`gog gmail (mail,email) history [flags]`](commands/gog-gmail-history.md) - Gmail history - [`gog gmail (mail,email) import [flags]`](commands/gog-gmail-import.md) - Import an RFC822/EML message into Gmail diff --git a/docs/commands/README.md b/docs/commands/README.md index 77e56aa1e..3ec57c491 100644 --- a/docs/commands/README.md +++ b/docs/commands/README.md @@ -2,7 +2,7 @@ Every `gog` command has a generated docs page. The source of truth is the live CLI schema; run `make docs-commands` after changing command names, flags, help text, aliases, or arguments. -Generated pages: 709. +Generated pages: 712. ## Top-level Commands @@ -456,8 +456,11 @@ Generated pages: 709. - [gog gmail drafts](gog-gmail-drafts.md) - Draft operations - [gog gmail drafts create](gog-gmail-drafts-create.md) - Create a draft - [gog gmail drafts delete](gog-gmail-drafts-delete.md) - Permanently delete a draft (not recoverable; drafts are not moved to Trash) + - [gog gmail drafts forward](gog-gmail-drafts-forward.md) - Save a forward as a draft - [gog gmail drafts get](gog-gmail-drafts-get.md) - Get draft details - [gog gmail drafts list](gog-gmail-drafts-list.md) - List drafts + - [gog gmail drafts reply](gog-gmail-drafts-reply.md) - Save a reply as a draft + - [gog gmail drafts reply-all](gog-gmail-drafts-reply-all.md) - Save a reply-all as a draft - [gog gmail drafts send](gog-gmail-drafts-send.md) - Send a draft - [gog gmail drafts update](gog-gmail-drafts-update.md) - Update a draft - [gog gmail forward](gog-gmail-forward.md) - Forward a message to new recipients diff --git a/docs/commands/gog-gmail-drafts-forward.md b/docs/commands/gog-gmail-drafts-forward.md new file mode 100644 index 000000000..8a21b23be --- /dev/null +++ b/docs/commands/gog-gmail-drafts-forward.md @@ -0,0 +1,53 @@ +# `gog gmail drafts forward` + +> Generated from `gog schema --json`. Do not edit this page by hand; run `make docs-commands`. + +Save a forward as a draft + +## Usage + +```bash +gog gmail (mail,email) drafts (draft) forward (fwd) [flags] +``` + +## Parent + +- [gog gmail drafts](gog-gmail-drafts.md) + +## Flags + +| Flag | Type | Default | Help | +| --- | --- | --- | --- | +| `--access-token` | `string` | | Use provided access token directly (bypasses stored refresh tokens; token expires in ~1h) | +| `-a`
`--account`
`--acct` | `string` | | Account email, alias, or auto for authenticated Google API commands | +| `--bcc` | `string` | | BCC recipients (comma-separated) | +| `--cc` | `string` | | CC recipients (comma-separated) | +| `--client` | `string` | | OAuth client name (selects stored credentials + token bucket) | +| `--color` | `string` | auto | Color output: auto\|always\|never | +| `--disable-commands` | `string` | | Comma-separated list of disabled commands; dot paths allowed | +| `-n`
`--dry-run`
`--dryrun`
`--noop`
`--preview` | `bool` | | Do not make changes; print intended actions and exit successfully | +| `--enable-commands` | `string` | | Comma-separated list of enabled command prefixes; dot paths allowed (restricts CLI) | +| `--enable-commands-exact` | `string` | | Comma-separated list of exact enabled commands; dot paths allowed and parent commands do not enable children | +| `-y`
`--force`
`--assume-yes`
`--yes` | `bool` | | Skip confirmations for destructive commands | +| `--from` | `string` | | Send from this email address (must be a verified send-as alias) | +| `--gmail-no-send` | `bool` | false | Block Gmail send operations (agent safety) | +| `-h`
`--help` | `kong.helpFlag` | | Show context-sensitive help. | +| `--home` | `string` | | Override gogcli config/data/state/cache root (equivalent to GOG_HOME) | +| `-j`
`--json`
`--machine` | `bool` | false | Output JSON to stdout (best for scripting) | +| `--no-input`
`--non-interactive`
`--noninteractive` | `bool` | | Never prompt; fail instead (useful for CI) | +| `--note`
`--intro` | `string` | | Introductory text above the forwarded message | +| `--note-file` | `string` | | Note file path (plain text; '-' for stdin) | +| `-p`
`--plain`
`--tsv` | `bool` | false | Output stable, parseable text to stdout (TSV; no colors) | +| `--readonly` | `bool` | false | Block mutating API requests at runtime; auth add also requests read-only OAuth scopes | +| `--results-only` | `bool` | | In JSON mode, emit only the primary result (drops envelope fields like nextPageToken) | +| `--select`
`--pick`
`--project` | `string` | | In JSON mode, select comma-separated fields (best-effort; supports dot paths). Desire path: use --fields for most commands. | +| `--skip-attachments` | `bool` | | Do not include original attachments | +| `--to` | `string` | | Recipients (comma-separated; required when sending, optional when saving a draft) | +| `-v`
`--verbose` | `bool` | | Enable verbose logging | +| `--version` | `kong.VersionFlag` | | Print version and exit | +| `--wrap-untrusted` | `bool` | false | In JSON/raw output, wrap fetched text fields in external untrusted-content markers | + +## See Also + +- [gog gmail drafts](gog-gmail-drafts.md) +- [Command index](README.md) diff --git a/docs/commands/gog-gmail-drafts-reply-all.md b/docs/commands/gog-gmail-drafts-reply-all.md new file mode 100644 index 000000000..fc26d1725 --- /dev/null +++ b/docs/commands/gog-gmail-drafts-reply-all.md @@ -0,0 +1,62 @@ +# `gog gmail drafts reply-all` + +> Generated from `gog schema --json`. Do not edit this page by hand; run `make docs-commands`. + +Save a reply-all as a draft + +## Usage + +```bash +gog gmail (mail,email) drafts (draft) reply-all (replyall) [flags] +``` + +## Parent + +- [gog gmail drafts](gog-gmail-drafts.md) + +## Flags + +| Flag | Type | Default | Help | +| --- | --- | --- | --- | +| `--access-token` | `string` | | Use provided access token directly (bypasses stored refresh tokens; token expires in ~1h) | +| `-a`
`--account`
`--acct` | `string` | | Account email, alias, or auto for authenticated Google API commands | +| `--attach` | `[]string` | | Attachment file path (repeatable) | +| `--auto-from-addressed-alias` | `bool` | | When --from is omitted, reply from the verified send-as alias addressed by the original message | +| `--bcc` | `[]string` | | Add or move recipients to Bcc (repeatable) | +| `--body` | `string` | | Body (plain text; required unless --body-html is set) | +| `--body-file` | `string` | | Body file path (plain text; '-' for stdin) | +| `--body-html` | `string` | | Body (HTML; optional) | +| `--body-html-file` | `string` | | HTML body file path ('-' for stdin) | +| `--cc` | `[]string` | | Add or move recipients to Cc (repeatable) | +| `--client` | `string` | | OAuth client name (selects stored credentials + token bucket) | +| `--color` | `string` | auto | Color output: auto\|always\|never | +| `--disable-commands` | `string` | | Comma-separated list of disabled commands; dot paths allowed | +| `-n`
`--dry-run`
`--dryrun`
`--noop`
`--preview` | `bool` | | Do not make changes; print intended actions and exit successfully | +| `--enable-commands` | `string` | | Comma-separated list of enabled command prefixes; dot paths allowed (restricts CLI) | +| `--enable-commands-exact` | `string` | | Comma-separated list of exact enabled commands; dot paths allowed and parent commands do not enable children | +| `-y`
`--force`
`--assume-yes`
`--yes` | `bool` | | Skip confirmations for destructive commands | +| `--from` | `string` | | Send from this email address (must be a verified send-as alias) | +| `--gmail-no-send` | `bool` | false | Block Gmail send operations (agent safety) | +| `-h`
`--help` | `kong.helpFlag` | | Show context-sensitive help. | +| `--home` | `string` | | Override gogcli config/data/state/cache root (equivalent to GOG_HOME) | +| `-j`
`--json`
`--machine` | `bool` | false | Output JSON to stdout (best for scripting) | +| `--no-input`
`--non-interactive`
`--noninteractive` | `bool` | | Never prompt; fail instead (useful for CI) | +| `--no-quote` | `bool` | | Do not include the original message below the reply | +| `-p`
`--plain`
`--tsv` | `bool` | false | Output stable, parseable text to stdout (TSV; no colors) | +| `--readonly` | `bool` | false | Block mutating API requests at runtime; auth add also requests read-only OAuth scopes | +| `--remove` | `[]string` | | Remove recipients from all fields (repeatable) | +| `--results-only` | `bool` | | In JSON mode, emit only the primary result (drops envelope fields like nextPageToken) | +| `--select`
`--pick`
`--project` | `string` | | In JSON mode, select comma-separated fields (best-effort; supports dot paths). Desire path: use --fields for most commands. | +| `--signature` | `bool` | | Append the Gmail signature from the active send-as address | +| `--signature-file` | `string` | | Append a local signature file (plain text or HTML) | +| `--signature-from` | `string` | | Append the Gmail signature from this send-as email address | +| `--subject` | `string` | | Override reply subject (a changed subject starts a new Gmail thread) | +| `--to` | `[]string` | | Add or move recipients to To (repeatable) | +| `-v`
`--verbose` | `bool` | | Enable verbose logging | +| `--version` | `kong.VersionFlag` | | Print version and exit | +| `--wrap-untrusted` | `bool` | false | In JSON/raw output, wrap fetched text fields in external untrusted-content markers | + +## See Also + +- [gog gmail drafts](gog-gmail-drafts.md) +- [Command index](README.md) diff --git a/docs/commands/gog-gmail-drafts-reply.md b/docs/commands/gog-gmail-drafts-reply.md new file mode 100644 index 000000000..83dae531e --- /dev/null +++ b/docs/commands/gog-gmail-drafts-reply.md @@ -0,0 +1,62 @@ +# `gog gmail drafts reply` + +> Generated from `gog schema --json`. Do not edit this page by hand; run `make docs-commands`. + +Save a reply as a draft + +## Usage + +```bash +gog gmail (mail,email) drafts (draft) reply [flags] +``` + +## Parent + +- [gog gmail drafts](gog-gmail-drafts.md) + +## Flags + +| Flag | Type | Default | Help | +| --- | --- | --- | --- | +| `--access-token` | `string` | | Use provided access token directly (bypasses stored refresh tokens; token expires in ~1h) | +| `-a`
`--account`
`--acct` | `string` | | Account email, alias, or auto for authenticated Google API commands | +| `--attach` | `[]string` | | Attachment file path (repeatable) | +| `--auto-from-addressed-alias` | `bool` | | When --from is omitted, reply from the verified send-as alias addressed by the original message | +| `--bcc` | `[]string` | | Add or move recipients to Bcc (repeatable) | +| `--body` | `string` | | Body (plain text; required unless --body-html is set) | +| `--body-file` | `string` | | Body file path (plain text; '-' for stdin) | +| `--body-html` | `string` | | Body (HTML; optional) | +| `--body-html-file` | `string` | | HTML body file path ('-' for stdin) | +| `--cc` | `[]string` | | Add or move recipients to Cc (repeatable) | +| `--client` | `string` | | OAuth client name (selects stored credentials + token bucket) | +| `--color` | `string` | auto | Color output: auto\|always\|never | +| `--disable-commands` | `string` | | Comma-separated list of disabled commands; dot paths allowed | +| `-n`
`--dry-run`
`--dryrun`
`--noop`
`--preview` | `bool` | | Do not make changes; print intended actions and exit successfully | +| `--enable-commands` | `string` | | Comma-separated list of enabled command prefixes; dot paths allowed (restricts CLI) | +| `--enable-commands-exact` | `string` | | Comma-separated list of exact enabled commands; dot paths allowed and parent commands do not enable children | +| `-y`
`--force`
`--assume-yes`
`--yes` | `bool` | | Skip confirmations for destructive commands | +| `--from` | `string` | | Send from this email address (must be a verified send-as alias) | +| `--gmail-no-send` | `bool` | false | Block Gmail send operations (agent safety) | +| `-h`
`--help` | `kong.helpFlag` | | Show context-sensitive help. | +| `--home` | `string` | | Override gogcli config/data/state/cache root (equivalent to GOG_HOME) | +| `-j`
`--json`
`--machine` | `bool` | false | Output JSON to stdout (best for scripting) | +| `--no-input`
`--non-interactive`
`--noninteractive` | `bool` | | Never prompt; fail instead (useful for CI) | +| `--no-quote` | `bool` | | Do not include the original message below the reply | +| `-p`
`--plain`
`--tsv` | `bool` | false | Output stable, parseable text to stdout (TSV; no colors) | +| `--readonly` | `bool` | false | Block mutating API requests at runtime; auth add also requests read-only OAuth scopes | +| `--remove` | `[]string` | | Remove recipients from all fields (repeatable) | +| `--results-only` | `bool` | | In JSON mode, emit only the primary result (drops envelope fields like nextPageToken) | +| `--select`
`--pick`
`--project` | `string` | | In JSON mode, select comma-separated fields (best-effort; supports dot paths). Desire path: use --fields for most commands. | +| `--signature` | `bool` | | Append the Gmail signature from the active send-as address | +| `--signature-file` | `string` | | Append a local signature file (plain text or HTML) | +| `--signature-from` | `string` | | Append the Gmail signature from this send-as email address | +| `--subject` | `string` | | Override reply subject (a changed subject starts a new Gmail thread) | +| `--to` | `[]string` | | Add or move recipients to To (repeatable) | +| `-v`
`--verbose` | `bool` | | Enable verbose logging | +| `--version` | `kong.VersionFlag` | | Print version and exit | +| `--wrap-untrusted` | `bool` | false | In JSON/raw output, wrap fetched text fields in external untrusted-content markers | + +## See Also + +- [gog gmail drafts](gog-gmail-drafts.md) +- [Command index](README.md) diff --git a/docs/commands/gog-gmail-drafts.md b/docs/commands/gog-gmail-drafts.md index c3db990c8..91bc29c25 100644 --- a/docs/commands/gog-gmail-drafts.md +++ b/docs/commands/gog-gmail-drafts.md @@ -18,8 +18,11 @@ gog gmail (mail,email) drafts (draft) - [gog gmail drafts create](gog-gmail-drafts-create.md) - Create a draft - [gog gmail drafts delete](gog-gmail-drafts-delete.md) - Permanently delete a draft (not recoverable; drafts are not moved to Trash) +- [gog gmail drafts forward](gog-gmail-drafts-forward.md) - Save a forward as a draft - [gog gmail drafts get](gog-gmail-drafts-get.md) - Get draft details - [gog gmail drafts list](gog-gmail-drafts-list.md) - List drafts +- [gog gmail drafts reply](gog-gmail-drafts-reply.md) - Save a reply as a draft +- [gog gmail drafts reply-all](gog-gmail-drafts-reply-all.md) - Save a reply-all as a draft - [gog gmail drafts send](gog-gmail-drafts-send.md) - Send a draft - [gog gmail drafts update](gog-gmail-drafts-update.md) - Update a draft diff --git a/docs/commands/gog-gmail-forward.md b/docs/commands/gog-gmail-forward.md index ac4a445df..48e860ce4 100644 --- a/docs/commands/gog-gmail-forward.md +++ b/docs/commands/gog-gmail-forward.md @@ -7,7 +7,7 @@ Forward a message to new recipients ## Usage ```bash -gog gmail (mail,email) forward (fwd) --to=STRING [flags] +gog gmail (mail,email) forward (fwd) [flags] ``` ## Parent @@ -42,7 +42,7 @@ gog gmail (mail,email) forward (fwd) --to=STRING [flags] | `--results-only` | `bool` | | In JSON mode, emit only the primary result (drops envelope fields like nextPageToken) | | `--select`
`--pick`
`--project` | `string` | | In JSON mode, select comma-separated fields (best-effort; supports dot paths). Desire path: use --fields for most commands. | | `--skip-attachments` | `bool` | | Do not include original attachments | -| `--to` | `string` | | Recipients (comma-separated; required) | +| `--to` | `string` | | Recipients (comma-separated; required when sending, optional when saving a draft) | | `-v`
`--verbose` | `bool` | | Enable verbose logging | | `--version` | `kong.VersionFlag` | | Print version and exit | | `--wrap-untrusted` | `bool` | false | In JSON/raw output, wrap fetched text fields in external untrusted-content markers | diff --git a/docs/gmail-workflows.md b/docs/gmail-workflows.md index 4e4f7956e..191cff31a 100644 --- a/docs/gmail-workflows.md +++ b/docs/gmail-workflows.md @@ -125,6 +125,29 @@ An explicit `--subject` override is supported. A changed subject cannot meet Gmail's thread-matching requirement, so gog keeps the RFC reply headers but does not force the original `threadId`; Gmail creates a new conversation. +To stage a reply for review instead of sending it, use the draft-side +counterparts. They accept the same flags and build the same message; only the +finalize step differs (the draft is saved, not sent), so they work under +no-send guardrails: + +```bash +gog gmail drafts reply --body-file reply.txt +gog gmail drafts reply-all --body "Thanks all" +``` + +## Forward + +`gog gmail forward` sends a message on with a `Fwd:` subject, a Gmail-style +forwarded-message block, and the original attachments (skip them with +`--skip-attachments`). `gog gmail drafts forward` saves the same composition +as a draft instead; unlike the send side it does not require `--to`, matching +Gmail's UI, which allows an addressless forward draft: + +```bash +gog gmail forward --to colleague@example.com --note "FYI" +gog gmail drafts forward --note "FYI" +``` + Remote HTTP images remain remote references. Only MIME parts referenced with `cid:` are copied into the outgoing message. @@ -142,8 +165,9 @@ Official behavior references: ## Attachment Confirmation -`gmail send --json` and `gmail drafts create|update --json` include an -`attachments` array when the resulting message contains attachments: +`gmail send --json`, `gmail drafts create|update --json`, and +`gmail drafts reply|reply-all --json` include an `attachments` array when the +resulting message contains attachments: ```json {"attachments":[{"filename":"report.pdf","size":2411233}]} diff --git a/docs/spec.md b/docs/spec.md index 63671db53..459c8d28e 100644 --- a/docs/spec.md +++ b/docs/spec.md @@ -397,6 +397,9 @@ after the bounded retry window, the command exits with retryable code `8`. - `gog gmail drafts get [--download]` - `gog gmail drafts create [--subject S] [--to a@b.com] [--body B] [--body-html H] [--cc ...] [--bcc ...] [--reply-to-message-id |--thread-id ] [--reply-all] [--reply-to addr] [--from addr|--auto-from-addressed-alias] [--attach ...]` - `gog gmail drafts update [--subject S] [--to a@b.com] [--body B] [--body-html H] [--cc ...] [--bcc ...] [--reply-to-message-id |--thread-id ] [--reply-all] [--reply-to addr] [--from addr|--auto-from-addressed-alias] [--attach ...]` +- `gog gmail drafts reply [--body B|--body-file PATH|--body-html HTML|--body-html-file PATH] [--to ...] [--cc ...] [--bcc ...] [--remove ...] [--subject S] [--no-quote] [--from addr|--auto-from-addressed-alias] [--signature|--signature-from addr|--signature-file path] [--attach ...]` (same flags as `gmail reply`, but saves a draft instead of sending; works under no-send) +- `gog gmail drafts reply-all [--body B|--body-file PATH|--body-html HTML|--body-html-file PATH] [--to ...] [--cc ...] [--bcc ...] [--remove ...] [--subject S] [--no-quote] [--from addr|--auto-from-addressed-alias] [--signature|--signature-from addr|--signature-file path] [--attach ...]` (same flags as `gmail reply-all`, but saves a draft; works under no-send) +- `gog gmail drafts forward [--to a@b.com] [--cc ...] [--bcc ...] [--note TEXT|--note-file PATH] [--from addr] [--skip-attachments]` (same flags as `gmail forward`, but saves a draft; `--to` is optional for a draft; works under no-send) - `gog gmail drafts send ` - `gog gmail drafts delete ` - `gog gmail watch start|status|renew|stop|serve` diff --git a/internal/cmd/gmail_drafts.go b/internal/cmd/gmail_drafts.go index 6a54a8f6a..08dbbecc7 100644 --- a/internal/cmd/gmail_drafts.go +++ b/internal/cmd/gmail_drafts.go @@ -17,12 +17,15 @@ import ( ) type GmailDraftsCmd struct { - List GmailDraftsListCmd `cmd:"" name:"list" aliases:"ls" help:"List drafts"` - Get GmailDraftsGetCmd `cmd:"" name:"get" aliases:"info,show" help:"Get draft details"` - Delete GmailDraftsDeleteCmd `cmd:"" name:"delete" aliases:"rm,del,remove" help:"Permanently delete a draft (not recoverable; drafts are not moved to Trash)"` - Send GmailDraftsSendCmd `cmd:"" name:"send" aliases:"post" help:"Send a draft"` - Create GmailDraftsCreateCmd `cmd:"" name:"create" aliases:"add,new" help:"Create a draft"` - Update GmailDraftsUpdateCmd `cmd:"" name:"update" aliases:"edit,set" help:"Update a draft"` + List GmailDraftsListCmd `cmd:"" name:"list" aliases:"ls" help:"List drafts"` + Get GmailDraftsGetCmd `cmd:"" name:"get" aliases:"info,show" help:"Get draft details"` + Delete GmailDraftsDeleteCmd `cmd:"" name:"delete" aliases:"rm,del,remove" help:"Permanently delete a draft (not recoverable; drafts are not moved to Trash)"` + Send GmailDraftsSendCmd `cmd:"" name:"send" aliases:"post" help:"Send a draft"` + Create GmailDraftsCreateCmd `cmd:"" name:"create" aliases:"add,new" help:"Create a draft"` + Update GmailDraftsUpdateCmd `cmd:"" name:"update" aliases:"edit,set" help:"Update a draft"` + Reply GmailDraftsReplyCmd `cmd:"" name:"reply" help:"Save a reply as a draft"` + ReplyAll GmailDraftsReplyAllCmd `cmd:"" name:"reply-all" aliases:"replyall" help:"Save a reply-all as a draft"` + Forward GmailDraftsForwardCmd `cmd:"" name:"forward" aliases:"fwd" help:"Save a forward as a draft"` } type GmailDraftsListCmd struct { diff --git a/internal/cmd/gmail_drafts_compose.go b/internal/cmd/gmail_drafts_compose.go new file mode 100644 index 000000000..9ff703a61 --- /dev/null +++ b/internal/cmd/gmail_drafts_compose.go @@ -0,0 +1,117 @@ +package cmd + +import ( + "context" + "fmt" + + "google.golang.org/api/gmail/v1" + + "github.com/openclaw/gogcli/internal/ui" +) + +// GmailDraftsReplyCmd saves a reply as a draft. It mirrors GmailReplyCmd exactly +// (same positional arg + embedded GmailReplyOptions) so it inherits every flag +// and ergonomic of the send-side reply; the only difference is that it creates a +// draft instead of sending. +type GmailDraftsReplyCmd struct { + MessageID string `arg:"" name:"messageId" help:"Gmail message ID to reply to"` + Options GmailReplyOptions `embed:""` +} + +// GmailDraftsReplyAllCmd saves a reply-all as a draft. Mirrors GmailReplyAllCmd. +type GmailDraftsReplyAllCmd struct { + MessageID string `arg:"" name:"messageId" help:"Gmail message ID to reply to"` + Options GmailReplyOptions `embed:""` +} + +// GmailDraftsForwardCmd saves a forward as a draft. Mirrors GmailForwardCmd. +type GmailDraftsForwardCmd struct { + MessageID string `arg:"" name:"messageId" help:"Gmail message ID to forward"` + GmailForwardOptions `embed:""` +} + +func (c *GmailDraftsReplyCmd) Run(ctx context.Context, flags *RootFlags) error { + return c.Options.runDraft(ctx, flags, c.MessageID, false) +} + +func (c *GmailDraftsReplyAllCmd) Run(ctx context.Context, flags *RootFlags) error { + return c.Options.runDraft(ctx, flags, c.MessageID, true) +} + +func (c *GmailDraftsForwardCmd) Run(ctx context.Context, flags *RootFlags) error { + return c.runDraft(ctx, flags, c.MessageID) +} + +// runDraft is the draft-saving counterpart to GmailReplyOptions.run. It reuses +// the shared resolve/build helpers verbatim and differs only in the dry-run +// action name, the service gate (the non-send gate, since saving a draft is not +// a send), and the finalize step (Drafts.Create instead of Messages.Send). +func (c *GmailReplyOptions) runDraft(ctx context.Context, flags *RootFlags, messageID string, replyAll bool) error { + u := ui.FromContext(ctx) + + inputs, err := c.resolveReplyInputs(ctx, messageID) + if err != nil { + return err + } + + if dryRunErr := dryRunExit(ctx, flags, "gmail.drafts."+replyModeName(replyAll), c.dryRunFields(inputs)); dryRunErr != nil { + return dryRunErr + } + + // Drafts use the non-send gate: a draft is not a send, so this works under + // --gmail-no-send and the config no-send, matching gmail drafts create. + account, svc, err := requireGmailService(ctx, flags) + if err != nil { + return err + } + + built, err := c.buildReplyComposeMessage(ctx, svc, account, inputs, replyAll) + if err != nil { + return err + } + + draft, err := svc.Users.Drafts.Create("me", &gmail.Draft{Message: built.message}).Context(ctx).Do() + if err != nil { + return fmt.Errorf("create reply draft: %w", err) + } + + return writeDraftResult(ctx, u, draft, built.threading, built.attachmentMetadata) +} + +// runDraft is the draft-saving counterpart to GmailForwardCmd.Run. Like the +// reply draft path it reuses the shared resolve/build helpers verbatim and only +// changes the dry-run action name, the service gate, and the finalize step. +func (c *GmailForwardOptions) runDraft(ctx context.Context, flags *RootFlags, messageID string) error { + u := ui.FromContext(ctx) + + // Drafts may have no recipients, so the draft path does not require --to. + inputs, err := c.resolveForwardInputs(ctx, messageID, recipientsOptional) + if err != nil { + return err + } + + if dryRunErr := dryRunExit(ctx, flags, "gmail.drafts.forward", c.dryRunFields(inputs)); dryRunErr != nil { + return dryRunErr + } + + account, svc, err := requireGmailService(ctx, flags) + if err != nil { + return err + } + + built, err := c.buildForwardComposeMessage(ctx, svc, account, inputs) + if err != nil { + return err + } + + draft, err := svc.Users.Drafts.Create("me", &gmail.Draft{Message: built.message}).Context(ctx).Do() + if err != nil { + return fmt.Errorf("create forward draft: %w", err) + } + + // Forward results intentionally omit attachment metadata, so pass nil here to + // stay consistent with the send path. A forward starts a new thread and + // carries no reply headers, so the threading is empty; writeDraftResult then + // falls back to the thread id Gmail assigns on the Drafts.Create response. + return writeDraftResult(ctx, u, draft, draftThreading{}, nil) +} diff --git a/internal/cmd/gmail_drafts_compose_test.go b/internal/cmd/gmail_drafts_compose_test.go new file mode 100644 index 000000000..a43d0db24 --- /dev/null +++ b/internal/cmd/gmail_drafts_compose_test.go @@ -0,0 +1,821 @@ +package cmd + +import ( + "encoding/base64" + "encoding/json" + "io" + "net/http" + "os" + "path/filepath" + "regexp" + "strings" + "testing" + + "google.golang.org/api/gmail/v1" + + "github.com/openclaw/gogcli/internal/config" +) + +// mockReplySourceMessage returns a gmail.Message JSON payload suitable as the +// target of a reply: it has From/To/Cc/Subject/Message-ID headers and a plain +// text body. +func mockReplySourceMessage() map[string]any { + plain := base64.RawURLEncoding.EncodeToString([]byte("Original plain body.")) + return map[string]any{ + "id": "msg-1", + "threadId": "thread-1", + "payload": map[string]any{ + "mimeType": "text/plain", + "headers": []map[string]any{ + {"name": "Message-ID", "value": ""}, + {"name": "References", "value": ""}, + {"name": "From", "value": `"Alice Sender" `}, + {"name": "To", "value": `"Me Person" , "Other Person" `}, + {"name": "Cc", "value": `"CC Person" `}, + {"name": "Date", "value": "Fri, 12 Jun 2026 10:00:00 +0000"}, + {"name": "Subject", "value": "Project update"}, + }, + "body": map[string]any{"data": plain, "size": len(plain)}, + }, + } +} + +// mockReplySourceMessageWithInlineImage returns a reply target whose HTML body +// references a CID inline image carried as a multipart/related part. The image +// bytes are embedded inline, so no separate attachment fetch is needed. +func mockReplySourceMessageWithInlineImage() map[string]any { + htmlBody := `

Original HTML

` + return map[string]any{ + "id": "msg-1", + "threadId": "thread-1", + "payload": map[string]any{ + "mimeType": "multipart/related", + "headers": []map[string]any{ + {"name": "Message-ID", "value": ""}, + {"name": "References", "value": ""}, + {"name": "From", "value": `"Alice Sender" `}, + {"name": "To", "value": `"Me Person" `}, + {"name": "Date", "value": "Fri, 12 Jun 2026 10:00:00 +0000"}, + {"name": "Subject", "value": "Project update"}, + }, + "parts": []map[string]any{ + { + "mimeType": "multipart/alternative", + "parts": []map[string]any{ + { + "mimeType": "text/plain", + "body": map[string]any{"data": base64.RawURLEncoding.EncodeToString([]byte("Original plain")), "size": 14}, + }, + { + "mimeType": "text/html", + "body": map[string]any{"data": base64.RawURLEncoding.EncodeToString([]byte(htmlBody)), "size": len(htmlBody)}, + }, + }, + }, + { + "mimeType": "image/png", + "filename": "inline.png", + "headers": []map[string]any{ + {"name": "Content-ID", "value": ""}, + {"name": "Content-Disposition", "value": `inline; filename="inline.png"`}, + }, + "body": map[string]any{"data": base64.RawURLEncoding.EncodeToString([]byte("png-data")), "size": 8}, + }, + }, + }, + } +} + +func sendAsListHandler(w http.ResponseWriter) { + _ = json.NewEncoder(w).Encode(map[string]any{ + "sendAs": []map[string]any{ + {"sendAsEmail": "me@example.com", "displayName": "Me Person", "isPrimary": true, "verificationStatus": "accepted"}, + {"sendAsEmail": "alias@example.com", "displayName": "Alias", "verificationStatus": "accepted"}, + }, + }) +} + +// normalizeRawForParity strips the nondeterministic parts of a built RFC822 +// message so two independent builds can be compared byte-for-byte: the Date and +// Message-ID header lines, and the randomly generated MIME boundary tokens +// (gogcli_...). Everything else must match exactly. +func normalizeRawForParity(raw string) string { + dateRE := regexp.MustCompile(`(?m)^Date: .*\r?$`) + msgIDRE := regexp.MustCompile(`(?m)^Message-ID: .*\r?$`) + // Boundaries are "gogcli_" + base64url, so the charset is [A-Za-z0-9_-]. + boundaryRE := regexp.MustCompile(`gogcli_[A-Za-z0-9_-]+`) + raw = dateRE.ReplaceAllString(raw, "Date: NORMALIZED") + raw = msgIDRE.ReplaceAllString(raw, "Message-ID: NORMALIZED") + raw = boundaryRE.ReplaceAllString(raw, "gogcli_BOUNDARY") + return raw +} + +// handleFinalizeRaw services the finalize POST shared by the send and draft +// paths: it decodes the outgoing message (from a Draft body when finalizePath is +// the drafts endpoint, otherwise from a bare Message), writes the canned finalize +// response, and returns the decoded RFC822 raw plus the stamped ThreadId. +func handleFinalizeRaw(t *testing.T, w http.ResponseWriter, r *http.Request, finalizePath string) (raw, threadID string) { + t.Helper() + var msg *gmail.Message + if finalizePath == "/gmail/v1/users/me/drafts" { + var draft gmail.Draft + if err := json.NewDecoder(r.Body).Decode(&draft); err != nil { + t.Fatalf("decode draft: %v", err) + } + msg = draft.Message + _ = json.NewEncoder(w).Encode(map[string]any{"id": "d1", "message": map[string]any{"id": "m1"}}) + } else { + var m gmail.Message + if err := json.NewDecoder(r.Body).Decode(&m); err != nil { + t.Fatalf("decode send: %v", err) + } + msg = &m + _ = json.NewEncoder(w).Encode(map[string]any{"id": "sent-1", "threadId": "thread-1"}) + } + if msg == nil { + t.Fatalf("nil message in finalize body") + } + decoded, err := base64.RawURLEncoding.DecodeString(msg.Raw) + if err != nil { + t.Fatalf("decode raw: %v", err) + } + return string(decoded), msg.ThreadId +} + +// captureReplyRaw runs a reply-style command (either send or draft) against a +// mock Gmail server and returns the raw RFC822 of the outgoing message plus the +// stamped ThreadId. finalizePath is the API path the command finalizes through +// ("/gmail/v1/users/me/messages/send" or "/gmail/v1/users/me/drafts"). source +// supplies the reply-target message payload. +func captureReplyRaw(t *testing.T, args []string, finalizePath string, source func() map[string]any) (raw, threadID string) { + t.Helper() + svc, cleanup := newGmailServiceForTest(t, func(w http.ResponseWriter, r *http.Request) { + w.Header().Set("Content-Type", "application/json") + switch { + case r.Method == http.MethodGet && r.URL.Path == "/gmail/v1/users/me/settings/sendAs": + sendAsListHandler(w) + case r.Method == http.MethodGet && r.URL.Path == "/gmail/v1/users/me/messages/msg-1": + _ = json.NewEncoder(w).Encode(source()) + case r.Method == http.MethodPost && r.URL.Path == finalizePath: + raw, threadID = handleFinalizeRaw(t, w, r, finalizePath) + default: + http.NotFound(w, r) + } + }) + defer cleanup() + + result := executeWithGmailTestService(t, args, svc) + if result.err != nil { + t.Fatalf("Execute(%v): %v", args, result.err) + } + return raw, threadID +} + +// TestGmailDraftsReply_ByteIdenticalToReply proves that gmail reply and gmail +// drafts reply build the exact same outgoing message from identical inputs; the +// only difference is the finalize step (Messages.Send vs Drafts.Create). +func TestGmailDraftsReply_ByteIdenticalToReply(t *testing.T) { + t.Setenv("GOG_TIMEZONE", "UTC") + base := []string{"--account", "me@example.com"} + replyArgs := append(append([]string{}, base...), "gmail", "reply", "msg-1", "--body", "Thanks for the update") + draftArgs := append(append([]string{}, base...), "gmail", "drafts", "reply", "msg-1", "--body", "Thanks for the update") + + sentRaw, sentThread := captureReplyRaw(t, replyArgs, "/gmail/v1/users/me/messages/send", mockReplySourceMessage) + draftRaw, draftThread := captureReplyRaw(t, draftArgs, "/gmail/v1/users/me/drafts", mockReplySourceMessage) + + if normalizeRawForParity(sentRaw) != normalizeRawForParity(draftRaw) { + t.Fatalf("reply vs drafts reply raw differ:\n--- send ---\n%s\n--- draft ---\n%s", sentRaw, draftRaw) + } + if sentThread != draftThread { + t.Fatalf("threadId differs: send=%q draft=%q", sentThread, draftThread) + } + if draftThread != "thread-1" { + t.Fatalf("expected draft to stamp thread-1, got %q", draftThread) + } +} + +// TestGmailDraftsReply_ByteIdenticalToReply_WithInlineImage proves the draft +// path preserves CID inline images identically to the send path: the full +// RFC822 raw (multipart/related with the image part) must match byte-for-byte. +func TestGmailDraftsReply_ByteIdenticalToReply_WithInlineImage(t *testing.T) { + t.Setenv("GOG_TIMEZONE", "UTC") + base := []string{"--account", "me@example.com"} + replyArgs := append(append([]string{}, base...), "gmail", "reply", "msg-1", "--body", "Thanks for the update") + draftArgs := append(append([]string{}, base...), "gmail", "drafts", "reply", "msg-1", "--body", "Thanks for the update") + + sentRaw, _ := captureReplyRaw(t, replyArgs, "/gmail/v1/users/me/messages/send", mockReplySourceMessageWithInlineImage) + draftRaw, _ := captureReplyRaw(t, draftArgs, "/gmail/v1/users/me/drafts", mockReplySourceMessageWithInlineImage) + + if normalizeRawForParity(sentRaw) != normalizeRawForParity(draftRaw) { + t.Fatalf("reply vs drafts reply raw differ (inline image):\n--- send ---\n%s\n--- draft ---\n%s", sentRaw, draftRaw) + } + // Sanity-check the inline image actually rode along on both paths. + for _, want := range []string{"Content-ID: ", "cid:image-1@example.com"} { + if !strings.Contains(draftRaw, want) { + t.Fatalf("drafts reply missing inline-image marker %q:\n%s", want, draftRaw) + } + } +} + +func mockForwardSourceMessage() map[string]any { + return mockOriginalMessage(false) +} + +// captureForwardRaw runs a forward-style command and returns the raw RFC822 and +// stamped ThreadId. source supplies the original-message payload; when it +// references attachmentIds (e.g. mockOriginalMessage(true)), the attachment +// bytes are served from the attachments endpoint. +func captureForwardRaw(t *testing.T, args []string, finalizePath string, source func() map[string]any) (raw, threadID string) { + t.Helper() + svc, cleanup := newGmailServiceForTest(t, func(w http.ResponseWriter, r *http.Request) { + w.Header().Set("Content-Type", "application/json") + switch { + case r.Method == http.MethodGet && r.URL.Path == "/gmail/v1/users/me/settings/sendAs": + sendAsListHandler(w) + case r.Method == http.MethodGet && strings.Contains(r.URL.Path, "/attachments/"): + // Original message attachments (e.g. report.pdf / att-123) re-attached + // on forward. Deterministic bytes so the parity comparison holds. + _ = json.NewEncoder(w).Encode(map[string]any{ + "data": base64.RawURLEncoding.EncodeToString([]byte("pdf-file-contents")), + "size": 100, + }) + case r.Method == http.MethodGet && strings.Contains(r.URL.Path, "/gmail/v1/users/me/messages/orig-msg-1"): + _ = json.NewEncoder(w).Encode(source()) + case r.Method == http.MethodPost && r.URL.Path == finalizePath: + raw, threadID = handleFinalizeRaw(t, w, r, finalizePath) + default: + http.NotFound(w, r) + } + }) + defer cleanup() + + result := executeWithGmailTestService(t, args, svc) + if result.err != nil { + t.Fatalf("Execute(%v): %v", args, result.err) + } + return raw, threadID +} + +func TestGmailDraftsForward_ByteIdenticalToForward(t *testing.T) { + t.Setenv("GOG_TIMEZONE", "UTC") + base := []string{"--account", "me@example.com"} + fwdArgs := append(append([]string{}, base...), "gmail", "forward", "orig-msg-1", "--to", "recipient@example.com", "--note", "FYI") + draftArgs := append(append([]string{}, base...), "gmail", "drafts", "forward", "orig-msg-1", "--to", "recipient@example.com", "--note", "FYI") + + sentRaw, sentThread := captureForwardRaw(t, fwdArgs, "/gmail/v1/users/me/messages/send", mockForwardSourceMessage) + draftRaw, draftThread := captureForwardRaw(t, draftArgs, "/gmail/v1/users/me/drafts", mockForwardSourceMessage) + + if normalizeRawForParity(sentRaw) != normalizeRawForParity(draftRaw) { + t.Fatalf("forward vs drafts forward raw differ:\n--- send ---\n%s\n--- draft ---\n%s", sentRaw, draftRaw) + } + if sentThread != "" || draftThread != "" { + t.Fatalf("forward must not stamp a thread: send=%q draft=%q", sentThread, draftThread) + } + // Forward draft must carry no reply headers and no ThreadId. + for _, h := range []string{"In-Reply-To:", "References:"} { + if strings.Contains(draftRaw, h) { + t.Fatalf("forward draft unexpectedly contains %q:\n%s", h, draftRaw) + } + } +} + +// TestGmailDraftsForward_ByteIdenticalToForward_WithAttachments proves the draft +// path re-attaches the original message's attachments identically to the send +// path: the full RFC822 raw (including the re-attached file part) must match +// byte-for-byte. This also exercises the forward path's nil-metadata wiring. +func TestGmailDraftsForward_ByteIdenticalToForward_WithAttachments(t *testing.T) { + t.Setenv("GOG_TIMEZONE", "UTC") + withAttachment := func() map[string]any { return mockOriginalMessage(true) } + base := []string{"--account", "me@example.com"} + fwdArgs := append(append([]string{}, base...), "gmail", "forward", "orig-msg-1", "--to", "recipient@example.com", "--note", "FYI") + draftArgs := append(append([]string{}, base...), "gmail", "drafts", "forward", "orig-msg-1", "--to", "recipient@example.com", "--note", "FYI") + + sentRaw, _ := captureForwardRaw(t, fwdArgs, "/gmail/v1/users/me/messages/send", withAttachment) + draftRaw, _ := captureForwardRaw(t, draftArgs, "/gmail/v1/users/me/drafts", withAttachment) + + if normalizeRawForParity(sentRaw) != normalizeRawForParity(draftRaw) { + t.Fatalf("forward vs drafts forward raw differ (with attachment):\n--- send ---\n%s\n--- draft ---\n%s", sentRaw, draftRaw) + } + // Sanity-check the attachment actually rode along on the draft path. + if !strings.Contains(draftRaw, "report.pdf") { + t.Fatalf("drafts forward missing re-attached file:\n%s", draftRaw) + } +} + +// --- No-send guard: drafts compose works under no-send; reply is blocked. --- + +func writeNoSendConfig(t *testing.T) { + t.Helper() + setTestConfigHome(t) + if err := defaultConfigStoreForTest(t).Write(config.File{GmailNoSend: true}); err != nil { + t.Fatalf("write no-send config: %v", err) + } +} + +func TestGmailDraftsReply_SucceedsUnderNoSendFlag(t *testing.T) { + created := false + svc, cleanup := newGmailServiceForTest(t, func(w http.ResponseWriter, r *http.Request) { + w.Header().Set("Content-Type", "application/json") + switch { + case r.Method == http.MethodGet && r.URL.Path == "/gmail/v1/users/me/settings/sendAs": + sendAsListHandler(w) + case r.Method == http.MethodGet && r.URL.Path == "/gmail/v1/users/me/messages/msg-1": + _ = json.NewEncoder(w).Encode(mockReplySourceMessage()) + case r.Method == http.MethodPost && r.URL.Path == "/gmail/v1/users/me/drafts": + created = true + _ = json.NewEncoder(w).Encode(map[string]any{"id": "d1", "message": map[string]any{"id": "m1"}}) + case r.Method == http.MethodPost && r.URL.Path == "/gmail/v1/users/me/messages/send": + t.Fatalf("drafts reply must not call Messages.Send") + default: + http.NotFound(w, r) + } + }) + defer cleanup() + + result := executeWithGmailTestService(t, []string{ + "--gmail-no-send", "--account", "me@example.com", + "gmail", "drafts", "reply", "msg-1", "--body", "hi", + }, svc) + if result.err != nil { + t.Fatalf("drafts reply under --gmail-no-send: %v", result.err) + } + if !created { + t.Fatal("expected Drafts.Create to be called") + } +} + +func TestGmailDraftsReplyAll_SucceedsUnderNoSendConfig(t *testing.T) { + writeNoSendConfig(t) + created := false + svc, cleanup := newGmailServiceForTest(t, func(w http.ResponseWriter, r *http.Request) { + w.Header().Set("Content-Type", "application/json") + switch { + case r.Method == http.MethodGet && r.URL.Path == "/gmail/v1/users/me/settings/sendAs": + sendAsListHandler(w) + case r.Method == http.MethodGet && r.URL.Path == "/gmail/v1/users/me/messages/msg-1": + _ = json.NewEncoder(w).Encode(mockReplySourceMessage()) + case r.Method == http.MethodPost && r.URL.Path == "/gmail/v1/users/me/drafts": + created = true + _ = json.NewEncoder(w).Encode(map[string]any{"id": "d1", "message": map[string]any{"id": "m1"}}) + default: + http.NotFound(w, r) + } + }) + defer cleanup() + + result := executeWithGmailTestService(t, []string{ + "--account", "me@example.com", + "gmail", "drafts", "reply-all", "msg-1", "--body", "hi", + }, svc) + if result.err != nil { + t.Fatalf("drafts reply-all under config no-send: %v", result.err) + } + if !created { + t.Fatal("expected Drafts.Create to be called") + } +} + +func TestGmailDraftsForward_SucceedsUnderNoSendFlag(t *testing.T) { + created := false + svc, cleanup := newGmailServiceForTest(t, func(w http.ResponseWriter, r *http.Request) { + w.Header().Set("Content-Type", "application/json") + switch { + case r.Method == http.MethodGet && r.URL.Path == "/gmail/v1/users/me/settings/sendAs": + sendAsListHandler(w) + case r.Method == http.MethodGet && strings.Contains(r.URL.Path, "/gmail/v1/users/me/messages/orig-msg-1"): + _ = json.NewEncoder(w).Encode(mockForwardSourceMessage()) + case r.Method == http.MethodPost && r.URL.Path == "/gmail/v1/users/me/drafts": + created = true + _ = json.NewEncoder(w).Encode(map[string]any{"id": "d1", "message": map[string]any{"id": "m1"}}) + default: + http.NotFound(w, r) + } + }) + defer cleanup() + + result := executeWithGmailTestService(t, []string{ + "--gmail-no-send", "--account", "me@example.com", + "gmail", "drafts", "forward", "orig-msg-1", "--to", "recipient@example.com", + }, svc) + if result.err != nil { + t.Fatalf("drafts forward under --gmail-no-send: %v", result.err) + } + if !created { + t.Fatal("expected Drafts.Create to be called") + } +} + +func TestGmailReply_BlockedUnderNoSendFlag(t *testing.T) { + requests := 0 + svc, cleanup := newGmailServiceForTest(t, func(w http.ResponseWriter, r *http.Request) { + requests++ + http.NotFound(w, r) + }) + defer cleanup() + + result := executeWithGmailTestService(t, []string{ + "--gmail-no-send", "--account", "me@example.com", + "gmail", "reply", "msg-1", "--body", "hi", + }, svc) + if result.err == nil { + t.Fatal("expected gmail reply to be blocked by --gmail-no-send") + } + if !strings.Contains(result.err.Error(), "no-send") { + t.Fatalf("unexpected error: %v", result.err) + } + if requests != 0 { + t.Fatalf("expected no Gmail API requests, got %d", requests) + } +} + +// --- Per-command behavior parity with the send path. --- + +func TestGmailDraftsReply_QuoteByDefaultAndAdditiveRecipients(t *testing.T) { + t.Setenv("GOG_TIMEZONE", "UTC") + raw, threadID := captureReplyRaw(t, []string{ + "--account", "me@example.com", + "gmail", "drafts", "reply", "msg-1", + "--body", "Thanks", + "--cc", "extra@example.com", + "--remove", "cc@example.com", + }, "/gmail/v1/users/me/drafts", mockReplySourceMessage) + + for _, want := range []string{ + "Subject: Re: Project update", + "In-Reply-To: ", + "References: ", + "Original plain body.", // quote-by-default includes the original + "Cc: extra@example.com", + } { + if !strings.Contains(raw, want) { + t.Fatalf("drafts reply missing %q:\n%s", want, raw) + } + } + if strings.Contains(raw, "cc@example.com") { + t.Fatalf("removed Cc recipient still present:\n%s", raw) + } + if threadID != "thread-1" { + t.Fatalf("threadId = %q, want thread-1", threadID) + } +} + +func TestGmailDraftsReplyAll_DerivesRecipients(t *testing.T) { + t.Setenv("GOG_TIMEZONE", "UTC") + raw, _ := captureReplyRaw(t, []string{ + "--account", "me@example.com", + "gmail", "drafts", "reply-all", "msg-1", + "--body", "Thanks", + }, "/gmail/v1/users/me/drafts", mockReplySourceMessage) + + // reply-all addresses the original sender (To) and keeps the other original + // recipients, while dropping the account's own address from To/Cc. + if !strings.Contains(raw, "alice@example.com") { + t.Fatalf("reply-all missing original sender:\n%s", raw) + } + if !strings.Contains(raw, "other@example.com") || !strings.Contains(raw, "cc@example.com") { + t.Fatalf("reply-all missing carried recipients:\n%s", raw) + } + // Self-exclusion: the account address must not appear in the To/Cc recipient + // header fields (it legitimately appears in From:). + headerBlock, _, _ := strings.Cut(raw, "\r\n\r\n") + for _, field := range []string{"To", "Cc"} { + fieldRE := regexp.MustCompile(`(?mi)^` + field + `:.*$`) + for _, line := range fieldRE.FindAllString(headerBlock, -1) { + if strings.Contains(line, "me@example.com") { + t.Fatalf("reply-all leaked account address into %s header: %q", field, line) + } + } + } +} + +func TestGmailDraftsReply_SubjectOverrideClearsThread(t *testing.T) { + raw, threadID := captureReplyRaw(t, []string{ + "--account", "me@example.com", + "gmail", "drafts", "reply", "msg-1", + "--body", "New topic", + "--subject", "Completely different", + "--no-quote", + }, "/gmail/v1/users/me/drafts", mockReplySourceMessage) + + if threadID != "" { + t.Fatalf("edited subject should clear thread, got %q", threadID) + } + if !strings.Contains(raw, "Subject: Completely different") { + t.Fatalf("expected overridden subject:\n%s", raw) + } +} + +func TestGmailDraftsReply_NoQuoteOmitsOriginal(t *testing.T) { + raw, _ := captureReplyRaw(t, []string{ + "--account", "me@example.com", + "gmail", "drafts", "reply", "msg-1", + "--body", "Short reply", + "--no-quote", + }, "/gmail/v1/users/me/drafts", mockReplySourceMessage) + + if strings.Contains(raw, "Original plain body.") { + t.Fatalf("--no-quote should omit original body:\n%s", raw) + } +} + +func TestGmailDraftsForward_BodyFormatAndNoReplyHeaders(t *testing.T) { + t.Setenv("GOG_TIMEZONE", "UTC") + raw, threadID := captureForwardRaw(t, []string{ + "--account", "me@example.com", + "gmail", "drafts", "forward", "orig-msg-1", + "--to", "recipient@example.com", + "--note", "FYI see below", + }, "/gmail/v1/users/me/drafts", mockForwardSourceMessage) + + for _, want := range []string{ + "Subject: Fwd: Original Subject", + "---------- Forwarded message ---------", + "From: Alice ", + "FYI see below", + "Hello, this is the body.", + } { + if !strings.Contains(raw, want) { + t.Fatalf("forward draft missing %q:\n%s", want, raw) + } + } + if threadID != "" { + t.Fatalf("forward draft must not stamp a thread, got %q", threadID) + } + for _, h := range []string{"In-Reply-To:", "References:"} { + if strings.Contains(raw, h) { + t.Fatalf("forward draft unexpectedly contains %q:\n%s", h, raw) + } + } +} + +// --- Addressless forward draft: allowed for draft, rejected for send. --- + +// newForwardDraftCaptureService builds a mock Gmail service for the drafts-forward +// path: it serves sendAs, the orig-msg-1 source message, and captures the Raw of +// the created draft. The returned created/raw pointers are populated when the +// Drafts.Create POST fires. +func newForwardDraftCaptureService(t *testing.T) (svc *gmail.Service, cleanup func(), created *bool, raw *string) { + t.Helper() + created = new(bool) + raw = new(string) + svc, cleanup = newGmailServiceForTest(t, func(w http.ResponseWriter, r *http.Request) { + w.Header().Set("Content-Type", "application/json") + switch { + case r.Method == http.MethodGet && r.URL.Path == "/gmail/v1/users/me/settings/sendAs": + sendAsListHandler(w) + case r.Method == http.MethodGet && strings.Contains(r.URL.Path, "/gmail/v1/users/me/messages/orig-msg-1"): + _ = json.NewEncoder(w).Encode(mockForwardSourceMessage()) + case r.Method == http.MethodPost && r.URL.Path == "/gmail/v1/users/me/drafts": + *created = true + var draft gmail.Draft + if err := json.NewDecoder(r.Body).Decode(&draft); err != nil { + t.Fatalf("decode draft: %v", err) + } + decoded, _ := base64.RawURLEncoding.DecodeString(draft.Message.Raw) + *raw = string(decoded) + _ = json.NewEncoder(w).Encode(map[string]any{"id": "d1", "message": map[string]any{"id": "m1"}}) + default: + http.NotFound(w, r) + } + }) + return svc, cleanup, created, raw +} + +func TestGmailDraftsForward_NoRecipientsSucceeds(t *testing.T) { + t.Setenv("GOG_TIMEZONE", "UTC") + svc, cleanup, created, rawPtr := newForwardDraftCaptureService(t) + defer cleanup() + + result := executeWithGmailTestService(t, []string{ + "--account", "me@example.com", + "gmail", "drafts", "forward", "orig-msg-1", + }, svc) + if result.err != nil { + t.Fatalf("addressless drafts forward: %v", result.err) + } + if !*created { + t.Fatal("expected Drafts.Create to be called") + } + // Inspect only the envelope headers (before the first blank line); the + // forwarded body legitimately quotes the original "To:" header. + headerBlock, _, _ := strings.Cut(*rawPtr, "\r\n\r\n") + if regexp.MustCompile(`(?m)^To:`).MatchString(headerBlock) { + t.Fatalf("addressless draft unexpectedly has To header:\n%s", headerBlock) + } +} + +func TestGmailForward_NoRecipientsStillErrors(t *testing.T) { + requests := 0 + svc, cleanup := newGmailServiceForTest(t, func(w http.ResponseWriter, r *http.Request) { + requests++ + http.NotFound(w, r) + }) + defer cleanup() + + result := executeWithGmailTestService(t, []string{ + "--account", "me@example.com", + "gmail", "forward", "orig-msg-1", + }, svc) + if result.err == nil { + t.Fatal("expected gmail forward to require --to") + } + if !strings.Contains(result.err.Error(), "--to") { + t.Fatalf("unexpected error: %v", result.err) + } +} + +// --- Drafts compose dry-run action names. --- + +func TestGmailDraftsCompose_DryRunActionNames(t *testing.T) { + cases := []struct { + name string + args []string + op string + }{ + {"reply", []string{"gmail", "drafts", "reply", "msg-1", "--body", "hi"}, "gmail.drafts.reply"}, + {"reply-all", []string{"gmail", "drafts", "reply-all", "msg-1", "--body", "hi"}, "gmail.drafts.reply-all"}, + {"forward", []string{"gmail", "drafts", "forward", "orig-msg-1", "--to", "a@example.com"}, "gmail.drafts.forward"}, + // Addressless draft-forward dry-run: the action must fire even with no + // --to (the behavior distinguishing it from send-forward, which requires + // --to before the dry-run). nil service proves the gate is never reached. + {"forward-no-to", []string{"gmail", "drafts", "forward", "orig-msg-1"}, "gmail.drafts.forward"}, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + args := append([]string{"--dry-run", "--account", "me@example.com"}, tc.args...) + result := executeWithGmailTestService(t, args, nil) + if result.err != nil { + t.Fatalf("dry-run: %v", result.err) + } + if !strings.Contains(result.stdout, tc.op) { + t.Fatalf("dry-run output missing action %q:\n%s", tc.op, result.stdout) + } + }) + } +} + +// --- Stdin read-once on the resolve step. --- + +func TestGmailDraftsReply_BodyFileStdinReadOnce(t *testing.T) { + created := false + var raw string + svc, cleanup := newGmailServiceForTest(t, func(w http.ResponseWriter, r *http.Request) { + w.Header().Set("Content-Type", "application/json") + switch { + case r.Method == http.MethodGet && r.URL.Path == "/gmail/v1/users/me/settings/sendAs": + sendAsListHandler(w) + case r.Method == http.MethodGet && r.URL.Path == "/gmail/v1/users/me/messages/msg-1": + _ = json.NewEncoder(w).Encode(mockReplySourceMessage()) + case r.Method == http.MethodPost && r.URL.Path == "/gmail/v1/users/me/drafts": + created = true + var draft gmail.Draft + if err := json.NewDecoder(r.Body).Decode(&draft); err != nil { + t.Fatalf("decode draft: %v", err) + } + decoded, _ := base64.RawURLEncoding.DecodeString(draft.Message.Raw) + raw = string(decoded) + _ = json.NewEncoder(w).Encode(map[string]any{"id": "d1", "message": map[string]any{"id": "m1"}}) + default: + http.NotFound(w, r) + } + }) + defer cleanup() + + ctx := withGmailTestService( + newCmdRuntimeIOContext(t, strings.NewReader("Body from stdin\n"), io.Discard, io.Discard), + svc, + ) + if err := runKong(t, &GmailDraftsReplyCmd{}, []string{"msg-1", "--body-file", "-"}, ctx, &RootFlags{Account: "me@example.com"}); err != nil { + t.Fatalf("execute: %v", err) + } + if !created { + t.Fatal("expected Drafts.Create to be called") + } + if !strings.Contains(raw, "Body from stdin") { + t.Fatalf("expected stdin body in draft:\n%s", raw) + } +} + +func TestGmailDraftsForward_NoteFileStdinReadOnce(t *testing.T) { + t.Setenv("GOG_TIMEZONE", "UTC") + svc, cleanup, created, rawPtr := newForwardDraftCaptureService(t) + defer cleanup() + + ctx := withGmailTestService( + newCmdRuntimeIOContext(t, strings.NewReader("Note from stdin\n"), io.Discard, io.Discard), + svc, + ) + if err := runKong(t, &GmailDraftsForwardCmd{}, []string{"orig-msg-1", "--to", "r@example.com", "--note-file", "-"}, ctx, &RootFlags{Account: "me@example.com"}); err != nil { + t.Fatalf("execute: %v", err) + } + if !*created { + t.Fatal("expected Drafts.Create to be called") + } + if !strings.Contains(*rawPtr, "Note from stdin") { + t.Fatalf("expected stdin note in draft:\n%s", *rawPtr) + } +} + +// --- Signature support on drafts reply (closes a pre-existing gap). --- + +func TestGmailDraftsReply_WithSignatureFile(t *testing.T) { + dir := t.TempDir() + path := filepath.Join(dir, "signature.txt") + if err := os.WriteFile(path, []byte("Local Sig\nhttps://example.com"), 0o600); err != nil { + t.Fatalf("write signature file: %v", err) + } + + var raw string + svc, cleanup := newGmailServiceForTest(t, func(w http.ResponseWriter, r *http.Request) { + w.Header().Set("Content-Type", "application/json") + switch { + case r.Method == http.MethodGet && r.URL.Path == "/gmail/v1/users/me/settings/sendAs": + sendAsListHandler(w) + case r.Method == http.MethodGet && r.URL.Path == "/gmail/v1/users/me/messages/msg-1": + _ = json.NewEncoder(w).Encode(mockReplySourceMessage()) + case r.Method == http.MethodPost && r.URL.Path == "/gmail/v1/users/me/drafts": + var draft gmail.Draft + if err := json.NewDecoder(r.Body).Decode(&draft); err != nil { + t.Fatalf("decode draft: %v", err) + } + decoded, _ := base64.RawURLEncoding.DecodeString(draft.Message.Raw) + raw = string(decoded) + _ = json.NewEncoder(w).Encode(map[string]any{"id": "d1", "message": map[string]any{"id": "m1"}}) + default: + http.NotFound(w, r) + } + }) + defer cleanup() + + result := executeWithGmailTestService(t, []string{ + "--account", "me@example.com", + "gmail", "drafts", "reply", "msg-1", + "--body", "Body", + "--no-quote", + "--signature-file", path, + }, svc) + if result.err != nil { + t.Fatalf("drafts reply with signature: %v", result.err) + } + if !strings.Contains(raw, "Body\r\n\r\n--\r\nLocal Sig\r\nhttps://example.com") { + t.Fatalf("signature missing from draft:\n%s", raw) + } +} + +// --- Drafts.Create failure path: the new error wrappings are surfaced. --- + +func TestGmailDraftsReply_CreateFailureWrapsError(t *testing.T) { + svc, cleanup := newGmailServiceForTest(t, func(w http.ResponseWriter, r *http.Request) { + w.Header().Set("Content-Type", "application/json") + switch { + case r.Method == http.MethodGet && r.URL.Path == "/gmail/v1/users/me/settings/sendAs": + sendAsListHandler(w) + case r.Method == http.MethodGet && r.URL.Path == "/gmail/v1/users/me/messages/msg-1": + _ = json.NewEncoder(w).Encode(mockReplySourceMessage()) + case r.Method == http.MethodPost && r.URL.Path == "/gmail/v1/users/me/drafts": + http.Error(w, `{"error":{"message":"boom"}}`, http.StatusInternalServerError) + default: + http.NotFound(w, r) + } + }) + defer cleanup() + + result := executeWithGmailTestService(t, []string{ + "--account", "me@example.com", + "gmail", "drafts", "reply", "msg-1", "--body", "hi", "--no-quote", + }, svc) + if result.err == nil { + t.Fatal("expected Drafts.Create failure to surface as an error") + } + if !strings.Contains(result.err.Error(), "create reply draft") { + t.Fatalf("error not wrapped with %q: %v", "create reply draft", result.err) + } +} + +func TestGmailDraftsForward_CreateFailureWrapsError(t *testing.T) { + svc, cleanup := newGmailServiceForTest(t, func(w http.ResponseWriter, r *http.Request) { + w.Header().Set("Content-Type", "application/json") + switch { + case r.Method == http.MethodGet && r.URL.Path == "/gmail/v1/users/me/settings/sendAs": + sendAsListHandler(w) + case r.Method == http.MethodGet && strings.Contains(r.URL.Path, "/gmail/v1/users/me/messages/orig-msg-1"): + _ = json.NewEncoder(w).Encode(mockForwardSourceMessage()) + case r.Method == http.MethodPost && r.URL.Path == "/gmail/v1/users/me/drafts": + http.Error(w, `{"error":{"message":"boom"}}`, http.StatusInternalServerError) + default: + http.NotFound(w, r) + } + }) + defer cleanup() + + result := executeWithGmailTestService(t, []string{ + "--account", "me@example.com", + "gmail", "drafts", "forward", "orig-msg-1", "--to", "recipient@example.com", + }, svc) + if result.err == nil { + t.Fatal("expected Drafts.Create failure to surface as an error") + } + if !strings.Contains(result.err.Error(), "create forward draft") { + t.Fatalf("error not wrapped with %q: %v", "create forward draft", result.err) + } +} diff --git a/internal/cmd/gmail_forward.go b/internal/cmd/gmail_forward.go index 1bf978668..3464ebe2c 100644 --- a/internal/cmd/gmail_forward.go +++ b/internal/cmd/gmail_forward.go @@ -20,7 +20,7 @@ type GmailForwardCmd struct { } type GmailForwardOptions struct { - To string `name:"to" help:"Recipients (comma-separated; required)" required:""` + To string `name:"to" help:"Recipients (comma-separated; required when sending, optional when saving a draft)"` Cc string `name:"cc" help:"CC recipients (comma-separated)"` Bcc string `name:"bcc" help:"BCC recipients (comma-separated)"` Note string `name:"note" aliases:"intro" help:"Introductory text above the forwarded message"` @@ -29,6 +29,15 @@ type GmailForwardOptions struct { SkipAttachments bool `name:"skip-attachments" help:"Do not include original attachments"` } +// recipientRequirement records whether a compose path must have recipients. The +// send path requires them; a draft may be saved without any (like Gmail's UI). +type recipientRequirement bool + +const ( + recipientsRequired recipientRequirement = true + recipientsOptional recipientRequirement = false +) + // forwardComposeInputs holds the validated, service-free inputs for a forward // compose. The note is resolved exactly once here because '-' reads stdin, // which cannot be read twice. @@ -36,6 +45,10 @@ type forwardComposeInputs struct { messageID string note string toRecipients []string + // allowMissingTo carries the recipient requirement forward to the build + // step: false on the send path (so buildGmailMessage keeps its missing-To + // backstop), true on the draft path (which permits an addressless forward). + allowMissingTo bool } // forwardComposeMessage carries the built forward message plus the metadata the @@ -48,20 +61,12 @@ type forwardComposeMessage struct { func (c *GmailForwardCmd) Run(ctx context.Context, flags *RootFlags) error { u := ui.FromContext(ctx) - inputs, err := c.resolveForwardInputs(ctx, c.MessageID) + inputs, err := c.resolveForwardInputs(ctx, c.MessageID, recipientsRequired) if err != nil { return err } - if dryRunErr := dryRunExit(ctx, flags, "gmail.forward", map[string]any{ - "message_id": inputs.messageID, - "to": inputs.toRecipients, - "cc": splitCSV(c.Cc), - "bcc": splitCSV(c.Bcc), - "from": strings.TrimSpace(c.From), - "note_len": len(inputs.note), - "skip_attachments": c.SkipAttachments, - }); dryRunErr != nil { + if dryRunErr := dryRunExit(ctx, flags, "gmail.forward", c.dryRunFields(inputs)); dryRunErr != nil { return dryRunErr } @@ -87,29 +92,49 @@ func (c *GmailForwardCmd) Run(ctx context.Context, flags *RootFlags) error { }}) } -// resolveForwardInputs normalizes the message ID, resolves the note input, and -// runs all validation that does not require a Gmail service. It reads the note -// exactly once so '-' (stdin) is consumed a single time. -func (c *GmailForwardOptions) resolveForwardInputs(ctx context.Context, messageID string) (forwardComposeInputs, error) { +// dryRunFields builds the dry-run request dictionary shared by the send-side +// forward and the draft-side forward, so both report the same fields and only +// the action name differs. +func (c *GmailForwardOptions) dryRunFields(inputs forwardComposeInputs) map[string]any { + return map[string]any{ + "message_id": inputs.messageID, + "to": inputs.toRecipients, + "cc": splitCSV(c.Cc), + "bcc": splitCSV(c.Bcc), + "from": strings.TrimSpace(c.From), + "note_len": len(inputs.note), + "skip_attachments": c.SkipAttachments, + } +} + +// resolveForwardInputs normalizes the message ID, runs the service-free +// validation, and resolves the note input. It reads the note +// exactly once so '-' (stdin) is consumed a single time. When req is +// recipientsRequired (the send path) an empty --to is rejected; a draft may have +// no recipients, so the draft path passes recipientsOptional. +func (c *GmailForwardOptions) resolveForwardInputs(ctx context.Context, messageID string, req recipientRequirement) (forwardComposeInputs, error) { messageID = normalizeGmailMessageID(messageID) if messageID == "" { return forwardComposeInputs{}, usage("required: messageId") } + toRecipients := splitCSV(c.To) + if req == recipientsRequired && len(toRecipients) == 0 { + return forwardComposeInputs{}, usage("required: --to") + } + + // Resolve the note after the required-recipient check so a missing --to on + // the send path fails fast without consuming stdin (--note-file -). note, err := resolveBodyInput(ctx, c.Note, c.NoteFile) if err != nil { return forwardComposeInputs{}, err } - toRecipients := splitCSV(c.To) - if len(toRecipients) == 0 { - return forwardComposeInputs{}, usage("required: --to") - } - return forwardComposeInputs{ - messageID: messageID, - note: note, - toRecipients: toRecipients, + messageID: messageID, + note: note, + toRecipients: toRecipients, + allowMissingTo: req == recipientsOptional, }, nil } @@ -166,6 +191,9 @@ func (c *GmailForwardOptions) buildForwardComposeMessage(ctx context.Context, sv ccRecipients := splitCSV(c.Cc) bccRecipients := splitCSV(c.Bcc) + // allowMissingTo comes from the recipient requirement resolved up front: the + // send path keeps buildGmailMessage's missing-To backstop (false), while the + // draft path opts out (true) to permit an addressless forward like Gmail's UI. msg, err := buildGmailMessage(ctx, sendMessageOptions{ FromAddr: from.header, Subject: fwdSubject, @@ -176,7 +204,7 @@ func (c *GmailForwardOptions) buildForwardComposeMessage(ctx context.Context, sv To: inputs.toRecipients, Cc: ccRecipients, Bcc: bccRecipients, - }, false) + }, inputs.allowMissingTo) if err != nil { return forwardComposeMessage{}, fmt.Errorf("build message: %w", err) } diff --git a/internal/cmd/gmail_reply_commands.go b/internal/cmd/gmail_reply_commands.go index 641d1c473..940898db0 100644 --- a/internal/cmd/gmail_reply_commands.go +++ b/internal/cmd/gmail_reply_commands.go @@ -64,6 +64,10 @@ type replyComposeMessage struct { fromHeader string to []string attachmentMetadata []mailmime.AttachmentMetadata + // threading records the reply headers the message was built with, for the + // draft path's result report. The send path reports the sent message's + // thread instead and ignores it. + threading draftThreading } func (c *GmailReplyOptions) run(ctx context.Context, flags *RootFlags, messageID string, replyAll bool) error { @@ -74,23 +78,7 @@ func (c *GmailReplyOptions) run(ctx context.Context, flags *RootFlags, messageID return err } - if dryRunErr := dryRunExit(ctx, flags, "gmail."+replyModeName(replyAll), map[string]any{ - "message_id": inputs.messageID, - "to_add": c.To, - "cc_add": c.Cc, - "bcc_add": c.Bcc, - "remove": c.Remove, - "subject_override": strings.TrimSpace(c.Subject), - "quote": !c.NoQuote, - "from": strings.TrimSpace(c.From), - "auto_from_addressed_alias": c.AutoFromAddressedAlias, - "body_len": len(inputs.body), - "body_html_len": len(inputs.htmlBody), - "attachments": inputs.attachPaths, - "signature": c.Signature, - "signature_from": strings.TrimSpace(c.SignatureFrom), - "signature_file": strings.TrimSpace(c.SignatureFile), - }); dryRunErr != nil { + if dryRunErr := dryRunExit(ctx, flags, "gmail."+replyModeName(replyAll), c.dryRunFields(inputs)); dryRunErr != nil { return dryRunErr } @@ -118,6 +106,29 @@ func (c *GmailReplyOptions) run(ctx context.Context, flags *RootFlags, messageID }}) } +// dryRunFields builds the dry-run request dictionary shared by the send-side +// reply/reply-all and the draft-side reply/reply-all, so both report the same +// fields and only the action name differs. +func (c *GmailReplyOptions) dryRunFields(inputs replyComposeInputs) map[string]any { + return map[string]any{ + "message_id": inputs.messageID, + "to_add": c.To, + "cc_add": c.Cc, + "bcc_add": c.Bcc, + "remove": c.Remove, + "subject_override": strings.TrimSpace(c.Subject), + "quote": !c.NoQuote, + "from": strings.TrimSpace(c.From), + "auto_from_addressed_alias": c.AutoFromAddressedAlias, + "body_len": len(inputs.body), + "body_html_len": len(inputs.htmlBody), + "attachments": inputs.attachPaths, + "signature": c.Signature, + "signature_from": strings.TrimSpace(c.SignatureFrom), + "signature_file": strings.TrimSpace(c.SignatureFile), + } +} + // resolveReplyInputs normalizes the message ID, resolves body/HTML inputs, and // runs all validation that does not require a Gmail service. It reads body // inputs exactly once so '-' (stdin) is consumed a single time. @@ -253,10 +264,20 @@ func (c *GmailReplyOptions) buildReplyComposeMessage(ctx context.Context, svc *g return replyComposeMessage{}, fmt.Errorf("build reply: %w", err) } + threading := draftThreading{ + ThreadID: info.ThreadID, + InReplyTo: strings.TrimSpace(info.InReplyTo), + References: strings.TrimSpace(info.References), + } + if threading.InReplyTo != "" { + threading.Source = replyContextCaller + } + return replyComposeMessage{ message: msg, fromHeader: from.header, to: toRecipients, attachmentMetadata: attachmentMetadata, + threading: threading, }, nil } diff --git a/internal/cmd/safety_profile_test.go b/internal/cmd/safety_profile_test.go index d3ba212c7..06b5745ca 100644 --- a/internal/cmd/safety_profile_test.go +++ b/internal/cmd/safety_profile_test.go @@ -248,8 +248,13 @@ func TestAgentSafeProfileFiltersHelp(t *testing.T) { } }) }) - if !strings.Contains(out, "\n create") { - t.Fatalf("expected create in filtered help, got: %q", out) + // The drafts compose leaves must all be allowed: an unlisted leaf fails + // closed, which would reject drafts reply/reply-all/forward in the + // agent-safe build — the exact audience of those commands. + for _, want := range []string{"\n create", "\n reply", "\n reply-all", "\n forward"} { + if !strings.Contains(out, want) { + t.Fatalf("expected %q in filtered agent-safe help, got: %q", want, out) + } } if strings.Contains(out, "\n send ") { t.Fatalf("expected send to be hidden from agent-safe help, got: %q", out) diff --git a/safety-profiles/agent-safe.yaml b/safety-profiles/agent-safe.yaml index 846c8a704..911d14bbf 100644 --- a/safety-profiles/agent-safe.yaml +++ b/safety-profiles/agent-safe.yaml @@ -39,6 +39,9 @@ gmail: get: true create: true update: true + reply: true + reply-all: true + forward: true delete: false send: false settings: false diff --git a/safety-profiles/readonly.yaml b/safety-profiles/readonly.yaml index 381146d43..362417d9b 100644 --- a/safety-profiles/readonly.yaml +++ b/safety-profiles/readonly.yaml @@ -41,6 +41,9 @@ gmail: get: true create: false update: false + reply: false + reply-all: false + forward: false delete: false send: false settings: false From 612e439b921acfed706bc71ddc555d2eb91f9038 Mon Sep 17 00:00:00 2001 From: Malo Bourgon Date: Mon, 29 Jun 2026 11:51:19 -0400 Subject: [PATCH 3/3] fix(gmail): parse compose recipients address-aware across all commands gmail send, forward, and drafts create/update parsed --to/--cc/--bcc with naive comma-splitting, while reply already parsed them as addresses. The splitting mangled quoted display names ("Smith, John" ) wherever the fragment list was consumed directly: --track counted such a recipient as two and refused to send, --track-split built one message per fragment, and dry-runs reported the broken fragments. Unparseable input was not rejected: send transmitted messages with garbage or empty To headers, and RFC 5322 group syntax ("undisclosed-recipients:;", which parses to zero addresses without an error) was silently dropped or silently saved. Route every compose command through shared address-aware parsing (parseComposeRecipients -> mail.ParseAddressList), with reply's within-flag case-insensitive dedup applying everywhere. Recipients are parsed service-free before the dry-run, so the dry-run reports the same lists the built message carries; malformed input and non-empty flags that parse to zero recipients now fail fast with a flag-named error and no API call, on every compose command. A draft update without --to keeps the existing draft's To header leniently (verbatim if it does not parse), so legacy drafts stay editable. Nameless addresses whose bare form is not valid on the wire (quoted local parts) are re-quoted instead of silently emitted broken. The tracking pixel identity is now always the bare email address, independent of how the recipient was typed. Includes final-review test and comment polish across the compose paths. Co-Authored-By: Claude Fable 5 --- internal/cmd/execute_gmail_forward_test.go | 124 +++++++ .../cmd/execute_gmail_send_recipients_test.go | 334 +++++++++++++++++ internal/cmd/gmail_body_file_newlines_test.go | 2 +- internal/cmd/gmail_drafts.go | 82 +++-- internal/cmd/gmail_drafts_cmd_test.go | 282 +++++++++++++-- internal/cmd/gmail_drafts_compose.go | 23 +- internal/cmd/gmail_drafts_compose_test.go | 339 ++++++++---------- internal/cmd/gmail_forward.go | 45 +-- internal/cmd/gmail_recipients.go | 88 ++++- internal/cmd/gmail_reply.go | 11 +- internal/cmd/gmail_send.go | 63 ++-- internal/cmd/gmail_send_batches_test.go | 145 ++++++++ internal/cmd/gmail_send_test.go | 21 +- internal/cmd/gmail_send_tracking_test.go | 49 +++ internal/cmd/gmail_testutil_test.go | 225 ++++++++++++ internal/mailmime/mime.go | 16 +- 16 files changed, 1534 insertions(+), 315 deletions(-) create mode 100644 internal/cmd/execute_gmail_send_recipients_test.go diff --git a/internal/cmd/execute_gmail_forward_test.go b/internal/cmd/execute_gmail_forward_test.go index b30bebc58..4304a648b 100644 --- a/internal/cmd/execute_gmail_forward_test.go +++ b/internal/cmd/execute_gmail_forward_test.go @@ -383,3 +383,127 @@ func TestFormatForwardedMessageHTML(t *testing.T) { t.Error("missing original HTML content") } } + +// TestExecute_GmailForward_CommaInDisplayName proves that a recipient whose +// display name contains a comma ("Smith, John" ) is parsed as +// a single recipient, not naively split on the comma — across --to, --cc, and +// --bcc (all emitted as real headers) and for both gmail forward and gmail +// drafts forward, which share GmailForwardOptions. +func TestExecute_GmailForward_CommaInDisplayName(t *testing.T) { + t.Setenv("GOG_TIMEZONE", "UTC") + cases := []struct { + name string + verb []string + finalizePath string + }{ + {name: "send", verb: []string{"gmail", "forward"}, finalizePath: "/gmail/v1/users/me/messages/send"}, + {name: "draft", verb: []string{"gmail", "drafts", "forward"}, finalizePath: "/gmail/v1/users/me/drafts"}, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + args := append([]string{"--json", "--account", "me@example.com"}, tc.verb...) + args = append(args, "orig-msg-1", + "--to", `"Smith, John" , other@example.com`, + "--cc", `"Roe, Jane" , z@y.com`, + "--bcc", `"Loe, Jane" , w@y.com`, + ) + raw, _ := captureForwardRaw(t, args, tc.finalizePath, mockForwardSourceMessage) + + assertHeaderRecipients(t, raw, "To", []wantAddr{ + {name: "Smith, John", address: "john@example.com"}, + {address: "other@example.com"}, + }) + assertHeaderRecipients(t, raw, "Cc", []wantAddr{ + {name: "Roe, Jane", address: "jane2@example.com"}, + {address: "z@y.com"}, + }) + assertHeaderRecipients(t, raw, "Bcc", []wantAddr{ + {name: "Loe, Jane", address: "jane3@example.com"}, + {address: "w@y.com"}, + }) + }) + } +} + +// TestExecute_GmailForward_OrdinaryMultiRecipient guards the regression case: +// a plain comma-separated list still yields the expected recipients. +func TestExecute_GmailForward_OrdinaryMultiRecipient(t *testing.T) { + t.Setenv("GOG_TIMEZONE", "UTC") + raw, _ := captureForwardRaw(t, []string{ + "--json", "--account", "me@example.com", + "gmail", "forward", "orig-msg-1", + "--to", "a@x.com, b@y.com", + }, "/gmail/v1/users/me/messages/send", mockForwardSourceMessage) + + assertHeaderRecipients(t, raw, "To", []wantAddr{{address: "a@x.com"}, {address: "b@y.com"}}) +} + +// TestExecute_GmailForward_MalformedRecipientNoAPICall proves a malformed +// recipient on any of --to/--cc/--bcc surfaces a clear, flag-named error before +// any Gmail API request is made (no original-message fetch, no send/draft) — for +// both gmail forward and gmail drafts forward. +func TestExecute_GmailForward_MalformedRecipientNoAPICall(t *testing.T) { + verbs := []struct { + name string + verb []string + }{ + {name: "send", verb: []string{"gmail", "forward"}}, + {name: "draft", verb: []string{"gmail", "drafts", "forward"}}, + } + flags := []string{"--to", "--cc", "--bcc"} + for _, v := range verbs { + for _, flag := range flags { + t.Run(v.name+"/"+flag, func(t *testing.T) { + requests := 0 + svc, cleanup := newGmailServiceForTest(t, func(w http.ResponseWriter, r *http.Request) { + requests++ + http.NotFound(w, r) + }) + defer cleanup() + + args := append([]string{"--account", "me@example.com"}, v.verb...) + // A valid --to keeps the failure attributable to the flag under test + // (the send path requires --to, which is checked after parsing). + args = append(args, "orig-msg-1", "--to", "recipient@example.com", flag, "not an address <<>") + result := executeWithGmailTestService(t, args, svc) + if result.err == nil { + t.Fatalf("expected error for malformed %s", flag) + } + if !strings.Contains(result.err.Error(), flag) { + t.Fatalf("expected %s validation error, got: %v", flag, result.err) + } + if requests != 0 { + t.Fatalf("expected no Gmail API requests, got %d", requests) + } + }) + } + } +} + +// TestExecute_GmailForward_DryRunReportsParsedRecipients proves the forward +// dry-run dict reports --to parsed address-aware (the display-name comma did not +// split into a third element) and that the dry-run makes no API call. +func TestExecute_GmailForward_DryRunReportsParsedRecipients(t *testing.T) { + requests := 0 + svc, cleanup := newGmailServiceForTest(t, func(w http.ResponseWriter, r *http.Request) { + requests++ + http.NotFound(w, r) + }) + defer cleanup() + + result := executeWithGmailTestService(t, []string{ + "--json", "--dry-run", "--account", "me@example.com", + "gmail", "forward", "orig-msg-1", + "--to", `"Smith, John" , other@example.com`, + }, svc) + if code := ExitCode(result.err); code != 0 { + t.Fatalf("expected clean dry-run exit (code 0), got code %d: %v", code, result.err) + } + assertDryRunRequestList(t, result.stdout, "to", []string{ + `"Smith, John" `, + "other@example.com", + }) + if requests != 0 { + t.Fatalf("expected no Gmail API requests, got %d", requests) + } +} diff --git a/internal/cmd/execute_gmail_send_recipients_test.go b/internal/cmd/execute_gmail_send_recipients_test.go new file mode 100644 index 000000000..99839a4d8 --- /dev/null +++ b/internal/cmd/execute_gmail_send_recipients_test.go @@ -0,0 +1,334 @@ +package cmd + +import ( + "encoding/json" + "net/http" + "strings" + "testing" +) + +// assertGmailComposeFailsFast runs a gmail compose command with args against a +// counting mock server and asserts the command fails with an error mentioning +// flag (and wantErr, when non-empty) before any Gmail API request is made. The +// config home is isolated so a developer's real no-send config cannot shadow +// the expected validation error on send-side commands. +func assertGmailComposeFailsFast(t *testing.T, args []string, flag, wantErr string) { + t.Helper() + + setTestConfigHome(t) + requests := 0 + svc, cleanup := newGmailServiceForTest(t, func(w http.ResponseWriter, r *http.Request) { + requests++ + http.NotFound(w, r) + }) + defer cleanup() + + result := executeWithGmailTestService(t, args, svc) + if result.err == nil || !strings.Contains(result.err.Error(), flag) { + t.Fatalf("expected %s validation error, got: %v", flag, result.err) + } + if wantErr != "" && !strings.Contains(result.err.Error(), wantErr) { + t.Fatalf("expected error containing %q, got: %v", wantErr, result.err) + } + if requests != 0 { + t.Fatalf("expected no Gmail API requests, got %d", requests) + } +} + +// TestExecute_GmailSend_CommaInDisplayName proves that a gmail send recipient +// whose display name contains a comma ("Smith, John" ) is +// parsed as a single recipient, not naively split on the comma — across --to, +// --cc, and --bcc. +func TestExecute_GmailSend_CommaInDisplayName(t *testing.T) { + raw, _ := captureComposeRaw(t, []string{ + "--json", "--account", "me@example.com", + "gmail", "send", + "--to", `"Smith, John" , other@example.com`, + "--cc", `"Day, Ada" , c2@y.com`, + "--bcc", `"Poe, Edgar" , b2@y.com`, + "--subject", "Hi", + "--body", "Hello", + }, "/gmail/v1/users/me/messages/send", mockReplySourceMessage) + + assertHeaderRecipients(t, raw, "To", []wantAddr{ + {name: "Smith, John", address: "john@example.com"}, + {address: "other@example.com"}, + }) + assertHeaderRecipients(t, raw, "Cc", []wantAddr{ + {name: "Day, Ada", address: "ada@example.com"}, + {address: "c2@y.com"}, + }) + assertHeaderRecipients(t, raw, "Bcc", []wantAddr{ + {name: "Poe, Edgar", address: "edgar@example.com"}, + {address: "b2@y.com"}, + }) +} + +// TestExecute_GmailSend_OrdinaryMultiRecipient guards the regression case: a +// plain comma-separated --to still yields the expected recipients. +func TestExecute_GmailSend_OrdinaryMultiRecipient(t *testing.T) { + raw, _ := captureComposeRaw(t, []string{ + "--json", "--account", "me@example.com", + "gmail", "send", + "--to", "a@x.com, b@y.com", + "--subject", "Hi", + "--body", "Hello", + }, "/gmail/v1/users/me/messages/send", mockReplySourceMessage) + + assertHeaderRecipients(t, raw, "To", []wantAddr{{address: "a@x.com"}, {address: "b@y.com"}}) +} + +// TestExecute_GmailSend_QuotedLocalPartRoundTrips proves an RFC-valid quoted +// local part ("john smith"@example.com) survives to the wire in re-parseable +// form: mail.ParseAddressList strips the quotes internally, and emitting the +// stripped form bare would produce an invalid To header. +func TestExecute_GmailSend_QuotedLocalPartRoundTrips(t *testing.T) { + raw, _ := captureComposeRaw(t, []string{ + "--json", "--account", "me@example.com", + "gmail", "send", + "--to", `"john smith"@example.com`, + "--subject", "Hi", + "--body", "Hello", + }, "/gmail/v1/users/me/messages/send", mockReplySourceMessage) + + // assertHeaderRecipients re-parses the built header, so it fails if the + // quoting was lost. + assertHeaderRecipients(t, raw, "To", []wantAddr{{address: "john smith@example.com"}}) +} + +// TestExecute_GmailCompose_GroupSyntaxRejectedEverywhere proves the zero-parse +// guard is shared by every recipient-flag consumer, not just gmail send: group +// syntax on forward (send and draft verbs), drafts create/update, and reply +// (--to and --remove) errors with the same flag-named message and makes no API +// call, instead of silently dropping the value. +func TestExecute_GmailCompose_GroupSyntaxRejectedEverywhere(t *testing.T) { + const group = "undisclosed-recipients:;" + cases := []struct { + name string + args []string + flag string + }{ + { + name: "forward to", + args: []string{"gmail", "forward", "orig-msg-1", "--to", group}, + flag: "--to", + }, + { + name: "forward cc", + args: []string{"gmail", "forward", "orig-msg-1", "--to", "r@example.com", "--cc", group}, + flag: "--cc", + }, + { + name: "forward bcc", + args: []string{"gmail", "forward", "orig-msg-1", "--to", "r@example.com", "--bcc", group}, + flag: "--bcc", + }, + { + name: "drafts forward to", + args: []string{"gmail", "drafts", "forward", "orig-msg-1", "--to", group}, + flag: "--to", + }, + { + name: "drafts create to", + args: []string{"gmail", "drafts", "create", "--subject", "S", "--body", "B", "--to", group}, + flag: "--to", + }, + { + name: "drafts create cc", + args: []string{"gmail", "drafts", "create", "--subject", "S", "--body", "B", "--cc", group}, + flag: "--cc", + }, + { + name: "drafts update to", + args: []string{"gmail", "drafts", "update", "d1", "--subject", "S", "--body", "B", "--to", group}, + flag: "--to", + }, + { + name: "drafts update cc", + args: []string{"gmail", "drafts", "update", "d1", "--subject", "S", "--body", "B", "--cc", group}, + flag: "--cc", + }, + { + name: "reply to", + args: []string{"gmail", "reply", "msg-1", "--body", "B", "--to", group}, + flag: "--to", + }, + { + name: "reply remove", + args: []string{"gmail", "reply", "msg-1", "--body", "B", "--remove", group}, + flag: "--remove", + }, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + args := append([]string{"--account", "me@example.com"}, tc.args...) + assertGmailComposeFailsFast(t, args, tc.flag, "contains no recipients") + }) + } +} + +// TestExecute_GmailSend_MalformedRecipientNoAPICall proves a malformed +// recipient on any of --to/--cc/--bcc surfaces a clear, flag-named error +// before any Gmail API request is made (no sender resolution, no send). +func TestExecute_GmailSend_MalformedRecipientNoAPICall(t *testing.T) { + cases := []struct { + name string + flag string + value string + }{ + {name: "to", flag: "--to", value: "not an address <<>"}, + {name: "cc", flag: "--cc", value: "not an address <<>"}, + {name: "bcc", flag: "--bcc", value: "not an address <<>"}, + {name: "to commas only", flag: "--to", value: ", ,"}, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + args := []string{ + "--account", "me@example.com", + "gmail", "send", + "--subject", "Hi", + "--body", "Hello", + tc.flag, tc.value, + } + if tc.flag != "--to" { + // A valid --to keeps the failure attributable to the flag under + // test (--to is required on the send path). + args = append(args, "--to", "recipient@example.com") + } + assertGmailComposeFailsFast(t, args, tc.flag, "") + }) + } +} + +// TestExecute_GmailSend_GroupSyntaxRejectedBeforeDryRun proves RFC 5322 group +// syntax ("undisclosed-recipients:;"), which parses to zero addresses without +// a parse error, is rejected up front with a flag-named error in both plain +// and --reply-all modes. The --dry-run flag pins the ordering: without the +// check the dry-run would exit 0 reporting an empty list while the real send +// failed late, after service acquisition. +func TestExecute_GmailSend_GroupSyntaxRejectedBeforeDryRun(t *testing.T) { + const group = "undisclosed-recipients:;" + cases := []struct { + name string + args []string + flag string + }{ + { + name: "plain to", + args: []string{"--to", group}, + flag: "--to", + }, + { + name: "plain cc", + args: []string{"--to", "recipient@example.com", "--cc", group}, + flag: "--cc", + }, + { + name: "reply-all to", + args: []string{"--reply-all", "--reply-to-message-id", "msg-1", "--to", group}, + flag: "--to", + }, + { + name: "reply-all cc", + args: []string{"--reply-all", "--reply-to-message-id", "msg-1", "--cc", group}, + flag: "--cc", + }, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + args := append([]string{ + "--dry-run", + "--account", "me@example.com", + "gmail", "send", + "--subject", "Hi", + "--body", "Hello", + }, tc.args...) + assertGmailComposeFailsFast(t, args, tc.flag, "contains no recipients") + }) + } +} + +// TestExecute_GmailSend_ReplyAllExplicitToOverrides proves an explicit --to +// still replaces (not merges with) the auto-populated reply-all To now that it +// is parsed address-aware, while the untouched Cc keeps the auto-populated Cc +// from the original message. +func TestExecute_GmailSend_ReplyAllExplicitToOverrides(t *testing.T) { + raw, _ := captureComposeRaw(t, []string{ + "--json", "--account", "me@example.com", + "gmail", "send", + "--reply-all", "--reply-to-message-id", "msg-1", + "--to", `"Smith, John" `, + "--body", "Hello", + }, "/gmail/v1/users/me/messages/send", mockReplySourceMessage) + + assertHeaderRecipients(t, raw, "To", []wantAddr{{name: "Smith, John", address: "john@example.com"}}) + assertHeaderRecipients(t, raw, "Cc", []wantAddr{{name: "CC Person", address: "cc@example.com"}}) +} + +// TestExecute_GmailSend_ReplyAllExplicitCcOverrides proves an explicit --cc +// replaces (not merges with) the auto-populated reply-all Cc while the +// untouched To keeps the auto-populated reply-all recipients (original sender +// plus its To minus self). +func TestExecute_GmailSend_ReplyAllExplicitCcOverrides(t *testing.T) { + raw, _ := captureComposeRaw(t, []string{ + "--json", "--account", "me@example.com", + "gmail", "send", + "--reply-all", "--reply-to-message-id", "msg-1", + "--cc", "x@y.com", + "--body", "Hello", + }, "/gmail/v1/users/me/messages/send", mockReplySourceMessage) + + assertHeaderRecipients(t, raw, "To", []wantAddr{ + {name: "Alice Sender", address: "alice@example.com"}, + {name: "Other Person", address: "other@example.com"}, + }) + assertHeaderRecipients(t, raw, "Cc", []wantAddr{{address: "x@y.com"}}) +} + +// TestExecute_GmailSend_DryRunReportsParsedRecipients proves the send dry-run +// dict reports the recipient flags parsed address-aware (the display-name +// comma did not split into an extra element), that an omitted flag serializes +// as null (the pre-parse wire shape), and that the dry-run makes no API call — +// the reported lists are the same ones the built message would carry. +func TestExecute_GmailSend_DryRunReportsParsedRecipients(t *testing.T) { + requests := 0 + svc, cleanup := newGmailServiceForTest(t, func(w http.ResponseWriter, r *http.Request) { + requests++ + http.NotFound(w, r) + }) + defer cleanup() + + result := executeWithGmailTestService(t, []string{ + "--json", "--dry-run", "--account", "me@example.com", + "gmail", "send", + "--to", `"Smith, John" , other@example.com`, + "--bcc", `"Roe, Jane" `, + "--subject", "Hi", + "--body", "Hello", + }, svc) + if code := ExitCode(result.err); code != 0 { + t.Fatalf("expected clean dry-run exit (code 0), got code %d: %v", code, result.err) + } + + assertDryRunRequestList(t, result.stdout, "to", []string{ + `"Smith, John" `, + "other@example.com", + }) + assertDryRunRequestList(t, result.stdout, "bcc", []string{`"Roe, Jane" `}) + + // The omitted --cc must serialize as null (nil slice), not [] — the wire + // shape the pre-parse splitCSV code produced. + var payload struct { + Request map[string]any `json:"request"` + } + if err := json.Unmarshal([]byte(result.stdout), &payload); err != nil { + t.Fatalf("decode dry-run output: %v\n%s", err, result.stdout) + } + if v, ok := payload.Request["cc"]; !ok || v != nil { + t.Fatalf("expected omitted --cc to be null, got %#v", v) + } + + if requests != 0 { + t.Fatalf("expected no Gmail API requests, got %d", requests) + } +} diff --git a/internal/cmd/gmail_body_file_newlines_test.go b/internal/cmd/gmail_body_file_newlines_test.go index b4c00328a..8c15884e6 100644 --- a/internal/cmd/gmail_body_file_newlines_test.go +++ b/internal/cmd/gmail_body_file_newlines_test.go @@ -147,7 +147,7 @@ func TestGmailForward_NoteFilePreservesTrailingNewlines(t *testing.T) { cmd := &GmailForwardCmd{ MessageID: "msg1", - GmailForwardOptions: GmailForwardOptions{ + Options: GmailForwardOptions{ To: "x@example.com", NoteFile: notePath, }, diff --git a/internal/cmd/gmail_drafts.go b/internal/cmd/gmail_drafts.go index 08dbbecc7..5085fc19c 100644 --- a/internal/cmd/gmail_drafts.go +++ b/internal/cmd/gmail_drafts.go @@ -305,7 +305,12 @@ type draftComposeInput struct { Attach []string // PrebuiltAttachments carry already-resolved attachment bytes (e.g. existing // draft attachments preserved across an update) alongside any --attach paths. - PrebuiltAttachments []mailmime.Attachment + PrebuiltAttachments []mailmime.Attachment + // KeptToRecipients carries the existing draft's To recipients when an + // update omits --to (kept-existing). They bypass the strict flag parse: + // the header was already resolved by keptDraftRecipients, so a + // legacy-malformed header cannot fail a body-only edit. + KeptToRecipients []string From string AutoFromAddressedAlias bool } @@ -327,6 +332,22 @@ func (c draftComposeInput) validate() error { return nil } +// keptDraftRecipients converts an existing draft's To header into the +// recipient list for a rebuild when an update omits --to. It prefers the +// address-aware parse (display-name commas stay one recipient) but must not +// reject a draft that predates strict parsing: on any parse failure it falls +// back to splitCSV's verbatim fragments, so a body-only edit of a +// legacy-malformed header (e.g. "alice@", or Outlook-style semicolon +// separators) cannot fail. The kept fragments then render exactly as they +// did before this parser: the MIME writer passes an unextractable value +// through verbatim and normalizes one it can extract addresses from. +func keptDraftRecipients(header string) []string { + if recipients, err := parseRecipientCSV("--to", header); err == nil { + return recipients + } + return splitCSV(header) +} + func buildDraftMessage(ctx context.Context, svc *gmail.Service, account string, input draftComposeInput) (*gmail.Message, draftThreading, []mailmime.AttachmentMetadata, error) { sendAs, sendAsErr := listSendAs(ctx, svc) from, err := resolveComposeFrom(ctx, svc, account, input.From, sendAs, sendAsErr) @@ -384,9 +405,16 @@ func buildDraftMessage(ctx context.Context, svc *gmail.Service, account string, subject = autoReplySubject("", info.Subject) } - toRecipients := splitCSV(input.To) - ccRecipients := splitCSV(input.Cc) - bccRecipients := splitCSV(input.Bcc) + toRecipients, ccRecipients, bccRecipients, err := parseComposeRecipients(input.To, input.Cc, input.Bcc) + if err != nil { + return nil, draftThreading{}, nil, err + } + // A kept-existing To (update without --to) was resolved leniently by + // keptDraftRecipients; input.To is empty then, so the parse above yields + // nil and the kept recipients take over. + if len(input.KeptToRecipients) > 0 { + toRecipients = input.KeptToRecipients + } if input.ReplyAll { recipients, recipientErr := buildReplyRecipients( info, @@ -400,17 +428,16 @@ func buildDraftMessage(ctx context.Context, svc *gmail.Service, account string, if recipientErr != nil { return nil, draftThreading{}, nil, recipientErr } - toRecipients = formatMailboxes(recipients.To) - ccRecipients = formatMailboxes(recipients.Cc) - bccRecipients = formatMailboxes(recipients.Bcc) - if strings.TrimSpace(input.To) != "" { - toRecipients = splitCSV(input.To) + // --reply-all auto-populates from the original message, but an explicit + // flag (parsed above) overrides the corresponding auto-populated field. + if strings.TrimSpace(input.To) == "" { + toRecipients = formatMailboxes(recipients.To) } - if strings.TrimSpace(input.Cc) != "" { - ccRecipients = splitCSV(input.Cc) + if strings.TrimSpace(input.Cc) == "" { + ccRecipients = formatMailboxes(recipients.Cc) } - if strings.TrimSpace(input.Bcc) != "" { - bccRecipients = splitCSV(input.Bcc) + if strings.TrimSpace(input.Bcc) == "" { + bccRecipients = formatMailboxes(recipients.Bcc) } } @@ -731,10 +758,16 @@ func (c *GmailDraftsCreateCmd) Run(ctx context.Context, flags *RootFlags) error return headerErr } + // Parsed identically in buildDraftMessage, so the dry-run reports the built lists. + toRecipients, ccRecipients, bccRecipients, err := parseComposeRecipients(input.To, input.Cc, input.Bcc) + if err != nil { + return err + } + if dryRunErr := dryRunExit(ctx, flags, "gmail.drafts.create", map[string]any{ - "to": splitCSV(input.To), - "cc": splitCSV(input.Cc), - "bcc": splitCSV(input.Bcc), + "to": toRecipients, + "cc": ccRecipients, + "bcc": bccRecipients, "subject": strings.TrimSpace(input.Subject), "body_len": len(input.Body), "body_html_len": len(input.BodyHTML), @@ -852,12 +885,20 @@ func (c *GmailDraftsUpdateCmd) Run(ctx context.Context, flags *RootFlags) error return headerErr } + // Parsed identically in buildDraftMessage, so the dry-run reports the built + // lists. (A kept-existing To is empty here; it is resolved later from the + // draft's own header, leniently, via keptDraftRecipients.) + toRecipients, ccRecipients, bccRecipients, err := parseComposeRecipients(input.To, input.Cc, input.Bcc) + if err != nil { + return err + } + if dryRunErr := dryRunExit(ctx, flags, "gmail.drafts.update", map[string]any{ "draft_id": draftID, "to_keep_existing": !toWasSet && !input.ReplyAll, - "to": splitCSV(input.To), - "cc": splitCSV(input.Cc), - "bcc": splitCSV(input.Bcc), + "to": toRecipients, + "cc": ccRecipients, + "bcc": bccRecipients, "subject": strings.TrimSpace(input.Subject), "body_len": len(input.Body), "body_html_len": len(input.BodyHTML), @@ -916,7 +957,7 @@ func (c *GmailDraftsUpdateCmd) Run(ctx context.Context, flags *RootFlags) error } } if !toWasSet && !c.ReplyAll { - to = existingTo + input.KeptToRecipients = keptDraftRecipients(existingTo) } // gmail drafts update rebuilds the whole message, so updating a rich-text @@ -972,7 +1013,6 @@ func (c *GmailDraftsUpdateCmd) Run(ctx context.Context, flags *RootFlags) error carriedReferences = existingReferences } - input.To = to input.ReplyToMessageID = replyToMessageID input.ReplyToThreadID = replyToThreadID input.ThreadContinuityID = targetThreadID diff --git a/internal/cmd/gmail_drafts_cmd_test.go b/internal/cmd/gmail_drafts_cmd_test.go index 5194e19cc..356095a3c 100644 --- a/internal/cmd/gmail_drafts_cmd_test.go +++ b/internal/cmd/gmail_drafts_cmd_test.go @@ -412,41 +412,11 @@ func TestGmailDraftsCreateCmd_BodyHTMLFile(t *testing.T) { t.Fatalf("write html: %v", err) } - var rawCreated string - srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - if strings.Contains(r.URL.Path, "/gmail/v1/users/me/drafts") && r.Method == http.MethodPost { - var draft gmail.Draft - if err := json.NewDecoder(r.Body).Decode(&draft); err != nil { - t.Fatalf("decode draft: %v", err) - } - if draft.Message == nil { - t.Fatalf("expected message in create") - } - raw, err := base64.RawURLEncoding.DecodeString(draft.Message.Raw) - if err != nil { - t.Fatalf("decode raw: %v", err) - } - rawCreated = string(raw) - w.Header().Set("Content-Type", "application/json") - _ = json.NewEncoder(w).Encode(map[string]any{ - "id": "d-html", - "message": map[string]any{"id": "m-html"}, - }) - return - } - http.NotFound(w, r) - })) - defer srv.Close() - - svc := newGmailServiceFromServer(t, srv) - ctx := withGmailTestService(newCmdRuntimeOutputContext(t, io.Discard, io.Discard), svc) - if err := runKong(t, &GmailDraftsCreateCmd{}, []string{ + rawCreated := captureDraftCreateRaw(t, []string{ "--to", "a@example.com", "--subject", "Hello", "--body-html-file", htmlPath, - }, ctx, &RootFlags{Account: "a@b.com"}); err != nil { - t.Fatalf("execute: %v", err) - } + }) if !strings.Contains(rawCreated, "Content-Type: text/html") || !strings.Contains(rawCreated, "

Hello

") { @@ -828,15 +798,28 @@ func TestGmailDraftsUpdateCmd_JSON(t *testing.T) { } } -func TestGmailDraftsUpdateCmd_BodyHTMLFileFromStdin(t *testing.T) { - var rawUpdated string +// newDraftUpdateCaptureServer builds an httptest server for the drafts-update +// path: GET draft/d1 returns a thread-bound draft (with existingTo as its To +// header when non-empty), GET threads/t1 returns the thread's Message-ID, and +// PUT draft/d1 captures the Raw of the updated message into the returned +// pointer. It is the shared mock for update tests that only assert on the PUT +// body. +func newDraftUpdateCaptureServer(t *testing.T, existingTo string) (*httptest.Server, *string) { + t.Helper() + rawUpdated := new(string) + message := map[string]any{"id": "m1", "threadId": "t1"} + if existingTo != "" { + message["payload"] = map[string]any{ + "headers": []map[string]any{{"name": "To", "value": existingTo}}, + } + } srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { switch { case strings.Contains(r.URL.Path, "/gmail/v1/users/me/drafts/d1") && r.Method == http.MethodGet: w.Header().Set("Content-Type", "application/json") _ = json.NewEncoder(w).Encode(map[string]any{ "id": "d1", - "message": map[string]any{"id": "m1", "threadId": "t1"}, + "message": message, }) return case strings.Contains(r.URL.Path, "/gmail/v1/users/me/threads/t1") && r.Method == http.MethodGet: @@ -868,7 +851,7 @@ func TestGmailDraftsUpdateCmd_BodyHTMLFileFromStdin(t *testing.T) { if err != nil { t.Fatalf("decode raw: %v", err) } - rawUpdated = string(raw) + *rawUpdated = string(raw) w.Header().Set("Content-Type", "application/json") _ = json.NewEncoder(w).Encode(map[string]any{ "id": "d1", @@ -880,6 +863,11 @@ func TestGmailDraftsUpdateCmd_BodyHTMLFileFromStdin(t *testing.T) { return } })) + return srv, rawUpdated +} + +func TestGmailDraftsUpdateCmd_BodyHTMLFileFromStdin(t *testing.T) { + srv, rawUpdatedPtr := newDraftUpdateCaptureServer(t, "") defer srv.Close() svc := newGmailServiceFromServer(t, srv) @@ -896,9 +884,9 @@ func TestGmailDraftsUpdateCmd_BodyHTMLFileFromStdin(t *testing.T) { t.Fatalf("execute: %v", err) } - if !strings.Contains(rawUpdated, "Content-Type: text/html") || - !strings.Contains(rawUpdated, "

Updated

") { - t.Fatalf("expected HTML stdin body in updated draft, got:\n%s", rawUpdated) + if !strings.Contains(*rawUpdatedPtr, "Content-Type: text/html") || + !strings.Contains(*rawUpdatedPtr, "

Updated

") { + t.Fatalf("expected HTML stdin body in updated draft, got:\n%s", *rawUpdatedPtr) } } @@ -2249,3 +2237,219 @@ func TestGmailDraftsUpdateCmd_AttachAndClearMutuallyExclusive(t *testing.T) { t.Fatalf("expected mutual-exclusion error, got %v", err) } } + +// captureDraftCreateRaw runs gmail drafts create against a mock server and +// returns the raw RFC822 of the created draft message. +func captureDraftCreateRaw(t *testing.T, args []string) string { + t.Helper() + var raw string + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if strings.Contains(r.URL.Path, "/gmail/v1/users/me/drafts") && r.Method == http.MethodPost { + var draft gmail.Draft + if err := json.NewDecoder(r.Body).Decode(&draft); err != nil { + t.Fatalf("decode draft: %v", err) + } + if draft.Message == nil { + t.Fatalf("expected message in create") + } + decoded, err := base64.RawURLEncoding.DecodeString(draft.Message.Raw) + if err != nil { + t.Fatalf("decode raw: %v", err) + } + raw = string(decoded) + w.Header().Set("Content-Type", "application/json") + _ = json.NewEncoder(w).Encode(map[string]any{"id": "d1", "message": map[string]any{"id": "m1"}}) + return + } + http.NotFound(w, r) + })) + defer srv.Close() + + svc := newGmailServiceFromServer(t, srv) + ctx := withGmailTestService(newCmdRuntimeOutputContext(t, io.Discard, io.Discard), svc) + if err := runKong(t, &GmailDraftsCreateCmd{}, args, ctx, &RootFlags{Account: "a@b.com"}); err != nil { + t.Fatalf("execute(%v): %v", args, err) + } + return raw +} + +// TestGmailDraftsCreateCmd_CommaInDisplayName proves a recipient whose display +// name contains a comma ("Doe, Jane" ) stays a single +// recipient instead of being naively split on the comma — across --to, --cc, +// and --bcc. +func TestGmailDraftsCreateCmd_CommaInDisplayName(t *testing.T) { + raw := captureDraftCreateRaw(t, []string{ + "--to", `"Doe, Jane" , x@y.com`, + "--cc", `"Roe, Jane" , z@y.com`, + "--bcc", `"Loe, Jane" , w@y.com`, + "--subject", "Hi", + "--body", "Hello", + }) + + assertHeaderRecipients(t, raw, "To", []wantAddr{ + {name: "Doe, Jane", address: "jane@example.com"}, + {address: "x@y.com"}, + }) + assertHeaderRecipients(t, raw, "Cc", []wantAddr{ + {name: "Roe, Jane", address: "jane2@example.com"}, + {address: "z@y.com"}, + }) + assertHeaderRecipients(t, raw, "Bcc", []wantAddr{ + {name: "Loe, Jane", address: "jane3@example.com"}, + {address: "w@y.com"}, + }) +} + +// TestGmailDraftsCreateCmd_OrdinaryMultiRecipient guards the regression case for +// a plain comma-separated recipient list. +func TestGmailDraftsCreateCmd_OrdinaryMultiRecipient(t *testing.T) { + raw := captureDraftCreateRaw(t, []string{ + "--to", "a@x.com, b@y.com", + "--subject", "Hi", + "--body", "Hello", + }) + assertHeaderRecipients(t, raw, "To", []wantAddr{{address: "a@x.com"}, {address: "b@y.com"}}) +} + +// TestGmailDraftsCreateCmd_MalformedRecipientNoAPICall proves a malformed +// recipient on any of --to/--cc/--bcc surfaces a clear, flag-named error and +// makes no Gmail API request. +func TestGmailDraftsCreateCmd_MalformedRecipientNoAPICall(t *testing.T) { + for _, flag := range []string{"--to", "--cc", "--bcc"} { + t.Run(flag, func(t *testing.T) { + requests := 0 + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + requests++ + http.NotFound(w, r) + })) + defer srv.Close() + + svc := newGmailServiceFromServer(t, srv) + ctx := withGmailTestService(newCmdRuntimeOutputContext(t, io.Discard, io.Discard), svc) + // A valid --to keeps the failure attributable to the flag under test. + err := runKong(t, &GmailDraftsCreateCmd{}, []string{ + "--to", "recipient@example.com", flag, "not an address <<>", + "--subject", "Hi", + "--body", "Hello", + }, ctx, &RootFlags{Account: "a@b.com"}) + if err == nil { + t.Fatalf("expected error for malformed %s", flag) + } + if !strings.Contains(err.Error(), flag) { + t.Fatalf("expected %s validation error, got: %v", flag, err) + } + if requests != 0 { + t.Fatalf("expected no Gmail API requests, got %d", requests) + } + }) + } +} + +// TestGmailDraftsUpdateCmd_CommaInDisplayName proves the update path (which +// shares parseComposeRecipients) parses an explicit --to with a comma in the +// display name address-aware: the PUT body's To header holds exactly two +// recipients with the "Doe, Jane" display name intact. +func TestGmailDraftsUpdateCmd_CommaInDisplayName(t *testing.T) { + srv, rawUpdatedPtr := newDraftUpdateCaptureServer(t, "") + defer srv.Close() + + svc := newGmailServiceFromServer(t, srv) + ctx := withGmailTestService(newCmdRuntimeOutputContext(t, io.Discard, io.Discard), svc) + if err := runKong(t, &GmailDraftsUpdateCmd{}, []string{ + "d1", + "--to", `"Doe, Jane" , x@y.com`, + "--subject", "Updated", + "--body", "Hello", + }, ctx, &RootFlags{Account: "a@b.com"}); err != nil { + t.Fatalf("execute: %v", err) + } + + assertHeaderRecipients(t, *rawUpdatedPtr, "To", []wantAddr{ + {name: "Doe, Jane", address: "jane@example.com"}, + {address: "x@y.com"}, + }) +} + +// TestGmailDraftsUpdateCmd_KeepsLegacyMalformedTo proves a body-only update of +// a draft whose existing To header predates strict parsing (e.g. "alice@", +// creatable by older gogcli) succeeds and preserves the header verbatim, +// instead of failing with a --to validation error for a flag the user never +// passed. +func TestGmailDraftsUpdateCmd_KeepsLegacyMalformedTo(t *testing.T) { + srv, rawUpdatedPtr := newDraftUpdateCaptureServer(t, "alice@") + defer srv.Close() + + svc := newGmailServiceFromServer(t, srv) + ctx := withGmailTestService(newCmdRuntimeOutputContext(t, io.Discard, io.Discard), svc) + if err := runKong(t, &GmailDraftsUpdateCmd{}, []string{ + "d1", "--subject", "S", "--body", "B", + }, ctx, &RootFlags{Account: "a@b.com"}); err != nil { + t.Fatalf("body-only update with legacy To: %v", err) + } + if !strings.Contains(*rawUpdatedPtr, "To: alice@\r\n") { + t.Fatalf("expected legacy To header preserved verbatim, got:\n%s", *rawUpdatedPtr) + } +} + +// TestGmailDraftsCreateCmd_DryRunReportsParsedRecipients proves the create +// dry-run dict reports --to parsed address-aware (no third element from the +// display-name comma) and makes no API call. +func TestGmailDraftsCreateCmd_DryRunReportsParsedRecipients(t *testing.T) { + requests := 0 + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + requests++ + http.NotFound(w, r) + })) + defer srv.Close() + + svc := newGmailServiceFromServer(t, srv) + var stdout bytes.Buffer + ctx := withGmailTestService(newCmdRuntimeJSONOutputContext(t, &stdout, io.Discard), svc) + err := runKong(t, &GmailDraftsCreateCmd{}, []string{ + "--to", `"Smith, John" , other@example.com`, + "--subject", "Hi", + "--body", "Hello", + }, ctx, &RootFlags{Account: "a@b.com", DryRun: true}) + if code := ExitCode(err); code != 0 { + t.Fatalf("expected clean dry-run exit (code 0), got code %d: %v", code, err) + } + assertDryRunRequestList(t, stdout.String(), "to", []string{ + `"Smith, John" `, + "other@example.com", + }) + if requests != 0 { + t.Fatalf("expected no Gmail API requests, got %d", requests) + } +} + +// TestGmailDraftsUpdateCmd_DryRunReportsParsedRecipients proves the update +// dry-run dict reports an explicit --to parsed address-aware and makes no API +// call. +func TestGmailDraftsUpdateCmd_DryRunReportsParsedRecipients(t *testing.T) { + requests := 0 + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + requests++ + http.NotFound(w, r) + })) + defer srv.Close() + + svc := newGmailServiceFromServer(t, srv) + var stdout bytes.Buffer + ctx := withGmailTestService(newCmdRuntimeJSONOutputContext(t, &stdout, io.Discard), svc) + err := runKong(t, &GmailDraftsUpdateCmd{}, []string{ + "d1", + "--to", `"Smith, John" , other@example.com`, + "--subject", "Hi", + "--body", "Hello", + }, ctx, &RootFlags{Account: "a@b.com", DryRun: true}) + if code := ExitCode(err); code != 0 { + t.Fatalf("expected clean dry-run exit (code 0), got code %d: %v", code, err) + } + assertDryRunRequestList(t, stdout.String(), "to", []string{ + `"Smith, John" `, + "other@example.com", + }) + if requests != 0 { + t.Fatalf("expected no Gmail API requests, got %d", requests) + } +} diff --git a/internal/cmd/gmail_drafts_compose.go b/internal/cmd/gmail_drafts_compose.go index 9ff703a61..bec47b063 100644 --- a/internal/cmd/gmail_drafts_compose.go +++ b/internal/cmd/gmail_drafts_compose.go @@ -26,8 +26,8 @@ type GmailDraftsReplyAllCmd struct { // GmailDraftsForwardCmd saves a forward as a draft. Mirrors GmailForwardCmd. type GmailDraftsForwardCmd struct { - MessageID string `arg:"" name:"messageId" help:"Gmail message ID to forward"` - GmailForwardOptions `embed:""` + MessageID string `arg:"" name:"messageId" help:"Gmail message ID to forward"` + Options GmailForwardOptions `embed:""` } func (c *GmailDraftsReplyCmd) Run(ctx context.Context, flags *RootFlags) error { @@ -39,13 +39,14 @@ func (c *GmailDraftsReplyAllCmd) Run(ctx context.Context, flags *RootFlags) erro } func (c *GmailDraftsForwardCmd) Run(ctx context.Context, flags *RootFlags) error { - return c.runDraft(ctx, flags, c.MessageID) + return c.Options.runDraft(ctx, flags, c.MessageID) } // runDraft is the draft-saving counterpart to GmailReplyOptions.run. It reuses // the shared resolve/build helpers verbatim and differs only in the dry-run // action name, the service gate (the non-send gate, since saving a draft is not -// a send), and the finalize step (Drafts.Create instead of Messages.Send). +// a send), and the finalize/report step (Drafts.Create + writeDraftResult +// instead of Messages.Send + writeGmailMessageResults). func (c *GmailReplyOptions) runDraft(ctx context.Context, flags *RootFlags, messageID string, replyAll bool) error { u := ui.FromContext(ctx) @@ -58,8 +59,11 @@ func (c *GmailReplyOptions) runDraft(ctx context.Context, flags *RootFlags, mess return dryRunErr } - // Drafts use the non-send gate: a draft is not a send, so this works under - // --gmail-no-send and the config no-send, matching gmail drafts create. + // A draft is not a send, so drafts compose stays usable under no-send, + // matching gmail drafts create: the --gmail-no-send flag and config keys + // are enforced pre-dispatch by the gmailSendCommandPaths list (which omits + // the drafts compose paths), and using requireGmailService here (not + // requireGmailSendService) skips the per-account config no-send check. account, svc, err := requireGmailService(ctx, flags) if err != nil { return err @@ -79,12 +83,13 @@ func (c *GmailReplyOptions) runDraft(ctx context.Context, flags *RootFlags, mess } // runDraft is the draft-saving counterpart to GmailForwardCmd.Run. Like the -// reply draft path it reuses the shared resolve/build helpers verbatim and only -// changes the dry-run action name, the service gate, and the finalize step. +// reply draft path it reuses the shared resolve/build helpers verbatim and +// changes the dry-run action name, the service gate, the finalize/report step, +// and the recipient requirement: a draft may be addressless, so it resolves +// with recipientsOptional where the send path requires --to. func (c *GmailForwardOptions) runDraft(ctx context.Context, flags *RootFlags, messageID string) error { u := ui.FromContext(ctx) - // Drafts may have no recipients, so the draft path does not require --to. inputs, err := c.resolveForwardInputs(ctx, messageID, recipientsOptional) if err != nil { return err diff --git a/internal/cmd/gmail_drafts_compose_test.go b/internal/cmd/gmail_drafts_compose_test.go index a43d0db24..e6b1363c4 100644 --- a/internal/cmd/gmail_drafts_compose_test.go +++ b/internal/cmd/gmail_drafts_compose_test.go @@ -1,8 +1,11 @@ package cmd import ( + "context" "encoding/base64" "encoding/json" + "errors" + "fmt" "io" "net/http" "os" @@ -13,33 +16,10 @@ import ( "google.golang.org/api/gmail/v1" + "github.com/openclaw/gogcli/internal/app" "github.com/openclaw/gogcli/internal/config" ) -// mockReplySourceMessage returns a gmail.Message JSON payload suitable as the -// target of a reply: it has From/To/Cc/Subject/Message-ID headers and a plain -// text body. -func mockReplySourceMessage() map[string]any { - plain := base64.RawURLEncoding.EncodeToString([]byte("Original plain body.")) - return map[string]any{ - "id": "msg-1", - "threadId": "thread-1", - "payload": map[string]any{ - "mimeType": "text/plain", - "headers": []map[string]any{ - {"name": "Message-ID", "value": ""}, - {"name": "References", "value": ""}, - {"name": "From", "value": `"Alice Sender" `}, - {"name": "To", "value": `"Me Person" , "Other Person" `}, - {"name": "Cc", "value": `"CC Person" `}, - {"name": "Date", "value": "Fri, 12 Jun 2026 10:00:00 +0000"}, - {"name": "Subject", "value": "Project update"}, - }, - "body": map[string]any{"data": plain, "size": len(plain)}, - }, - } -} - // mockReplySourceMessageWithInlineImage returns a reply target whose HTML body // references a CID inline image carried as a multipart/related part. The image // bytes are embedded inline, so no separate attachment fetch is needed. @@ -86,89 +66,24 @@ func mockReplySourceMessageWithInlineImage() map[string]any { } } -func sendAsListHandler(w http.ResponseWriter) { - _ = json.NewEncoder(w).Encode(map[string]any{ - "sendAs": []map[string]any{ - {"sendAsEmail": "me@example.com", "displayName": "Me Person", "isPrimary": true, "verificationStatus": "accepted"}, - {"sendAsEmail": "alias@example.com", "displayName": "Alias", "verificationStatus": "accepted"}, - }, - }) -} - // normalizeRawForParity strips the nondeterministic parts of a built RFC822 // message so two independent builds can be compared byte-for-byte: the Date and -// Message-ID header lines, and the randomly generated MIME boundary tokens -// (gogcli_...). Everything else must match exactly. +// Message-ID header lines, and the randomly generated MIME boundary tokens. +// Boundaries are normalized by exact token: each boundary declared in a +// Content-Type header is replaced everywhere it appears, numbered by +// declaration order, so an undeclared or truncated boundary token still +// diverges instead of being masked by a pattern match. func normalizeRawForParity(raw string) string { dateRE := regexp.MustCompile(`(?m)^Date: .*\r?$`) msgIDRE := regexp.MustCompile(`(?m)^Message-ID: .*\r?$`) - // Boundaries are "gogcli_" + base64url, so the charset is [A-Za-z0-9_-]. - boundaryRE := regexp.MustCompile(`gogcli_[A-Za-z0-9_-]+`) raw = dateRE.ReplaceAllString(raw, "Date: NORMALIZED") raw = msgIDRE.ReplaceAllString(raw, "Message-ID: NORMALIZED") - raw = boundaryRE.ReplaceAllString(raw, "gogcli_BOUNDARY") - return raw -} - -// handleFinalizeRaw services the finalize POST shared by the send and draft -// paths: it decodes the outgoing message (from a Draft body when finalizePath is -// the drafts endpoint, otherwise from a bare Message), writes the canned finalize -// response, and returns the decoded RFC822 raw plus the stamped ThreadId. -func handleFinalizeRaw(t *testing.T, w http.ResponseWriter, r *http.Request, finalizePath string) (raw, threadID string) { - t.Helper() - var msg *gmail.Message - if finalizePath == "/gmail/v1/users/me/drafts" { - var draft gmail.Draft - if err := json.NewDecoder(r.Body).Decode(&draft); err != nil { - t.Fatalf("decode draft: %v", err) - } - msg = draft.Message - _ = json.NewEncoder(w).Encode(map[string]any{"id": "d1", "message": map[string]any{"id": "m1"}}) - } else { - var m gmail.Message - if err := json.NewDecoder(r.Body).Decode(&m); err != nil { - t.Fatalf("decode send: %v", err) - } - msg = &m - _ = json.NewEncoder(w).Encode(map[string]any{"id": "sent-1", "threadId": "thread-1"}) - } - if msg == nil { - t.Fatalf("nil message in finalize body") - } - decoded, err := base64.RawURLEncoding.DecodeString(msg.Raw) - if err != nil { - t.Fatalf("decode raw: %v", err) - } - return string(decoded), msg.ThreadId -} - -// captureReplyRaw runs a reply-style command (either send or draft) against a -// mock Gmail server and returns the raw RFC822 of the outgoing message plus the -// stamped ThreadId. finalizePath is the API path the command finalizes through -// ("/gmail/v1/users/me/messages/send" or "/gmail/v1/users/me/drafts"). source -// supplies the reply-target message payload. -func captureReplyRaw(t *testing.T, args []string, finalizePath string, source func() map[string]any) (raw, threadID string) { - t.Helper() - svc, cleanup := newGmailServiceForTest(t, func(w http.ResponseWriter, r *http.Request) { - w.Header().Set("Content-Type", "application/json") - switch { - case r.Method == http.MethodGet && r.URL.Path == "/gmail/v1/users/me/settings/sendAs": - sendAsListHandler(w) - case r.Method == http.MethodGet && r.URL.Path == "/gmail/v1/users/me/messages/msg-1": - _ = json.NewEncoder(w).Encode(source()) - case r.Method == http.MethodPost && r.URL.Path == finalizePath: - raw, threadID = handleFinalizeRaw(t, w, r, finalizePath) - default: - http.NotFound(w, r) - } - }) - defer cleanup() - result := executeWithGmailTestService(t, args, svc) - if result.err != nil { - t.Fatalf("Execute(%v): %v", args, result.err) + boundaryDeclRE := regexp.MustCompile(`boundary="?(gogcli_[A-Za-z0-9_-]+)"?`) + for i, m := range boundaryDeclRE.FindAllStringSubmatch(raw, -1) { + raw = strings.ReplaceAll(raw, m[1], fmt.Sprintf("gogcli_BOUNDARY_%d", i+1)) } - return raw, threadID + return raw } // TestGmailDraftsReply_ByteIdenticalToReply proves that gmail reply and gmail @@ -180,8 +95,8 @@ func TestGmailDraftsReply_ByteIdenticalToReply(t *testing.T) { replyArgs := append(append([]string{}, base...), "gmail", "reply", "msg-1", "--body", "Thanks for the update") draftArgs := append(append([]string{}, base...), "gmail", "drafts", "reply", "msg-1", "--body", "Thanks for the update") - sentRaw, sentThread := captureReplyRaw(t, replyArgs, "/gmail/v1/users/me/messages/send", mockReplySourceMessage) - draftRaw, draftThread := captureReplyRaw(t, draftArgs, "/gmail/v1/users/me/drafts", mockReplySourceMessage) + sentRaw, sentThread := captureComposeRaw(t, replyArgs, "/gmail/v1/users/me/messages/send", mockReplySourceMessage) + draftRaw, draftThread := captureComposeRaw(t, draftArgs, "/gmail/v1/users/me/drafts", mockReplySourceMessage) if normalizeRawForParity(sentRaw) != normalizeRawForParity(draftRaw) { t.Fatalf("reply vs drafts reply raw differ:\n--- send ---\n%s\n--- draft ---\n%s", sentRaw, draftRaw) @@ -203,8 +118,8 @@ func TestGmailDraftsReply_ByteIdenticalToReply_WithInlineImage(t *testing.T) { replyArgs := append(append([]string{}, base...), "gmail", "reply", "msg-1", "--body", "Thanks for the update") draftArgs := append(append([]string{}, base...), "gmail", "drafts", "reply", "msg-1", "--body", "Thanks for the update") - sentRaw, _ := captureReplyRaw(t, replyArgs, "/gmail/v1/users/me/messages/send", mockReplySourceMessageWithInlineImage) - draftRaw, _ := captureReplyRaw(t, draftArgs, "/gmail/v1/users/me/drafts", mockReplySourceMessageWithInlineImage) + sentRaw, _ := captureComposeRaw(t, replyArgs, "/gmail/v1/users/me/messages/send", mockReplySourceMessageWithInlineImage) + draftRaw, _ := captureComposeRaw(t, draftArgs, "/gmail/v1/users/me/drafts", mockReplySourceMessageWithInlineImage) if normalizeRawForParity(sentRaw) != normalizeRawForParity(draftRaw) { t.Fatalf("reply vs drafts reply raw differ (inline image):\n--- send ---\n%s\n--- draft ---\n%s", sentRaw, draftRaw) @@ -217,45 +132,6 @@ func TestGmailDraftsReply_ByteIdenticalToReply_WithInlineImage(t *testing.T) { } } -func mockForwardSourceMessage() map[string]any { - return mockOriginalMessage(false) -} - -// captureForwardRaw runs a forward-style command and returns the raw RFC822 and -// stamped ThreadId. source supplies the original-message payload; when it -// references attachmentIds (e.g. mockOriginalMessage(true)), the attachment -// bytes are served from the attachments endpoint. -func captureForwardRaw(t *testing.T, args []string, finalizePath string, source func() map[string]any) (raw, threadID string) { - t.Helper() - svc, cleanup := newGmailServiceForTest(t, func(w http.ResponseWriter, r *http.Request) { - w.Header().Set("Content-Type", "application/json") - switch { - case r.Method == http.MethodGet && r.URL.Path == "/gmail/v1/users/me/settings/sendAs": - sendAsListHandler(w) - case r.Method == http.MethodGet && strings.Contains(r.URL.Path, "/attachments/"): - // Original message attachments (e.g. report.pdf / att-123) re-attached - // on forward. Deterministic bytes so the parity comparison holds. - _ = json.NewEncoder(w).Encode(map[string]any{ - "data": base64.RawURLEncoding.EncodeToString([]byte("pdf-file-contents")), - "size": 100, - }) - case r.Method == http.MethodGet && strings.Contains(r.URL.Path, "/gmail/v1/users/me/messages/orig-msg-1"): - _ = json.NewEncoder(w).Encode(source()) - case r.Method == http.MethodPost && r.URL.Path == finalizePath: - raw, threadID = handleFinalizeRaw(t, w, r, finalizePath) - default: - http.NotFound(w, r) - } - }) - defer cleanup() - - result := executeWithGmailTestService(t, args, svc) - if result.err != nil { - t.Fatalf("Execute(%v): %v", args, result.err) - } - return raw, threadID -} - func TestGmailDraftsForward_ByteIdenticalToForward(t *testing.T) { t.Setenv("GOG_TIMEZONE", "UTC") base := []string{"--account", "me@example.com"} @@ -323,7 +199,7 @@ func TestGmailDraftsReply_SucceedsUnderNoSendFlag(t *testing.T) { _ = json.NewEncoder(w).Encode(mockReplySourceMessage()) case r.Method == http.MethodPost && r.URL.Path == "/gmail/v1/users/me/drafts": created = true - _ = json.NewEncoder(w).Encode(map[string]any{"id": "d1", "message": map[string]any{"id": "m1"}}) + writeDraftCreatedResponse(w) case r.Method == http.MethodPost && r.URL.Path == "/gmail/v1/users/me/messages/send": t.Fatalf("drafts reply must not call Messages.Send") default: @@ -356,7 +232,7 @@ func TestGmailDraftsReplyAll_SucceedsUnderNoSendConfig(t *testing.T) { _ = json.NewEncoder(w).Encode(mockReplySourceMessage()) case r.Method == http.MethodPost && r.URL.Path == "/gmail/v1/users/me/drafts": created = true - _ = json.NewEncoder(w).Encode(map[string]any{"id": "d1", "message": map[string]any{"id": "m1"}}) + writeDraftCreatedResponse(w) default: http.NotFound(w, r) } @@ -375,7 +251,11 @@ func TestGmailDraftsReplyAll_SucceedsUnderNoSendConfig(t *testing.T) { } } -func TestGmailDraftsForward_SucceedsUnderNoSendFlag(t *testing.T) { +func TestGmailDraftsForward_SucceedsUnderNoSend(t *testing.T) { + // Both no-send dimensions at once: the config key (blocks send paths + // pre-dispatch) and the --gmail-no-send flag. A draft is not a send, so + // neither may block it. + writeNoSendConfig(t) created := false svc, cleanup := newGmailServiceForTest(t, func(w http.ResponseWriter, r *http.Request) { w.Header().Set("Content-Type", "application/json") @@ -386,7 +266,7 @@ func TestGmailDraftsForward_SucceedsUnderNoSendFlag(t *testing.T) { _ = json.NewEncoder(w).Encode(mockForwardSourceMessage()) case r.Method == http.MethodPost && r.URL.Path == "/gmail/v1/users/me/drafts": created = true - _ = json.NewEncoder(w).Encode(map[string]any{"id": "d1", "message": map[string]any{"id": "m1"}}) + writeDraftCreatedResponse(w) default: http.NotFound(w, r) } @@ -432,12 +312,11 @@ func TestGmailReply_BlockedUnderNoSendFlag(t *testing.T) { func TestGmailDraftsReply_QuoteByDefaultAndAdditiveRecipients(t *testing.T) { t.Setenv("GOG_TIMEZONE", "UTC") - raw, threadID := captureReplyRaw(t, []string{ + raw, threadID := captureComposeRaw(t, []string{ "--account", "me@example.com", "gmail", "drafts", "reply", "msg-1", "--body", "Thanks", "--cc", "extra@example.com", - "--remove", "cc@example.com", }, "/gmail/v1/users/me/drafts", mockReplySourceMessage) for _, want := range []string{ @@ -451,17 +330,36 @@ func TestGmailDraftsReply_QuoteByDefaultAndAdditiveRecipients(t *testing.T) { t.Fatalf("drafts reply missing %q:\n%s", want, raw) } } - if strings.Contains(raw, "cc@example.com") { - t.Fatalf("removed Cc recipient still present:\n%s", raw) - } if threadID != "thread-1" { t.Fatalf("threadId = %q, want thread-1", threadID) } } +// TestGmailDraftsReplyAll_RemoveSubtractsDerivedRecipient proves --remove on a +// drafts reply-all subtracts a recipient that reply-all actually derived from +// the original message (its Cc), while the other derived recipients stay. +func TestGmailDraftsReplyAll_RemoveSubtractsDerivedRecipient(t *testing.T) { + t.Setenv("GOG_TIMEZONE", "UTC") + raw, _ := captureComposeRaw(t, []string{ + "--account", "me@example.com", + "gmail", "drafts", "reply-all", "msg-1", + "--body", "Thanks", + "--remove", "cc@example.com", + }, "/gmail/v1/users/me/drafts", mockReplySourceMessage) + + assertHeaderRecipients(t, raw, "To", []wantAddr{ + {name: "Alice Sender", address: "alice@example.com"}, + {name: "Other Person", address: "other@example.com"}, + }) + headerBlock, _, _ := strings.Cut(raw, "\r\n\r\n") + if strings.Contains(headerBlock, "cc@example.com") { + t.Fatalf("removed recipient still present in headers:\n%s", headerBlock) + } +} + func TestGmailDraftsReplyAll_DerivesRecipients(t *testing.T) { t.Setenv("GOG_TIMEZONE", "UTC") - raw, _ := captureReplyRaw(t, []string{ + raw, _ := captureComposeRaw(t, []string{ "--account", "me@example.com", "gmail", "drafts", "reply-all", "msg-1", "--body", "Thanks", @@ -489,7 +387,7 @@ func TestGmailDraftsReplyAll_DerivesRecipients(t *testing.T) { } func TestGmailDraftsReply_SubjectOverrideClearsThread(t *testing.T) { - raw, threadID := captureReplyRaw(t, []string{ + raw, threadID := captureComposeRaw(t, []string{ "--account", "me@example.com", "gmail", "drafts", "reply", "msg-1", "--body", "New topic", @@ -506,7 +404,7 @@ func TestGmailDraftsReply_SubjectOverrideClearsThread(t *testing.T) { } func TestGmailDraftsReply_NoQuoteOmitsOriginal(t *testing.T) { - raw, _ := captureReplyRaw(t, []string{ + raw, _ := captureComposeRaw(t, []string{ "--account", "me@example.com", "gmail", "drafts", "reply", "msg-1", "--body", "Short reply", @@ -567,13 +465,9 @@ func newForwardDraftCaptureService(t *testing.T) (svc *gmail.Service, cleanup fu _ = json.NewEncoder(w).Encode(mockForwardSourceMessage()) case r.Method == http.MethodPost && r.URL.Path == "/gmail/v1/users/me/drafts": *created = true - var draft gmail.Draft - if err := json.NewDecoder(r.Body).Decode(&draft); err != nil { - t.Fatalf("decode draft: %v", err) - } - decoded, _ := base64.RawURLEncoding.DecodeString(draft.Message.Raw) - *raw = string(decoded) - _ = json.NewEncoder(w).Encode(map[string]any{"id": "d1", "message": map[string]any{"id": "m1"}}) + // A forward draft never stamps a thread, so the request ThreadId must be "". + *raw = readGmailDraftRaw(t, r, "") + writeDraftCreatedResponse(w) default: http.NotFound(w, r) } @@ -596,6 +490,10 @@ func TestGmailDraftsForward_NoRecipientsSucceeds(t *testing.T) { if !*created { t.Fatal("expected Drafts.Create to be called") } + // Positive check first: the draft really is the forward composition. + if !strings.Contains(*rawPtr, "Subject: Fwd: Original Subject") { + t.Fatalf("addressless draft missing forward subject:\n%s", *rawPtr) + } // Inspect only the envelope headers (before the first blank line); the // forwarded body legitimately quotes the original "To:" header. headerBlock, _, _ := strings.Cut(*rawPtr, "\r\n\r\n") @@ -605,6 +503,7 @@ func TestGmailDraftsForward_NoRecipientsSucceeds(t *testing.T) { } func TestGmailForward_NoRecipientsStillErrors(t *testing.T) { + setTestConfigHome(t) requests := 0 svc, cleanup := newGmailServiceForTest(t, func(w http.ResponseWriter, r *http.Request) { requests++ @@ -622,6 +521,9 @@ func TestGmailForward_NoRecipientsStillErrors(t *testing.T) { if !strings.Contains(result.err.Error(), "--to") { t.Fatalf("unexpected error: %v", result.err) } + if requests != 0 { + t.Fatalf("expected no Gmail API requests, got %d", requests) + } } // --- Drafts compose dry-run action names. --- @@ -637,26 +539,35 @@ func TestGmailDraftsCompose_DryRunActionNames(t *testing.T) { {"forward", []string{"gmail", "drafts", "forward", "orig-msg-1", "--to", "a@example.com"}, "gmail.drafts.forward"}, // Addressless draft-forward dry-run: the action must fire even with no // --to (the behavior distinguishing it from send-forward, which requires - // --to before the dry-run). nil service proves the gate is never reached. + // --to before the dry-run). {"forward-no-to", []string{"gmail", "drafts", "forward", "orig-msg-1"}, "gmail.drafts.forward"}, } for _, tc := range cases { t.Run(tc.name, func(t *testing.T) { - args := append([]string{"--dry-run", "--account", "me@example.com"}, tc.args...) - result := executeWithGmailTestService(t, args, nil) + args := append([]string{"--plain", "--dry-run", "--account", "me@example.com"}, tc.args...) + // An error-returning factory proves the dry-run exits before any + // service acquisition; a nil service would succeed silently if the + // command never happened to dereference it. + result := executeWithTestRuntime(t, args, &app.Runtime{Services: app.Services{ + Gmail: func(context.Context, string) (*gmail.Service, error) { + return nil, errors.New("service must not be acquired during dry-run") + }, + }}) if result.err != nil { t.Fatalf("dry-run: %v", result.err) } - if !strings.Contains(result.stdout, tc.op) { + // Exact op line: a Contains on the bare op would let + // gmail.drafts.reply match reply-all output. + if !strings.Contains(result.stdout, "op\t"+tc.op+"\n") { t.Fatalf("dry-run output missing action %q:\n%s", tc.op, result.stdout) } }) } } -// --- Stdin read-once on the resolve step. --- +// --- Stdin-backed body/note inputs land in the built draft. --- -func TestGmailDraftsReply_BodyFileStdinReadOnce(t *testing.T) { +func TestGmailDraftsReply_BodyFileStdinBodyInDraft(t *testing.T) { created := false var raw string svc, cleanup := newGmailServiceForTest(t, func(w http.ResponseWriter, r *http.Request) { @@ -668,13 +579,8 @@ func TestGmailDraftsReply_BodyFileStdinReadOnce(t *testing.T) { _ = json.NewEncoder(w).Encode(mockReplySourceMessage()) case r.Method == http.MethodPost && r.URL.Path == "/gmail/v1/users/me/drafts": created = true - var draft gmail.Draft - if err := json.NewDecoder(r.Body).Decode(&draft); err != nil { - t.Fatalf("decode draft: %v", err) - } - decoded, _ := base64.RawURLEncoding.DecodeString(draft.Message.Raw) - raw = string(decoded) - _ = json.NewEncoder(w).Encode(map[string]any{"id": "d1", "message": map[string]any{"id": "m1"}}) + raw = readGmailDraftRaw(t, r, "thread-1") + writeDraftCreatedResponse(w) default: http.NotFound(w, r) } @@ -696,7 +602,7 @@ func TestGmailDraftsReply_BodyFileStdinReadOnce(t *testing.T) { } } -func TestGmailDraftsForward_NoteFileStdinReadOnce(t *testing.T) { +func TestGmailDraftsForward_NoteFileStdinNoteInDraft(t *testing.T) { t.Setenv("GOG_TIMEZONE", "UTC") svc, cleanup, created, rawPtr := newForwardDraftCaptureService(t) defer cleanup() @@ -734,13 +640,8 @@ func TestGmailDraftsReply_WithSignatureFile(t *testing.T) { case r.Method == http.MethodGet && r.URL.Path == "/gmail/v1/users/me/messages/msg-1": _ = json.NewEncoder(w).Encode(mockReplySourceMessage()) case r.Method == http.MethodPost && r.URL.Path == "/gmail/v1/users/me/drafts": - var draft gmail.Draft - if err := json.NewDecoder(r.Body).Decode(&draft); err != nil { - t.Fatalf("decode draft: %v", err) - } - decoded, _ := base64.RawURLEncoding.DecodeString(draft.Message.Raw) - raw = string(decoded) - _ = json.NewEncoder(w).Encode(map[string]any{"id": "d1", "message": map[string]any{"id": "m1"}}) + raw = readGmailDraftRaw(t, r, "thread-1") + writeDraftCreatedResponse(w) default: http.NotFound(w, r) } @@ -819,3 +720,79 @@ func TestGmailDraftsForward_CreateFailureWrapsError(t *testing.T) { t.Fatalf("error not wrapped with %q: %v", "create forward draft", result.err) } } + +// --- Alias gating: the no-send path list matches exact command paths, so the +// drafts compose aliases must stay usable under a no-send config too. --- + +func TestGmailDraftsComposeAliases_SucceedUnderNoSendConfig(t *testing.T) { + writeNoSendConfig(t) + + t.Run("replyall", func(t *testing.T) { + created := false + svc, cleanup := newGmailServiceForTest(t, func(w http.ResponseWriter, r *http.Request) { + w.Header().Set("Content-Type", "application/json") + switch { + case r.Method == http.MethodGet && r.URL.Path == "/gmail/v1/users/me/settings/sendAs": + sendAsListHandler(w) + case r.Method == http.MethodGet && r.URL.Path == "/gmail/v1/users/me/messages/msg-1": + _ = json.NewEncoder(w).Encode(mockReplySourceMessage()) + case r.Method == http.MethodPost && r.URL.Path == "/gmail/v1/users/me/drafts": + created = true + writeDraftCreatedResponse(w) + default: + http.NotFound(w, r) + } + }) + defer cleanup() + + result := executeWithGmailTestService(t, []string{ + "--account", "me@example.com", + "gmail", "drafts", "replyall", "msg-1", "--body", "hi", + }, svc) + if result.err != nil { + t.Fatalf("drafts replyall under config no-send: %v", result.err) + } + if !created { + t.Fatal("expected Drafts.Create to be called") + } + }) + + t.Run("fwd", func(t *testing.T) { + created := false + svc, cleanup := newGmailServiceForTest(t, func(w http.ResponseWriter, r *http.Request) { + w.Header().Set("Content-Type", "application/json") + switch { + case r.Method == http.MethodGet && r.URL.Path == "/gmail/v1/users/me/settings/sendAs": + sendAsListHandler(w) + case r.Method == http.MethodGet && strings.Contains(r.URL.Path, "/gmail/v1/users/me/messages/orig-msg-1"): + _ = json.NewEncoder(w).Encode(mockForwardSourceMessage()) + case r.Method == http.MethodPost && r.URL.Path == "/gmail/v1/users/me/drafts": + created = true + // A non-empty response threadId exercises writeDraftResult's + // fallback: a forward draft carries no built thread id, so the + // reported one must come from the Drafts.Create response. + _ = json.NewEncoder(w).Encode(map[string]any{ + "id": "d1", + "message": map[string]any{"id": "m1", "threadId": "t-created"}, + }) + default: + http.NotFound(w, r) + } + }) + defer cleanup() + + result := executeWithGmailTestService(t, []string{ + "--json", "--account", "me@example.com", + "gmail", "drafts", "fwd", "orig-msg-1", "--to", "recipient@example.com", + }, svc) + if result.err != nil { + t.Fatalf("drafts fwd under config no-send: %v", result.err) + } + if !created { + t.Fatal("expected Drafts.Create to be called") + } + if !strings.Contains(result.stdout, `"threadId": "t-created"`) { + t.Fatalf("expected create-response thread id fallback, got:\n%s", result.stdout) + } + }) +} diff --git a/internal/cmd/gmail_forward.go b/internal/cmd/gmail_forward.go index 3464ebe2c..5dea018a3 100644 --- a/internal/cmd/gmail_forward.go +++ b/internal/cmd/gmail_forward.go @@ -15,8 +15,8 @@ import ( ) type GmailForwardCmd struct { - MessageID string `arg:"" name:"messageId" help:"Gmail message ID to forward"` - GmailForwardOptions `embed:""` + MessageID string `arg:"" name:"messageId" help:"Gmail message ID to forward"` + Options GmailForwardOptions `embed:""` } type GmailForwardOptions struct { @@ -42,9 +42,11 @@ const ( // compose. The note is resolved exactly once here because '-' reads stdin, // which cannot be read twice. type forwardComposeInputs struct { - messageID string - note string - toRecipients []string + messageID string + note string + toRecipients []string + ccRecipients []string + bccRecipients []string // allowMissingTo carries the recipient requirement forward to the build // step: false on the send path (so buildGmailMessage keeps its missing-To // backstop), true on the draft path (which permits an addressless forward). @@ -61,12 +63,12 @@ type forwardComposeMessage struct { func (c *GmailForwardCmd) Run(ctx context.Context, flags *RootFlags) error { u := ui.FromContext(ctx) - inputs, err := c.resolveForwardInputs(ctx, c.MessageID, recipientsRequired) + inputs, err := c.Options.resolveForwardInputs(ctx, c.MessageID, recipientsRequired) if err != nil { return err } - if dryRunErr := dryRunExit(ctx, flags, "gmail.forward", c.dryRunFields(inputs)); dryRunErr != nil { + if dryRunErr := dryRunExit(ctx, flags, "gmail.forward", c.Options.dryRunFields(inputs)); dryRunErr != nil { return dryRunErr } @@ -75,7 +77,7 @@ func (c *GmailForwardCmd) Run(ctx context.Context, flags *RootFlags) error { return err } - built, err := c.buildForwardComposeMessage(ctx, svc, account, inputs) + built, err := c.Options.buildForwardComposeMessage(ctx, svc, account, inputs) if err != nil { return err } @@ -99,8 +101,8 @@ func (c *GmailForwardOptions) dryRunFields(inputs forwardComposeInputs) map[stri return map[string]any{ "message_id": inputs.messageID, "to": inputs.toRecipients, - "cc": splitCSV(c.Cc), - "bcc": splitCSV(c.Bcc), + "cc": inputs.ccRecipients, + "bcc": inputs.bccRecipients, "from": strings.TrimSpace(c.From), "note_len": len(inputs.note), "skip_attachments": c.SkipAttachments, @@ -108,17 +110,21 @@ func (c *GmailForwardOptions) dryRunFields(inputs forwardComposeInputs) map[stri } // resolveForwardInputs normalizes the message ID, runs the service-free -// validation, and resolves the note input. It reads the note -// exactly once so '-' (stdin) is consumed a single time. When req is -// recipientsRequired (the send path) an empty --to is rejected; a draft may have -// no recipients, so the draft path passes recipientsOptional. +// validation, and resolves the note input. It reads the note exactly once so +// '-' (stdin) is consumed a single time. When req is recipientsRequired (the +// send path) an empty --to is rejected; a draft may have no recipients, so the +// draft path passes recipientsOptional. func (c *GmailForwardOptions) resolveForwardInputs(ctx context.Context, messageID string, req recipientRequirement) (forwardComposeInputs, error) { messageID = normalizeGmailMessageID(messageID) if messageID == "" { return forwardComposeInputs{}, usage("required: messageId") } - toRecipients := splitCSV(c.To) + // Parsed before the dry-run so it reports the lists the build will use. + toRecipients, ccRecipients, bccRecipients, err := parseComposeRecipients(c.To, c.Cc, c.Bcc) + if err != nil { + return forwardComposeInputs{}, err + } if req == recipientsRequired && len(toRecipients) == 0 { return forwardComposeInputs{}, usage("required: --to") } @@ -134,6 +140,8 @@ func (c *GmailForwardOptions) resolveForwardInputs(ctx context.Context, messageI messageID: messageID, note: note, toRecipients: toRecipients, + ccRecipients: ccRecipients, + bccRecipients: bccRecipients, allowMissingTo: req == recipientsOptional, }, nil } @@ -188,9 +196,6 @@ func (c *GmailForwardOptions) buildForwardComposeMessage(ctx context.Context, sv return forwardComposeMessage{}, fmt.Errorf("preserve forwarded message parts: %w", err) } - ccRecipients := splitCSV(c.Cc) - bccRecipients := splitCSV(c.Bcc) - // allowMissingTo comes from the recipient requirement resolved up front: the // send path keeps buildGmailMessage's missing-To backstop (false), while the // draft path opts out (true) to permit an addressless forward like Gmail's UI. @@ -202,8 +207,8 @@ func (c *GmailForwardOptions) buildForwardComposeMessage(ctx context.Context, sv Attachments: attachments, }, sendBatch{ To: inputs.toRecipients, - Cc: ccRecipients, - Bcc: bccRecipients, + Cc: inputs.ccRecipients, + Bcc: inputs.bccRecipients, }, inputs.allowMissingTo) if err != nil { return forwardComposeMessage{}, fmt.Errorf("build message: %w", err) diff --git a/internal/cmd/gmail_recipients.go b/internal/cmd/gmail_recipients.go index bb409d012..b158885e9 100644 --- a/internal/cmd/gmail_recipients.go +++ b/internal/cmd/gmail_recipients.go @@ -124,6 +124,47 @@ func parseExplicitRecipientFields(to, cc, bcc []string) (replyRecipients, error) return out, nil } +// parseRecipientCSV parses a single comma-separated recipient flag (e.g. +// --to "a@x.com, b@y.com") into the []string representation that +// buildGmailMessage's sendBatch expects, the same form formatMailboxes produces +// for the reply path. Unlike splitCSV it is address-aware: a comma inside a +// quoted display name ("Smith, John" ) stays one recipient, +// so recipients are counted correctly (--track's exactly-one gate), malformed +// or zero-recipient input fails up front instead of late or not at all, +// dry-runs report the real list, and duplicates dedup by address. An +// empty/whitespace value yields no recipients (no error), preserving the +// addressless-draft path. +func parseRecipientCSV(flag, value string) ([]string, error) { + addrs, err := parseMailboxValues(flag, []string{value}) + if err != nil { + return nil, err + } + return formatMailboxes(addrs), nil +} + +// parseComposeRecipients parses the three explicit compose recipient flags +// (--to/--cc/--bcc) address-aware via parseRecipientCSV, returning the +// []string recipient lists that buildGmailMessage's sendBatch expects. +// +// Unlike the reply path's parseExplicitRecipientFields, it does not reject an +// address appearing in two flags. Reply's --to/--cc/--bcc are add-or-MOVE +// operations on an existing recipient set, so the same address in two flags is +// a contradictory instruction; compose flags assign whole fields, so overlap +// (including with --reply-all auto-populated fields) is well-defined and +// preserved. +func parseComposeRecipients(to, cc, bcc string) (toOut, ccOut, bccOut []string, err error) { + if toOut, err = parseRecipientCSV("--to", to); err != nil { + return nil, nil, nil, err + } + if ccOut, err = parseRecipientCSV("--cc", cc); err != nil { + return nil, nil, nil, err + } + if bccOut, err = parseRecipientCSV("--bcc", bcc); err != nil { + return nil, nil, nil, err + } + return toOut, ccOut, bccOut, nil +} + func parseMailboxValues(flag string, values []string) ([]mail.Address, error) { var out []mail.Address for _, value := range values { @@ -135,6 +176,13 @@ func parseMailboxValues(flag string, values []string) ([]mail.Address, error) { if err != nil { return nil, usagef("invalid %s recipient list %q: %v", flag, value, err) } + // RFC 5322 group syntax (e.g. "undisclosed-recipients:;") parses to + // zero addresses without a parse error. Reject it here so no flag + // consumer silently drops the value — and no dry-run reports an empty + // list while the real command fails late or proceeds without it. + if len(addrs) == 0 { + return nil, usagef("%s %q contains no recipients", flag, value) + } for _, addr := range addrs { if addr == nil || strings.TrimSpace(addr.Address) == "" { continue @@ -241,29 +289,51 @@ func removeMailbox(addrs []mail.Address, key string) []mail.Address { return out } -func canonicalEmail(value string) string { +// bareEmail extracts the bare address from a single formatted mailbox +// ("Name" → a@x.com), preserving its case. A value that does not +// parse as an address is returned trimmed as-is. +func bareEmail(value string) string { value = strings.TrimSpace(value) if addr, err := mail.ParseAddress(value); err == nil && addr != nil { - value = addr.Address + return strings.TrimSpace(addr.Address) } - return strings.ToLower(strings.TrimSpace(value)) + return value +} + +// canonicalEmail returns the same address lowercased, for use as a map key. +func canonicalEmail(value string) string { + return strings.ToLower(bareEmail(value)) } func formatMailboxes(addrs []mail.Address) []string { - out := make([]string, 0, len(addrs)) + // nil rather than an allocated empty slice when there are no mailboxes, so + // an omitted recipient flag serializes as null in dry-run JSON (the shape + // splitCSV produced), not []. + var out []string for _, addr := range addrs { if strings.TrimSpace(addr.Address) == "" { continue } - if strings.TrimSpace(addr.Name) == "" { - out = append(out, addr.Address) - } else { - out = append(out, addr.String()) - } + out = append(out, formatMailbox(addr)) } return out } +// formatMailbox renders one mailbox for a recipient header: the bare address +// when nameless, addr.String() otherwise. A nameless address whose bare form +// does not re-parse — a quoted local part like "john smith"@example.com is +// stored unquoted — is re-quoted via addr.String(), since the bare form is +// invalid on the wire. +func formatMailbox(addr mail.Address) string { + if strings.TrimSpace(addr.Name) != "" { + return addr.String() + } + if _, err := mail.ParseAddress(addr.Address); err != nil { + return addr.String() + } + return addr.Address +} + func selfEmailsForReply(account, sendingEmail string, sendAs []*gmail.SendAs) []string { out := []string{account, sendingEmail} for _, alias := range sendAs { diff --git a/internal/cmd/gmail_reply.go b/internal/cmd/gmail_reply.go index f592047b6..1706ecedd 100644 --- a/internal/cmd/gmail_reply.go +++ b/internal/cmd/gmail_reply.go @@ -19,7 +19,7 @@ import ( func buildReplyAllRecipients(info *replyInfo, selfEmail string) (to, cc []string) { recipients, err := buildReplyRecipients(info, []string{selfEmail}, true, nil, nil, nil, nil) if err != nil { - return []string{}, []string{} + return nil, nil } return formatMailboxes(recipients.To), formatMailboxes(recipients.Cc) } @@ -261,13 +261,16 @@ func filterOutSelf(addresses []string, selfEmail string) []string { return result } +// deduplicateAddresses removes duplicate recipients, keeping the first +// occurrence. It keys on the canonical bare address so a formatted mailbox +// ("Bob" ) and a bare a@x.com (in any case) count as one recipient. func deduplicateAddresses(addresses []string) []string { seen := make(map[string]bool) result := make([]string, 0, len(addresses)) for _, addr := range addresses { - lower := strings.ToLower(addr) - if !seen[lower] { - seen[lower] = true + key := canonicalEmail(addr) + if !seen[key] { + seen[key] = true result = append(result, addr) } } diff --git a/internal/cmd/gmail_send.go b/internal/cmd/gmail_send.go index e50ee44ab..ff6f93f51 100644 --- a/internal/cmd/gmail_send.go +++ b/internal/cmd/gmail_send.go @@ -2,6 +2,7 @@ package cmd import ( "context" + "errors" "fmt" "os" "strings" @@ -35,9 +36,14 @@ type GmailSendCmd struct { } type sendBatch struct { - To []string - Cc []string - Bcc []string + To []string + Cc []string + Bcc []string + // TrackingRecipient, when set (only buildSendBatches sets it), is the bare + // email address — never a formatted "Name" mailbox — so the + // identifier baked into the tracking pixel stays stable regardless of how + // the user typed the recipient. Batches built by the reply/forward/drafts + // paths leave it empty; sendGmailBatches normalizes defensively on read. TrackingRecipient string } @@ -110,15 +116,24 @@ func (c *GmailSendCmd) Run(ctx context.Context, flags *RootFlags) error { return headerErr } + // Parse the recipient flags before the dry-run so it reports the lists the + // built message will carry and bad input fails without any API call. Under + // --reply-all the dry-run's to/cc report the explicit overrides only: + // auto-populated recipients are resolved later, post-dry-run, by design. + explicitTo, explicitCc, explicitBcc, err := parseComposeRecipients(c.To, c.Cc, c.Bcc) + if err != nil { + return err + } + attachPaths, err := expandComposeAttachmentPaths(c.Attach) if err != nil { return err } if dryRunErr := dryRunExit(ctx, flags, "gmail.send", map[string]any{ - "to": splitCSV(c.To), - "cc": splitCSV(c.Cc), - "bcc": splitCSV(c.Bcc), + "to": explicitTo, + "cc": explicitCc, + "bcc": explicitBcc, "subject": subject, "reply_to_message_id": replyToMessageID, "thread_id": threadID, @@ -172,12 +187,16 @@ func (c *GmailSendCmd) Run(ctx context.Context, flags *RootFlags) error { toRecipients, ccRecipients = buildReplyAllRecipients(replyInfo, from.sendingEmail) } - // Explicit --to and --cc override (not merge with) auto-populated recipients + // Explicit --to and --cc override (not merge with) auto-populated + // recipients. Non-empty flags always parse to at least one recipient + // (parseMailboxValues rejects zero-parse input), so keying the override on + // the raw flag matches keying on the parsed list; the raw flag states the + // intent directly. if strings.TrimSpace(c.To) != "" { - toRecipients = splitCSV(c.To) + toRecipients = explicitTo } if strings.TrimSpace(c.Cc) != "" { - ccRecipients = splitCSV(c.Cc) + ccRecipients = explicitCc } // Final validation: we must have at least one recipient @@ -185,8 +204,6 @@ func (c *GmailSendCmd) Run(ctx context.Context, flags *RootFlags) error { return usage("no recipients: specify --to or use --reply-all with a message that has recipients") } - bccRecipients := splitCSV(c.Bcc) - atts, attachmentMetadata, err := mailmime.PrepareAttachments(attachmentsFromPaths(attachPaths), os.ReadFile) if err != nil { return err @@ -195,13 +212,13 @@ func (c *GmailSendCmd) Run(ctx context.Context, flags *RootFlags) error { var trackingCfg *tracking.Config if c.Track { - trackingCfg, err = c.resolveTrackingConfig(ctx, account, toRecipients, ccRecipients, bccRecipients, htmlBody) + trackingCfg, err = c.resolveTrackingConfig(ctx, account, toRecipients, ccRecipients, explicitBcc, htmlBody) if err != nil { return err } } - batches := buildSendBatches(toRecipients, ccRecipients, bccRecipients, c.Track, c.TrackSplit) + batches := buildSendBatches(toRecipients, ccRecipients, explicitBcc, c.Track, c.TrackSplit) results, err := sendGmailBatches(ctx, svc, sendMessageOptions{ FromAddr: from.header, ReplyTo: c.ReplyTo, @@ -292,6 +309,8 @@ func primaryDisplayNameFromSendAsList(sendAs []*gmail.SendAs, account string) st return "" } +// buildSendBatches splits a tracked multi-recipient send into per-recipient +// batches (--track-split) or returns a single batch carrying all recipients. func buildSendBatches(toRecipients, ccRecipients, bccRecipients []string, track, trackSplit bool) []sendBatch { totalRecipients := len(toRecipients) + len(ccRecipients) + len(bccRecipients) if track && trackSplit && totalRecipients > 1 { @@ -302,14 +321,14 @@ func buildSendBatches(toRecipients, ccRecipients, bccRecipients []string, track, for _, recipient := range recipients { batches = append(batches, sendBatch{ To: []string{recipient}, - TrackingRecipient: recipient, + TrackingRecipient: bareEmail(recipient), }) } return batches } - trackingRecipient := firstRecipient(toRecipients, ccRecipients, bccRecipients) + trackingRecipient := bareEmail(firstRecipient(toRecipients, ccRecipients, bccRecipients)) return []sendBatch{{ To: toRecipients, Cc: ccRecipients, @@ -324,9 +343,12 @@ func sendGmailBatches(ctx context.Context, svc *gmail.Service, opts sendMessageO htmlBody := opts.BodyHTML trackingID := "" if opts.Track { - recipient := strings.TrimSpace(batch.TrackingRecipient) + recipient := bareEmail(batch.TrackingRecipient) + if recipient == "" { + recipient = bareEmail(firstRecipient(batch.To, batch.Cc, batch.Bcc)) + } if recipient == "" { - recipient = strings.TrimSpace(firstRecipient(batch.To, batch.Cc, batch.Bcc)) + return nil, errors.New("tracking requires a recipient") } pixelURL, blob, pixelErr := tracking.GeneratePixelURL(opts.TrackingCfg, recipient, opts.Subject) if pixelErr != nil { @@ -351,10 +373,9 @@ func sendGmailBatches(ctx context.Context, svc *gmail.Service, opts sendMessageO return nil, err } - resultRecipient := strings.TrimSpace(batch.TrackingRecipient) - if resultRecipient == "" { - resultRecipient = strings.TrimSpace(firstRecipient(batch.To, batch.Cc, batch.Bcc)) - } + // Results report the mailbox as actually sent (possibly a formatted + // "Name" ); only the tracking identity above is bare. + resultRecipient := strings.TrimSpace(firstRecipient(batch.To, batch.Cc, batch.Bcc)) results = append(results, sendResult{ To: resultRecipient, MessageID: sent.Id, diff --git a/internal/cmd/gmail_send_batches_test.go b/internal/cmd/gmail_send_batches_test.go index 242d2451a..7e1469bf2 100644 --- a/internal/cmd/gmail_send_batches_test.go +++ b/internal/cmd/gmail_send_batches_test.go @@ -2,6 +2,7 @@ package cmd import ( "context" + "encoding/base64" "encoding/json" "fmt" "io" @@ -81,6 +82,150 @@ func TestSendGmailBatches_WithTracking(t *testing.T) { } } +// TestSendGmailBatches_TrackSplitBareTrackingRecipient proves a track-split +// send correlates each tracking pixel to the bare email address even when the +// recipient was typed as a formatted "Name" mailbox: the message's +// To header and the reported result keep the formatted mailbox, while the +// encrypted pixel payload carries only the address, so opens correlate +// regardless of how the recipient was typed. +func TestSendGmailBatches_TrackSplitBareTrackingRecipient(t *testing.T) { + var raws []string + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if r.Method != http.MethodPost || !strings.HasSuffix(r.URL.Path, "/users/me/messages/send") { + http.NotFound(w, r) + return + } + var msg gmail.Message + if err := json.NewDecoder(r.Body).Decode(&msg); err != nil { + t.Errorf("decode sent message: %v", err) + } + decoded, err := base64.RawURLEncoding.DecodeString(msg.Raw) + if err != nil { + t.Errorf("decode raw: %v", err) + } + raws = append(raws, string(decoded)) + w.Header().Set("Content-Type", "application/json") + _ = json.NewEncoder(w).Encode(map[string]any{"id": fmt.Sprintf("m%d", len(raws)), "threadId": "t1"}) + })) + defer srv.Close() + + key := mustTrackingKey(t) + batches := buildSendBatches( + []string{`"Smith, John" `, "other@example.com"}, + nil, + nil, + true, + true, + ) + results, err := sendGmailBatches(context.Background(), newGmailServiceFromServer(t, srv), sendMessageOptions{ + FromAddr: "me@example.com", + Subject: "Hello", + BodyHTML: "Hi", + Track: true, + TrackingCfg: &tracking.Config{ + Enabled: true, + WorkerURL: "https://example.com", + TrackingKey: key, + }, + }, batches) + if err != nil { + t.Fatalf("sendGmailBatches: %v", err) + } + if len(results) != 2 || len(raws) != 2 { + t.Fatalf("expected 2 results and 2 sent messages, got %d and %d", len(results), len(raws)) + } + + want := []struct { + formatted string + header wantAddr + bare string + }{ + { + formatted: `"Smith, John" `, + header: wantAddr{name: "Smith, John", address: "john@example.com"}, + bare: "john@example.com", + }, + { + formatted: "other@example.com", + header: wantAddr{address: "other@example.com"}, + bare: "other@example.com", + }, + } + for i, w := range want { + payload, decErr := tracking.Decrypt(results[i].TrackingID, key) + if decErr != nil { + t.Fatalf("decrypt tracking blob %d: %v", i, decErr) + } + if payload.Recipient != w.bare { + t.Errorf("tracking recipient[%d] = %q, want bare %q", i, payload.Recipient, w.bare) + } + if results[i].To != w.formatted { + t.Errorf("results[%d].To = %q, want formatted %q", i, results[i].To, w.formatted) + } + assertHeaderRecipients(t, raws[i], "To", []wantAddr{w.header}) + } +} + +// TestSendGmailBatches_NonSplitTrackedBarePixelRecipient proves the non-split +// tracked path (single recipient, no --track-split) also bakes the bare email +// address into the encrypted pixel payload when the recipient was typed as a +// formatted mailbox. +func TestSendGmailBatches_NonSplitTrackedBarePixelRecipient(t *testing.T) { + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if r.Method != http.MethodPost || !strings.HasSuffix(r.URL.Path, "/users/me/messages/send") { + http.NotFound(w, r) + return + } + w.Header().Set("Content-Type", "application/json") + _ = json.NewEncoder(w).Encode(map[string]any{"id": "m1", "threadId": "t1"}) + })) + defer srv.Close() + + key := mustTrackingKey(t) + batches := buildSendBatches([]string{`"Smith, John" `}, nil, nil, true, false) + results, err := sendGmailBatches(context.Background(), newGmailServiceFromServer(t, srv), sendMessageOptions{ + FromAddr: "me@example.com", + Subject: "Hello", + BodyHTML: "Hi", + Track: true, + TrackingCfg: &tracking.Config{ + Enabled: true, + WorkerURL: "https://example.com", + TrackingKey: key, + }, + }, batches) + if err != nil { + t.Fatalf("sendGmailBatches: %v", err) + } + if len(results) != 1 { + t.Fatalf("expected 1 result, got %d", len(results)) + } + payload, err := tracking.Decrypt(results[0].TrackingID, key) + if err != nil { + t.Fatalf("decrypt tracking blob: %v", err) + } + if payload.Recipient != "john@example.com" { + t.Fatalf("pixel recipient = %q, want bare john@example.com", payload.Recipient) + } +} + +// TestBuildSendBatches_TrackSplitDedupesFormattedAndBare proves the +// track-split dedup keys on the canonical bare address across to/cc/bcc: +// "Bob" in --to and A@X.COM in --cc are the same person and yield +// one batch, keeping the first-seen spelling. +func TestBuildSendBatches_TrackSplitDedupesFormattedAndBare(t *testing.T) { + batches := buildSendBatches([]string{`"Bob" `}, []string{"A@X.COM"}, nil, true, true) + if len(batches) != 1 { + t.Fatalf("expected 1 batch after cross-field dedup, got %d: %#v", len(batches), batches) + } + if got := batches[0].To; len(got) != 1 || got[0] != `"Bob" ` { + t.Fatalf("expected first-seen formatted mailbox, got %#v", got) + } + if batches[0].TrackingRecipient != "a@x.com" { + t.Fatalf("tracking recipient = %q, want bare a@x.com", batches[0].TrackingRecipient) + } +} + func TestReplyHeaders_Message(t *testing.T) { srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { path := strings.TrimPrefix(r.URL.Path, "/gmail/v1") diff --git a/internal/cmd/gmail_send_test.go b/internal/cmd/gmail_send_test.go index defeabb9f..3ad376bf3 100644 --- a/internal/cmd/gmail_send_test.go +++ b/internal/cmd/gmail_send_test.go @@ -985,6 +985,11 @@ func TestDeduplicateAddresses(t *testing.T) { addresses: []string{"alice@example.com", "ALICE@EXAMPLE.COM", "bob@example.com"}, expect: []string{"alice@example.com", "bob@example.com"}, }, + { + name: "formatted mailbox and bare address", + addresses: []string{`"Bob" `, "A@X.COM", "b@y.com"}, + expect: []string{`"Bob" `, "b@y.com"}, + }, } for _, tc := range tests { @@ -1025,7 +1030,7 @@ func TestBuildReplyAllRecipients(t *testing.T) { }, selfEmail: "me@example.com", expectTo: []string{`"Sender Name" `}, - expectCc: []string{}, + expectCc: nil, }, { name: "deduplication across To", @@ -1036,7 +1041,7 @@ func TestBuildReplyAllRecipients(t *testing.T) { }, selfEmail: "me@example.com", expectTo: []string{"sender@example.com", "alice@example.com"}, - expectCc: []string{}, + expectCc: nil, }, { name: "Cc address already in To is excluded from Cc", @@ -1069,7 +1074,7 @@ func TestBuildReplyAllRecipients(t *testing.T) { }, selfEmail: "me@example.com", expectTo: []string{"sender@example.com", "alice@example.com"}, - expectCc: []string{}, + expectCc: nil, }, { name: "empty recipients", @@ -1079,8 +1084,8 @@ func TestBuildReplyAllRecipients(t *testing.T) { CcAddrs: nil, }, selfEmail: "me@example.com", - expectTo: []string{}, - expectCc: []string{}, + expectTo: nil, + expectCc: nil, }, { name: "Reply-To header takes precedence over From (RFC 5322)", @@ -1092,7 +1097,7 @@ func TestBuildReplyAllRecipients(t *testing.T) { }, selfEmail: "me@example.com", expectTo: []string{"reply-here@example.com", "alice@example.com"}, - expectCc: []string{}, + expectCc: nil, }, { name: "Reply-To with display name", @@ -1104,7 +1109,7 @@ func TestBuildReplyAllRecipients(t *testing.T) { }, selfEmail: "me@example.com", expectTo: []string{`"Mailing List" `, "alice@example.com"}, - expectCc: []string{}, + expectCc: nil, }, { name: "Empty Reply-To falls back to From", @@ -1116,7 +1121,7 @@ func TestBuildReplyAllRecipients(t *testing.T) { }, selfEmail: "me@example.com", expectTo: []string{"sender@example.com", "alice@example.com"}, - expectCc: []string{}, + expectCc: nil, }, } diff --git a/internal/cmd/gmail_send_tracking_test.go b/internal/cmd/gmail_send_tracking_test.go index 8c19efde5..e2447640e 100644 --- a/internal/cmd/gmail_send_tracking_test.go +++ b/internal/cmd/gmail_send_tracking_test.go @@ -6,6 +6,7 @@ import ( "io" "os" "path/filepath" + "strings" "testing" "github.com/openclaw/gogcli/internal/mailmime" @@ -64,6 +65,54 @@ func TestResolveTrackingConfig(t *testing.T) { } } +// TestResolveTrackingConfig_CountsParsedRecipients proves the --track +// exactly-1-recipient gate counts address-aware parsed recipients: a single +// display-name mailbox whose comma splitCSV used to miscount as two +// recipients passes the gate, and a same-address pair deduped within the flag +// counts as one. Each call stops at the empty-HTML-body check — the error +// must be the HTML one, not the count one — so the test proves the gate +// passed without touching the env-dependent tracking config load. The same +// parsed list must also yield a single send batch. +func TestResolveTrackingConfig_CountsParsedRecipients(t *testing.T) { + ctx := newCmdRuntimeOutputContext(t, io.Discard, io.Discard) + cases := []struct { + name string + value string + bare string + }{ + {name: "display name comma", value: `"Smith, John" `, bare: "john@example.com"}, + {name: "case duplicate pair", value: "a@x.com, A@X.COM", bare: "a@x.com"}, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + to, err := parseRecipientCSV("--to", tc.value) + if err != nil { + t.Fatalf("parseRecipientCSV: %v", err) + } + if len(to) != 1 { + t.Fatalf("expected 1 parsed recipient, got %#v", to) + } + + cmd := &GmailSendCmd{Track: true} + _, err = cmd.resolveTrackingConfig(ctx, "a@b.com", to, nil, nil, "") + if err == nil || !strings.Contains(err.Error(), "HTML body") { + t.Fatalf("expected HTML-body error (count gate passed), got: %v", err) + } + if strings.Contains(err.Error(), "exactly 1 recipient") { + t.Fatalf("count gate rejected a single parsed recipient: %v", err) + } + + batches := buildSendBatches(to, nil, nil, true, false) + if len(batches) != 1 { + t.Fatalf("expected 1 batch, got %#v", batches) + } + if batches[0].TrackingRecipient != tc.bare { + t.Fatalf("tracking recipient = %q, want bare %q", batches[0].TrackingRecipient, tc.bare) + } + }) + } +} + func TestFirstRecipient(t *testing.T) { if got := firstRecipient([]string{"a"}, []string{"b"}, []string{"c"}); got != "a" { t.Fatalf("unexpected first recipient: %q", got) diff --git a/internal/cmd/gmail_testutil_test.go b/internal/cmd/gmail_testutil_test.go index 76d752c39..ea239a6b8 100644 --- a/internal/cmd/gmail_testutil_test.go +++ b/internal/cmd/gmail_testutil_test.go @@ -2,9 +2,11 @@ package cmd import ( "context" + "encoding/base64" "encoding/json" "net/http" "net/http/httptest" + "net/mail" "strings" "testing" @@ -13,6 +15,229 @@ import ( "github.com/openclaw/gogcli/internal/app" ) +// mockReplySourceMessage returns a gmail.Message JSON payload suitable as the +// target of a reply: it has From/To/Cc/Subject/Message-ID headers and a plain +// text body. +func mockReplySourceMessage() map[string]any { + plain := base64.RawURLEncoding.EncodeToString([]byte("Original plain body.")) + return map[string]any{ + "id": "msg-1", + "threadId": "thread-1", + "payload": map[string]any{ + "mimeType": "text/plain", + "headers": []map[string]any{ + {"name": "Message-ID", "value": ""}, + {"name": "References", "value": ""}, + {"name": "From", "value": `"Alice Sender" `}, + {"name": "To", "value": `"Me Person" , "Other Person" `}, + {"name": "Cc", "value": `"CC Person" `}, + {"name": "Date", "value": "Fri, 12 Jun 2026 10:00:00 +0000"}, + {"name": "Subject", "value": "Project update"}, + }, + "body": map[string]any{"data": plain, "size": len(plain)}, + }, + } +} + +func mockForwardSourceMessage() map[string]any { + return mockOriginalMessage(false) +} + +func sendAsListHandler(w http.ResponseWriter) { + _ = json.NewEncoder(w).Encode(map[string]any{ + "sendAs": []map[string]any{ + {"sendAsEmail": "me@example.com", "displayName": "Me Person", "isPrimary": true, "verificationStatus": "accepted"}, + {"sendAsEmail": "alias@example.com", "displayName": "Alias", "verificationStatus": "accepted"}, + }, + }) +} + +// writeDraftCreatedResponse writes the canned Drafts.Create response shared by +// the drafts compose tests. +func writeDraftCreatedResponse(w http.ResponseWriter) { + _ = json.NewEncoder(w).Encode(map[string]any{"id": "d1", "message": map[string]any{"id": "m1"}}) +} + +// handleFinalizeRaw services the finalize POST shared by the send and draft +// paths: it decodes the outgoing message (from a Draft body when finalizePath is +// the drafts endpoint, otherwise from a bare Message), writes the canned finalize +// response, and returns the decoded RFC822 raw plus the stamped ThreadId. +func handleFinalizeRaw(t *testing.T, w http.ResponseWriter, r *http.Request, finalizePath string) (raw, threadID string) { + t.Helper() + var msg *gmail.Message + if finalizePath == "/gmail/v1/users/me/drafts" { + var draft gmail.Draft + if err := json.NewDecoder(r.Body).Decode(&draft); err != nil { + t.Fatalf("decode draft: %v", err) + } + msg = draft.Message + writeDraftCreatedResponse(w) + } else { + var m gmail.Message + if err := json.NewDecoder(r.Body).Decode(&m); err != nil { + t.Fatalf("decode send: %v", err) + } + msg = &m + _ = json.NewEncoder(w).Encode(map[string]any{"id": "sent-1", "threadId": "thread-1"}) + } + if msg == nil { + t.Fatalf("nil message in finalize body") + } + decoded, err := base64.RawURLEncoding.DecodeString(msg.Raw) + if err != nil { + t.Fatalf("decode raw: %v", err) + } + return string(decoded), msg.ThreadId +} + +// captureComposeRaw runs a compose command — send, reply, or their draft +// counterparts — against a mock Gmail server and returns the raw RFC822 of the +// outgoing message plus the stamped ThreadId. finalizePath is the API path the +// command finalizes through ("/gmail/v1/users/me/messages/send" or +// "/gmail/v1/users/me/drafts"). source supplies the msg-1 payload for commands +// that fetch a reply target; a plain gmail send never requests it. The config +// home is isolated so a developer's real no-send config cannot block the +// send-side commands. +func captureComposeRaw(t *testing.T, args []string, finalizePath string, source func() map[string]any) (raw, threadID string) { + t.Helper() + setTestConfigHome(t) + svc, cleanup := newGmailServiceForTest(t, func(w http.ResponseWriter, r *http.Request) { + w.Header().Set("Content-Type", "application/json") + switch { + case r.Method == http.MethodGet && r.URL.Path == "/gmail/v1/users/me/settings/sendAs": + sendAsListHandler(w) + case r.Method == http.MethodGet && r.URL.Path == "/gmail/v1/users/me/messages/msg-1": + _ = json.NewEncoder(w).Encode(source()) + case r.Method == http.MethodPost && r.URL.Path == finalizePath: + raw, threadID = handleFinalizeRaw(t, w, r, finalizePath) + default: + http.NotFound(w, r) + } + }) + defer cleanup() + + result := executeWithGmailTestService(t, args, svc) + if result.err != nil { + t.Fatalf("Execute(%v): %v", args, result.err) + } + return raw, threadID +} + +// captureForwardRaw runs a forward-style command and returns the raw RFC822 and +// stamped ThreadId. source supplies the original-message payload; when it +// references attachmentIds (e.g. mockOriginalMessage(true)), the attachment +// bytes are served from the attachments endpoint. The config home is isolated +// so a developer's real no-send config cannot block the send-side forward. +func captureForwardRaw(t *testing.T, args []string, finalizePath string, source func() map[string]any) (raw, threadID string) { + t.Helper() + setTestConfigHome(t) + svc, cleanup := newGmailServiceForTest(t, func(w http.ResponseWriter, r *http.Request) { + w.Header().Set("Content-Type", "application/json") + switch { + case r.Method == http.MethodGet && r.URL.Path == "/gmail/v1/users/me/settings/sendAs": + sendAsListHandler(w) + case r.Method == http.MethodGet && strings.Contains(r.URL.Path, "/attachments/"): + // Original message attachments (e.g. report.pdf / att-123) re-attached + // on forward. Deterministic bytes so the parity comparison holds. + _ = json.NewEncoder(w).Encode(map[string]any{ + "data": base64.RawURLEncoding.EncodeToString([]byte("pdf-file-contents")), + "size": 100, + }) + case r.Method == http.MethodGet && strings.Contains(r.URL.Path, "/gmail/v1/users/me/messages/orig-msg-1"): + _ = json.NewEncoder(w).Encode(source()) + case r.Method == http.MethodPost && r.URL.Path == finalizePath: + raw, threadID = handleFinalizeRaw(t, w, r, finalizePath) + default: + http.NotFound(w, r) + } + }) + defer cleanup() + + result := executeWithGmailTestService(t, args, svc) + if result.err != nil { + t.Fatalf("Execute(%v): %v", args, result.err) + } + return raw, threadID +} + +// rawMessageHeader extracts a single RFC822 header value from a raw MIME +// message (the headers precede the first blank line). It returns "" when the +// header is absent. +func rawMessageHeader(t *testing.T, raw, name string) string { + t.Helper() + msg, err := mail.ReadMessage(strings.NewReader(raw)) + if err != nil { + t.Fatalf("parse raw message: %v\nraw:\n%s", err, raw) + } + return msg.Header.Get(name) +} + +// wantAddr is one expected recipient (display name + address) for +// assertHeaderRecipients. +type wantAddr struct { + name string + address string +} + +// assertHeaderRecipients parses header from raw as an RFC822 address list and +// asserts it contains exactly want, in order, with display names intact. It +// proves a comma inside a quoted display name did not split a recipient in two. +func assertHeaderRecipients(t *testing.T, raw, header string, want []wantAddr) { + t.Helper() + value := rawMessageHeader(t, raw, header) + addrs, err := mail.ParseAddressList(value) + if err != nil { + t.Fatalf("parse %s header %q: %v", header, value, err) + } + if len(addrs) != len(want) { + t.Fatalf("%s: expected exactly %d recipients, got %d from %q", header, len(want), len(addrs), value) + } + for i, w := range want { + if addrs[i].Name != w.name || addrs[i].Address != w.address { + t.Errorf("%s[%d] = %q <%s>, want %q <%s>", header, i, addrs[i].Name, addrs[i].Address, w.name, w.address) + } + } +} + +// assertDryRunRequestList decodes a --json --dry-run result and asserts that +// request[field] is exactly want (a single-element-per-recipient list). This +// locks the invariant that the dry-run dict reports recipients parsed the same +// way buildGmailMessage builds them — a display-name comma must not split into +// an extra element. +func assertDryRunRequestList(t *testing.T, stdout, field string, want []string) { + t.Helper() + var payload struct { + DryRun bool `json:"dry_run"` + Request map[string]any `json:"request"` + } + if err := json.Unmarshal([]byte(stdout), &payload); err != nil { + t.Fatalf("decode dry-run output: %v\n%s", err, stdout) + } + if !payload.DryRun { + t.Fatalf("expected dry_run=true, got:\n%s", stdout) + } + raw, ok := payload.Request[field].([]any) + if !ok { + t.Fatalf("request[%q] is not a list: %#v", field, payload.Request[field]) + } + got := make([]string, len(raw)) + for i, v := range raw { + s, ok := v.(string) + if !ok { + t.Fatalf("request[%q][%d] is not a string: %#v", field, i, v) + } + got[i] = s + } + if len(got) != len(want) { + t.Fatalf("request[%q] = %#v, want %#v", field, got, want) + } + for i := range want { + if got[i] != want[i] { + t.Fatalf("request[%q][%d] = %q, want %q", field, i, got[i], want[i]) + } + } +} + func gmailSearchTestHandler() http.Handler { return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { path := r.URL.Path diff --git a/internal/mailmime/mime.go b/internal/mailmime/mime.go index 12fa1f058..e8c63759f 100644 --- a/internal/mailmime/mime.go +++ b/internal/mailmime/mime.go @@ -410,12 +410,24 @@ func formatAddressHeader(value string) string { } if strings.TrimSpace(addr.Name) == "" { - return addr.Address + return bareAddressWireForm(addr) } return addr.String() } +// bareAddressWireForm returns the wire form of a nameless address: the bare +// address when it re-parses, addr.String() otherwise. A quoted local part +// ("john smith"@example.com) is stored unquoted in addr.Address and is +// invalid bare, so it must be re-quoted. +func bareAddressWireForm(addr *mail.Address) string { + if _, err := mail.ParseAddress(addr.Address); err != nil { + return addr.String() + } + + return addr.Address +} + func formatAddressHeaders(values []string) string { parts := make([]string, 0, len(values)) for _, value := range values { @@ -436,7 +448,7 @@ func formatAddressHeaders(values []string) string { formatted := make([]string, 0, len(addrs)) for _, addr := range addrs { if strings.TrimSpace(addr.Name) == "" { - formatted = append(formatted, addr.Address) + formatted = append(formatted, bareAddressWireForm(addr)) } else { formatted = append(formatted, addr.String()) }