diff --git a/CHANGELOG.md b/CHANGELOG.md index c13f86e5a..5f468861e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,7 @@ ## 0.35.1 - Unreleased +- Gmail: add draft-only reply, reply-all, and forward workflows with shared send-side composition, no-send compatibility, and address-aware recipient validation. (#977) — thanks @malob. - Safety: allow custom baked profiles to lock boolean CLI flags against command-line, environment, and config overrides without echoing the locked value in override errors. (#976) — thanks @ronny-rentner. - Auth: offer one-time re-authorization for expired or revoked stored OAuth refresh tokens after interactive confirmation, while preserving non-interactive recovery guidance. (#973) — thanks @inamiy. - Dependencies: update Kong, Google protobuf/genproto, Cloudflare Workers types, and pnpm to their latest releases. 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/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 45cdc7bae..8c15884e6 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, + Options: 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_drafts.go b/internal/cmd/gmail_drafts.go index 6a54a8f6a..5085fc19c 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 { @@ -302,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 } @@ -324,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) @@ -381,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, @@ -397,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) } } @@ -728,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), @@ -849,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), @@ -913,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 @@ -969,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 new file mode 100644 index 000000000..bec47b063 --- /dev/null +++ b/internal/cmd/gmail_drafts_compose.go @@ -0,0 +1,122 @@ +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"` + Options 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.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/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) + + 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 + } + + // 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 + } + + 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 +// 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) + + 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..e6b1363c4 --- /dev/null +++ b/internal/cmd/gmail_drafts_compose_test.go @@ -0,0 +1,798 @@ +package cmd + +import ( + "context" + "encoding/base64" + "encoding/json" + "errors" + "fmt" + "io" + "net/http" + "os" + "path/filepath" + "regexp" + "strings" + "testing" + + "google.golang.org/api/gmail/v1" + + "github.com/openclaw/gogcli/internal/app" + "github.com/openclaw/gogcli/internal/config" +) + +// 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}, + }, + }, + }, + } +} + +// 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. +// 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?$`) + raw = dateRE.ReplaceAllString(raw, "Date: NORMALIZED") + raw = msgIDRE.ReplaceAllString(raw, "Message-ID: NORMALIZED") + + 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 +} + +// 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 := 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) + } + 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, _ := 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) + } + // 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 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 + 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: + 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 + writeDraftCreatedResponse(w) + 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_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") + 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 + writeDraftCreatedResponse(w) + 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 := captureComposeRaw(t, []string{ + "--account", "me@example.com", + "gmail", "drafts", "reply", "msg-1", + "--body", "Thanks", + "--cc", "extra@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 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, _ := captureComposeRaw(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 := captureComposeRaw(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, _ := captureComposeRaw(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 + // A forward draft never stamps a thread, so the request ThreadId must be "". + *raw = readGmailDraftRaw(t, r, "") + writeDraftCreatedResponse(w) + 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") + } + // 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") + if regexp.MustCompile(`(?m)^To:`).MatchString(headerBlock) { + t.Fatalf("addressless draft unexpectedly has To header:\n%s", headerBlock) + } +} + +func TestGmailForward_NoRecipientsStillErrors(t *testing.T) { + 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, []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) + } + if requests != 0 { + t.Fatalf("expected no Gmail API requests, got %d", requests) + } +} + +// --- 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). + {"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{"--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) + } + // 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-backed body/note inputs land in the built draft. --- + +func TestGmailDraftsReply_BodyFileStdinBodyInDraft(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 + raw = readGmailDraftRaw(t, r, "thread-1") + writeDraftCreatedResponse(w) + 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_NoteFileStdinNoteInDraft(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": + raw = readGmailDraftRaw(t, r, "thread-1") + writeDraftCreatedResponse(w) + 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) + } +} + +// --- 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 8020e9315..5dea018a3 100644 --- a/internal/cmd/gmail_forward.go +++ b/internal/cmd/gmail_forward.go @@ -8,13 +8,19 @@ 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"` - To string `name:"to" help:"Recipients (comma-separated; required)" required:""` + MessageID string `arg:"" name:"messageId" help:"Gmail message ID to forward"` + Options GmailForwardOptions `embed:""` +} + +type GmailForwardOptions struct { + 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"` @@ -23,50 +29,137 @@ type GmailForwardCmd 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. +type forwardComposeInputs struct { + 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). + allowMissingTo bool +} + +// 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") + inputs, err := c.Options.resolveForwardInputs(ctx, c.MessageID, recipientsRequired) + if err != nil { + return err } - note, err := resolveBodyInput(ctx, c.Note, c.NoteFile) + if dryRunErr := dryRunExit(ctx, flags, "gmail.forward", c.Options.dryRunFields(inputs)); dryRunErr != nil { + return dryRunErr + } + + account, svc, err := requireGmailSendService(ctx, flags) + if err != nil { + return err + } + + built, err := c.Options.buildForwardComposeMessage(ctx, svc, account, inputs) if err != nil { return err } - toRecipients := splitCSV(c.To) - if len(toRecipients) == 0 { - return usage("required: --to") + sent, err := svc.Users.Messages.Send("me", built.message).Context(ctx).Do() + if err != nil { + return fmt.Errorf("send forward: %w", err) } - if dryRunErr := dryRunExit(ctx, flags, "gmail.forward", map[string]any{ - "message_id": messageID, - "to": toRecipients, - "cc": splitCSV(c.Cc), - "bcc": splitCSV(c.Bcc), + return writeGmailMessageResults(ctx, u, []gmailMessageResult{{ + From: built.fromHeader, + MessageID: sent.Id, + ThreadID: sent.ThreadId, + }}) +} + +// 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": inputs.ccRecipients, + "bcc": inputs.bccRecipients, "from": strings.TrimSpace(c.From), - "note_len": len(note), + "note_len": len(inputs.note), "skip_attachments": c.SkipAttachments, - }); dryRunErr != nil { - return dryRunErr } +} - account, svc, err := requireGmailSendService(ctx, flags) +// 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") + } + + // 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 err + return forwardComposeInputs{}, err + } + 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 } + return forwardComposeInputs{ + messageID: messageID, + note: note, + toRecipients: toRecipients, + ccRecipients: ccRecipients, + bccRecipients: bccRecipients, + allowMissingTo: req == recipientsOptional, + }, 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 err + 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,28 +177,28 @@ 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) - 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, @@ -113,24 +206,18 @@ func (c *GmailForwardCmd) Run(ctx context.Context, flags *RootFlags) error { BodyHTML: fwdHTML, Attachments: attachments, }, sendBatch{ - To: toRecipients, - Cc: ccRecipients, - Bcc: bccRecipients, - }, false) + To: inputs.toRecipients, + Cc: inputs.ccRecipients, + Bcc: inputs.bccRecipients, + }, inputs.allowMissingTo) 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_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_reply_commands.go b/internal/cmd/gmail_reply_commands.go index 47540dd21..940898db0 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,71 @@ func (c *GmailReplyAllCmd) Run(ctx context.Context, flags *RootFlags) error { return c.Options.run(ctx, flags, c.MessageID, true) } +// 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 +} + +// 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 + // 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 { u := ui.FromContext(ctx) - messageID = normalizeGmailMessageID(messageID) - if messageID == "" { - return usage("required: messageId") - } - body, htmlBody, err := resolveComposeBodyInputs(ctx, c.Body, c.BodyFile, c.BodyHTML, c.BodyHTMLFile) + inputs, err := c.resolveReplyInputs(ctx, messageID) 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 - } - signatureCmd := GmailSendCmd{ - Signature: c.Signature, - SignatureFrom: c.SignatureFrom, - SignatureFile: c.SignatureFile, + if dryRunErr := dryRunExit(ctx, flags, "gmail."+replyModeName(replyAll), c.dryRunFields(inputs)); dryRunErr != nil { + return dryRunErr } - if signatureErr := signatureCmd.validateSignatureOptions(); signatureErr != nil { - return signatureErr + + account, svc, err := requireGmailSendService(ctx, flags) + if err != nil { + return err } - attachPaths, err := expandComposeAttachmentPaths(c.Attach) + built, err := c.buildReplyComposeMessage(ctx, svc, account, inputs, replyAll) if err != nil { return err } - if dryRunErr := dryRunExit(ctx, flags, "gmail."+replyModeName(replyAll), map[string]any{ - "message_id": messageID, + 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, + }}) +} + +// 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, @@ -98,29 +120,77 @@ 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), - }); dryRunErr != nil { - return dryRunErr } +} - account, svc, err := requireGmailSendService(ctx, flags) +// 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 err + 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 +201,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 +214,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 +226,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 +240,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 +256,28 @@ 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) + return replyComposeMessage{}, 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) + threading := draftThreading{ + ThreadID: info.ThreadID, + InReplyTo: strings.TrimSpace(info.InReplyTo), + References: strings.TrimSpace(info.References), + } + if threading.InReplyTo != "" { + threading.Source = replyContextCaller } - 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, + threading: threading, + }, nil } diff --git a/internal/cmd/gmail_send.go b/internal/cmd/gmail_send.go index b8e83642a..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" @@ -14,32 +15,35 @@ 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 { - 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 } @@ -112,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, @@ -174,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 @@ -187,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 @@ -197,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, @@ -294,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 { @@ -304,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, @@ -326,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 { @@ -353,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_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) 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/cmd/safety_profile_test.go b/internal/cmd/safety_profile_test.go index db63709a7..896ea58bf 100644 --- a/internal/cmd/safety_profile_test.go +++ b/internal/cmd/safety_profile_test.go @@ -253,8 +253,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/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()) } 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