From 02893f22a3d3bf66f2031e7ee1451a5d2b5685fb Mon Sep 17 00:00:00 2001 From: Rob Zolkos Date: Sun, 20 Sep 2026 16:55:59 -0400 Subject: [PATCH 1/2] Add reply recipient overrides and dry run (#463) --- .surface | 5 + docs/cli.md | 4 + internal/cmd/compose.go | 3 + internal/cmd/compose_test.go | 13 ++ internal/cmd/reply.go | 122 +++++++++++++++- internal/cmd/thread_reply.go | 146 +++++++++++++++++-- internal/cmd/thread_reply_test.go | 218 +++++++++++++++++++++++++++- internal/mail/reply_prefill.go | 13 ++ internal/mail/reply_prefill_test.go | 6 + skills/hey/SKILL.md | 9 +- tests/smoke/threads_test.go | 67 +++++++++ 11 files changed, 576 insertions(+), 30 deletions(-) diff --git a/.surface b/.surface index 9dee36fb..38644877 100644 --- a/.surface +++ b/.surface @@ -284,9 +284,14 @@ hey move hey move --to hey reply hey reply --attach +hey reply --bcc +hey reply --cc hey reply --draft +hey reply --dry-run hey reply --message hey reply --message-html +hey reply --replace-recipients +hey reply --to hey screener hey screener approve hey screener approve --box diff --git a/docs/cli.md b/docs/cli.md index 72cbb684..ee1ddbe2 100644 --- a/docs/cli.md +++ b/docs/cli.md @@ -224,6 +224,8 @@ hey unshare 123 # turn off the sharing link hey attachment list 123 # list files attached to the thread hey attachment save 456:1 # save a file using its attachment ID hey reply 123 -m "Friday works for me — I'll send an agenda." # or omit -m for $EDITOR +hey reply 123 --to support@example.com -m "The replacement is on the way." +hey reply 123 --to support@example.com --replace-recipients --dry-run --json hey reply 123 -m "Here is the wiring diagram." --attach ./diagram.png hey bulk-reply preview 12345 67890 # inspect threads and exact To/CC/BCC recipients hey bulk-reply send 12345 67890 -m "Thanks for the update." @@ -262,6 +264,8 @@ hey stop-ignoring 12345 # resume attention for a thread `hey thread read` reads a whole thread, oldest entry first, however many pages HEY serves it in — within limits it states: a hundred pages past the first, two thousand entries, as many bodies, 64 MiB of retained thread data and two minutes in all. The byte budget covers entry-index metadata, message bodies and metadata, the recipient identities retained for thread output, and inbound delivery addresses and resolved contact identities retained for JSON. A thread that could only be read in part — a body HEY would not serve, a limit reached — is refused rather than passed off as whole; `--allow-partial` takes what was read, with a `notice` saying what is missing and each entry's `body_state` saying whether its body was `hydrated`, `bodyless` (HEY served none), `over_limit` or `failed`. Each entry whose message was read carries `recipients`, with `to`, `cc` and `bcc` contact lists; the object is absent when the message was not read, while a known-empty line is `[]`. In JSON, an inbound entry also carries `received_via`: every exact account address HEY recorded the message arriving through, including plus tags and catch-all aliases. These are delivery records, not the visible To/CC/BCC recipients. A record's `contact` is optional and is omitted when HEY did not resolve one; the whole field is omitted for sent or generated messages and whenever the message was not read. `--count` and `--ids-only` read the entry index and no messages, so only a truncated index can make them partial. `--markdown` writes the thread as one Markdown document — a heading per entry naming the sender, date and ID, then the body — which is the shape to hand an agent or a notes app. `hey attachment list` reads the bodies in every format, since that is where attachment metadata lives, and answers a partial thread the same way. `hey reply` answers the thread's latest entry and addresses the reply the way HEY does: it asks HEY for the reply's recipients — everyone that entry was addressed to, its sender moved onto the To line, and your own addresses, aliases and catch-alls excluded — falling back to computing them from the entry when that read is unavailable. +Repeatable `hey reply --to`, `--cc` and `--bcc` flags add recipients to that envelope; comma-separated addresses also work. An explicitly named address moves to that line instead of appearing twice. `--replace-recipients` discards HEY's prefill and requires at least one explicit address. `--dry-run` needs no body, uploads nothing and sends nothing; its JSON data reports the account, thread, entry, subject, resolved sender, and final To, CC and BCC lists. + Email bodies come back as Markdown. `hey thread read` and the TUI render that Markdown for the terminal — headings, emphasis, lists, quotes, tables and code survive, and links keep their URLs and stay clickable where the terminal supports it. `--json` carries the same Markdown in `body`, so an agent reading a thread sees the structure a human sees rather than a flattened wall of text. `--html` keeps HEY's original body HTML and frames each entry with its From, To, CC and BCC headers. Writing is Markdown too, everywhere text goes in: `-m`, `--content`, `--note`, positional content, stdin, and `$EDITOR` (which opens prefilled with the existing entry or note as Markdown). Every such flag has a raw-HTML twin — `--message-html`, `--content-html`, `--note-html` — for sending markup verbatim; each pair is mutually exclusive. The TUI's compose and bulk-reply forms convert Markdown the same way, and the compose editor renders it live as you type — `**bold**` turns bold, markers and all. A fenced code block's language (` ```ruby `) is carried the way HEY's own editor stores it, so the web app syntax-highlights it. diff --git a/internal/cmd/compose.go b/internal/cmd/compose.go index 987f9136..55c58933 100644 --- a/internal/cmd/compose.go +++ b/internal/cmd/compose.go @@ -125,6 +125,9 @@ func (c *composeCommand) run(cmd *cobra.Command, args []string) error { if resolveErr != nil { return resolveErr } + if !replyHasRecipients(target.Addressed) { + return apierr.ErrUsage("could not determine thread recipients; use hey reply with --to, --cc or --bcc") + } replySDK := target.client messageWithAttachments, attachErr := attachFilesWithClient(ctx, replySDK, message, c.attachments) if attachErr != nil { diff --git a/internal/cmd/compose_test.go b/internal/cmd/compose_test.go index 0ce3cf15..bb2d9359 100644 --- a/internal/cmd/compose_test.go +++ b/internal/cmd/compose_test.go @@ -105,6 +105,19 @@ func TestComposeSubjectRequiredOnlyForANewMessage(t *testing.T) { } } +func TestComposeThreadReplyWithoutRecipientsIsRefusedBeforeWriting(t *testing.T) { + server, sent := threadReplyServer(t, messageWithoutRecipients, 11, 12) + + err := runCLI(t, server, "--account", "8", "compose", "--thread-id", "7", "-m", "must not send") + var cliErr *apierr.Error + if !errors.As(err, &cliErr) || cliErr.Code != "usage" || !strings.Contains(err.Error(), "use hey reply with --to") { + t.Fatalf("expected an actionable usage error, got %v", err) + } + if sent.Path != "" { + t.Errorf("unaddressed compose reply wrote to %q", sent.Path) + } +} + func TestComposeSendsTheMessageAsMarkdown(t *testing.T) { server, sent := threadReplyServer(t, messageAddressedToJane, 11, 12) diff --git a/internal/cmd/reply.go b/internal/cmd/reply.go index a6b3ef84..27fc960a 100644 --- a/internal/cmd/reply.go +++ b/internal/cmd/reply.go @@ -3,21 +3,40 @@ package cmd import ( "fmt" "strconv" + "strings" "github.com/spf13/cobra" "github.com/basecamp/hey-cli/internal/apierr" "github.com/basecamp/hey-cli/internal/editor" "github.com/basecamp/hey-cli/internal/htmlutil" + "github.com/basecamp/hey-cli/internal/mail" "github.com/basecamp/hey-cli/internal/output" + "github.com/basecamp/hey-cli/internal/terminal" ) type replyCommand struct { - cmd *cobra.Command - message string - messageHTML string - attachments []string - draft bool + cmd *cobra.Command + message string + messageHTML string + attachments []string + to []string + cc []string + bcc []string + draft bool + replaceRecipients bool + dryRun bool +} + +type replyPreview struct { + ThreadID int64 `json:"thread_id"` + EntryID int64 `json:"entry_id"` + AccountID int64 `json:"account_id"` + Subject string `json:"subject"` + From mail.ReplySender `json:"from"` + To []string `json:"to"` + CC []string `json:"cc"` + BCC []string `json:"bcc"` } func newReplyCommand() *replyCommand { @@ -28,12 +47,16 @@ func newReplyCommand() *replyCommand { Long: `Reply to a thread's latest entry. The reply is addressed the way HEY's own web app addresses one: everyone that entry was -addressed to, with whoever wrote it on the To line. HEY saves an unaddressed reply as a -draft rather than sending it, so the command fails when it cannot work the recipients out.`, +addressed to, with whoever wrote it on the To line. Repeatable --to, --cc and --bcc +values add or move recipients on those lines; each value can also be comma-separated. +--replace-recipients uses only the explicitly supplied recipients instead. A dry run +resolves and prints the complete envelope without requiring a message or sending one.`, Annotations: map[string]string{ - "agent_notes": "Replies to the latest entry in a thread, addressed the way HEY addresses a reply: everyone that entry was addressed to, plus its sender on the To line, minus the acting user's own addresses. Accepts message via -m, stdin, or $EDITOR, plus repeatable --attach files; an attachment can be sent without body text. The message is Markdown; use --message-html to send raw HTML instead. --draft saves the reply as a draft — carrying those recipients — and answers the draft ID for hey draft show/edit/send/delete.", + "agent_notes": "Replies to the latest entry in a thread, addressed the way HEY addresses a reply: everyone that entry was addressed to, plus its sender on the To line, minus the acting user's own addresses. Repeatable --to/--cc/--bcc flags merge explicit recipients into that prefill; --replace-recipients uses only the explicit lists. --dry-run is read-only, needs no message, and returns the resolved sender, recipients, subject, account, thread and entry. Accepts message via -m, stdin, or $EDITOR, plus repeatable --attach files; an attachment can be sent without body text. The message is Markdown; use --message-html to send raw HTML instead. --draft saves the reply as a draft, carries the resolved recipients, and answers the draft ID for hey draft show/edit/send/delete.", }, Example: ` hey reply 12345 -m "Friday works for me — I'll send an agenda." + hey reply 12345 --to support@example.com -m "The replacement is on the way." + hey reply 12345 --to support@example.com --replace-recipients --dry-run --json hey reply 12345 -m "Attached is the report." --attach ./report.pdf hey reply 12345 -m "Drafting a longer answer — sending tomorrow." --draft echo "Longer reply from a file or a heredoc" | hey reply 12345`, @@ -44,8 +67,14 @@ draft rather than sending it, so the command fails when it cannot work the recip replyCommand.cmd.Flags().StringVarP(&replyCommand.message, "message", "m", "", "Reply message as Markdown (or opens $EDITOR)") replyCommand.cmd.Flags().StringVar(&replyCommand.messageHTML, "message-html", "", "Reply message as raw HTML instead of Markdown") replyCommand.cmd.Flags().StringArrayVar(&replyCommand.attachments, "attach", nil, "File to attach (repeatable)") + replyCommand.cmd.Flags().StringArrayVar(&replyCommand.to, "to", nil, "To recipient email address (repeatable or comma-separated)") + replyCommand.cmd.Flags().StringArrayVar(&replyCommand.cc, "cc", nil, "CC recipient email address (repeatable or comma-separated)") + replyCommand.cmd.Flags().StringArrayVar(&replyCommand.bcc, "bcc", nil, "BCC recipient email address (repeatable or comma-separated)") replyCommand.cmd.Flags().BoolVar(&replyCommand.draft, "draft", false, "Save as a draft instead of sending") + replyCommand.cmd.Flags().BoolVar(&replyCommand.replaceRecipients, "replace-recipients", false, "Replace HEY's reply recipients with the explicit To, CC and BCC lists") + replyCommand.cmd.Flags().BoolVar(&replyCommand.dryRun, "dry-run", false, "Print the resolved reply envelope without sending") replyCommand.cmd.MarkFlagsMutuallyExclusive("message", "message-html") + replyCommand.cmd.MarkFlagsMutuallyExclusive("draft", "dry-run") return replyCommand } @@ -55,6 +84,15 @@ func (c *replyCommand) run(cmd *cobra.Command, args []string) error { return err } + overrides := replyRecipients{ + To: parseReplyAddresses(c.to), + CC: parseReplyAddresses(c.cc), + BCC: parseReplyAddresses(c.bcc), + } + if _, err := applyReplyRecipientOverrides(replyRecipients{}, overrides, c.replaceRecipients); err != nil { + return err + } + threadID, err := strconv.ParseInt(args[0], 10, 64) if err != nil { return apierr.ErrUsage(fmt.Sprintf("invalid thread ID: %s", args[0])) @@ -66,6 +104,16 @@ func (c *replyCommand) run(cmd *cobra.Command, args []string) error { if err != nil { return err } + target.Addressed, err = applyReplyRecipientOverrides(target.Addressed, overrides, c.replaceRecipients) + if err != nil { + return err + } + if !replyHasRecipients(target.Addressed) { + return apierr.ErrUsage("could not determine thread recipients; supply --to, --cc or --bcc") + } + if c.dryRun { + return writeReplyPreview(cmd, threadID, target) + } replySDK := target.client message := c.messageHTML @@ -115,3 +163,61 @@ func (c *replyCommand) run(cmd *cobra.Command, args []string) error { }), ) } + +func parseReplyAddresses(values []string) []string { + addresses := make([]string, 0, len(values)) + for _, value := range values { + addresses = append(addresses, parseAddresses(value)...) + } + return addresses +} + +func writeReplyPreview(cmd *cobra.Command, threadID int64, target *threadReplyTarget) error { + from, err := replySenderForPreview(cmd.Context(), target) + if err != nil { + return err + } + preview := replyPreview{ + ThreadID: threadID, + EntryID: target.EntryID, + AccountID: target.AccountID, + Subject: target.Subject, + From: from, + To: nonNilAddresses(target.Addressed.To), + CC: nonNilAddresses(target.Addressed.CC), + BCC: nonNilAddresses(target.Addressed.BCC), + } + if writer.IsStyled() { + fmt.Fprintf(cmd.OutOrStdout(), "Thread: %d\n", preview.ThreadID) + fmt.Fprintf(cmd.OutOrStdout(), "Entry: %d\n", preview.EntryID) + fmt.Fprintf(cmd.OutOrStdout(), "Account: %d\n", preview.AccountID) + fmt.Fprintf(cmd.OutOrStdout(), "Subject: %s\n", terminal.SanitizeLine(preview.Subject)) + fmt.Fprintf(cmd.OutOrStdout(), "From: %s\n", terminal.SanitizeLine(formatReplySender(preview.From))) + fmt.Fprintf(cmd.OutOrStdout(), "To: %s\n", terminal.SanitizeLine(strings.Join(preview.To, ", "))) + fmt.Fprintf(cmd.OutOrStdout(), "CC: %s\n", terminal.SanitizeLine(strings.Join(preview.CC, ", "))) + fmt.Fprintf(cmd.OutOrStdout(), "BCC: %s\n", terminal.SanitizeLine(strings.Join(preview.BCC, ", "))) + fmt.Fprintln(cmd.OutOrStdout(), "Nothing sent.") + return nil + } + return writeOK(preview, output.WithSummary("Reply preview; nothing sent")) +} + +func nonNilAddresses(addresses []string) []string { + if addresses == nil { + return []string{} + } + return addresses +} + +func formatReplySender(sender mail.ReplySender) string { + name := terminal.SanitizeLine(sender.Name) + email := terminal.SanitizeLine(sender.EmailAddress) + switch { + case name != "" && email != "": + return fmt.Sprintf("%s <%s>", name, email) + case email != "": + return email + default: + return strconv.FormatInt(sender.ID, 10) + } +} diff --git a/internal/cmd/thread_reply.go b/internal/cmd/thread_reply.go index 54d79bfd..f836ed16 100644 --- a/internal/cmd/thread_reply.go +++ b/internal/cmd/thread_reply.go @@ -17,18 +17,19 @@ import ( 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 -// unaddressed reply as a draft, so the recipients are not optional. The subject is not -// optional either: HEY never derives one, so a reply sent without it saves drafts that -// read "No subject" in Drafts. ActingSenderID is the identity the reply goes out as — -// the sender HEY resolved for the thread, which on a shared or alternate address is -// not the account default; zero (the prefill named none, or was unreachable) leaves -// the SDK on the account default. +// recipients, and an immutable client bound to the thread's mail account. Recipients +// can be empty when HEY cannot resolve them: an explicit reply override can still make +// that target addressable. The subject is not optional either: HEY never derives one, +// so a reply sent without it saves drafts that read "No subject" in Drafts. +// ActingSenderID is the identity the reply goes out as: the sender HEY resolved for +// the thread, which on a shared or alternate address is not the account default; zero +// (the prefill named none, or was unreachable) leaves the SDK on the account default. type threadReplyTarget struct { EntryID int64 AccountID int64 ActingSenderID int64 Subject string + Sender mail.ReplySender Addressed replyRecipients client *hey.Client } @@ -58,6 +59,7 @@ func resolveThreadReply(ctx context.Context, threadID int64) (*threadReplyTarget prefill, ok := mail.ReplyPrefillFromServer(ctx, threadSDK, entryID) target.ActingSenderID = prefill.ActingSenderID target.Subject = prefill.Subject + target.Sender = prefill.Sender if ok { target.Addressed = prefill.Addressed return target, nil @@ -71,20 +73,138 @@ func resolveThreadReply(ctx context.Context, threadID int64) (*threadReplyTarget return nil, apierr.ErrNotFound("message", fmt.Sprintf("%d", entryID)) } - addressed := recipientsForReplyTo(*message) - if len(addressed.To) == 0 && len(addressed.CC) == 0 && len(addressed.BCC) == 0 { - return nil, apierr.ErrUsage("could not determine thread recipients") - } - // The prefill's subject survives an empty recipient list: only the recipients // needed the local computation. if target.Subject == "" { target.Subject = replySubject(message.Subject) } - target.Addressed = addressed + target.Addressed = recipientsForReplyTo(*message) return target, nil } +func replyHasRecipients(addressed replyRecipients) bool { + return len(addressed.To)+len(addressed.CC)+len(addressed.BCC) > 0 +} + +// applyReplyRecipientOverrides merges explicit recipient lines into HEY's prefill, or +// replaces the prefill whole. An explicitly named address moves to the requested line +// rather than being sent twice. Conflicting explicit lines are refused because there +// is no honest way to decide which one the caller intended. +func applyReplyRecipientOverrides(addressed, overrides replyRecipients, replace bool) (replyRecipients, error) { + overrides, err := distinctReplyRecipients(overrides) + if err != nil { + return replyRecipients{}, err + } + if !replyHasRecipients(overrides) { + if replace { + return replyRecipients{}, apierr.ErrUsage("--replace-recipients requires at least one --to, --cc or --bcc address") + } + return addressed, nil + } + if replace { + return overrides, nil + } + + explicit := make(map[string]bool) + for _, line := range [][]string{overrides.To, overrides.CC, overrides.BCC} { + for _, address := range line { + explicit[strings.ToLower(address)] = true + } + } + seen := make(map[string]bool) + merged := replyRecipients{ + To: keepReplyRecipients(addressed.To, explicit, seen), + CC: keepReplyRecipients(addressed.CC, explicit, seen), + BCC: keepReplyRecipients(addressed.BCC, explicit, seen), + } + merged.To = append(merged.To, overrides.To...) + merged.CC = append(merged.CC, overrides.CC...) + merged.BCC = append(merged.BCC, overrides.BCC...) + return merged, nil +} + +func distinctReplyRecipients(addressed replyRecipients) (replyRecipients, error) { + seen := make(map[string]string) + distinct := replyRecipients{} + lines := []struct { + name string + addresses []string + target *[]string + }{ + {name: "to", addresses: addressed.To, target: &distinct.To}, + {name: "cc", addresses: addressed.CC, target: &distinct.CC}, + {name: "bcc", addresses: addressed.BCC, target: &distinct.BCC}, + } + for _, line := range lines { + for _, address := range line.addresses { + address = strings.TrimSpace(address) + if address == "" { + continue + } + key := strings.ToLower(address) + if previous, found := seen[key]; found { + if previous != line.name { + return replyRecipients{}, apierr.ErrUsage(fmt.Sprintf("recipient %s was specified for both --%s and --%s", address, previous, line.name)) + } + continue + } + seen[key] = line.name + *line.target = append(*line.target, address) + } + } + return distinct, nil +} + +func keepReplyRecipients(addresses []string, excluded, seen map[string]bool) []string { + var kept []string + for _, address := range addresses { + address = strings.TrimSpace(address) + key := strings.ToLower(address) + if address == "" || excluded[key] || seen[key] { + continue + } + seen[key] = true + kept = append(kept, address) + } + return kept +} + +func replySenderForPreview(ctx context.Context, target *threadReplyTarget) (mail.ReplySender, error) { + if target.Sender.ID > 0 && target.Sender.EmailAddress != "" { + return target.Sender, nil + } + + identity, err := rootSDK.Identity().GetIdentity(ctx) + if err != nil { + return mail.ReplySender{}, apierr.FromSDK(err) + } + if identity == nil { + return mail.ReplySender{}, apierr.ErrAPI(0, "HEY returned no identity data") + } + + var fallback *generated.Sender + for i := range identity.Senders { + sender := &identity.Senders[i] + if sender.AccountId != target.AccountID { + continue + } + if target.ActingSenderID > 0 && sender.Id == target.ActingSenderID { + return previewReplySender(*sender), nil + } + if fallback == nil || sender.Default { + fallback = sender + } + } + if target.ActingSenderID == 0 && fallback != nil { + return previewReplySender(*fallback), nil + } + return mail.ReplySender{}, apierr.ErrNotFound("sender for account", fmt.Sprintf("%d", target.AccountID)) +} + +func previewReplySender(sender generated.Sender) mail.ReplySender { + return mail.ReplySender{ID: sender.Id, Name: sender.Name, EmailAddress: sender.EmailAddress} +} + // 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/cmd/thread_reply_test.go b/internal/cmd/thread_reply_test.go index 0deccfe4..9ee580b3 100644 --- a/internal/cmd/thread_reply_test.go +++ b/internal/cmd/thread_reply_test.go @@ -15,6 +15,7 @@ import ( "github.com/basecamp/hey-sdk/go/pkg/generated" "github.com/basecamp/hey-cli/internal/apierr" + "github.com/basecamp/hey-cli/internal/mail" ) // messageAddressedToJane is entry 12 as HEY serves it: Rick wrote it, Jane was on the @@ -108,7 +109,7 @@ func threadReplyServer(t *testing.T, messageJSON string, entryIDs ...int64) (*ht t.Errorf("identity account = %q, want unscoped", got) } w.Header().Set("Content-Type", "application/json") - fmt.Fprint(w, `{"id":1,"accounts":[{"id":8,"status":"active"},{"id":9,"status":"active"}],"senders":[{"id":42,"account_id":9,"default":true}]}`) + fmt.Fprint(w, `{"id":1,"accounts":[{"id":8,"status":"active"},{"id":9,"status":"active"}],"senders":[{"id":42,"account_id":9,"name":"Jane Doe","email_address":"me@example.com","default":true}]}`) case r.URL.Path == "/topics/7.json": sent.TopicAccountFilter = r.URL.Query().Get("filtered_account_id") w.Header().Set("Content-Type", "application/json") @@ -216,17 +217,43 @@ func TestResolveThreadReplyFollowsTheLatestEntrysRecipients(t *testing.T) { } } -// An unaddressed reply is saved as a draft rather than sent, so a thread we cannot read -// recipients from is refused before anything is written. +// Resolution keeps an unaddressed target so an explicit recipient override can make +// the reply addressable. The command still refuses to send it without that override. func TestResolveThreadReplyWithoutRecipients(t *testing.T) { server, _ := threadReplyServer(t, messageWithoutRecipients, 11, 12) withSDKPointedAt(t, server) - _, err := resolveThreadReply(context.Background(), 7) + target, err := resolveThreadReply(context.Background(), 7) + if err != nil { + t.Fatalf("resolve unaddressed reply: %v", err) + } + if replyHasRecipients(target.Addressed) { + t.Errorf("recipients = %+v, want none", target.Addressed) + } +} + +func TestReplyOverrideCanAddressAnOtherwiseUnaddressedThread(t *testing.T) { + server, sent := threadReplyServer(t, messageWithoutRecipients, 11, 12) + + err := runCLI(t, server, "--account", "8", "reply", "7", "-m", "please route this", "--to", "support@example.com") + if err != nil { + t.Fatalf("reply with explicit recipient: %v", err) + } + if want := []string{"support@example.com"}; !reflect.DeepEqual(sent.To, want) { + t.Errorf("to = %v, want %v", sent.To, want) + } +} +func TestReplyWithoutResolvedOrExplicitRecipientsIsRefused(t *testing.T) { + server, sent := threadReplyServer(t, messageWithoutRecipients, 11, 12) + + err := runCLI(t, server, "--account", "8", "reply", "7", "-m", "must not send") var cliErr *apierr.Error - if !errors.As(err, &cliErr) || cliErr.Code != "usage" { - t.Fatalf("expected a usage error, got %v", err) + if !errors.As(err, &cliErr) || cliErr.Code != "usage" || !strings.Contains(err.Error(), "supply --to") { + t.Fatalf("expected an actionable usage error, got %v", err) + } + if sent.Path != "" { + t.Errorf("unaddressed reply wrote to %q", sent.Path) } } @@ -347,6 +374,12 @@ func TestReplySendsRawHTMLVerbatim(t *testing.T) { // runCLI drives a command the way the binary does — through the root command, so the // output writer and auth are set up — against a test server. func runCLI(t *testing.T, server *httptest.Server, args ...string) error { + t.Helper() + _, err := runCLIOutput(t, server, args...) + return err +} + +func runCLIOutput(t *testing.T, server *httptest.Server, args ...string) (string, error) { t.Helper() t.Setenv("HEY_TOKEN", "test-token") t.Setenv("HEY_NO_KEYRING", "1") @@ -362,7 +395,178 @@ func runCLI(t *testing.T, server *httptest.Server, args ...string) error { root.SetErr(&buf) root.SetArgs(append([]string{"--json", "--base-url", server.URL}, args...)) - return root.Execute() + err := root.Execute() + return buf.String(), err +} + +func TestReplyMergesRecipientOverridesIntoHEYsPrefill(t *testing.T) { + server, sent := threadReplyServer(t, messageAddressedToJane, 11, 12) + sent.ReplyNewJSON = `{"subject":"Re: Weekly sync","content":"
quoted
","is_reply":true, + "addressed":{ + "directly":[{"email_address":"rick@example.com"}], + "copied":[{"email_address":"moved@example.com"},{"email_address":"cc@example.com"}] + }}` + + err := runCLI(t, server, "--account", "8", "reply", "7", "-m", "sounds good", + "--to", "moved@example.com", "--to", "support@example.com,billing@example.com", + "--cc", "manager@example.com", "--bcc", "audit@example.com") + if err != nil { + t.Fatalf("reply with recipient overrides: %v", err) + } + + if want := []string{"rick@example.com", "moved@example.com", "support@example.com", "billing@example.com"}; !reflect.DeepEqual(sent.To, want) { + t.Errorf("to = %v, want %v", sent.To, want) + } + if want := []string{"cc@example.com", "manager@example.com"}; !reflect.DeepEqual(sent.CC, want) { + t.Errorf("cc = %v, want %v", sent.CC, want) + } + if want := []string{"audit@example.com"}; !reflect.DeepEqual(sent.BCC, want) { + t.Errorf("bcc = %v, want %v", sent.BCC, want) + } + if !strings.Contains(sent.Path, "/entries/12/replies") { + t.Errorf("path = %q, want the existing thread's reply endpoint", sent.Path) + } +} + +func TestReplyCanReplaceHEYsPrefilledRecipients(t *testing.T) { + server, sent := threadReplyServer(t, messageAddressedToJane, 11, 12) + sent.ReplyNewJSON = `{"subject":"Re: Weekly sync","content":"
quoted
","is_reply":true, + "addressed":{"directly":[{"email_address":"rick@example.com"}],"copied":[{"email_address":"cc@example.com"}]}}` + + err := runCLI(t, server, "--account", "8", "reply", "7", "-m", "sounds good", + "--replace-recipients", "--to", "support@example.com", "--bcc", "archive@example.com") + if err != nil { + t.Fatalf("reply with replacement recipients: %v", err) + } + + if want := []string{"support@example.com"}; !reflect.DeepEqual(sent.To, want) { + t.Errorf("to = %v, want %v", sent.To, want) + } + if len(sent.CC) != 0 { + t.Errorf("cc = %v, want none", sent.CC) + } + if want := []string{"archive@example.com"}; !reflect.DeepEqual(sent.BCC, want) { + t.Errorf("bcc = %v, want %v", sent.BCC, want) + } +} + +func TestReplyDryRunReportsTheResolvedEnvelopeWithoutSending(t *testing.T) { + server, sent := threadReplyServer(t, messageAddressedToJane, 11, 12) + sent.ReplyNewJSON = `{"subject":"Re: Weekly sync","content":"
quoted
","is_reply":true, + "sender":{"id":215,"name":"Support","email_address":"support@example.com"}, + "addressed":{"directly":[{"email_address":"rick@example.com"}]}}` + + out, err := runCLIOutput(t, server, "--account", "8", "reply", "7", "--dry-run", + "--to", "customer@example.org", "--cc", "manager@example.com") + if err != nil { + t.Fatalf("reply dry run: %v", err) + } + if sent.Path != "" { + t.Fatalf("dry run wrote a reply to %q", sent.Path) + } + + var response struct { + Data struct { + ThreadID int64 `json:"thread_id"` + EntryID int64 `json:"entry_id"` + AccountID int64 `json:"account_id"` + Subject string `json:"subject"` + From struct { + ID int64 `json:"id"` + Name string `json:"name"` + EmailAddress string `json:"email_address"` + } `json:"from"` + To []string `json:"to"` + CC []string `json:"cc"` + BCC []string `json:"bcc"` + } `json:"data"` + Summary string `json:"summary"` + } + if err := json.Unmarshal([]byte(out), &response); err != nil { + t.Fatalf("decode dry run: %v\n%s", err, out) + } + if response.Data.ThreadID != 7 || response.Data.EntryID != 12 || response.Data.AccountID != 9 { + t.Errorf("reply identifiers = thread %d entry %d account %d", response.Data.ThreadID, response.Data.EntryID, response.Data.AccountID) + } + if response.Data.Subject != "Re: Weekly sync" { + t.Errorf("subject = %q", response.Data.Subject) + } + if response.Data.From.ID != 215 || response.Data.From.Name != "Support" || response.Data.From.EmailAddress != "support@example.com" { + t.Errorf("from = %+v", response.Data.From) + } + if want := []string{"rick@example.com", "customer@example.org"}; !reflect.DeepEqual(response.Data.To, want) { + t.Errorf("to = %v, want %v", response.Data.To, want) + } + if want := []string{"manager@example.com"}; !reflect.DeepEqual(response.Data.CC, want) { + t.Errorf("cc = %v, want %v", response.Data.CC, want) + } + if response.Data.BCC == nil || len(response.Data.BCC) != 0 { + t.Errorf("bcc = %#v, want an empty list", response.Data.BCC) + } + if response.Summary != "Reply preview; nothing sent" { + t.Errorf("summary = %q", response.Summary) + } +} + +func TestReplyDryRunResolvesTheAccountDefaultSender(t *testing.T) { + server, sent := threadReplyServer(t, messageAddressedToJane, 11, 12) + sent.ReplyNewJSON = `{"subject":"Re: Weekly sync","content":"
quoted
","is_reply":true, + "addressed":{"directly":[{"email_address":"rick@example.com"}]}}` + + out, err := runCLIOutput(t, server, "--account", "8", "reply", "7", "--dry-run") + if err != nil { + t.Fatalf("reply dry run: %v", err) + } + var response struct { + Data struct { + From mail.ReplySender `json:"from"` + } `json:"data"` + } + if err := json.Unmarshal([]byte(out), &response); err != nil { + t.Fatalf("decode dry run: %v\n%s", err, out) + } + want := mail.ReplySender{ID: 42, Name: "Jane Doe", EmailAddress: "me@example.com"} + if response.Data.From != want { + t.Errorf("from = %+v, want account default %+v", response.Data.From, want) + } +} + +func TestReplyRecipientCannotBeExplicitlyNamedOnTwoLines(t *testing.T) { + server, sent := threadReplyServer(t, messageAddressedToJane, 11, 12) + + err := runCLI(t, server, "--account", "8", "reply", "7", "--dry-run", + "--to", "support@example.com", "--cc", "SUPPORT@example.com") + if err == nil || !strings.Contains(err.Error(), "both --to and --cc") { + t.Fatalf("error = %v, want a conflicting recipient refusal", err) + } + if sent.Path != "" { + t.Errorf("invalid overrides wrote to %q", sent.Path) + } +} + +func TestReplyDraftCarriesRecipientOverrides(t *testing.T) { + server, sent := threadReplyServer(t, messageAddressedToJane, 11, 12) + + err := runCLI(t, server, "--account", "8", "reply", "7", "--draft", "-m", "please review", + "--replace-recipients", "--to", "support@example.com") + if err != nil { + t.Fatalf("reply draft with recipient override: %v", err) + } + if sent.Status != "drafted" || !reflect.DeepEqual(sent.To, []string{"support@example.com"}) { + t.Errorf("draft status = %q, to = %v", sent.Status, sent.To) + } +} + +func TestReplyReplacementRequiresExplicitRecipients(t *testing.T) { + server, sent := threadReplyServer(t, messageAddressedToJane, 11, 12) + + err := runCLI(t, server, "--account", "8", "reply", "7", "--replace-recipients", "--dry-run") + if err == nil || !strings.Contains(err.Error(), "--replace-recipients requires") { + t.Fatalf("error = %v, want a replacement recipient refusal", err) + } + if sent.Path != "" { + t.Errorf("invalid replacement wrote to %q", sent.Path) + } } // HEY's replies/new endpoint answers the reply's recipients with the acting user's own diff --git a/internal/mail/reply_prefill.go b/internal/mail/reply_prefill.go index 31e0000f..796d9e8a 100644 --- a/internal/mail/reply_prefill.go +++ b/internal/mail/reply_prefill.go @@ -15,6 +15,13 @@ type ReplyRecipients struct { BCC []string } +// ReplySender is the configured identity a reply goes out as. +type ReplySender struct { + ID int64 `json:"id"` + Name string `json:"name"` + EmailAddress string `json:"email_address"` +} + // 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 @@ -23,6 +30,7 @@ type ReplyRecipients struct { type ReplyPrefill struct { Subject string ActingSenderID int64 + Sender ReplySender Addressed ReplyRecipients } @@ -47,6 +55,11 @@ func ReplyPrefillFromServer(ctx context.Context, client *hey.Client, entryID int prefill := ReplyPrefill{ Subject: prefilled.Subject, ActingSenderID: prefilled.Sender.Id, + Sender: ReplySender{ + ID: prefilled.Sender.Id, + Name: prefilled.Sender.Name, + EmailAddress: prefilled.Sender.EmailAddress, + }, Addressed: ReplyRecipients{ To: contactEmails(prefilled.Addressed.Directly), CC: contactEmails(prefilled.Addressed.Copied), diff --git a/internal/mail/reply_prefill_test.go b/internal/mail/reply_prefill_test.go index c3b49e76..8da923d4 100644 --- a/internal/mail/reply_prefill_test.go +++ b/internal/mail/reply_prefill_test.go @@ -46,6 +46,9 @@ func TestReplyPrefillFromServer(t *testing.T) { if prefill.ActingSenderID != 215 { t.Errorf("acting sender = %d, want the prefill's 215", prefill.ActingSenderID) } + if want := (ReplySender{ID: 215, Name: "Support", EmailAddress: "support@example.com"}); prefill.Sender != want { + t.Errorf("sender = %+v, want %+v", prefill.Sender, want) + } // 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. @@ -74,6 +77,9 @@ func TestReplyPrefillFromServerWithoutRecipients(t *testing.T) { t.Errorf("subject = %q, sender = %d — both survive an empty recipient list", prefill.Subject, prefill.ActingSenderID) } + if prefill.Sender.ID != 215 || prefill.Sender.EmailAddress != "support@example.com" { + t.Errorf("sender details = %+v, want the prefilled identity", prefill.Sender) + } if !reflect.DeepEqual(prefill.Addressed, ReplyRecipients{}) { t.Errorf("addressed = %+v, want none", prefill.Addressed) } diff --git a/skills/hey/SKILL.md b/skills/hey/SKILL.md index 273c5def..98c6c3ed 100644 --- a/skills/hey/SKILL.md +++ b/skills/hey/SKILL.md @@ -475,6 +475,8 @@ Direct attachment IDs combine the message ID and position, so `67890:1` identifi ```bash hey reply -m "Friday works for me — I'll send an agenda." # Inline message hey reply # Reply via $EDITOR +hey reply --to support@example.com -m "The replacement is on the way." +hey reply --to support@example.com --replace-recipients --dry-run --json hey reply -m "Here is the wiring diagram." --attach ./diagram.png hey forward --to alice@example.com # Forward the latest message hey forward --to alice@example.com -m "Please review before Thursday." @@ -490,8 +492,11 @@ hey compose --to alice@example.com --subject "Newsletter draft" --message-html " `hey reply` answers the thread's **latest** entry. HEY addresses the reply the way its own web app does: everyone that entry was addressed to, plus whoever wrote it, on the To line. -A reply HEY cannot address is saved as a draft rather than sent, so the command fails -rather than guessing when it cannot work out the recipients. +Repeatable `--to`, `--cc` and `--bcc` flags add or move explicit recipients. Use +`--replace-recipients` to discard HEY's prefill. Run `--dry-run --json` first when an +agent changes the envelope; it needs no message and reports the resolved sender and final +recipients without sending. A reply HEY cannot address is refused unless an explicit +recipient makes it addressable. Everything you send is Markdown by default — `-m`, `--content`, `--note`, positional content, stdin, and `$EDITOR` alike — and is converted to rich text on the way out. To diff --git a/tests/smoke/threads_test.go b/tests/smoke/threads_test.go index 689c6542..ec75b3f2 100644 --- a/tests/smoke/threads_test.go +++ b/tests/smoke/threads_test.go @@ -36,6 +36,18 @@ type threadRecipient struct { EmailAddress string `json:"email_address"` } +type replyPreview struct { + ThreadID int64 `json:"thread_id"` + EntryID int64 `json:"entry_id"` + From struct { + ID int64 `json:"id"` + EmailAddress string `json:"email_address"` + } `json:"from"` + To []string `json:"to"` + CC []string `json:"cc"` + BCC []string `json:"bcc"` +} + // firstMessage is the Markdown the long thread starts with. compose sends -m as // Markdown, so in the thread the emphasis and the list survive as structure, and the // bare URL stays a literal the reader can follow. @@ -97,6 +109,61 @@ func longThread(t *testing.T, replies int) (topicID string, subject string) { return topicID, subject } +// Recipient overrides first preview the exact envelope without writing, then replace +// HEY's prefill while still creating the message inside the original thread. +func TestReplyRecipientOverrides(t *testing.T) { + topicID, _ := longThread(t, 0) + const ( + toAddress = "jane.doe@example.com" + ccAddress = "morty.smith@example.org" + bccAddress = "beth.smith@example.org" + ) + + previewResponse := heyJSON(t, "reply", topicID, "--dry-run", "--replace-recipients", + "--to", toAddress, "--cc", ccAddress, "--bcc", bccAddress) + preview := dataAs[replyPreview](t, previewResponse) + parsedTopicID, err := strconv.ParseInt(topicID, 10, 64) + if err != nil { + t.Fatalf("parse topic ID %q: %v", topicID, err) + } + if preview.ThreadID != parsedTopicID || preview.EntryID <= 0 { + t.Errorf("preview identifiers = thread %d entry %d", preview.ThreadID, preview.EntryID) + } + if preview.From.ID <= 0 || preview.From.EmailAddress == "" { + t.Errorf("preview sender = %+v, want a resolved identity", preview.From) + } + if !slices.Equal(preview.To, []string{toAddress}) || !slices.Equal(preview.CC, []string{ccAddress}) || !slices.Equal(preview.BCC, []string{bccAddress}) { + t.Errorf("preview recipients = to %v cc %v bcc %v", preview.To, preview.CC, preview.BCC) + } + + before := dataAs[[]threadEntry](t, heyJSON(t, "thread", "read", topicID)) + if len(before) != 1 { + t.Fatalf("dry run left %d entries, want the original entry alone", len(before)) + } + + _, stderr, code := hey(t, "reply", topicID, "--replace-recipients", + "--to", toAddress, "--cc", ccAddress, "--bcc", bccAddress, + "-m", "The replacement is on the way.", "--json") + if code != 0 { + skipf(t, "reply with recipient overrides failed (exit %d): %s", code, stderr) + } + + after := dataAs[[]threadEntry](t, heyJSON(t, "thread", "read", topicID)) + if len(after) != 2 { + t.Fatalf("thread has %d entries after reply, want 2", len(after)) + } + latest := after[len(after)-1] + if !strings.Contains(latest.Body, "The replacement is on the way.") { + t.Errorf("latest body = %q", latest.Body) + } + if latest.Recipients == nil || + !slices.ContainsFunc(latest.Recipients.To, func(recipient threadRecipient) bool { return strings.EqualFold(recipient.EmailAddress, toAddress) }) || + !slices.ContainsFunc(latest.Recipients.CC, func(recipient threadRecipient) bool { return strings.EqualFold(recipient.EmailAddress, ccAddress) }) || + !slices.ContainsFunc(latest.Recipients.BCC, func(recipient threadRecipient) bool { return strings.EqualFold(recipient.EmailAddress, bccAddress) }) { + t.Errorf("latest recipients = %+v, want the replacement envelope", latest.Recipients) + } +} + // A thread longer than a page reads whole, oldest first, with every entry's body as // Markdown: the composed text survives as prose, its URL intact, and no HTML tag does. func TestThreadsReadsALongThreadAsMarkdown(t *testing.T) { From 77f2039cd3cb060247aa1c57fc0bf8ec638743f5 Mon Sep 17 00:00:00 2001 From: Rob Zolkos Date: Sun, 20 Sep 2026 17:23:52 -0400 Subject: [PATCH 2/2] Keep reply previews body-free --- docs/cli.md | 4 +- internal/cmd/reply.go | 12 ++++- internal/cmd/thread_reply.go | 68 +++++++++++++++++------ internal/cmd/thread_reply_test.go | 89 ++++++++++++++++++++++++++++++- skills/hey/SKILL.md | 6 ++- 5 files changed, 157 insertions(+), 22 deletions(-) diff --git a/docs/cli.md b/docs/cli.md index ee1ddbe2..dabbd90d 100644 --- a/docs/cli.md +++ b/docs/cli.md @@ -262,9 +262,9 @@ hey ignore 12345 # ignore future activity on a thread hey stop-ignoring 12345 # resume attention for a thread ``` -`hey thread read` reads a whole thread, oldest entry first, however many pages HEY serves it in — within limits it states: a hundred pages past the first, two thousand entries, as many bodies, 64 MiB of retained thread data and two minutes in all. The byte budget covers entry-index metadata, message bodies and metadata, the recipient identities retained for thread output, and inbound delivery addresses and resolved contact identities retained for JSON. A thread that could only be read in part — a body HEY would not serve, a limit reached — is refused rather than passed off as whole; `--allow-partial` takes what was read, with a `notice` saying what is missing and each entry's `body_state` saying whether its body was `hydrated`, `bodyless` (HEY served none), `over_limit` or `failed`. Each entry whose message was read carries `recipients`, with `to`, `cc` and `bcc` contact lists; the object is absent when the message was not read, while a known-empty line is `[]`. In JSON, an inbound entry also carries `received_via`: every exact account address HEY recorded the message arriving through, including plus tags and catch-all aliases. These are delivery records, not the visible To/CC/BCC recipients. A record's `contact` is optional and is omitted when HEY did not resolve one; the whole field is omitted for sent or generated messages and whenever the message was not read. `--count` and `--ids-only` read the entry index and no messages, so only a truncated index can make them partial. `--markdown` writes the thread as one Markdown document — a heading per entry naming the sender, date and ID, then the body — which is the shape to hand an agent or a notes app. `hey attachment list` reads the bodies in every format, since that is where attachment metadata lives, and answers a partial thread the same way. `hey reply` answers the thread's latest entry and addresses the reply the way HEY does: it asks HEY for the reply's recipients — everyone that entry was addressed to, its sender moved onto the To line, and your own addresses, aliases and catch-alls excluded — falling back to computing them from the entry when that read is unavailable. +`hey thread read` reads a whole thread, oldest entry first, however many pages HEY serves it in — within limits it states: a hundred pages past the first, two thousand entries, as many bodies, 64 MiB of retained thread data and two minutes in all. The byte budget covers entry-index metadata, message bodies and metadata, the recipient identities retained for thread output, and inbound delivery addresses and resolved contact identities retained for JSON. A thread that could only be read in part — a body HEY would not serve, a limit reached — is refused rather than passed off as whole; `--allow-partial` takes what was read, with a `notice` saying what is missing and each entry's `body_state` saying whether its body was `hydrated`, `bodyless` (HEY served none), `over_limit` or `failed`. Each entry whose message was read carries `recipients`, with `to`, `cc` and `bcc` contact lists; the object is absent when the message was not read, while a known-empty line is `[]`. In JSON, an inbound entry also carries `received_via`: every exact account address HEY recorded the message arriving through, including plus tags and catch-all aliases. These are delivery records, not the visible To/CC/BCC recipients. A record's `contact` is optional and is omitted when HEY did not resolve one; the whole field is omitted for sent or generated messages and whenever the message was not read. `--count` and `--ids-only` read the entry index and no messages, so only a truncated index can make them partial. `--markdown` writes the thread as one Markdown document — a heading per entry naming the sender, date and ID, then the body — which is the shape to hand an agent or a notes app. `hey attachment list` reads the bodies in every format, since that is where attachment metadata lives, and answers a partial thread the same way. `hey reply` answers the thread's latest entry and addresses the reply the way HEY does: it asks HEY for the reply's recipients — everyone that entry was addressed to, its sender moved onto the To line, and your own addresses, aliases and catch-alls excluded — falling back to the latest message's metadata for a send when that read is unavailable. -Repeatable `hey reply --to`, `--cc` and `--bcc` flags add recipients to that envelope; comma-separated addresses also work. An explicitly named address moves to that line instead of appearing twice. `--replace-recipients` discards HEY's prefill and requires at least one explicit address. `--dry-run` needs no body, uploads nothing and sends nothing; its JSON data reports the account, thread, entry, subject, resolved sender, and final To, CC and BCC lists. +Repeatable `hey reply --to`, `--cc` and `--bcc` flags add recipients to that envelope; comma-separated addresses also work. An explicitly named address moves to that line instead of appearing twice. `--replace-recipients` discards HEY's prefill and requires at least one explicit address. `--dry-run` needs no body, does not read the original message body, uploads nothing and sends nothing; its JSON data reports the account, thread, entry, subject, resolved sender, and final To, CC and BCC lists. If HEY's envelope prefill is unavailable, a dry run refuses to guess the original recipient lists; use `--replace-recipients` with explicit addresses to preview a complete replacement instead. Email bodies come back as Markdown. `hey thread read` and the TUI render that Markdown for the terminal — headings, emphasis, lists, quotes, tables and code survive, and links keep their URLs and stay clickable where the terminal supports it. `--json` carries the same Markdown in `body`, so an agent reading a thread sees the structure a human sees rather than a flattened wall of text. `--html` keeps HEY's original body HTML and frames each entry with its From, To, CC and BCC headers. diff --git a/internal/cmd/reply.go b/internal/cmd/reply.go index 27fc960a..641de4ca 100644 --- a/internal/cmd/reply.go +++ b/internal/cmd/reply.go @@ -100,10 +100,20 @@ func (c *replyCommand) run(cmd *cobra.Command, args []string) error { ctx := cmd.Context() - target, err := resolveThreadReply(ctx, threadID) + resolve := resolveThreadReply + if c.dryRun || c.replaceRecipients { + resolve = resolveThreadReplyWithoutMessage + } + target, err := resolve(ctx, threadID) if err != nil { return err } + if target.Subject == "" { + return apierr.ErrUsage("could not determine the reply subject") + } + if c.dryRun && !c.replaceRecipients && !target.RecipientsResolved { + return apierr.ErrUsageHint("could not resolve HEY's reply recipients without reading the message", "use --replace-recipients with explicit --to, --cc or --bcc addresses") + } target.Addressed, err = applyReplyRecipientOverrides(target.Addressed, overrides, c.replaceRecipients) if err != nil { return err diff --git a/internal/cmd/thread_reply.go b/internal/cmd/thread_reply.go index f836ed16..3620b423 100644 --- a/internal/cmd/thread_reply.go +++ b/internal/cmd/thread_reply.go @@ -18,25 +18,39 @@ 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. Recipients -// can be empty when HEY cannot resolve them: an explicit reply override can still make -// that target addressable. The subject is not optional either: HEY never derives one, +// can be unresolved when the body-free path cannot get HEY's prefill; only a complete +// replacement envelope can safely address that target. The subject is not optional +// either: HEY never derives one, // so a reply sent without it saves drafts that read "No subject" in Drafts. // ActingSenderID is the identity the reply goes out as: the sender HEY resolved for // the thread, which on a shared or alternate address is not the account default; zero // (the prefill named none, or was unreachable) leaves the SDK on the account default. type threadReplyTarget struct { - EntryID int64 - AccountID int64 - ActingSenderID int64 - Subject string - Sender mail.ReplySender - Addressed replyRecipients - client *hey.Client + EntryID int64 + AccountID int64 + ActingSenderID int64 + Subject string + Sender mail.ReplySender + Addressed replyRecipients + RecipientsResolved bool + client *hey.Client } // resolveThreadReply returns the thread's latest entry, linked account, and the -// recipients a reply to that entry goes to. +// recipients a reply to that entry goes to. If HEY's reply prefill is unavailable, +// it reads the message for the most complete fallback metadata. func resolveThreadReply(ctx context.Context, threadID int64) (*threadReplyTarget, error) { + return resolveThreadReplyTarget(ctx, threadID, true) +} + +// resolveThreadReplyWithoutMessage resolves a reply from the topic entry summary and +// HEY's reply prefill only. Dry runs must not hydrate a body, and a replacement envelope +// does not need the original recipients. +func resolveThreadReplyWithoutMessage(ctx context.Context, threadID int64) (*threadReplyTarget, error) { + return resolveThreadReplyTarget(ctx, threadID, false) +} + +func resolveThreadReplyTarget(ctx context.Context, threadID int64, readMessageFallback bool) (*threadReplyTarget, error) { topic, err := rootSDK.Topics().Get(ctx, threadID) if err != nil { return nil, apierr.FromSDK(err) @@ -49,7 +63,8 @@ func resolveThreadReply(ctx context.Context, threadID int64) (*threadReplyTarget if err != nil { return nil, err } - entryID := topic.Entries[len(topic.Entries)-1].Id + entry := topic.Entries[len(topic.Entries)-1] + entryID := entry.Id target := &threadReplyTarget{ EntryID: entryID, @@ -62,6 +77,17 @@ func resolveThreadReply(ctx context.Context, threadID int64) (*threadReplyTarget target.Sender = prefill.Sender if ok { target.Addressed = prefill.Addressed + target.RecipientsResolved = true + return target, nil + } + + if !readMessageFallback { + if target.Subject == "" { + target.Subject = replySubject(entry.Subject) + if target.Subject == "" { + target.Subject = replySubject(topic.Name) + } + } return target, nil } @@ -73,12 +99,19 @@ func resolveThreadReply(ctx context.Context, threadID int64) (*threadReplyTarget return nil, apierr.ErrNotFound("message", fmt.Sprintf("%d", entryID)) } - // The prefill's subject survives an empty recipient list: only the recipients - // needed the local computation. + // The prefill's subject survives the fallback. Otherwise the message is the source + // of truth; entry and topic names are the last body-free fallback. if target.Subject == "" { target.Subject = replySubject(message.Subject) + if target.Subject == "" { + target.Subject = replySubject(entry.Subject) + } + if target.Subject == "" { + target.Subject = replySubject(topic.Name) + } } target.Addressed = recipientsForReplyTo(*message) + target.RecipientsResolved = true return target, nil } @@ -231,11 +264,14 @@ func recipientsForReplyTo(message generated.Message) replyRecipients { if sender == "" { sender = message.Creator.EmailAddress } + return recipientsForReplyAddressed(message.Addressed, sender) +} +func recipientsForReplyAddressed(addressed generated.Addressed, sender string) replyRecipients { recipients := replyRecipients{ - To: addressesOf(message.Addressed.Directly, sender), - CC: addressesOf(message.Addressed.Copied, sender), - BCC: addressesOf(message.Addressed.Blindcopied, sender), + To: addressesOf(addressed.Directly, sender), + CC: addressesOf(addressed.Copied, sender), + BCC: addressesOf(addressed.Blindcopied, sender), } if sender != "" { recipients.To = append(recipients.To, sender) diff --git a/internal/cmd/thread_reply_test.go b/internal/cmd/thread_reply_test.go index 9ee580b3..39e8b109 100644 --- a/internal/cmd/thread_reply_test.go +++ b/internal/cmd/thread_reply_test.go @@ -46,6 +46,9 @@ type sentReply struct { To []string CC []string BCC []string + MessageReads int + MessageStatus int + TopicEntryJSON string // ReplyNewJSON, when set before the command runs, is what GET // /entries/{id}/replies/new answers — HEY's own computed reply recipients. @@ -117,9 +120,17 @@ func threadReplyServer(t *testing.T, messageJSON string, entryIDs ...int64) (*ht for _, id := range entryIDs { entries = append(entries, fmt.Sprintf(`{"id":%d}`, id)) } - fmt.Fprintf(w, `{"id":7,"account_id":9,"entries":[%s]}`, strings.Join(entries, ",")) + if sent.TopicEntryJSON != "" { + entries = []string{sent.TopicEntryJSON} + } + fmt.Fprintf(w, `{"id":7,"account_id":9,"name":"Weekly sync","entries":[%s]}`, strings.Join(entries, ",")) case strings.HasPrefix(r.URL.Path, "/messages/"): + sent.MessageReads++ sent.MessageAccountFilter = r.URL.Query().Get("filtered_account_id") + if sent.MessageStatus != 0 { + http.Error(w, `{"message":"message unavailable"}`, sent.MessageStatus) + return + } if r.URL.Path != "/messages/12.json" { t.Errorf("read %s, want the thread's latest entry", r.URL.Path) } @@ -508,6 +519,82 @@ func TestReplyDryRunReportsTheResolvedEnvelopeWithoutSending(t *testing.T) { } } +func TestReplyDryRunUsesEntryMetadataWithoutHydratingTheMessage(t *testing.T) { + server, sent := threadReplyServer(t, messageAddressedToJane, 11, 12) + sent.TopicEntryJSON = `{"id":12,"subject":"Weekly sync"}` + sent.MessageStatus = http.StatusInternalServerError + + out, err := runCLIOutput(t, server, "--account", "8", "reply", "7", "--dry-run", + "--replace-recipients", "--to", "support@example.com", "--cc", "manager@example.com") + if err != nil { + t.Fatalf("reply dry run: %v", err) + } + if sent.MessageReads != 0 { + t.Fatalf("dry run hydrated the message %d times", sent.MessageReads) + } + var response struct { + Data struct { + Subject string `json:"subject"` + To []string `json:"to"` + CC []string `json:"cc"` + } `json:"data"` + } + if err := json.Unmarshal([]byte(out), &response); err != nil { + t.Fatalf("decode dry run: %v\n%s", err, out) + } + if response.Data.Subject != "Re: Weekly sync" { + t.Errorf("subject = %q", response.Data.Subject) + } + if want := []string{"support@example.com"}; !reflect.DeepEqual(response.Data.To, want) { + t.Errorf("to = %v, want %v", response.Data.To, want) + } + if want := []string{"manager@example.com"}; !reflect.DeepEqual(response.Data.CC, want) { + t.Errorf("cc = %v, want %v", response.Data.CC, want) + } +} + +func TestReplyDryRunRefusesToGuessWhenThePrefillIsUnavailable(t *testing.T) { + server, sent := threadReplyServer(t, messageAddressedToJane, 11, 12) + sent.TopicEntryJSON = `{"id":12,"subject":"Weekly sync"}` + sent.MessageStatus = http.StatusInternalServerError + + _, err := runCLIOutput(t, server, "--account", "8", "reply", "7", "--dry-run", "--to", "support@example.com") + var cliErr *apierr.Error + if !errors.As(err, &cliErr) || !strings.Contains(cliErr.Hint, "use --replace-recipients") { + t.Fatalf("error = %v, want an actionable replacement hint", err) + } + if sent.MessageReads != 0 { + t.Fatalf("refused dry run hydrated the message %d times", sent.MessageReads) + } + if sent.Path != "" { + t.Errorf("refused dry run wrote to %q", sent.Path) + } +} + +func TestReplyReplacementDoesNotNeedTheOriginalMessage(t *testing.T) { + server, sent := threadReplyServer(t, messageAddressedToJane, 11, 12) + sent.TopicEntryJSON = `{"id":12,"subject":"Weekly sync"}` + sent.MessageStatus = http.StatusRequestEntityTooLarge + + err := runCLI(t, server, "--account", "8", "reply", "7", "--replace-recipients", + "--to", "support@example.com", "-m", "please route this") + if err != nil { + t.Fatalf("reply with replacement recipient: %v", err) + } + if sent.MessageReads != 0 { + t.Fatalf("replacement reply hydrated the message %d times", sent.MessageReads) + } + if want := []string{"support@example.com"}; !reflect.DeepEqual(sent.To, want) { + t.Errorf("to = %v, want %v", sent.To, want) + } + if sent.Subject != "Re: Weekly sync" { + t.Errorf("subject = %q", sent.Subject) + } + if !strings.Contains(sent.Path, "/entries/12/replies") { + t.Errorf("path = %q, want the existing thread's reply endpoint", sent.Path) + } +} + func TestReplyDryRunResolvesTheAccountDefaultSender(t *testing.T) { server, sent := threadReplyServer(t, messageAddressedToJane, 11, 12) sent.ReplyNewJSON = `{"subject":"Re: Weekly sync","content":"
quoted
","is_reply":true, diff --git a/skills/hey/SKILL.md b/skills/hey/SKILL.md index 98c6c3ed..97b1ad23 100644 --- a/skills/hey/SKILL.md +++ b/skills/hey/SKILL.md @@ -494,8 +494,10 @@ hey compose --to alice@example.com --subject "Newsletter draft" --message-html " web app does: everyone that entry was addressed to, plus whoever wrote it, on the To line. Repeatable `--to`, `--cc` and `--bcc` flags add or move explicit recipients. Use `--replace-recipients` to discard HEY's prefill. Run `--dry-run --json` first when an -agent changes the envelope; it needs no message and reports the resolved sender and final -recipients without sending. A reply HEY cannot address is refused unless an explicit +agent changes the envelope; it does not read the original message body and reports the +resolved sender and final recipients without sending. If HEY's envelope prefill is +unavailable, use `--replace-recipients` with explicit addresses because a dry run will not +guess the original lists. A reply HEY cannot address is refused unless an explicit recipient makes it addressable. Everything you send is Markdown by default — `-m`, `--content`, `--note`, positional