Phase 5 follow-up: discover the \Sent mailbox; record the approved real send (AUD-71)

The approved real test reply was sent and the thread updated (same
X-GM-THRID as the original). The live test exposed one bug: the thread
check hardcoded Gmail's sent mailbox as "[Gmail]/Sent Mail", but this
account exposes it as "[Google Mail]/Sent Mail".

- internal/mailfetch: LIST parser, ListMailboxes, and SentMailbox that
  finds the mailbox flagged \Sent (RFC 6154) with name fallbacks.
- internal/mailops: VerifyThreadUpdated selects the discovered \Sent
  mailbox instead of a hardcoded name.
- cmd/email-agent: -check-thread <id> exercises the check against real
  Gmail without sending mail.
- Tests for LIST parsing, sent-mailbox discovery, and the discovery
  error path.
- docs/phase5-replies.md: record send-as verification, the sent reply's
  thread id, the app confirmation (sent_reply_in_thread=true), and the
  fix.

Co-Authored-By: Paperclip <noreply@paperclip.ing>
This commit is contained in:
Paperclip Builder
2026-10-11 22:18:05 +11:00
parent 7ca888f0c6
commit 7c66e4ad31
7 changed files with 341 additions and 34 deletions

View File

@@ -9,6 +9,7 @@ import (
"net/http" "net/http"
"os" "os"
"os/signal" "os/signal"
"strings"
"syscall" "syscall"
"time" "time"
@@ -29,6 +30,7 @@ func main() {
classifyNow = flag.Bool("classify", false, "classify pending messages with JEV and exit") classifyNow = flag.Bool("classify", false, "classify pending messages with JEV and exit")
classifyAll = flag.Bool("classify-all", false, "re-classify every stored message with JEV and exit") classifyAll = flag.Bool("classify-all", false, "re-classify every stored message with JEV and exit")
checkSMTP = flag.Bool("check-smtp", false, "verify the Gmail SMTP connection (IPv4 + STARTTLS + login) and exit; sends no mail") checkSMTP = flag.Bool("check-smtp", false, "verify the Gmail SMTP connection (IPv4 + STARTTLS + login) and exit; sends no mail")
checkThread = flag.Int64("check-thread", 0, "verify that a stored message's Gmail thread has a sent reply (read-only), then exit")
) )
flag.Parse() flag.Parse()
@@ -79,6 +81,14 @@ func main() {
return return
} }
if *checkThread > 0 {
if err := runCheckThread(cfg, logger, *checkThread); err != nil {
fmt.Fprintf(os.Stderr, "thread check failed: %v\n", err)
os.Exit(1)
}
return
}
if err := run(cfg, logger); err != nil { if err := run(cfg, logger); err != nil {
logger.Error("fatal", "error", err) logger.Error("fatal", "error", err)
os.Exit(1) os.Exit(1)
@@ -194,6 +204,37 @@ func runCheckSMTP(cfg *config.Config, logger *slog.Logger) error {
return mailops.New(cfg, logger).CheckSMTP(ctx) return mailops.New(cfg, logger).CheckSMTP(ctx)
} }
// runCheckThread reports whether a stored message's Gmail thread already has a
// message in the Sent mailbox. It is read-only and exists so the Phase 5 thread
// check can be exercised against real Gmail without sending anything.
func runCheckThread(cfg *config.Config, logger *slog.Logger, messageID int64) error {
if !cfg.GmailConfigured() {
return mailops.ErrNotConfigured
}
ctx, stop := signal.NotifyContext(context.Background(), syscall.SIGINT, syscall.SIGTERM)
defer stop()
st, err := store.Open(ctx, cfg.DBPath)
if err != nil {
return fmt.Errorf("open store: %w", err)
}
defer func() { _ = st.Close() }()
msg, err := st.MessageByID(ctx, messageID)
if err != nil {
return err
}
if strings.TrimSpace(msg.ThreadID) == "" {
return fmt.Errorf("message %d has no Gmail thread id", messageID)
}
ok, err := mailops.New(cfg, logger).VerifyThreadUpdated(ctx, msg.ThreadID)
if err != nil {
return err
}
fmt.Printf("message=%d thread=%s sent_reply_in_thread=%v\n", messageID, msg.ThreadID, ok)
return nil
}
// probeHealth requests the local health endpoint. It exists so the container // probeHealth requests the local health endpoint. It exists so the container
// healthcheck does not need an external HTTP client in the image. // healthcheck does not need an external HTTP client in the image.
func probeHealth(port int) error { func probeHealth(port int) error {

View File

@@ -1,15 +1,13 @@
# Phase 5 — Reply drafts (templates, LLM draft, edit, send) # Phase 5 — Reply drafts (templates, LLM draft, edit, send)
Status: **built, tested, and deployed. The one real test send is awaiting Sam's Status: **complete.** Built, tested, deployed, and the approved real test reply
explicit approval** (issue interaction `080c500d-5968-49ad-9c40-0b2d3f445cc0` on was sent and confirmed to have updated the thread. Sam approved the test send via
[AUD-71](/AUD/issues/AUD-71)). Parent [AUD-68](/AUD/issues/AUD-68). Phase 5 of issue interaction `080c500d-5968-49ad-9c40-0b2d3f445cc0` on
[AUD-71](/AUD/issues/AUD-71). Parent [AUD-68](/AUD/issues/AUD-68). Phase 5 of
AGENTS.md §11. AGENTS.md §11.
**DoD:** a reply is drafted, edited, and sent, and the thread updates. **DoD:** a reply is drafted, edited, and sent, and the thread updates. — met
(§2.6).
Every part of the DoD except the real send is verified below. The real send needs
Sam's approval per the phase rules and AGENTS.md §10; it is the only step that
cannot be tested without sending mail.
## 1. What was built ## 1. What was built
@@ -84,19 +82,23 @@ and the send path is verified up to (but not including) DATA.
### 2.3 Gmail "Send mail as" verification (AGENTS.md §13 "Remaining") ### 2.3 Gmail "Send mail as" verification (AGENTS.md §13 "Remaining")
AGENTS.md records send-as as "reported as already set up in Gmail … not verifiable AGENTS.md recorded send-as as "reported as already set up in Gmail … not
from the sent folder". Gmail exposes no read-only alias list over IMAP/SMTP, so verifiable from the sent folder". Gmail exposes no read-only alias list over
the only reliable check is a real send: Gmail accepts the `From` header only when IMAP/SMTP, so the check is a real send: Gmail accepts the `From` header only when
the address is a registered send-as alias, and rejects it otherwise during DATA. the address is a registered send-as alias, and rejects it during DATA otherwise.
The SMTP connection, STARTTLS upgrade, and app-password authentication are **Result: verified.** The approved test reply was sent as
verified (`-check-smtp`). The `From`-header/send-as check is **pending the `"Sam Rolfe" <hello@where-woof.com>` (the `X-Pm-Original-To` address) from the
approved test send**. `samuelrolfe@gmail.com` account. Gmail accepted it and stored it in the Sent
mailbox (`[Google Mail]/Sent Mail`) with that exact `From` header, so
`hello@where-woof.com` is a working send-as alias. The SMTP connection, STARTTLS
upgrade, and app-password authentication were already verified separately with
`-check-smtp`.
### 2.4 Draft generated, edited, and stored (screenshot of the live data) ### 2.4 Draft generated, edited, and sent
Draft `2` for message `99` ("Test email", arrived at `hello@where-woof.com`), Draft `2` for message `99` ("Test email", arrived at `hello@where-woof.com`),
generated through the live UI path and then edited: generated through the live UI path, edited, and then sent after Sam's approval:
``` ```
Thanks, got it. This came through to hello@where-woof.com. Since it's a test, no action needed on my side. If you need anything specific, let me know. Thanks, got it. This came through to hello@where-woof.com. Since it's a test, no action needed on my side. If you need anything specific, let me know.
@@ -104,13 +106,44 @@ Thanks, got it. This came through to hello@where-woof.com. Since it's a test, no
Sam Sam
``` ```
### 2.5 Thread update after a sent reply After the send the draft status is `sent`, the message left the inbox and now
lives under the `Replied-to/hello@where-woof.com` label (1 row in the Replied-to
view), and the message history shows `draft`, `reply_sent`, and the `move` rows
(FR-13).
Pending the approved test send. The plan is to reply to message `99` as ### 2.5 Thread update after the sent reply
`"Sam Rolfe" <hello@where-woof.com>` to `samuelrolfe@gmail.com` (Sam himself — no
third party), then confirm the thread by finding the original Gmail thread id in **Result: verified — the reply joined the original thread.** Read directly from
`[Gmail]/Sent Mail` (`X-GM-THRID` search). The exact message and draft are on the Gmail over IMAP (read-only):
pending approval card.
| Message | From | To | `X-GM-THRID` | `In-Reply-To` / `References` |
|---|---|---|---|---|
| Original (`99`) | `sam rolfe <samuelrolfe@gmail.com>` | `hello@where-woof.com` | `1878738835102629816` | — |
| Sent reply | `"Sam Rolfe" <hello@where-woof.com>` | `samuelrolfe@gmail.com` | `1878738835102629816` | `<CAMAgT7S04G5E=…@mail.gmail.com>` |
The reply carries the original `Message-ID` in `In-Reply-To` and `References` and
Gmail assigned it the **same thread id**, so the thread updated. The app's own
check now agrees:
```
$ docker exec email-agent email-agent -check-thread 99
message=99 thread=1878738835102629816 sent_reply_in_thread=true
```
Status code `sent_reply_in_thread=true`. No third party received the test: the
only recipient was `samuelrolfe@gmail.com`.
### 2.6 Bug found by the live test and fixed
The first send reported "Could not yet confirm the thread update". The cause was
not threading: `VerifyThreadUpdated` hardcoded Gmail's mailbox as
`[Gmail]/Sent Mail`, but this account exposes the special-use mailboxes as
`[Google Mail]/…`. The fix discovers the mailbox flagged `\Sent` (RFC 6154) with
`LIST`, falling back to the common names. New coverage:
`internal/mailfetch` (`parseList`, `ListMailboxes`, `SentMailbox`) and
`internal/mailops` (discovered sent mailbox, discovery error). `-check-thread
<id>` was added so the check can be exercised against real Gmail without sending
mail. After the fix, `-check-thread 99` returns true (above).
## 3. Configuration ## 3. Configuration
@@ -129,9 +162,13 @@ docker compose up -d # migration v5 runs on start; data volume kept
docker exec email-agent email-agent -check-smtp # safe: no mail sent docker exec email-agent email-agent -check-smtp # safe: no mail sent
``` ```
Two commits were needed: the Phase 5 implementation and the `\Sent` mailbox
discovery fix from §2.6.
## 5. Rollback ## 5. Rollback
1. `git revert <phase-5 commit>` and `docker compose up -d --build`. 1. `git revert <sent-mailbox-fix commit> <phase-5 commit>` and
`docker compose up -d --build`.
2. Migration v5 is additive (`messages.message_id_header`, `drafts.sent_at`); the 2. Migration v5 is additive (`messages.message_id_header`, `drafts.sent_at`); the
Phase 4 binary ignores the new columns. Phase 4 binary ignores the new columns.
3. A sent reply cannot be unsent. Gmail's Sent copy and the thread stay; the 3. A sent reply cannot be unsent. Gmail's Sent copy and the thread stay; the
@@ -139,8 +176,6 @@ docker exec email-agent email-agent -check-smtp # safe: no mail sent
## 6. Not done / deferred ## 6. Not done / deferred
- **One real send + thread confirmation** — awaiting Sam's approval (the pending
interaction above). This is the last DoD item.
- A **template editor** (user-defined templates) is not in the Phase 5 - A **template editor** (user-defined templates) is not in the Phase 5
requirements; templates are built in. The Phase 6 rules editor still feeds the requirements; templates are built in. The Phase 6 rules editor still feeds the
classifier. classifier.

View File

@@ -0,0 +1,118 @@
// Mailbox discovery for the IMAP client.
//
// Gmail exposes its special-use mailboxes with RFC 6154 attributes. The Sent
// mailbox is named "[Gmail]/Sent Mail" on most accounts but "[Google Mail]/Sent
// Mail" on others (and "Sent" on non-Gmail servers). Phase 5 asks the server for
// the mailbox flagged \Sent instead of hardcoding a name.
package mailfetch
import (
"fmt"
"strings"
)
// Mailbox is one entry from an IMAP LIST response.
type Mailbox struct {
Name string
Attributes []string
}
// Has reports whether the mailbox carries the given attribute (for example
// `\Sent` or `\All`). The comparison is case-insensitive.
func (m Mailbox) Has(attr string) bool {
for _, a := range m.Attributes {
if strings.EqualFold(a, attr) {
return true
}
}
return false
}
// ListMailboxes returns every mailbox the server advertises.
func (c *Client) ListMailboxes() ([]Mailbox, error) {
var out []Mailbox
err := c.exec(`LIST "" "*"`, func(r *response) error {
if mb, ok := parseList(r.line); ok {
out = append(out, mb)
}
return nil
})
return out, err
}
// sentFallbacks are used when the server does not advertise the \Sent
// attribute.
var sentFallbacks = []string{
"[Gmail]/Sent Mail",
"[Google Mail]/Sent Mail",
"Sent",
"Sent Mail",
}
// SentMailbox returns the mailbox the server flags \Sent. It falls back to the
// common names when the attribute is absent.
func (c *Client) SentMailbox() (string, error) {
boxes, err := c.ListMailboxes()
if err != nil {
return "", err
}
for _, mb := range boxes {
if mb.Has(`\Sent`) {
return mb.Name, nil
}
}
for _, want := range sentFallbacks {
for _, mb := range boxes {
if strings.EqualFold(mb.Name, want) {
return mb.Name, nil
}
}
}
return "", fmt.Errorf("no sent mailbox advertised by the server")
}
// parseList parses one "* LIST (attrs) delim name" response line. XLIST is
// accepted as well, for servers that only implement the older extension.
func parseList(line string) (Mailbox, bool) {
upper := strings.ToUpper(line)
if !strings.HasPrefix(upper, "* LIST ") && !strings.HasPrefix(upper, "* XLIST ") {
return Mailbox{}, false
}
open := strings.IndexByte(line, '(')
if open < 0 {
return Mailbox{}, false
}
closeIdx := strings.IndexByte(line[open:], ')')
if closeIdx < 0 {
return Mailbox{}, false
}
closeIdx += open
attrs := strings.Fields(line[open+1 : closeIdx])
tail := strings.TrimSpace(line[closeIdx+1:])
name := lastQuoted(tail)
if name == "" {
fields := strings.Fields(tail)
if len(fields) == 0 {
return Mailbox{}, false
}
name = strings.Trim(fields[len(fields)-1], `"`)
}
if name == "" {
return Mailbox{}, false
}
return Mailbox{Name: name, Attributes: attrs}, true
}
// lastQuoted returns the contents of the last "..." segment in s.
func lastQuoted(s string) string {
end := strings.LastIndexByte(s, '"')
if end <= 0 {
return ""
}
start := strings.LastIndexByte(s[:end], '"')
if start < 0 {
return ""
}
return s[start+1 : end]
}

View File

@@ -0,0 +1,77 @@
package mailfetch
import "testing"
func TestParseList(t *testing.T) {
// Gmail on this account advertises "[Google Mail]/..." rather than
// "[Gmail]/...". The \Sent attribute is what matters.
cases := []struct {
line string
name string
attr string
ok bool
}{
{`* LIST (\HasChildren \Noselect) "/" "[Gmail]"`, "[Gmail]", `\Noselect`, true},
{`* LIST (\HasNoChildren \Sent) "/" "[Google Mail]/Sent Mail"`, "[Google Mail]/Sent Mail", `\Sent`, true},
{`* LIST (\HasNoChildren) "/" "Replied-to/hello@where-woof.com"`, "Replied-to/hello@where-woof.com", `\HasNoChildren`, true},
{`* LIST (\All \HasNoChildren) "/" "[Google Mail]/All Mail"`, "[Google Mail]/All Mail", `\All`, true},
{`* LIST (\HasNoChildren) NIL "Sent"`, "Sent", `\HasNoChildren`, true},
{`* OK no list here`, "", "", false},
}
for _, tc := range cases {
mb, ok := parseList(tc.line)
if ok != tc.ok {
t.Errorf("parseList(%q) ok = %v, want %v", tc.line, ok, tc.ok)
continue
}
if !ok {
continue
}
if mb.Name != tc.name {
t.Errorf("parseList(%q) name = %q, want %q", tc.line, mb.Name, tc.name)
}
if !mb.Has(tc.attr) {
t.Errorf("parseList(%q) missing attribute %q (got %v)", tc.line, tc.attr, mb.Attributes)
}
}
}
func TestListMailboxesAndSentMailbox(t *testing.T) {
script := "* LIST (\\HasChildren \\Noselect) \"/\" \"[Gmail]\"\r\n" +
"* LIST (\\HasNoChildren) \"/\" \"Replied-to/hello@where-woof.com\"\r\n" +
"* LIST (\\All \\HasNoChildren) \"/\" \"[Google Mail]/All Mail\"\r\n" +
"* LIST (\\HasChildren \\Sent) \"/\" \"[Google Mail]/Sent Mail\"\r\n" +
"A0001 OK LIST completed\r\n"
c, fc := newFakeClient(script)
boxes, err := c.ListMailboxes()
if err != nil {
t.Fatalf("ListMailboxes() error = %v", err)
}
if len(boxes) != 4 {
t.Fatalf("ListMailboxes() = %d, want 4", len(boxes))
}
if got := fc.w.String(); got == "" || !contains(got, `LIST "" "*"`) {
t.Errorf("command sent = %q, want LIST", got)
}
// SentMailbox issues its own LIST, so it needs a fresh script (a live
// connection would simply answer again).
c2, _ := newFakeClient(script)
sent, err := c2.SentMailbox()
if err != nil {
t.Fatalf("SentMailbox() error = %v", err)
}
if sent != "[Google Mail]/Sent Mail" {
t.Errorf("SentMailbox() = %q, want [Google Mail]/Sent Mail", sent)
}
}
func contains(haystack, needle string) bool {
for i := 0; i+len(needle) <= len(haystack); i++ {
if haystack[i:i+len(needle)] == needle {
return true
}
}
return false
}

View File

@@ -31,9 +31,6 @@ const (
LabelDelete = "Delete" LabelDelete = "Delete"
LabelSaved = "Saved" LabelSaved = "Saved"
repliedPrefix = "Replied-to/" repliedPrefix = "Replied-to/"
// LabelSent is Gmail's IMAP name for the Sent mailbox. Phase 5 checks it to
// confirm a reply joined the original thread.
LabelSent = "[Gmail]/Sent Mail"
) )
// ErrNotConfigured means Gmail credentials are missing, so no mailbox action can // ErrNotConfigured means Gmail credentials are missing, so no mailbox action can
@@ -50,6 +47,9 @@ type Conn interface {
StoreFlags(uid uint32, op string, flag string) error StoreFlags(uid uint32, op string, flag string) error
SearchGmailID(gmailID string) ([]uint32, error) SearchGmailID(gmailID string) ([]uint32, error)
SearchGmailThreadID(threadID string) ([]uint32, error) SearchGmailThreadID(threadID string) ([]uint32, error)
// SentMailbox returns the server's \Sent mailbox (Gmail may call it
// "[Gmail]/Sent Mail" or "[Google Mail]/Sent Mail").
SentMailbox() (string, error)
Logout() error Logout() error
} }
@@ -164,12 +164,16 @@ func (c *Client) VerifyThreadUpdated(ctx context.Context, threadID string) (bool
} }
defer func() { _ = conn.Logout() }() defer func() { _ = conn.Logout() }()
if _, err := conn.Select(LabelSent); err != nil { sent, err := conn.SentMailbox()
return false, fmt.Errorf("select %q: %w", LabelSent, err) if err != nil {
return false, fmt.Errorf("find the sent mailbox: %w", err)
}
if _, err := conn.Select(sent); err != nil {
return false, fmt.Errorf("select %q: %w", sent, err)
} }
uids, err := conn.SearchGmailThreadID(threadID) uids, err := conn.SearchGmailThreadID(threadID)
if err != nil { if err != nil {
return false, fmt.Errorf("search thread %s in %q: %w", threadID, LabelSent, err) return false, fmt.Errorf("search thread %s in %q: %w", threadID, sent, err)
} }
return len(uids) > 0, nil return len(uids) > 0, nil
} }

