fix(imap): remap local id to new UID after MOVE so caches survive

IMAP UIDs are mailbox-scoped, so MOVE assigns a fresh UID in the
destination folder. The flush previously discarded the response
from `client.uidMove(...)`, so the local row kept the *source*
UID while its `mailbox_path` already pointed at the destination.
Two things broke:

- Deletion reconciliation, which runs per mailbox and compares
  local UIDs to the server's `ALL` search result, would not find
  the source UID in the destination mailbox and wipe the row —
  taking the cached body and queued undo with it.
- `UndoLog` rows kept referencing the old `accountId:mailbox:uid`
  id, so undo had to fall back to a Message-ID lookup just to
  rediscover the moved message.

The fix captures the RFC 4315 `COPYUID` response code that
modern `UIDPLUS` servers attach to `MOVE`/`COPY` (already exposed
as `GenericImapResult.responseCodeCopyUid` in `enough_mail`).
When that's missing — i.e. the server doesn't support UIDPLUS —
we fall back to `UID SEARCH HEADER Message-ID …` in the
destination mailbox. Either way the local id is rewritten in
place to `accountId:destMailbox:newUid` and the cascading
`email_bodies`, `threads`, `pending_changes`, and `undo_actions`
references are updated in the same transaction.

`_reconcileDeletedImap` now also skips rows whose
`move`/`snooze`/`unsnooze` is still queued in `pending_changes`,
so the optimistic local move can't be wiped between the
optimistic write and the server flush.

