Skip to content

Commit 9a4131c

Browse files
authored
feat(mail): reply-to-thread drafts (#115)
* feat(mail): reply-to-thread drafts Extend `gro mail draft` with --reply-to and --reply-all flags, building on the existing DraftMessage/buildMIME/CreateDraft pipeline rather than forking a new command. - gmail.Message: expose Cc, RFCMessageID, References, In-Reply-To from the source message so the command layer can derive defaults. - gmail.DraftMessage: add ThreadID, InReplyTo, References fields; buildMIME emits the two new headers; guardHeaderInjection covers them; CreateDraft sets Draft.Message.ThreadId. - mail draft command: --reply-to fetches the source via existing GetMessage, derives To/Subject/threading; --reply-all fetches the profile + reads any --from alias to filter self out of the source To/Cc. Local validation runs before any Gmail API call. deriveReplyDefaults is fallible and rejects empty/malformed From or Message-Id headers, so we never silently produce a broken reply. - Re: prefixing is case-insensitive and locale-naive (we don't recognise Aw/Sv/Sp variants — matches Gmail's own behaviour). - Tests: 19 new cases across both layers (derivation, overrides, alias filtering, missing/malformed headers, local-before-remote validation, injection guard for the new flag and headers). - Integration: T21-T26 verify wire-format headers via gro mail read --json rather than UI inspection. Closes #114 * fix(mail): address Codex PR review — lazy reply derivation + restored local guards - deriveReplyDefaults now takes a replyNeeds struct and only parses source headers for slots the caller actually consumes. An explicit --to override no longer fails on a malformed source From; an explicit --cc override no longer fails on malformed source To/Cc. - Restore the local 'len(toAddrs) == 0' guard in non-reply mode so empty --to fails before any Gmail API call (regression introduced by reply mode). - Treat --reply-to "" as non-reply mode (don't call GetMessage with empty ID, don't skip --to/--subject requirements). --reply-all --reply-to "" still errors with the existing requires-reply-to message. - New tests pin exact alias-filter expectations and cover the three new edge cases above. * test(mail): cover the most-valuable TDD review gaps Added 4 tests for the highest-value defensive branches flagged by TDD review: - TestDraftCommand_ReplyAll_GetProfileFails: error from GetProfile aborts before CreateDraft (without this test, a regression that swallows the error could leak the user's address into Cc). - TestDraftCommand_ReplyAll_EmptyProfileFallsBackToFrom: empty profile + --from alias still produces correct self-filtering. - TestDraftCommand_ReplyTo_FirstReplyEmptyReferences: source with no prior References produces a 1-element chain (just the source Message-Id). - TestDraftCommand_ReplyTo_WithAttachment: threading headers survive the multipart path when --attach is also supplied. Other suggestions in the TDD comment (addRePrefix edge cases, empty --subject "" override, srcReply() shared-pointer concern) are low-yield and skipped. * fix(mail): reject empty --reply-to and empty --to override locally Address Codex re-review: --reply-to with an empty value (a common agentic-flow footgun when shell variables don't expand) now errors explicitly rather than silently falling back to non-reply mode. In reply mode, an explicit --to "" override is also rejected before any GetMessage call. * fix(mail): minor daemon polish — Refs dedup, T25 command, override test - buildReferences dedupes the trailing Message-Id when the source's References header already ends with it (some MUAs do this) and drops the redundant empty-token guard inside strings.Fields. - addRePrefix comment clarified to document that 're:foo' (no trailing space) is treated as already-prefixed. - New TestDraftCommand_ReplyAll_ExplicitToOverride pins the needs.To=false + needs.Cc=true combination. - T25 integration row now has an executable command instead of prose. * ci: gofmt + revive (ThreadId -> ThreadID)
1 parent 4aebff7 commit 9a4131c

9 files changed

Lines changed: 958 additions & 13 deletions

File tree

CLAUDE.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@ gro is a **non-destructive** command-line interface for Google services written
1010
**Module:** `github.com/open-cli-collective/google-readonly`
1111

1212
### Features
13-
- Gmail: Search, read, thread viewing, labels, attachments, archive, star, mark read/unread, label, categorize, draft (compose-only, never sent)
13+
- Gmail: Search, read, thread viewing, labels, attachments, archive, star, mark read/unread, label, categorize, draft (compose-only, never sent — supports reply-to-thread)
1414
- Google Calendar: List calendars, view events, today/week shortcuts
1515
- Google Contacts: List contacts, search, view details, list groups
1616
- Google Drive: List files, search, get details, download, tree view, shared drives

README.md

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -288,10 +288,17 @@ gro mail draft --to a@x.com --subject "See attached" --body "..." --attach repor
288288

289289
# From a send-as alias
290290
gro mail draft --from work@me.com --to a@x.com --subject "Hi" --body "..."
291+
292+
# Reply to an existing message (preserves thread, adds In-Reply-To / References)
293+
gro mail draft --reply-to <message-id> --body "thanks, will review"
294+
gro mail draft --reply-to <message-id> --reply-all --body "looping everyone in"
295+
gro mail draft --reply-to <message-id> --subject "Re: customised" --body "..."
291296
```
292297

293298
Drafts always land in your Gmail Drafts folder for human review. The CLI never calls `drafts.send` — sending requires explicit action in Gmail.
294299

300+
With `--reply-to`, `--to` and `--subject` are derived from the source message (To = original From; Subject = `Re: <original>`, no double prefix). Explicit `--to` / `--cc` / `--subject` flags override the derived values. `--reply-all` adds the original To and Cc as Cc on the reply, filtered to remove your own address (and any `--from` alias).
301+
295302
### Calendar Commands
296303

297304
All Calendar commands are under `gro calendar` (or `gro cal`):
@@ -733,12 +740,16 @@ Flags:
733740
--html Send body as raw HTML (no markdown rendering)
734741
-a, --attach strings File path to attach (repeat for multiple)
735742
-j, --json Output result as JSON
743+
--reply-to string Source Gmail message ID to reply to (derives To/Subject/threading)
744+
--reply-all Include the source To/Cc as Cc on the reply (requires --reply-to)
736745
```
737746

738747
`--body`, `--stdin`, and `--file` are mutually exclusive (exactly one required). `--plain` and `--html` are mutually exclusive.
739748

740749
Display names in `--to`/`--cc`/`--bcc` (e.g., `Alice <alice@example.com>`) are stripped; only the email address is sent. Edit the draft in Gmail to set display names.
741750

751+
With `--reply-to`, the draft is threaded onto the source conversation (`In-Reply-To` and `References` headers are set; `Draft.Message.ThreadId` is set to the source thread). `--to` and `--subject` are derived from the source (To = original From; Subject = `Re: <original>` with no double prefix). Explicit `--to`/`--cc`/`--subject` flags override the derived values. `--reply-all` populates Cc with the source To+Cc minus your authenticated account and any `--from` alias.
752+
742753
### gro calendar list
743754

744755
List all calendars the user has access to.

integration-tests.md

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -565,6 +565,34 @@ Skip if no Gmail send-as alias is configured for the test user.
565565
| T19 | Valid alias | `./bin/gro mail draft --from <configured-alias> --to "$ME" --subject "T19 alias" --body "x" --plain` | Exit 0. Gmail Drafts shows the alias in From line. |
566566
| T20 | Unconfigured alias | `./bin/gro mail draft --from "not-my-alias@example.com" --to "$ME" --subject "T20 bad alias" --body "x" --plain; echo "exit=$?"` | Non-zero exit. API error surfaced verbatim from `creating draft: ...`. |
567567

568+
### Reply-to-thread (`--reply-to` / `--reply-all`)
569+
570+
**Setup additions:**
571+
```bash
572+
# Pick any existing message in your inbox as the source. The query can be
573+
# anything that resolves to a single recent message.
574+
SRC_ID=$(./bin/gro mail search "in:inbox" --json | jq -r '.[0].id')
575+
SRC_RFC=$(./bin/gro mail read "$SRC_ID" --json | jq -r '.rfcMessageId')
576+
SRC_THREAD=$(./bin/gro mail read "$SRC_ID" --json | jq -r '.threadId')
577+
echo "source: $SRC_ID / thread: $SRC_THREAD / rfc-id: $SRC_RFC"
578+
```
579+
580+
For each happy-path scenario, capture the created draft's message ID and read it back to verify the wire-format headers:
581+
582+
```bash
583+
DRAFT_MSG_ID=$(./bin/gro mail draft --reply-to "$SRC_ID" --body "T21 reply" --plain --json | jq -r '.messageId')
584+
./bin/gro mail read "$DRAFT_MSG_ID" --json | jq '{inReplyTo, references, threadId}'
585+
```
586+
587+
| # | Test Case | Command | Expected Result |
588+
|---|-----------|---------|-----------------|
589+
| T21 | Simple reply | `./bin/gro mail draft --reply-to "$SRC_ID" --body "T21 reply" --plain --json` | Exit 0. Read-back JSON shows `threadId == $SRC_THREAD`, `inReplyTo == $SRC_RFC`, `references` ends with `$SRC_RFC`, subject prefixed `Re: `. Gmail UI shows the draft grouped under the original conversation. |
590+
| T22 | Reply-all | `./bin/gro mail draft --reply-to "$SRC_ID" --reply-all --body "T22 all" --plain --json` | Exit 0. Cc on the draft contains the source To+Cc minus `$ME`. Read-back JSON same threading checks as T21. |
591+
| T23 | Explicit subject override | `./bin/gro mail draft --reply-to "$SRC_ID" --subject "T23 custom" --body "x" --plain --json` | Exit 0. Draft subject is `T23 custom` (no `Re:` prefix). Threading headers still set. |
592+
| T24 | Explicit Cc override | `./bin/gro mail draft --reply-to "$SRC_ID" --cc "$ME" --body "T24 cc" --plain --json` | Exit 0. Draft Cc is exactly `$ME` (replaces, not merges with, derived Cc). |
593+
| T25 | No double Re: prefix | First find a source message whose subject already starts with `Re:`: `RE_SRC=$(./bin/gro mail search "subject:Re:" --json \| jq -r '.[0].id')`. Then: `./bin/gro mail draft --reply-to "$RE_SRC" --body "T25" --plain --json` | Exit 0. Read-back subject is unchanged (no `Re: Re: ` doubling). |
594+
| T26 | Reply-all without reply-to | `./bin/gro mail draft --to "$ME" --subject "x" --body "y" --reply-all; echo "exit=$?"` | Non-zero exit. Error mentions `--reply-all requires --reply-to`. |
595+
568596
### Cleanup
569597

570598
```bash

internal/cmd/mail/draft.go

Lines changed: 212 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,8 @@ func newDraftCommand() *cobra.Command {
3131
htmlMode bool
3232
attach []string
3333
jsonOut bool
34+
replyTo string
35+
replyAll bool
3436
)
3537

3638
cmd := &cobra.Command{
@@ -50,34 +52,48 @@ Examples:
5052
gro mail draft --to "a@x.com, b@x.com" --subject "Sync" --file notes.md
5153
echo "# Hello" | gro mail draft --to a@x.com --subject "Hi" --stdin
5254
gro mail draft --to a@x.com --subject "Plain" --body "no md" --plain
53-
gro mail draft --to a@x.com --subject "Report" --body "see attached" --attach report.pdf`,
55+
gro mail draft --to a@x.com --subject "Report" --body "see attached" --attach report.pdf
56+
gro mail draft --reply-to <message-id> --body "thanks, will review"
57+
gro mail draft --reply-to <message-id> --reply-all --body "..."`,
5458
RunE: func(cmd *cobra.Command, _ []string) error {
55-
// 1. Required-flag checks.
56-
if !cmd.Flags().Changed("to") {
57-
return fmt.Errorf("--to is required")
59+
// --reply-to with an empty value is a user error (often a shell
60+
// variable that didn't expand). Reject it explicitly so we don't
61+
// silently produce an unthreaded draft.
62+
if cmd.Flags().Changed("reply-to") && strings.TrimSpace(replyTo) == "" {
63+
return fmt.Errorf("--reply-to requires a non-empty message ID")
5864
}
59-
if !cmd.Flags().Changed("subject") {
60-
return fmt.Errorf("--subject is required (use --subject \"\" for empty subject)")
65+
isReply := cmd.Flags().Changed("reply-to")
66+
67+
// 1. Required-flag checks. In reply mode, --to and --subject are
68+
// derived from the source message and only required if not derivable.
69+
if replyAll && !isReply {
70+
return fmt.Errorf("--reply-all requires --reply-to")
71+
}
72+
if !isReply {
73+
if !cmd.Flags().Changed("to") {
74+
return fmt.Errorf("--to is required")
75+
}
76+
if !cmd.Flags().Changed("subject") {
77+
return fmt.Errorf("--subject is required (use --subject \"\" for empty subject)")
78+
}
6179
}
6280

6381
// 2. Header-injection guard on raw flag values.
6482
for _, f := range []struct{ name, val string }{
6583
{"--to", to}, {"--cc", cc}, {"--bcc", bcc},
6684
{"--from", fromAddr}, {"--subject", subject},
85+
{"--reply-to", replyTo},
6786
} {
6887
if strings.ContainsAny(f.val, "\r\n") {
6988
return fmt.Errorf("%s contains illegal CR or LF", f.name)
7089
}
7190
}
7291

73-
// 3. Address parsing.
92+
// 3. Address parsing on user-supplied flags.
7493
toAddrs, err := parseAddressList("--to", to)
7594
if err != nil {
7695
return err
7796
}
78-
if len(toAddrs) == 0 {
79-
return fmt.Errorf("--to must contain at least one address")
80-
}
8197
ccAddrs, err := parseAddressList("--cc", cc)
8298
if err != nil {
8399
return err
@@ -98,6 +114,17 @@ Examples:
98114
fromAddr = parsed.Address
99115
}
100116

117+
// 3b. Local recipient validation (no I/O yet).
118+
// - Non-reply mode: --to is required and must parse to ≥1 address.
119+
// - Reply mode: an explicit --to override must still parse to ≥1
120+
// address; otherwise we defer the check until after derivation.
121+
if !isReply && len(toAddrs) == 0 {
122+
return fmt.Errorf("--to must contain at least one address")
123+
}
124+
if isReply && cmd.Flags().Changed("to") && len(toAddrs) == 0 {
125+
return fmt.Errorf("--to must contain at least one address")
126+
}
127+
101128
// 4. Body source: exactly one of --body, --stdin, --file.
102129
bodySources := 0
103130
if cmd.Flags().Changed("body") {
@@ -169,11 +196,63 @@ Examples:
169196
})
170197
}
171198

172-
// 9. Call client.
199+
// 9. Build client; in reply mode, fetch source + (optionally) profile and derive defaults.
173200
client, err := newGmailClient(cmd.Context())
174201
if err != nil {
175202
return fmt.Errorf("creating Gmail client: %w", err)
176203
}
204+
205+
var (
206+
threadID string
207+
inReplyTo string
208+
references []string
209+
)
210+
if isReply {
211+
src, err := client.GetMessage(cmd.Context(), replyTo, false)
212+
if err != nil {
213+
return fmt.Errorf("fetching reply source %s: %w", replyTo, err)
214+
}
215+
needs := replyNeeds{
216+
To: !cmd.Flags().Changed("to"),
217+
Cc: replyAll && !cmd.Flags().Changed("cc"),
218+
Subject: !cmd.Flags().Changed("subject"),
219+
}
220+
selfSet := map[string]bool{}
221+
if needs.Cc {
222+
profile, err := client.GetProfile(cmd.Context())
223+
if err != nil {
224+
return fmt.Errorf("fetching profile for --reply-all: %w", err)
225+
}
226+
if profile != nil && profile.EmailAddress != "" {
227+
selfSet[strings.ToLower(profile.EmailAddress)] = true
228+
}
229+
if fromAddr != "" {
230+
selfSet[strings.ToLower(fromAddr)] = true
231+
}
232+
}
233+
derived, err := deriveReplyDefaults(src, needs, selfSet)
234+
if err != nil {
235+
return err
236+
}
237+
threadID = derived.ThreadID
238+
inReplyTo = derived.InReplyTo
239+
references = derived.References
240+
if needs.To {
241+
toAddrs = derived.To
242+
}
243+
if needs.Cc {
244+
ccAddrs = derived.Cc
245+
}
246+
if needs.Subject {
247+
subject = derived.Subject
248+
}
249+
}
250+
251+
// 9b. Post-derivation resolved-recipient validation.
252+
if len(toAddrs) == 0 {
253+
return fmt.Errorf("--to must contain at least one address")
254+
}
255+
177256
result, err := client.CreateDraft(cmd.Context(), gmailapi.DraftMessage{
178257
From: fromAddr,
179258
To: toAddrs,
@@ -183,6 +262,9 @@ Examples:
183262
Body: outBody,
184263
BodyKind: kind,
185264
Attachments: attachments,
265+
ThreadID: threadID,
266+
InReplyTo: inReplyTo,
267+
References: references,
186268
})
187269
if err != nil {
188270
return fmt.Errorf("creating draft: %w", err)
@@ -226,10 +308,129 @@ Examples:
226308
cmd.Flags().BoolVar(&htmlMode, "html", false, "Treat body as raw HTML (no markdown rendering)")
227309
cmd.Flags().StringArrayVarP(&attach, "attach", "a", nil, "File path to attach (repeat for multiple)")
228310
cmd.Flags().BoolVarP(&jsonOut, "json", "j", false, "Output result as JSON")
311+
cmd.Flags().StringVar(&replyTo, "reply-to", "", "Source Gmail message ID to reply to (derives To/Subject/threading)")
312+
cmd.Flags().BoolVar(&replyAll, "reply-all", false, "Include the source To/Cc as Cc on the reply (requires --reply-to)")
229313

230314
return cmd
231315
}
232316

317+
// replyDerivation is the result of analysing the source message for reply mode.
318+
type replyDerivation struct {
319+
ThreadID string
320+
InReplyTo string
321+
References []string
322+
To []string
323+
Cc []string
324+
Subject string
325+
}
326+
327+
// replyNeeds tells deriveReplyDefaults which slots the caller will actually
328+
// consume. Source headers are only parsed for needed slots, so an explicit
329+
// --to/--cc/--subject override is unaffected by a malformed/missing source
330+
// header for that slot.
331+
type replyNeeds struct {
332+
To bool
333+
Cc bool
334+
Subject bool
335+
}
336+
337+
func deriveReplyDefaults(src *gmailapi.Message, needs replyNeeds, selfSet map[string]bool) (replyDerivation, error) {
338+
if src == nil {
339+
return replyDerivation{}, fmt.Errorf("source message is nil")
340+
}
341+
// Message-Id is always required: it feeds In-Reply-To and References,
342+
// which are emitted regardless of override flags.
343+
if strings.TrimSpace(src.RFCMessageID) == "" {
344+
return replyDerivation{}, fmt.Errorf("source message %s has no Message-Id header", src.ID)
345+
}
346+
out := replyDerivation{
347+
ThreadID: src.ThreadID,
348+
InReplyTo: src.RFCMessageID,
349+
References: buildReferences(src.References, src.RFCMessageID),
350+
}
351+
if needs.To {
352+
if strings.TrimSpace(src.From) == "" {
353+
return replyDerivation{}, fmt.Errorf("source message %s has no From header", src.ID)
354+
}
355+
fromAddr, err := mail.ParseAddress(src.From)
356+
if err != nil {
357+
return replyDerivation{}, fmt.Errorf("parsing source From header %q: %w", src.From, err)
358+
}
359+
out.To = []string{fromAddr.Address}
360+
}
361+
if needs.Subject {
362+
out.Subject = addRePrefix(src.Subject)
363+
}
364+
if needs.Cc {
365+
ccAddrs, err := splitAddressHeaders(src.To, src.Cc)
366+
if err != nil {
367+
return replyDerivation{}, fmt.Errorf("parsing source To/Cc headers: %w", err)
368+
}
369+
out.Cc = filterSelf(ccAddrs, selfSet)
370+
}
371+
return out, nil
372+
}
373+
374+
// buildReferences appends the source Message-Id to the existing References
375+
// chain, splitting the raw header on whitespace (RFC 5322 §3.6.4). If the
376+
// chain already ends with msgID, it is not duplicated.
377+
func buildReferences(rawRefs, msgID string) []string {
378+
out := strings.Fields(rawRefs)
379+
if len(out) > 0 && out[len(out)-1] == msgID {
380+
return out
381+
}
382+
return append(out, msgID)
383+
}
384+
385+
// splitAddressHeaders parses each non-empty raw header value via
386+
// mail.ParseAddressList and concatenates the bare addresses. Returns an
387+
// error on the first malformed header rather than silently dropping content.
388+
func splitAddressHeaders(values ...string) ([]string, error) {
389+
var out []string
390+
for _, v := range values {
391+
if strings.TrimSpace(v) == "" {
392+
continue
393+
}
394+
addrs, err := mail.ParseAddressList(v)
395+
if err != nil {
396+
return nil, fmt.Errorf("%q: %w", v, err)
397+
}
398+
for _, a := range addrs {
399+
out = append(out, a.Address)
400+
}
401+
}
402+
return out, nil
403+
}
404+
405+
// filterSelf removes addresses present in selfSet (case-insensitive lookup)
406+
// and de-duplicates while preserving first-seen order.
407+
func filterSelf(addrs []string, selfSet map[string]bool) []string {
408+
seen := map[string]bool{}
409+
out := make([]string, 0, len(addrs))
410+
for _, a := range addrs {
411+
k := strings.ToLower(a)
412+
if selfSet[k] || seen[k] {
413+
continue
414+
}
415+
seen[k] = true
416+
out = append(out, a)
417+
}
418+
return out
419+
}
420+
421+
// addRePrefix prefixes "Re: " unless the subject already begins with a
422+
// case-insensitive "re:" (the colon is the delimiter; "re:foo" without a
423+
// trailing space still counts as already-prefixed). Locale variants
424+
// (Aw, Sv, etc.) are intentionally not recognised — Gmail itself
425+
// doesn't normalise them.
426+
func addRePrefix(subject string) string {
427+
trimmed := strings.TrimLeft(subject, " \t")
428+
if len(trimmed) >= 3 && strings.EqualFold(trimmed[:3], "re:") {
429+
return subject
430+
}
431+
return "Re: " + subject
432+
}
433+
233434
// parseAddressList parses a comma-separated address list. An empty input is OK
234435
// and returns nil — the caller decides whether emptiness is an error.
235436
func parseAddressList(flag, raw string) ([]string, error) {

0 commit comments

Comments
 (0)