fix(imap): markøren er vores, ikke serverens

Samme mail blev en opgave hver tiende minut, og markøren stod stille på 1271.
Målt i produktion: en manuel synk lavede én opgave og flyttede ikke last_uid en
eneste plads.

Årsagen er ikke vores regnestykke, men hvad søgningen svarer. Vi beder om
1272:* — og en rigtig server læser det som 0:1272 og sender hele postkassen
tilbage. Alt, hvad den finder, ligger så under markøren, og markøren kan derfor
aldrig komme videre.

Uid'er ved eller under markøren frasorteres nu hos os. Det koster ét gennemløb
af nogle hundrede tal og afhænger ikke af nogen andens læsning af specifikationen.

De to tidligere forsøg var ikke forkerte, de var bare ikke årsagen: at tage
uid'en fra søgningen frem for fra fetch-svaret, og at give hver postkasse sin
egen lås, var begge reelle fejl. Denne er den, der fik den til at gentage sig.

Prøven læser en postkasse med markøren over alt i den og kræver nul tilbage —
og gentager det for hver enkelt besked.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

internal/imap/imap.go

@@ -135,6 +135,21 @@ func (c *Client) Since(uid uint32, limit int) ([]Message, uint32, error) {
 	}
 
 	uids := found.AllUIDs()
+
+	// Anything at or below the marker is dropped here rather than trusted to the
+	// range in the query. A server that reads `1272:*` as `0:1272` — and one does,
+	// which is how the same mail became a task every ten minutes with the marker
+	// standing still at 1271 — hands back the whole mailbox. The marker is ours;
+	// enforcing it here costs one pass over a list of at most a few hundred
+	// numbers and does not depend on anybody else's reading of the spec.
+	kept := make([]imap.UID, 0, len(uids))
+	for _, u := range uids {
+		if uint32(u) > uid {
+			kept = append(kept, u)
+		}
+	}
+	uids = kept
+
 	if len(uids) == 0 {
 		return nil, uid, nil
 	}

internal/imap/roundtrip_test.go

@@ -262,3 +262,57 @@ func TestAMultipartMailReadsAsItsText(t *testing.T) {
 		t.Errorf("the snippet does not start with the text of the mail: %q", got)
 	}
 }
+
+// The marker is ours, not the server's.
+//
+// A mailbox read with a marker above everything in it must come back empty. The
+// query says so, but a server is free to read the range differently — one reads
+// `1272:*` as `0:1272` and hands back the lot, which is how the same mail became
+// a task every ten minutes while the marker stood still at 1271. The filter is
+// on this side now, so the invariant holds whatever comes back.
+func TestNothingAtOrBelowTheMarkerComesBack(t *testing.T) {
+	addr, pool := serveMailbox(t, []mail{
+		{subject: "En", from: "a@example.dk", body: "en", flagged: true},
+		{subject: "To", from: "b@example.dk", body: "to", flagged: true},
+		{subject: "Tre", from: "c@example.dk", body: "tre", flagged: true},
+	})
+	client, err := Dial(Account{Host: addr, Username: "kw", Password: "hemmelig", RootCAs: pool})
+	if err != nil {
+		t.Fatal(err)
+	}
+	defer client.Close()
+
+	all, highest, err := client.Since(0, 25)
+	if err != nil {
+		t.Fatal(err)
+	}
+	if len(all) != 3 {
+		t.Fatalf("got %d messages on the first read, want 3", len(all))
+	}
+
+	// Read again from the marker the first run would have written.
+	rest, again, err := client.Since(highest, 25)
+	if err != nil {
+		t.Fatal(err)
+	}
+	if len(rest) != 0 {
+		t.Errorf("%d message(s) came back at or below the marker: %v", len(rest), rest[0].Subject)
+	}
+	// And the marker must not go backwards, or the next run reads them all again.
+	if again < highest {
+		t.Errorf("the marker moved back from %d to %d", highest, again)
+	}
+
+	// Every message, individually: none of them may reappear once passed.
+	for _, m := range all {
+		later, _, err := client.Since(m.UID, 25)
+		if err != nil {
+			t.Fatal(err)
+		}
+		for _, l := range later {
+			if l.UID <= m.UID {
+				t.Errorf("reading from %d gave %d back", m.UID, l.UID)
+			}
+		}
+	}
+}

internal/store/savemailbox_test.go

@@ -0,0 +1,40 @@
+package store
+
+import (
+	"context"
+	"testing"
+)
+
+// Saving a mailbox that is already there must update it, not fail.
+//
+// The row is written again on every token refresh and every time Gmail's seen
+// list changes. If that write errors, the tasks a sync made are kept and the note
+// of having made them is not — so the same mail becomes a task again on the next
+// sweep, once every ten minutes, forever.
+func TestSavingAnExistingMailboxUpdatesIt(t *testing.T) {
+	db, userID := sealedStore(t)
+	ctx := context.Background()
+
+	m := &Mailbox{UserID: userID, Kind: "gmail", Username: "kw@nolimit.dk",
+		RefreshToken: "r", Seen: []string{"a"}}
+	if err := db.SaveMailbox(ctx, m); err != nil {
+		t.Fatal(err)
+	}
+
+	m.Seen = []string{"a", "b"}
+	m.AccessToken = "fresh"
+	if err := db.SaveMailbox(ctx, m); err != nil {
+		t.Fatalf("saving it a second time failed: %v", err)
+	}
+
+	back, err := db.MailboxOfKind(ctx, userID, "gmail")
+	if err != nil {
+		t.Fatal(err)
+	}
+	if len(back.Seen) != 2 {
+		t.Errorf("the seen list came back as %v", back.Seen)
+	}
+	if back.AccessToken != "fresh" {
+		t.Errorf("the token came back as %q", back.AccessToken)
+	}
+}