Closes #539

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
agentloop
2026-06-10 13:19:04 +00:00
co-authored by Claude Opus 4.7
parent f1f7de7b4d
commit 0141d86361
7 changed files with 490 additions and 5 deletions
+7
View File
@@ -54,6 +54,13 @@ This document covers the mail-to-database sync layer only, not the UI.
optimistic local update; `flushPendingChanges` drains the queue over a single
IMAP connection at the start of each sync cycle.
- Sent messages are appended to the Sent folder after SMTP delivery.
- IMAP move remap: after a `MOVE` is flushed, the local row id is rewritten in
place using the RFC 4315 `COPYUID` response code (UIDPLUS); if the server
doesn't support UIDPLUS, the new UID is looked up via `UID SEARCH HEADER
Message-ID …` in the destination mailbox. Cached bodies (`email_bodies`),
threads, queued pending changes, and undo entries follow the new id.
Deletion reconciliation skips rows whose `move`/`snooze`/`unsnooze` is still
in `pending_changes` so the optimistic local move isn't wiped mid-flight.
- Sync retries use exponential backoff after failures.
### Cross-protocol
+11
View File
@@ -81,6 +81,17 @@ start()
On each run, only UIDs greater than `lastUid` are fetched. If `uidValidity` changes the full
folder is re-scanned and the checkpoint is reset.
**IMAP move remap** — IMAP UIDs are mailbox-scoped, so a moved message gets a new UID in
its destination folder. When a `move`/`snooze`/`unsnooze` change is flushed, the local row
id (`accountId:mailboxPath:uid`) is rewritten in place to point at the new UID. The new
UID is taken from the RFC 4315 `COPYUID` response code returned by `MOVE`; if the server
does not advertise `UIDPLUS`, a `UID SEARCH HEADER Message-ID …` in the destination
mailbox is used as a fallback. `email_bodies`, `threads`, `pending_changes`, and
`undo_actions` rows that reference the old id are updated atomically so cached bodies and
pending undo operations keep tracking the same physical message. Deletion reconciliation
also skips rows whose move is still queued, so the optimistic local move never gets
wiped mid-flight.
**IDLE cap** — IDLE sessions are limited to 25 minutes per the RFC. The loop also wakes
immediately if `syncNow()` is called (e.g. user pulls-to-refresh).
@@ -6,6 +6,7 @@ import 'dart:math' as math;
import 'package:drift/drift.dart';
import 'package:enough_mail/enough_mail.dart' as imap;
import 'package:http/http.dart' as http;
import 'package:meta/meta.dart';
import 'package:path/path.dart' as p;
import 'package:path_provider/path_provider.dart';
@@ -728,6 +729,14 @@ class EmailRepositoryImpl implements EmailRepository {
await _saveSyncState(accountId, resourceType, jsonEncode(data));
}
@visibleForTesting
Future<void> reconcileDeletedImapForTest(
String accountId,
String mailboxPath,
List<int> serverUids,
) =>
_reconcileDeletedImap(accountId, mailboxPath, serverUids);
Future<void> _reconcileDeletedImap(
String accountId,
String mailboxPath,
@@ -752,10 +761,28 @@ class EmailRepositoryImpl implements EmailRepository {
return;
}
// Email IDs that still have a queued move/snooze/unsnooze waiting to be
// flushed. The optimistic local move has already updated mailbox_path, so
// these rows look orphaned from both the old and new mailbox until the
// server applies the change and we remap to the destination UID. Skipping
// them here avoids wiping the row mid-flight.
final inFlightIds = await (_db.selectOnly(_db.pendingChanges)
..addColumns([_db.pendingChanges.resourceId])
..where(
_db.pendingChanges.accountId.equals(accountId) &
_db.pendingChanges.changeType.isIn(
const ['move', 'snooze', 'unsnooze'],
),
))
.map((row) => row.read(_db.pendingChanges.resourceId)!)
.get();
final inFlightSet = inFlightIds.toSet();
final serverUidSet = serverUids.toSet();
final affectedThreads = <String>{};
for (final row in localRows) {
if (!serverUidSet.contains(row.uid)) {
if (inFlightSet.contains(row.id)) continue;
affectedThreads.add(row.threadId ?? row.id);
await (_db.delete(_db.emails)..where((t) => t.id.equals(row.id))).go();
}
@@ -2317,7 +2344,15 @@ class EmailRepositoryImpl implements EmailRepository {
? await client.uidMarkFlagged(seq)
: await client.uidMarkUnflagged(seq);
case 'move':
await client.uidMove(seq, targetMailboxPath: payload['dest'] as String);
final dest = payload['dest'] as String;
final result = await client.uidMove(seq, targetMailboxPath: dest);
await _remapEmailAfterImapMove(
client,
oldId: row.resourceId,
sourceUid: uid,
destMailboxPath: dest,
moveResult: result,
);
case 'delete':
await client.uidMarkDeleted(seq);
await client.uidExpunge(seq);
@@ -2332,7 +2367,14 @@ class EmailRepositoryImpl implements EmailRepository {
await client.createMailbox(dest);
} catch (_) {}
await client.uidStore(seq, [keyword], action: imap.StoreAction.add);
await client.uidMove(seq, targetMailboxPath: dest);
final snoozeResult = await client.uidMove(seq, targetMailboxPath: dest);
await _remapEmailAfterImapMove(
client,
oldId: row.resourceId,
sourceUid: uid,
destMailboxPath: dest,
moveResult: snoozeResult,
);
case 'unsnooze':
final dest = payload['dest'] as String;
try {
@@ -2351,7 +2393,151 @@ class EmailRepositoryImpl implements EmailRepository {
);
}
}
final unsnoozeResult =
await client.uidMove(seq, targetMailboxPath: dest);
await _remapEmailAfterImapMove(
client,
oldId: row.resourceId,
sourceUid: uid,
destMailboxPath: dest,
moveResult: unsnoozeResult,
);
}
}
/// Rewrites the local row identity after an IMAP MOVE so the cache keeps
/// tracking the same physical message under its new (mailbox, UID).
///
/// The new UID is taken from the RFC 4315 `COPYUID` response code first
/// (every modern server advertises `UIDPLUS`). If that's missing we fall
/// back to `UID SEARCH HEADER Message-ID …` in the destination mailbox.
/// When neither yields a UID we leave the row in place; the next sync
/// cycle will re-fetch it as a new message and reconciliation will drop
/// the stale source-side row.
Future<void> _remapEmailAfterImapMove(
imap.ImapClient client, {
required String oldId,
required int sourceUid,
required String destMailboxPath,
required imap.GenericImapResult moveResult,
}) async {
final row = await (_db.select(_db.emails)..where((t) => t.id.equals(oldId)))
.getSingleOrNull();
if (row == null) return;
final newUid = _resolveCopyUid(moveResult, sourceUid) ??
await _searchUidByMessageId(
client,
destMailboxPath,
row.messageId,
);
if (newUid == null) {
log(
'_remapEmailAfterImapMove: could not resolve new UID for $oldId '
'after move to $destMailboxPath (no COPYUID, '
'messageId=${row.messageId}); row will be re-fetched on next sync',
);
return;
}
final newId = '${row.accountId}:$destMailboxPath:$newUid';
if (newId == oldId) return;
await _db.transaction(() async {
await _db.customStatement('PRAGMA defer_foreign_keys = ON');
await _db.customStatement(
'UPDATE email_bodies SET email_id = ?1 WHERE email_id = ?2',
[newId, oldId],
);
await (_db.update(_db.emails)..where((t) => t.id.equals(oldId))).write(
EmailsCompanion(
id: Value(newId),
uid: Value(newUid),
mailboxPath: Value(destMailboxPath),
),
);
await (_db.update(_db.pendingChanges)
..where((t) => t.resourceId.equals(oldId)))
.write(PendingChangesCompanion(resourceId: Value(newId)));
// threads.latest_email_id is a plain equality match; threads.email_ids_json
// is a JSON array of email IDs — both are safe to update via REPLACE()
// because email IDs are unique opaque strings.
await _db.customStatement(
'UPDATE threads SET latest_email_id = ?1 '
'WHERE latest_email_id = ?2',
[newId, oldId],
);
await _db.customStatement(
'UPDATE threads SET email_ids_json = '
'REPLACE(email_ids_json, ?1, ?2) '
'WHERE email_ids_json LIKE ?3',
['"$oldId"', '"$newId"', '%"$oldId"%'],
);
// UndoAction.toJson() embeds email IDs as quoted JSON strings in both
// emailIds and originalEmails[].id, so the same REPLACE() works.
await _db.customStatement(
'UPDATE undo_actions SET data_json = '
'REPLACE(data_json, ?1, ?2) '
'WHERE data_json LIKE ?3',
['"$oldId"', '"$newId"', '%"$oldId"%'],
);
});
// Rebuild thread aggregates in both mailboxes from the now-updated emails.
final threadId = row.threadId ?? newId;
await _updateThread(row.accountId, row.mailboxPath, threadId);
await _updateThread(row.accountId, destMailboxPath, threadId);
}
/// Extracts the destination UID for [sourceUid] from a MOVE/COPY result's
/// `COPYUID` response code (RFC 4315). Returns null when the server did not
/// advertise UIDPLUS or the response code is malformed.
int? _resolveCopyUid(imap.GenericImapResult result, int sourceUid) {
final code = result.responseCodeCopyUid;
if (code == null) return null;
try {
final sources = code.originalSequence?.toList();
final targets = code.targetSequence.toList();
if (sources == null) {
// Some servers omit the source set when only one message moved.
return targets.length == 1 ? targets.first : null;
}
final idx = sources.indexOf(sourceUid);
if (idx < 0 || idx >= targets.length) return null;
return targets[idx];
} catch (_) {
return null;
}
}
/// Looks up the UID of a message in [mailboxPath] by its RFC 2822
/// `Message-ID` header. Used as a fallback when the server doesn't
/// support UIDPLUS so we can still relink the local row after a move.
Future<int?> _searchUidByMessageId(
imap.ImapClient client,
String mailboxPath,
String? messageId,
) async {
if (messageId == null || messageId.isEmpty) return null;
try {
await client.selectMailboxByPath(mailboxPath);
// RFC 3501 SEARCH HEADER uses an astring for the value; quoting is safe
// for typical Message-ID syntax (no embedded quotes or backslashes).
final escaped = messageId.replaceAll(r'\', r'\\').replaceAll('"', r'\"');
final result = await client.uidSearchMessages(
searchCriteria: 'HEADER Message-ID "$escaped"',
);
final uids = result.matchingSequence?.toList() ?? const <int>[];
if (uids.isEmpty) return null;
return uids.reduce((a, b) => a > b ? a : b);
} catch (e) {
log('_searchUidByMessageId failed for $messageId in $mailboxPath: $e');
return null;
}
}
+1 -1
View File
@@ -680,7 +680,7 @@ packages:
source: hosted
version: "0.13.0"
meta:
dependency: transitive
dependency: "direct main"
description:
name: meta
sha256: "1741988757a65eb6b36abe716829688cf01910bbf91c34354ff7ec1c3de2b349"
+3
View File
@@ -67,6 +67,9 @@ dependencies:
share_plus: ^13.1.0
device_info_plus: ^13.1.0
# @visibleForTesting annotation used in lib/data/repositories.
meta: ^1.16.0
dev_dependencies:
flutter_test:
sdk: flutter
+236
View File
@@ -1109,6 +1109,242 @@ void main() {
expect(spy.movedToMailbox, 'Snoozed');
},
);
test(
'move flush remaps local id/uid from COPYUID and rewrites cached bodies',
() async {
final spy = SnoozeSpyImapClient(
copyUidValidity: 1,
copyUidSourceToTarget: const {5: 42},
);
final r = _makeRepos(imapConnect: (_, __, ___) async => spy);
await r.accounts.addAccount(_account, 'pw');
const oldId = 'acc-1:INBOX:5';
await r.db.into(r.db.emails).insert(
EmailsCompanion.insert(
id: oldId,
accountId: 'acc-1',
mailboxPath: 'Archive', // already optimistically moved
uid: 5,
receivedAt: DateTime(2024),
messageId: const Value('<msg-1@example.com>'),
threadId: const Value('thr-1'),
),
);
await r.db.into(r.db.emailBodies).insert(
EmailBodiesCompanion.insert(
emailId: oldId,
textBody: const Value('cached body'),
),
);
await r.db.into(r.db.pendingChanges).insert(
PendingChangesCompanion.insert(
accountId: 'acc-1',
resourceType: 'Email',
resourceId: oldId,
changeType: 'move',
payload: jsonEncode({
'uid': 5,
'mailboxPath': 'INBOX',
'dest': 'Archive',
}),
createdAt: DateTime.now(),
),
);
await r.emails.flushPendingChanges('acc-1', 'pw');
// Pending change drained.
expect(await r.db.select(r.db.pendingChanges).get(), isEmpty);
// Old id is gone; new id reflects destination mailbox + new UID.
expect(await r.emails.getEmail(oldId), isNull);
const newId = 'acc-1:Archive:42';
final moved = await r.emails.getEmail(newId);
expect(moved, isNotNull);
expect(moved!.uid, 42);
expect(moved.mailboxPath, 'Archive');
// Body cache follows the new id.
final bodies = await r.db.select(r.db.emailBodies).get();
expect(bodies, hasLength(1));
expect(bodies.first.emailId, newId);
expect(bodies.first.textBody, 'cached body');
},
);
test(
'move flush falls back to UID SEARCH HEADER Message-ID without UIDPLUS',
() async {
const messageId = '<msg-1@example.com>';
const criteria = 'HEADER Message-ID "$messageId"';
final spy = SnoozeSpyImapClient(
// No copyUidValidity → no COPYUID in the MOVE response.
searchResults: {
criteria: const [99]
},
);
final r = _makeRepos(imapConnect: (_, __, ___) async => spy);
await r.accounts.addAccount(_account, 'pw');
const oldId = 'acc-1:INBOX:5';
await r.db.into(r.db.emails).insert(
EmailsCompanion.insert(
id: oldId,
accountId: 'acc-1',
mailboxPath: 'Archive',
uid: 5,
receivedAt: DateTime(2024),
messageId: const Value(messageId),
),
);
await r.db.into(r.db.pendingChanges).insert(
PendingChangesCompanion.insert(
accountId: 'acc-1',
resourceType: 'Email',
resourceId: oldId,
changeType: 'move',
payload: jsonEncode({
'uid': 5,
'mailboxPath': 'INBOX',
'dest': 'Archive',
}),
createdAt: DateTime.now(),
),
);
await r.emails.flushPendingChanges('acc-1', 'pw');
expect(spy.lastSearchCriteria, criteria);
const newId = 'acc-1:Archive:99';
final moved = await r.emails.getEmail(newId);
expect(moved, isNotNull);
expect(moved!.uid, 99);
},
);
test(
'move flush rewrites pending undo_actions referencing the old id',
() async {
final spy = SnoozeSpyImapClient(
copyUidValidity: 1,
copyUidSourceToTarget: const {5: 42},
);
final r = _makeRepos(imapConnect: (_, __, ___) async => spy);
await r.accounts.addAccount(_account, 'pw');
const oldId = 'acc-1:INBOX:5';
await r.db.into(r.db.emails).insert(
EmailsCompanion.insert(
id: oldId,
accountId: 'acc-1',
mailboxPath: 'Archive',
uid: 5,
receivedAt: DateTime(2024),
),
);
await r.db.into(r.db.pendingChanges).insert(
PendingChangesCompanion.insert(
accountId: 'acc-1',
resourceType: 'Email',
resourceId: oldId,
changeType: 'move',
payload: jsonEncode({
'uid': 5,
'mailboxPath': 'INBOX',
'dest': 'Archive',
}),
createdAt: DateTime.now(),
),
);
// An undo entry created when the user did the move, referencing oldId
// in both emailIds and originalEmails[].id.
await r.db.into(r.db.undoActions).insert(
UndoActionsCompanion.insert(
id: 'undo-1',
accountId: 'acc-1',
dataJson: jsonEncode({
'id': 'undo-1',
'accountId': 'acc-1',
'type': 'move',
'emailIds': [oldId],
'sourceMailboxPath': 'INBOX',
'destinationMailboxPath': 'Archive',
'timestamp': DateTime(2024).toIso8601String(),
'originalEmails': [
{
'id': oldId,
'accountId': 'acc-1',
'mailboxPath': 'INBOX',
'uid': 5,
'receivedAt': DateTime(2024).toIso8601String(),
'from': [],
'to': [],
'cc': [],
'isSeen': false,
'isFlagged': false,
'hasAttachment': false,
},
],
}),
createdAt: DateTime(2024),
),
);
await r.emails.flushPendingChanges('acc-1', 'pw');
const newId = 'acc-1:Archive:42';
final stored = await r.db.select(r.db.undoActions).getSingle();
final json = jsonDecode(stored.dataJson) as Map<String, dynamic>;
expect(json['emailIds'], [newId]);
expect(
(json['originalEmails'] as List).first as Map<String, dynamic>,
containsPair('id', newId),
);
},
);
test(
'reconciliation skips rows with a pending move so they are not wiped',
() async {
final r = _makeRepos();
await r.accounts.addAccount(_account, 'pw');
const oldId = 'acc-1:INBOX:5';
await r.db.into(r.db.emails).insert(
EmailsCompanion.insert(
id: oldId,
accountId: 'acc-1',
mailboxPath: 'Archive', // optimistically moved
uid: 5,
receivedAt: DateTime(2024),
),
);
await r.db.into(r.db.pendingChanges).insert(
PendingChangesCompanion.insert(
accountId: 'acc-1',
resourceType: 'Email',
resourceId: oldId,
changeType: 'move',
payload: jsonEncode({
'uid': 5,
'mailboxPath': 'INBOX',
'dest': 'Archive',
}),
createdAt: DateTime.now(),
),
);
// Run the deletion-reconciliation pass with a destination snapshot
// that does NOT contain UID 5 — the row would be wiped without the
// in-flight guard.
await r.emails
.reconcileDeletedImapForTest('acc-1', 'Archive', const []);
expect(await r.emails.getEmail(oldId), isNotNull);
},
);
});
group('Snooze', () {
+43 -1
View File
@@ -19,9 +19,25 @@ class FakeImapClient extends imap.ImapClient {
/// Spy IMAP client that records snooze-related operations and succeeds silently.
class SnoozeSpyImapClient extends FakeImapClient {
SnoozeSpyImapClient({
this.copyUidValidity,
this.copyUidSourceToTarget = const {},
this.searchResults = const {},
});
String? selectedMailbox;
String? createdMailbox;
String? movedToMailbox;
String? lastSearchCriteria;
/// When non-null, `uidMove` returns a `COPYUID` response code built from
/// these mappings (sourceUid → destinationUid) for the moved sequence.
final int? copyUidValidity;
final Map<int, int> copyUidSourceToTarget;
/// Maps a `UID SEARCH HEADER Message-ID …` search criteria (the literal
/// IMAP atom incl. quotes) to the UIDs the fake should return.
final Map<String, List<int>> searchResults;
imap.Mailbox _fakeMailbox(String path) => imap.Mailbox(
encodedName: path,
@@ -63,7 +79,33 @@ class SnoozeSpyImapClient extends FakeImapClient {
String? targetMailboxPath,
}) async {
movedToMailbox = targetMailboxPath;
return imap.GenericImapResult();
final result = imap.GenericImapResult();
if (copyUidValidity != null && copyUidSourceToTarget.isNotEmpty) {
final sources = sequence.toList();
final mapped = sources
.where(copyUidSourceToTarget.containsKey)
.map((uid) => copyUidSourceToTarget[uid]!)
.toList();
if (mapped.isNotEmpty) {
final src = sources.join(',');
final dst = mapped.join(',');
result.responseCode = 'COPYUID $copyUidValidity $src $dst';
}
}
return result;
}
@override
Future<imap.SearchImapResult> uidSearchMessages({
String searchCriteria = 'UNSEEN',
List<imap.ReturnOption>? returnOptions,
Duration? responseTimeout,
}) async {
lastSearchCriteria = searchCriteria;
final hits = searchResults[searchCriteria] ?? const <int>[];
final result = imap.SearchImapResult()
..matchingSequence = imap.MessageSequence.fromIds(hits, isUid: true);
return result;
}
@override