[2.x] Add incremental mailbox synchronization - #185
Conversation
There was a problem hiding this comment.
🟡 Changes recommended
FETCH filtering and mailbox selection tracking can return incorrect data, while empty synchronization sets generate invalid commands.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
Adds incremental IMAP synchronization and modernizes the v2 connection API.
Changes:
- Adds CONDSTORE/QRESYNC synchronization and conditional STORE support.
- Preserves complete fetched attributes and introduces typed results.
- Updates authentication, command parameters, selection state, and tests.
File summaries
| File | Description |
|---|---|
| tests/Unit/Support/StrTest.php | Tests sequence parsing and literals. |
| tests/Unit/MessageTest.php | Updates message construction tests. |
| tests/Unit/MessageQueryTest.php | Tests lookup and APPEND behavior. |
| tests/Unit/MessageDataTest.php | Tests MODSEQ fetch items. |
| tests/Unit/IncrementalSyncTest.php | Covers incremental synchronization. |
| tests/Unit/FolderTest.php | Tests folder examination. |
| tests/Unit/FetchedMessageDataTest.php | Covers attribute preservation and caching. |
| tests/Unit/Connection/ImapConnectionTest.php | Updates connection API tests. |
| tests/Unit/Connection/ImapConnectionParametersTest.php | Tests command parameter handling. |
| tests/Unit/Connection/ImapConnectionOperationsTest.php | Tests message operations and results. |
| tests/Unit/Connection/ImapConnectionAuthenticationTest.php | Tests SASL authentication. |
| tests/Integration/FoldersTest.php | Updates STATUS expectations. |
| src/Vanished.php | Models VANISHED responses. |
| src/Testing/FakeMessageQuery.php | Adds fake incremental fetching. |
| src/Testing/FakeMessage.php | Adds fake modification sequences. |
| src/Testing/FakeMailbox.php | Supports selection results and ENABLE. |
| src/Testing/FakeFolder.php | Updates fake folder selection. |
| src/Support/Str.php | Adds literal-list and sequence parsing. |
| src/StoreResult.php | Models STORE results. |
| src/StoreModifier.php | Defines STORE modifiers. |
| src/Store/UnchangedSince.php | Adds UNCHANGEDSINCE. |
| src/SelectionResult.php | Models SELECT/EXAMINE metadata. |
| src/SelectionOption.php | Defines selection options. |
| src/Selection/QuickResync.php | Adds QRESYNC parameters. |
| src/Selection/CondStore.php | Adds CONDSTORE selection. |
| src/MessageQueryInterface.php | Extends query synchronization API. |
| src/MessageQuery.php | Implements changed-message fetching. |
| src/MessageInterface.php | Exposes modification sequences. |
| src/MessageData/Attribute.php | Adds MODSEQ attribute. |
| src/MessageData.php | Adds MODSEQ factory. |
| src/Message.php | Stores and merges fetched attributes. |
| src/MailboxInterface.php | Extends mailbox selection API. |
| src/Mailbox.php | Tracks selection and enabled capabilities. |
| src/FolderRepository.php | Filters extended LIST responses. |
| src/FolderInterface.php | Returns typed selection results. |
| src/Folder.php | Updates selection and quota handling. |
| src/FileMessage.php | Implements modification sequence accessor. |
| src/FetchResult.php | Models FETCH results. |
| src/FetchModifier.php | Defines FETCH modifiers. |
| src/FetchedMessageData.php | Preserves arbitrary fetched attributes. |
| src/Fetch/ChangedSince.php | Adds CHANGEDSINCE/VANISHED. |
| src/Enums/ImapIdentifier.php | Generalizes message identifiers. |
| src/Connection/ImapTokenizer.php | Parses binary literals. |
| src/Connection/ImapConnection.php | Implements revised IMAP operations. |
| src/Connection/ConnectionInterface.php | Defines the v2 connection API. |
| src/Authenticator.php | Defines SASL authenticators. |
| src/Authentication/XOAuth2.php | Implements XOAUTH2 authentication. |
Review details
- Files reviewed: 47/47 changed files
- Comments generated: 4
- Review effort level: Balanced
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
There was a problem hiding this comment.
🟡 Changes recommended
Synchronization state, unsolicited response filtering, capability caching, and 64-bit checkpoint handling remain incorrect.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review details
Suppressed comments (4)
Previously missed (3) — in code that hasn't changed since the last review.
src/Mailbox.php:83
- The clone still inherits the cached server capabilities even though it opens a new connection. Capability sets can differ after reconnecting (for example across failover nodes or authentication state), so the clone may skip required checks or attempt unsupported QRESYNC/CONDSTORE operations. Clear
$capabilitieswith the other connection-specific fields.
This issue also appears on line 194 of the same file.
src/SelectionResult.php:62
- IMAP modification sequences are unsigned 64-bit values, but casting
HIGHESTMODSEQto PHP's signedintcorrupts valid checkpoints above9223372036854775807(for example,18446744073709551615saturates to PHP_INT_MAX). Such checkpoints cannot round-trip into CHANGEDSINCE/UNCHANGEDSINCE. Preserve them as decimal strings consistently across selection results, fetched data, query methods, and modifiers.
src/StoreResult.php:31 - Every untagged FETCH response is exposed as a successfully changed message, including unsolicited updates for messages outside the STORE set. This makes
messages()contradict its contract and can cause callers to apply unrelated state. Pass the command's set/addressing mode into result parsing and filter these entries, leaving unsolicited responses available throughresponses().
src/Mailbox.php:197
- Disconnecting clears the enabled and selection caches but leaves
$capabilitiescached for the next connection. A subsequent connection can therefore make capability decisions from the previous session, despite capabilities being connection/server-state dependent. Reset the capability cache here as well.
$this->connection = null;
$this->selected = null;
$this->selection = null;
$this->enabled = [];
- Files reviewed: 49/49 changed files
- Comments generated: 2
- Review effort level: Balanced
|
Addressed the latest review in 0844b7b, including the pending password-aware reconnect and explicit login/xoauth2 configuration changes from our discussion. The additional findings in the review summary were checked as well:
QRESYNC is now checked as an enabled session capability, not merely an advertised one. ENABLE must happen before folder selection; issuing it inside an already-selected query would violate the protocol. Verified: 625 tests pass, including the live-server suite, and formatting is clean. The separate Laravel adapter remains unchanged; its plain default will need to become login when adopting this v2 configuration. |
This adds the IMAP primitives needed to keep a local mailbox in sync: fetching changes since a saved checkpoint, discovering deleted messages, and updating flags without overwriting changes made by another client.
Incremental synchronization
Applications remain responsible for storing checkpoints, handling UIDVALIDITY changes, and falling back when the server does not support these extensions.
Extensible fetched data
FetchedMessageData now retains the returned attributes instead of copying a fixed list into constructor properties. It provides has(), get(), and immutable merge() alongside the existing typed accessors.
This preserves arbitrary body sections, partial offsets, extension attributes, nested lists, and explicit NIL values. It also distinguishes an attribute that was not fetched from one that was returned empty.
Message retains the complete data container, including through serialization. Lazy fetches merge into the existing data, and previously fetched complete body parts can be reused when peeking.
Command parameter fixes
V2 API changes
For example:
Before applying these changes, the application should compare the returned UIDVALIDITY with its saved value. It should only save the new checkpoint after successfully applying the synchronization results.
To fetch changes directly on the connection:
CHANGEDSINCE requires CONDSTORE support (also provided by QRESYNC). Requesting VANISHED additionally requires UID FETCH and QRESYNC to be enabled on the connection. New modifiers can implement FetchModifier without adding another fetch method.
Conditional flag updates follow the same pattern:
The set-first connection API follows the same ordering across message operations:
Higher-level message and query APIs keep their existing argument order. Extension options still require the corresponding server capabilities.