From 8e84aacfe387649d347bdb633bbe896d501536b2 Mon Sep 17 00:00:00 2001 From: Jeremy Daer Date: Tue, 1 Sep 2026 13:35:24 -0700 Subject: [PATCH 1/3] Consolidate the reply-prefill readers into one shared helper --- internal/cmd/thread_reply.go | 50 +--------------- internal/mail/reply_prefill.go | 73 ++++++++++++++++++++++++ internal/mail/reply_prefill_test.go | 88 +++++++++++++++++++++++++++++ internal/tui/compose.go | 55 ++++++++---------- 4 files changed, 186 insertions(+), 80 deletions(-) create mode 100644 internal/mail/reply_prefill.go create mode 100644 internal/mail/reply_prefill_test.go diff --git a/internal/cmd/thread_reply.go b/internal/cmd/thread_reply.go index ae85d507..54d79bfd 100644 --- a/internal/cmd/thread_reply.go +++ b/internal/cmd/thread_reply.go @@ -10,14 +10,11 @@ import ( "github.com/basecamp/hey-sdk/go/pkg/generated" "github.com/basecamp/hey-cli/internal/apierr" + "github.com/basecamp/hey-cli/internal/mail" ) // replyRecipients is who a reply goes out to, in HEY's three kinds of addressing. -type replyRecipients struct { - To []string - CC []string - BCC []string -} +type replyRecipients = mail.ReplyRecipients // threadReplyTarget carries the entry a reply answers, its subject, sender and // recipients, and an immutable client bound to the thread's mail account. HEY saves an @@ -58,7 +55,7 @@ func resolveThreadReply(ctx context.Context, threadID int64) (*threadReplyTarget AccountID: topic.AccountId, client: threadSDK, } - prefill, ok := replyPrefillFromServer(ctx, threadSDK, entryID) + prefill, ok := mail.ReplyPrefillFromServer(ctx, threadSDK, entryID) target.ActingSenderID = prefill.ActingSenderID target.Subject = prefill.Subject if ok { @@ -88,47 +85,6 @@ func resolveThreadReply(ctx context.Context, threadID int64) (*threadReplyTarget return target, nil } -// replyPrefill is how a reply starts out, as HEY prefills it: the "Re: …" subject it -// goes out under, the sender it goes out as, and who it goes out to. -type replyPrefill struct { - Subject string - ActingSenderID int64 - Addressed replyRecipients -} - -// replyPrefillFromServer asks HEY how a reply to the entry starts out -// (GET /entries/{id}/replies/new): the "Re: …" subject the reply carries; the sender -// it goes out as — resolved from the entry's own to and from addresses, so a thread on -// a shared or alternate address answers as that address, not the account default, and -// named only when it differs from the acting user; and its recipients — the entry's -// sender moved onto the To line and the acting user's own addresses, aliases and -// catch-alls excluded — the exclusion this CLI cannot compute locally, and the reason -// a reply used to be able to CC its writer back to themselves. A failed read falls -// back to the local computation, and so does an empty answer: on a thread with -// yourself, everyone HEY excludes is everyone there is, and the local list is what -// keeps that reply addressable. The subject and sender are answered even when the -// recipients are not — only they need the fallback, not what HEY already supplied. -func replyPrefillFromServer(ctx context.Context, client *hey.Client, entryID int64) (replyPrefill, bool) { - prefilled, err := client.Entries().NewReply(ctx, entryID) - if err != nil || prefilled == nil { - return replyPrefill{}, false - } - prefill := replyPrefill{ - Subject: prefilled.Subject, - ActingSenderID: prefilled.Sender.Id, - Addressed: replyRecipients{ - To: addressEmails(prefilled.Addressed.Directly), - CC: addressEmails(prefilled.Addressed.Copied), - BCC: addressEmails(prefilled.Addressed.Blindcopied), - }, - } - if len(prefill.Addressed.To)+len(prefill.Addressed.CC)+len(prefill.Addressed.BCC) == 0 { - prefill.Addressed = replyRecipients{} - return prefill, false - } - return prefill, true -} - // replySubject answers the subject a reply to the given subject carries, the way HEY // derives it in Entry::Replyable#reply_subject: a "Re: " prefix, without doubling one // already there in any casing. An empty subject stays empty rather than becoming a diff --git a/internal/mail/reply_prefill.go b/internal/mail/reply_prefill.go new file mode 100644 index 00000000..31e0000f --- /dev/null +++ b/internal/mail/reply_prefill.go @@ -0,0 +1,73 @@ +package mail + +import ( + "context" + + "github.com/basecamp/hey-sdk/go/pkg/generated" + + hey "github.com/basecamp/hey-sdk/go/pkg/hey" +) + +// ReplyRecipients is who a reply goes out to, in HEY's three kinds of addressing. +type ReplyRecipients struct { + To []string + CC []string + BCC []string +} + +// ReplyPrefill is how a reply starts out, as HEY prefills it: the "Re: …" subject it +// goes out under, the sender it goes out as, and who it goes out to. The prefill's +// quoted content is deliberately not carried: a reply's content is the writer's body +// alone — the server appends the quoted original at delivery (auto_quoting defaults +// on), so echoing the prefill's quote back would double it. +type ReplyPrefill struct { + Subject string + ActingSenderID int64 + Addressed ReplyRecipients +} + +// ReplyPrefillFromServer asks HEY how a reply to the entry starts out +// (GET /entries/{id}/replies/new): the "Re: …" subject the reply carries; the sender +// it goes out as — resolved from the entry's own to and from addresses, so a thread on +// a shared or alternate address answers as that address, not the account default, and +// named only when it differs from the acting user; and its recipients — the entry's +// sender moved onto the To line and the acting user's own addresses, aliases and +// catch-alls excluded — the exclusion no client can compute locally, and the reason +// a reply used to be able to CC its writer back to themselves. A false answer sends +// the caller to its local fallback: a failed read needs one, and so does an empty +// recipient list — on a thread with yourself, everyone HEY excludes is everyone there +// is, and the local list is what keeps that reply addressable. The subject and sender +// are answered even when the recipients are not — only they need the fallback, not +// what HEY already supplied. +func ReplyPrefillFromServer(ctx context.Context, client *hey.Client, entryID int64) (ReplyPrefill, bool) { + prefilled, err := client.Entries().NewReply(ctx, entryID) + if err != nil || prefilled == nil { + return ReplyPrefill{}, false + } + prefill := ReplyPrefill{ + Subject: prefilled.Subject, + ActingSenderID: prefilled.Sender.Id, + Addressed: ReplyRecipients{ + To: contactEmails(prefilled.Addressed.Directly), + CC: contactEmails(prefilled.Addressed.Copied), + BCC: contactEmails(prefilled.Addressed.Blindcopied), + }, + } + if len(prefill.Addressed.To)+len(prefill.Addressed.CC)+len(prefill.Addressed.BCC) == 0 { + prefill.Addressed = ReplyRecipients{} + return prefill, false + } + return prefill, true +} + +// contactEmails answers the contacts' email addresses verbatim, dropping blanks: the +// prefill's lists are HEY's own computation, not input to clean up. +func contactEmails(contacts []generated.Contact) []string { + var emails []string + for _, contact := range contacts { + if contact.EmailAddress != "" { + emails = append(emails, contact.EmailAddress) + } + } + return emails +} diff --git a/internal/mail/reply_prefill_test.go b/internal/mail/reply_prefill_test.go new file mode 100644 index 00000000..db2a6876 --- /dev/null +++ b/internal/mail/reply_prefill_test.go @@ -0,0 +1,88 @@ +package mail + +import ( + "context" + "fmt" + "net/http" + "reflect" + "testing" + + hey "github.com/basecamp/hey-sdk/go/pkg/hey" +) + +// replyPrefillClient answers GET /entries/12/replies/new.json with the given body, the +// way HEY serves a reply prefill. +func replyPrefillClient(t *testing.T, prefillJSON string) *hey.Client { + t.Helper() + return testClient(t, func(w http.ResponseWriter, r *http.Request) { + if r.URL.Path != "/entries/12/replies/new.json" { + t.Errorf("read %s %s, want the entry's reply prefill", r.Method, r.URL.Path) + http.NotFound(w, r) + return + } + w.Header().Set("Content-Type", "application/json") + fmt.Fprint(w, prefillJSON) + }) +} + +func TestReplyPrefillFromServer(t *testing.T) { + client := replyPrefillClient(t, `{ + "subject": "Re: Weekly sync", "content": "
quoted
", "is_reply": true, + "sender": {"id": 215, "name": "Support", "email_address": "support@example.com"}, + "addressed": { + "directly": [{"id": 31, "name": "Rick", "email_address": "rick@example.com"}, {"id": 32}], + "copied": [{"id": 33, "email_address": "cc@example.com"}] + } + }`) + + prefill, ok := ReplyPrefillFromServer(context.Background(), client, 12) + if !ok { + t.Fatal("an addressed prefill answers; no fallback is needed") + } + if prefill.Subject != "Re: Weekly sync" { + t.Errorf("subject = %q, want the prefilled one", prefill.Subject) + } + if prefill.ActingSenderID != 215 { + t.Errorf("acting sender = %d, want the prefill's 215", prefill.ActingSenderID) + } + // The addressless contact is dropped; the rest ride verbatim. The quoted content + // is not carried at all: HEY appends it at delivery, and echoing it back would + // double the quote. + want := ReplyRecipients{To: []string{"rick@example.com"}, CC: []string{"cc@example.com"}} + if !reflect.DeepEqual(prefill.Addressed, want) { + t.Errorf("addressed = %+v, want %+v", prefill.Addressed, want) + } +} + +// The subject and sender are answered even when the recipients are not: on a thread +// with yourself, everyone HEY excludes is everyone there is, and only the recipients +// need the caller's local fallback. +func TestReplyPrefillFromServerWithoutRecipients(t *testing.T) { + client := replyPrefillClient(t, `{"subject": "Re: Weekly sync", + "sender": {"id": 215, "email_address": "support@example.com"}, "addressed": {}}`) + + prefill, ok := ReplyPrefillFromServer(context.Background(), client, 12) + if ok { + t.Fatal("a recipientless prefill sends the caller to its local fallback") + } + if prefill.Subject != "Re: Weekly sync" || prefill.ActingSenderID != 215 { + t.Errorf("subject = %q, sender = %d — both survive an empty recipient list", + prefill.Subject, prefill.ActingSenderID) + } + if !reflect.DeepEqual(prefill.Addressed, ReplyRecipients{}) { + t.Errorf("addressed = %+v, want none", prefill.Addressed) + } +} + +// A read that fails answers nothing: subject, sender and recipients all fall back. +func TestReplyPrefillFromServerUnreachable(t *testing.T) { + client := testClient(t, func(w http.ResponseWriter, r *http.Request) { + w.Header().Set("Content-Type", "application/json") + http.Error(w, `{"message":"not found"}`, http.StatusNotFound) + }) + + prefill, ok := ReplyPrefillFromServer(context.Background(), client, 12) + if ok || !reflect.DeepEqual(prefill, ReplyPrefill{}) { + t.Errorf("prefill = %+v, ok = %v — an unreachable prefill answers nothing", prefill, ok) + } +} diff --git a/internal/tui/compose.go b/internal/tui/compose.go index 1f975cc6..fb7c7d7a 100644 --- a/internal/tui/compose.go +++ b/internal/tui/compose.go @@ -14,6 +14,7 @@ import ( hey "github.com/basecamp/hey-sdk/go/pkg/hey" "github.com/basecamp/hey-cli/internal/htmlutil" + "github.com/basecamp/hey-cli/internal/mail" ) // --- Messages --- @@ -422,37 +423,25 @@ func (v *mailView) loadReplyContext(topicID int64, topicName string) tea.Cmd { } entryID := topic.Entries[len(topic.Entries)-1].Id - // HEY's reply prefill (GET /entries/{id}/replies/new) is the authority on how - // a reply starts out: the "Re: …" subject it goes out under, the sender it - // goes out as — on a shared or alternate address, not the account default — - // and recipients with the acting user's own addresses, aliases and catch-alls - // excluded — an exclusion this client cannot compute locally. A failed read - // falls back to the local computation, and so does an empty answer: on a - // thread with yourself, everyone HEY excludes is everyone there is. The - // prefill's subject and sender survive that recipient fallback — only the - // recipients needed it. - var prefillSubject string - var prefillSenderID int64 - if prefilled, prefillErr := accountSDK.Entries().NewReply(ctx, entryID); prefillErr == nil && prefilled != nil { - prefillSubject = prefilled.Subject - prefillSenderID = prefilled.Sender.Id - to := addressesOf(prefilled.Addressed.Directly, "") - cc := addressesOf(prefilled.Addressed.Copied, "") - bcc := addressesOf(prefilled.Addressed.Blindcopied, "") - if len(to)+len(cc)+len(bcc) > 0 { - return replyContextLoadedMsg{ - requestID: requestID, - boxID: boxID, - topicID: topicID, - topicName: topicName, - entryID: entryID, - sdk: accountSDK, - actingSenderID: prefillSenderID, - subject: prefillSubject, - to: to, - cc: cc, - bcc: bcc, - } + // HEY's reply prefill is the authority on how a reply starts out — see + // mail.ReplyPrefillFromServer. A failed read falls back to the local + // computation, and so does an empty answer: on a thread with yourself, + // everyone HEY excludes is everyone there is. The prefill's subject and + // sender survive that recipient fallback — only the recipients needed it. + prefill, ok := mail.ReplyPrefillFromServer(ctx, accountSDK, entryID) + if ok { + return replyContextLoadedMsg{ + requestID: requestID, + boxID: boxID, + topicID: topicID, + topicName: topicName, + entryID: entryID, + sdk: accountSDK, + actingSenderID: prefill.ActingSenderID, + subject: prefill.Subject, + to: prefill.Addressed.To, + cc: prefill.Addressed.CC, + bcc: prefill.Addressed.BCC, } } @@ -468,7 +457,7 @@ func (v *mailView) loadReplyContext(topicID int64, topicName string) tea.Cmd { } } to, cc, bcc := recipientsForReplyTo(*message) - subject := prefillSubject + subject := prefill.Subject if subject == "" { subject = replySubjectFor(*message) } @@ -479,7 +468,7 @@ func (v *mailView) loadReplyContext(topicID int64, topicName string) tea.Cmd { topicName: topicName, entryID: entryID, sdk: accountSDK, - actingSenderID: prefillSenderID, + actingSenderID: prefill.ActingSenderID, subject: subject, to: to, cc: cc, From f437358a2d0bfb78ba27e4f4ceec7ef2672e1559 Mon Sep 17 00:00:00 2001 From: Jeremy Daer Date: Tue, 1 Sep 2026 13:42:41 -0700 Subject: [PATCH 2/3] Point the reply-flow guide at the shared prefill helper --- AGENTS.md | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index ec0e243c..d56a30d9 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -153,13 +153,14 @@ because both were mis-stated here before: out: its "Re: …" subject and its recipients, with the entry's sender moved onto the To line (haystack's `directly_address_sender`) *and* the acting user's own addresses, aliases, catch-alls and redelivery contacts removed — the exclusion this CLI cannot - compute locally. Both reply paths — `replyPrefillFromServer` in - `internal/cmd/thread_reply.go` for `hey reply`, and `loadReplyContext` in - `internal/tui/compose.go` for the TUI's reply form — ask the prefill first and fall - back to the local computation (`recipientsForReplyTo` plus the derived subject) on a - failed read or an empty recipient answer, which a thread with yourself produces; the - prefill's subject survives that recipient fallback. Extend the prefill flow rather - than reimplementing HEY's exclusion rules here. + compute locally. Both reply paths — `hey reply` in `internal/cmd/thread_reply.go`, + and the TUI's reply form via `loadReplyContext` in `internal/tui/compose.go` — ask + the shared `mail.ReplyPrefillFromServer` (`internal/mail/reply_prefill.go`) first and + fall back to their local computation (`recipientsForReplyTo` plus the derived subject) + on a failed read or an empty recipient answer, which a thread with yourself produces; + the prefill's subject survives that recipient fallback. Extend + `mail.ReplyPrefillFromServer` rather than reimplementing HEY's exclusion rules in + each caller. `internal/htmlutil` provides `ToMarkdown` (HTML→Markdown), `ToText` (HTML→plain text), `ExtractImageURLs` and `ExtractAttachments`, which are presentation helpers rather than From af22ba0e423f1c76ac7c0c5f14aa42c6a1b0ead5 Mon Sep 17 00:00:00 2001 From: Jeremy Daer Date: Tue, 1 Sep 2026 13:49:24 -0700 Subject: [PATCH 3/3] Cover the prefill's blindcopied list in the shared reader's test --- internal/mail/reply_prefill_test.go | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/internal/mail/reply_prefill_test.go b/internal/mail/reply_prefill_test.go index db2a6876..c3b49e76 100644 --- a/internal/mail/reply_prefill_test.go +++ b/internal/mail/reply_prefill_test.go @@ -31,7 +31,8 @@ func TestReplyPrefillFromServer(t *testing.T) { "sender": {"id": 215, "name": "Support", "email_address": "support@example.com"}, "addressed": { "directly": [{"id": 31, "name": "Rick", "email_address": "rick@example.com"}, {"id": 32}], - "copied": [{"id": 33, "email_address": "cc@example.com"}] + "copied": [{"id": 33, "email_address": "cc@example.com"}], + "blindcopied": [{"id": 34, "email_address": "bcc@example.com"}] } }`) @@ -48,7 +49,11 @@ func TestReplyPrefillFromServer(t *testing.T) { // The addressless contact is dropped; the rest ride verbatim. The quoted content // is not carried at all: HEY appends it at delivery, and echoing it back would // double the quote. - want := ReplyRecipients{To: []string{"rick@example.com"}, CC: []string{"cc@example.com"}} + want := ReplyRecipients{ + To: []string{"rick@example.com"}, + CC: []string{"cc@example.com"}, + BCC: []string{"bcc@example.com"}, + } if !reflect.DeepEqual(prefill.Addressed, want) { t.Errorf("addressed = %+v, want %+v", prefill.Addressed, want) }