View File

@@ -21,6 +21,8 @@ type fakeConn struct {
searched []string searched []string
searchUIDs []uint32 searchUIDs []uint32
searchErr error searchErr error
sentMailbox string
sentErr error
loggedOut bool loggedOut bool
} }
@@ -49,6 +51,15 @@ func (f *fakeConn) SearchGmailThreadID(id string) ([]uint32, error) {
f.searched = append(f.searched, "thrid:"+id) f.searched = append(f.searched, "thrid:"+id)
return f.searchUIDs, f.searchErr return f.searchUIDs, f.searchErr
} }
func (f *fakeConn) SentMailbox() (string, error) {
if f.sentErr != nil {
return "", f.sentErr
}
if f.sentMailbox == "" {
return "[Google Mail]/Sent Mail", nil
}
return f.sentMailbox, nil
}
func (f *fakeConn) Logout() error { f.loggedOut = true; return nil } func (f *fakeConn) Logout() error { f.loggedOut = true; return nil }
func configured() *config.Config { func configured() *config.Config {

View File

@@ -2,6 +2,7 @@ package mailops
import ( import (
"context" "context"
"errors"
"net" "net"
"strings" "strings"
"testing" "testing"
@@ -113,14 +114,34 @@ func TestVerifyThreadUpdatedSearchesSentMailbox(t *testing.T) {
if !ok { if !ok {
t.Error("VerifyThreadUpdated() = false, want true when the thread has a sent message") t.Error("VerifyThreadUpdated() = false, want true when the thread has a sent message")
} }
if fc.selected != LabelSent { if fc.selected != "[Google Mail]/Sent Mail" {
t.Errorf("selected = %q, want %q", fc.selected, LabelSent) t.Errorf("selected = %q, want the discovered sent mailbox", fc.selected)
} }
if len(fc.searched) != 1 || fc.searched[0] != "thrid:987654321" { if len(fc.searched) != 1 || fc.searched[0] != "thrid:987654321" {
t.Errorf("searched = %v, want [thrid:987654321]", fc.searched) t.Errorf("searched = %v, want [thrid:987654321]", fc.searched)
} }
} }
func TestVerifyThreadUpdatedUsesDiscoveredSentMailbox(t *testing.T) {
fc := &fakeConn{uidValidity: 3, searchUIDs: []uint32{5}, sentMailbox: "[Gmail]/Sent Mail"}
c := newFakeClient(fc)
ok, err := c.VerifyThreadUpdated(context.Background(), "77")
if err != nil || !ok {
t.Fatalf("VerifyThreadUpdated() ok=%v err=%v", ok, err)
}
if fc.selected != "[Gmail]/Sent Mail" {
t.Errorf("selected = %q, want the configured sent mailbox", fc.selected)
}
}
func TestVerifyThreadUpdatedSentMailboxError(t *testing.T) {
fc := &fakeConn{sentErr: errors.New("no sent mailbox")}
c := newFakeClient(fc)
if _, err := c.VerifyThreadUpdated(context.Background(), "77"); err == nil {
t.Fatal("VerifyThreadUpdated() error = nil, want a sent-mailbox discovery error")
}
}
func TestVerifyThreadUpdatedEmptyThread(t *testing.T) { func TestVerifyThreadUpdatedEmptyThread(t *testing.T) {
c := newFakeClient(&fakeConn{searchUIDs: nil}) c := newFakeClient(&fakeConn{searchUIDs: nil})
if _, err := c.VerifyThreadUpdated(context.Background(), ""); err == nil { if _, err := c.VerifyThreadUpdated(context.Background(), ""); err == nil {