fix(imap): error instead of silent success when STORE/COPY/MOVE target UIDs aren't present

[kochj23]

Summary

DeleteMessages, CopyMessages, and MoveMessages in
internal/imap/client.go reported success even when the target UIDs were not
present in the selected mailbox. Removing a label an email didn't have (a COPY
to the wrong Labels/* folder, or a STORE on an absent UID) printed e.g.
Label removed from 1 message(s) while nothing actually changed.

Root cause, as described in the issue:

• STORE against absent UIDs is a valid no-op per RFC 3501 and returns no
  error; the streamed FETCH responses (one per modified message) were never
  counted.
• COPY of a UID set that matches nothing is accepted without error by many
  servers (Proton Bridge included); the CopyData (SourceUIDs/DestUIDs)
  was discarded.
• MoveMessages reimplements COPY + STORE inline and ignored both.

Fix

• DeleteMessages: drain storeCmd.Next() and error when the affected count
  is zero.
• CopyMessages: error when CopyData.SourceUIDs and DestUIDs are both
  empty.
• MoveMessages: apply both checks before expunging the source.

No extra IMAP round-trips — the affected count and COPYUID data are already in
the response streams. SetFlagsMultiple is intentionally left unchanged (per
the issue): a server returns zero affected items both for an absent UID and for
a flag that is already set, so distinguishing them needs a different approach.

Tests

Added internal/imap/client_affected_test.go, which drives a real in-memory
IMAP server (go-imap's imapmemserver, no new module dependency) end-to-end,
organized into all seven categories:

• Security – a failed (no-match) delete must not partially mutate the
  mailbox.
• Performance – present-UID delete completes promptly, documenting that the
  affected count adds no extra/blocking round-trip.
• Retry – a no-match delete keeps failing deterministically, never leaking
  a false success.
• Unit – missing UID → error; present UID → success (STORE affected count).
• Integration – MOVE actually relocates the message from INBOX to Archive.
• Functional – COPY and MOVE on a missing UID surface an error rather than
  silent success (the memserver rejects an empty COPY at the protocol level;
  Proton Bridge returns an empty COPYUID set — both paths are handled).
• Frame – N/A (no new wire framing; response parsing covered by the
  integration path); explicit skip placeholder retained.

go build ./... && go vet ./... && go test ./... all green.

Closes #11

🤖 Generated with Claude Code

[Juan-de-Costa-Rica]

Ran this branch through the CI steps locally, since Actions does not appear to have run on the PR. All three checks from .github/workflows/ci.yml pass on go1.26.2:

gofmt -l .      (no output)
go vet ./...    clean
go test ./...   ok: internal/cli, internal/config, internal/imap, internal/output, internal/safetext, internal/smtp

One data point on the COPY path, from running pm-cli against Proton Bridge daily in an automated triage pipeline. Bridge does return COPYUID for a copy that matches nothing, so CopyData.SourceUIDs and DestUIDs come back non-nil and empty, and the check catches it there:

$ pm-cli mail label add uid:99999999 --label UNSORTED --json
{"error": "no messages matched the given ID(s) in INBOX", "success": false}

The affected count in MoveMessages still matters for the STORE half, where there is no COPYUID to inspect at all, and for servers that do omit COPYUID on COPY. Worth keeping both checks as written.

We had independently written a partial version of this fix (COPY guard only, no affected count) after hitting the same class in production. This one is more complete, so we are dropping ours and tracking this instead.

[bscott]

Reviewed and verified locally: gofmt/go vet/go test clean against master. The in-memory imapmemserver fixture is a genuinely good way to test this end-to-end — nice work.

Merging, with one follow-up I'm pushing immediately afterward:

The COPYUID check needs a capability guard. In go-imap v2, CopyCommand.Wait() returns &cmd.data unconditionally, so it is never nil — the copyData != nil half of the guard is dead code, and the check rests entirely on SourceUIDs/DestUIDs. But COPYUID is only emitted by servers advertising UIDPLUS (folded into IMAP4rev2). On a server without it, an empty pair means "the server never told us", not "nothing was copied" — which would turn every successful copy/label/move into no messages matched.

The fixture pins Caps: imap.CapSet{imap.CapIMAP4rev2: {}}, so the suite can't catch this. My follow-up extracts a copyMatchedNothing helper that returns false unless c.client.Caps().Has(imap.CapUIDPlus), plus a second fixture running plain IMAP4rev1 to cover it.

Worth knowing: on a non-UIDPLUS server this narrows COPY no-match detection, but MoveMessages still catches it via your STORE affected-count — I added TestMoveMissingUIDWithoutUIDPlusStillErrors to pin that down. All of your original tests still pass unchanged.

Closes #11.