fix: append a copy of every sent message to the sent folder

internal/desktop/bind_sync.go

@@ -124,10 +124,48 @@ func (t *accountTransmitter) Transmit(ctx context.Context, m outbox.Message) err
 	if err != nil {
 		return err
 	}
-	sender := psmtp.NewSender(cfg, psmtp.WithLogger(t.app.log))
+	sender := psmtp.NewSender(cfg,
+		psmtp.WithLogger(t.app.log),
+		psmtp.WithSentAppender(func(raw []byte) (string, error) {
+			return t.app.appendToSent(*account, raw)
+		}),
+	)
 	return sender.Transmit(ctx, m)
 }
 
+// appendToSent puts a copy of a message that has just been sent in the
+// account's Sent folder, so it is there in webmail and in every other client,
+// not only in Pelton (#451).
+//
+// It opens its own imap session rather than borrowing the sync engine's. That
+// one is parked in IDLE almost all the time, and interrupting it to append
+// would cost more than a second connection does for something that happens
+// once per sent message.
+//
+// The copy is appended to the server and not written locally: the folder's next
+// sync pulls it down like any other message, which keeps one path for how mail
+// arrives in the store.
+func (a *App) appendToSent(account storage.Account, raw []byte) (string, error) {
+	cfg, err := a.resolveIMAP(account)
+	if err != nil {
+		return "", err
+	}
+	// every other imap session in the package takes this, so a send during a
+	// sync waits its turn rather than opening a second login for the account.
+	syncMu.Lock()
+	defer syncMu.Unlock()
+	client, err := a.connectIMAP(cfg)
+	if err != nil {
+		return "", err
+	}
+	defer func() { _ = client.Close() }()
+	if err := client.Login(); err != nil {
+		return "", err
+	}
+	defer func() { _ = client.Logout() }()
+	return client.AppendToSent(raw)
+}
+
 // runInitialSyncAndIdle syncs every account once, then parks each on idle.
 func (a *App) runInitialSyncAndIdle() {
 	accounts, err := a.store.ListAccounts(a.ctx)

internal/desktop/bind_sync_sent_test.go

@@ -0,0 +1,198 @@
+package desktop
+
+import (
+	"bufio"
+	"context"
+	"crypto/tls"
+	"errors"
+	"log/slog"
+	"net"
+	"net/http"
+	"net/http/httptest"
+	"path/filepath"
+	"strconv"
+	"strings"
+	"testing"
+
+	"github.com/peltonapp/Pelton/internal/certtrust"
+	"github.com/peltonapp/Pelton/internal/credentials"
+	pimap "github.com/peltonapp/Pelton/internal/imap"
+	"github.com/peltonapp/Pelton/internal/outbox"
+	"github.com/peltonapp/Pelton/internal/storage"
+)
+
+// submissionServer is enough of a submission server to carry one message:
+// greeting, EHLO, AUTH, MAIL, RCPT, DATA, QUIT. It runs on implicit TLS with
+// httptest's self-signed certificate, and returns its port and the certificate
+// fingerprint so the account under test can pin it.
+//
+// The whole point of these tests is the step that happens after a send
+// succeeds, so the send has to actually succeed.
+func submissionServer(t *testing.T) (port int, fingerprint string) {
+	t.Helper()
+	certServer := httptest.NewTLSServer(http.HandlerFunc(func(http.ResponseWriter, *http.Request) {}))
+	certServer.Close()
+	tlsConfig := certServer.TLS.Clone()
+	tlsConfig.NextProtos = nil
+
+	ln, err := tls.Listen("tcp", "127.0.0.1:0", tlsConfig)
+	if err != nil {
+		t.Fatalf("listen: %v", err)
+	}
+	t.Cleanup(func() { _ = ln.Close() })
+	go func() {
+		for {
+			conn, err := ln.Accept()
+			if err != nil {
+				return
+			}
+			go speakSubmission(conn)
+		}
+	}()
+
+	_, p, _ := net.SplitHostPort(ln.Addr().String())
+	n, _ := strconv.Atoi(p)
+	return n, certtrust.Fingerprint(certServer.Certificate())
+}
+
+func speakSubmission(conn net.Conn) {
+	defer func() { _ = conn.Close() }()
+	r := bufio.NewReader(conn)
+	write := func(s string) { _, _ = conn.Write([]byte(s + "\r\n")) }
+
+	write("220 localhost ESMTP")
+	for {
+		line, err := r.ReadString('\n')
+		if err != nil {
+			return
+		}
+		cmd := strings.ToUpper(strings.TrimSpace(line))
+		switch {
+		case strings.HasPrefix(cmd, "EHLO"), strings.HasPrefix(cmd, "HELO"):
+			write("250-localhost")
+			write("250 AUTH PLAIN")
+		case strings.HasPrefix(cmd, "AUTH"):
+			write("235 2.7.0 authenticated")
+		case strings.HasPrefix(cmd, "DATA"):
+			write("354 go ahead")
+			// read to the end-of-data marker and accept whatever arrived.
+			for {
+				body, err := r.ReadString('\n')
+				if err != nil {
+					return
+				}
+				if strings.TrimSpace(body) == "." {
+					break
+				}
+			}
+			write("250 2.0.0 queued")
+		case strings.HasPrefix(cmd, "QUIT"):
+			write("221 bye")
+			return
+		default:
+			write("250 2.0.0 ok")
+		}
+	}
+}
+
+// sendingAccount builds an app and an account whose smtp points at a live test
+// server, with the server's certificate pinned and a password stored, and
+// points the imap side at client.
+func sendingAccount(t *testing.T, client *fakeIMAP) (*App, storage.Account) {
+	t.Helper()
+	ctx, stopBackground := testContext(t)
+	db, err := storage.Open(filepath.Join(t.TempDir(), "test.db"))
+	if err != nil {
+		t.Fatalf("open: %v", err)
+	}
+	t.Cleanup(func() { db.Close() })
+	t.Cleanup(stopBackground)
+	if err := db.RunMigrations(ctx); err != nil {
+		t.Fatalf("migrate: %v", err)
+	}
+
+	port, fingerprint := submissionServer(t)
+	account := storage.Account{
+		Email:        "me@example.com",
+		Username:     "me@example.com",
+		SMTPHost:     "127.0.0.1",
+		SMTPPort:     port,
+		SMTPTLS:      "ssl",
+		IMAPHost:     "127.0.0.1",
+		IMAPPort:     993,
+		IMAPTLS:      "ssl",
+		TrustedCerts: []string{fingerprint},
+	}
+	id, err := db.CreateAccount(ctx, &account)
+	if err != nil {
+		t.Fatalf("create account: %v", err)
+	}
+	account.ID = id
+
+	if err := credentials.Store(id, credentials.Secret{
+		Method:   credentials.MethodPassword,
+		Password: "hunter2",
+	}); err != nil {
+		t.Fatalf("store credentials: %v", err)
+	}
+	t.Cleanup(func() { _ = credentials.Delete(id) })
+
+	app := &App{ctx: ctx, store: db, log: slog.New(slog.DiscardHandler)}
+	app.newIMAPClient = func(pimap.Config) (mailClient, error) { return client, nil }
+	return app, account
+}
+
+// The bug this exists for (#451): the append-to-Sent step was wired through an
+// option the one place that builds the sender never passed, so every message
+// sent correctly and no copy was ever written. Nothing failed, because the
+// missing appender is a debug log and a send that returns nil.
+//
+// So the assertion is on the transmit path as the outbox worker drives it, not
+// on the helper underneath. A test of the helper alone would still have passed
+// the whole time the feature was dead.
+func TestTransmitAppendsACopyToSent(t *testing.T) {
+	client := &fakeIMAP{}
+	app, account := sendingAccount(t, client)
+
+	raw := []byte("From: me@example.com\r\nTo: you@example.com\r\nSubject: hi\r\n\r\nbody\r\n")
+	transmitter := &accountTransmitter{app: app}
+	err := transmitter.Transmit(context.Background(), outbox.Message{
+		AccountID:    account.ID,
+		EnvelopeFrom: "me@example.com",
+		Recipients:   []string{"you@example.com"},
+		Raw:          raw,
+	})
+	if err != nil {
+		t.Fatalf("Transmit: %v", err)
+	}
+
+	if len(client.appended) != 1 {
+		t.Fatalf("appended %d messages to Sent, want 1", len(client.appended))
+	}
+	if string(client.appended[0]) != string(raw) {
+		t.Errorf("appended %q, want the message that was sent", client.appended[0])
+	}
+	if !client.loggedOut {
+		t.Error("the imap session opened for the append was left open")
+	}
+}
+
+// The message has already left the building by the time the append runs, so a
+// server that refuses it must not turn a delivered message into a failed one.
+// The copy is lost, which is worth a log, but reporting failure would have the
+// outbox retry and send it twice.
+func TestTransmitSucceedsWhenTheSentCopyFails(t *testing.T) {
+	client := &fakeIMAP{failAppend: errors.New("permission denied")}
+	app, account := sendingAccount(t, client)
+
+	transmitter := &accountTransmitter{app: app}
+	err := transmitter.Transmit(context.Background(), outbox.Message{
+		AccountID:    account.ID,
+		EnvelopeFrom: "me@example.com",
+		Recipients:   []string{"you@example.com"},
+		Raw:          []byte("Subject: hi\r\n\r\nbody\r\n"),
+	})
+	if err != nil {
+		t.Errorf("Transmit reported failure for a message that was sent: %v", err)
+	}
+}

internal/desktop/imapseam.go

@@ -31,6 +31,10 @@ type mailClient interface {
 	RenameFolder(path, newPath string) error
 	DeleteFolder(path string) error
 
+	// AppendToSent puts a copy of a message that has just been sent in the
+	// account's Sent folder, and reports which folder it used.
+	AppendToSent(raw []byte) (string, error)
+
 	FetchMessage(uid imap.UID) (*pimap.Message, error)
 	FetchMessages(uids []imap.UID, fn func(uid imap.UID, msg *pimap.Message, err error) error) error
 	FetchRawMessage(uid imap.UID) ([]byte, error)

internal/desktop/imapseam_test.go

@@ -43,6 +43,11 @@ type fakeIMAP struct {
 	// refusing the next login.
 	loggedOut bool
 
+	// appended is every message handed to AppendToSent, in order, and
+	// failAppend makes the append fail so a test can check a send still
+	// succeeds without one.
+	appended   [][]byte
+	failAppend error
 	// failMove makes MOVE fail, for the paths that have to leave the local cache
 	// alone when the server says no.
 	failMove error
@@ -53,6 +58,14 @@ type movedMessage struct {
 	dest string
 }
 
+func (f *fakeIMAP) AppendToSent(raw []byte) (string, error) {
+	if f.failAppend != nil {
+		return "", f.failAppend
+	}
+	f.appended = append(f.appended, raw)
+	return "Sent", nil
+}
+
 func (f *fakeIMAP) Login() error  { return nil }
 func (f *fakeIMAP) Logout() error { f.loggedOut = true; return nil }
 func (f *fakeIMAP) Close() error  { return nil }