Skip to content

feat(imap): add folder listing and filtering for imap sync - #502

Merged
wesm merged 3 commits into
kenn-io:mainfrom
mattf:feat/imap-folder-filter
Jul 31, 2026
Merged

feat(imap): add folder listing and filtering for imap sync#502
wesm merged 3 commits into
kenn-io:mainfrom
mattf:feat/imap-folder-filter

Conversation

@mattf

@mattf mattf commented Jul 24, 2026

Copy link
Copy Markdown
Contributor

What changed

  • Adds msgvault list-folders [account] to show selectable IMAP folders and approximate message counts. Without an account name, it lists folders for every configured IMAP account.
  • Adds repeatable --folders and --skip-folders options to both sync-full and sync. Each flag takes one complete folder name, so names containing commas work correctly.
  • Supports the same folder filters when the CLI talks to a local daemon or a configured remote server.
  • Keeps filtered scans narrow, including on servers with Gmail-style All Mail, Trash, and Junk folders.
  • Preserves stable message identities during filtered or incomplete rescans. These scans add newly observed folder labels without removing labels learned earlier. An unfiltered scan replaces stale labels only when every mailbox was collected successfully.
  • Refreshes labels for messages that are already archived by fetching only lightweight Message-ID headers. Raw message bodies and attachments are downloaded only for new messages.
  • Handles metadata failures per message, so a UID that disappears after listing does not prevent other messages in the batch from updating.
  • Adds a Zensical guide, setup examples, and complete CLI reference entries for folder discovery, matching rules, and safe filtered syncs.

Why

A first sync can take a long time on a large IMAP account. Folder filtering lets someone start with a useful subset, confirm that msgvault fits their needs, and expand the archive later. It also lets ongoing syncs ignore folders that are not useful to the local archive.

How to use it

First, find the folder names reported by the server:

msgvault list-folders you@example.com

Sync only selected folders by repeating --folders:

msgvault sync-full you@example.com \
  --folders INBOX \
  --folders Archive

Or scan everything except selected folders by repeating --skip-folders:

msgvault sync you@example.com \
  --skip-folders Trash \
  --skip-folders Spam

Folder names use exact, case-insensitive matching. When include and exclude flags are combined, msgvault applies the include list first and then removes excluded folders. The flags affect IMAP accounts only and apply to the current command. With no folder flags, msgvault scans every selectable folder.

Filtering changes which remote folders are scanned; it does not delete messages from the provider or remove messages already stored in the local archive.

Closes #196.

@roborev-ci

roborev-ci Bot commented Jul 24, 2026

Copy link
Copy Markdown

roborev: Combined Review (c97a966)

The PR has four medium-severity correctness and portability issues in IMAP folder selection, error handling, and catalog counts.

Medium

  • internal/imap/client.go:547 — Gmail optimization bypasses folder filters. Gmail detection replaces the filtered mailbox list with All Mail, Trash, and Junk. As a result, --folders=INBOX can sync the entire account and excluded Trash/Junk folders are reintroduced.

    • Fix: Derive the Gmail optimization from the effective filtered mailbox set, or disable it when folder filters are active and enumerate only selected folders with deduplication.
  • internal/imap/client.go:531 — CLI folder selection has incorrect precedence and exclusions. CLI includes are applied after configured folders have already narrowed the list, so they cannot override configuration. Additionally, --skip-folders is ignored whenever --folders is non-empty.

    • Fix: Select one effective include list—CLI includes when provided, otherwise configured folders—then call filterMailboxes once against the original LIST results with both the effective includes and CLI exclusions.
  • internal/store/messages.go:1517 — Catalog counts use the canonical source mailbox instead of actual folder membership. Providers using All Mail as the canonical mailbox can incorrectly report zero cataloged messages for Inbox and other labeled folders. The query also relies on SQLite-specific INSTR, preventing counts on PostgreSQL.

    • Fix: Use portable SQL to count live messages by joining message_labels and labels, scoped to the source.
  • internal/imap/client.go:762 — IMAP LIST errors are silently reported as empty success. A failed LIST command becomes a nil mailbox slice, causing ListMailboxes and list-folders to return successful empty results instead of the server error.

    • Fix: Return the LIST error from the helper and propagate it through ListMailboxes.

Reviewers: 2 done | Synthesis: codex, 14s | Total: 6m32s

@mattf
mattf force-pushed the feat/imap-folder-filter branch from 5673786 to 719d445 Compare July 24, 2026 14:53
@roborev-ci

roborev-ci Bot commented Jul 24, 2026

Copy link
Copy Markdown

roborev: Combined Review (20b05cc)

The PR needs a medium-severity fix: Gmail folder filters can be bypassed by the \All optimization.

Medium

  • internal/imap/client.go:545 — Gmail’s \All optimization replaces the filtered mailbox list with All Mail, Trash, and Junk. Because these special mailboxes are discovered before filtering, --folders and --skip-folders are effectively ignored for Gmail IMAP accounts.

    Suggested fix: Disable the optimization when filters are active, or intersect the special mailboxes with the filtered list. Add coverage with [Gmail]/All Mail configured as the \All mailbox.


Reviewers: 2 done | Synthesis: codex, 6s | Total: 5m46s

@mattf
mattf force-pushed the feat/imap-folder-filter branch from 20b05cc to 29d1801 Compare July 24, 2026 15:32
@mattf

mattf commented Jul 24, 2026

Copy link
Copy Markdown
Contributor Author

please excuse the force-pushing. i messed up the history and decided to just squash everything.

i'm not using gmail (or postgres, oops), so how would you like me to resolve the \All feedback?

@roborev-ci

roborev-ci Bot commented Jul 24, 2026

Copy link
Copy Markdown

roborev: Combined Review (29d1801)

Medium-severity issue found: Gmail’s All Mail optimization can bypass configured folder filters.

Medium

  • Folder filtering bypassed for Gmail IMAP serversinternal/imap/client.go:540
    After allMailboxes is filtered, the Gmail \All optimization replaces listMailboxes with unfiltered All Mail, Trash, and Junk mailboxes. Excluded folders may therefore still be synced, and an include filter such as INBOX can import nearly the entire account.

    Fix: Disable or constrain the Gmail All Mail optimization whenever folder filters are active, ensuring canonical enumeration remains limited to the filtered mailbox set. Add a test with allMailFolder set to [Gmail]/All Mail.


Reviewers: 2 done | Synthesis: codex, 7s | Total: 4m26s

@roborev-ci

roborev-ci Bot commented Jul 24, 2026

Copy link
Copy Markdown

roborev: Combined Review (29d1801)

The IMAP folder-filtering changes have two medium-severity correctness and interface issues that should be addressed before merging.

Medium

  • Gmail canonical mailbox selection bypasses folder filtersinternal/imap/client.go:551
    Gmail-style servers replace the filtered mailbox list with All Mail, Trash, and Junk. Consequently, --folders INBOX may sync every message, while --skip-folders Trash may still enumerate Trash. Preserve the effective filter when selecting canonical Gmail mailboxes, and add coverage for a server advertising \All, \Trash, and \Junk.

  • Folder flags are unavailable on the normal sync commandcmd/msgvault/cmd/syncfull.go:829
    IMAP accounts invoked through sync use the same folder-based full-sync path, and list-folders help directs users to these flags on sync, yet msgvault sync --folders ... is rejected. Register and propagate the folder options through sync and its HTTP endpoint, or explicitly limit and document the feature as sync-full only.


Reviewers: 2 done | Synthesis: codex, 8s | Total: 2m39s

@mariusvniekerk

Copy link
Copy Markdown
Contributor

@mattf force pushing is very normal in this agent era. rebases are now basically free. huzzah!

@wesm

wesm commented Jul 25, 2026

Copy link
Copy Markdown
Member

taking a look

@roborev-ci

roborev-ci Bot commented Jul 25, 2026

Copy link
Copy Markdown

roborev: Combined Review (4060875)

The changes are generally sound, but mailbox names containing commas can be misparsed during API sync.

Medium

  • cmd/msgvault/cmd/serve.go:895 — Repeatable API folder values are collapsed into a comma-separated flag and later split on commas. A valid mailbox name containing a comma is silently treated as multiple folders, potentially syncing none of that mailbox’s messages.
    • Fix: Preserve values with repeatable string-array CLI flags end-to-end, or use an escaping format that supports arbitrary mailbox names.

Reviewers: 2 done | Synthesis: codex, 6s | Total: 7m18s

@roborev-ci

roborev-ci Bot commented Jul 26, 2026

Copy link
Copy Markdown

roborev: Combined Review (63f320b)

Code changes need revision due to one medium-severity folder-handling issue.

Medium

  • Comma-containing IMAP folder names are corruptedcmd/msgvault/cmd/serve.go:969, cmd/msgvault/cmd/syncfull.go:553
    Folder arrays are joined and re-split using commas, so valid mailbox names containing commas may select or exclude unintended folders. Preserve folder names as distinct repeatable arguments, such as Cobra string-array flags, instead of CSV encoding.

Reviewers: 2 done | Synthesis: codex, 10s | Total: 8m56s

@mattf

mattf commented Jul 26, 2026

Copy link
Copy Markdown
Contributor Author

i went with the repeated arg approach after trying an escaping approach w/ \,. i think the escaping was going to be too complicated for users as they'd contend with both shell & msgvault escaping.

granted, a , in a folder name is likely a rare event.

@roborev-ci

roborev-ci Bot commented Jul 26, 2026

Copy link
Copy Markdown

roborev: Combined Review (28513e1)

Medium-severity correctness issue found in filtered IMAP rescans; no security issues identified.

Medium

  • internal/imap/client.go:540 — Filtering allMailboxes before label-map construction and cross-mailbox deduplication makes a partial folder view appear authoritative. On accounts with an \All mailbox, a filtered rescan excluding All Mail can rewrite an existing message’s stable source_message_id to a selected folder’s UID and replace previously known labels with only the filtered labels.

    Recommended fix: When folder filtering is active, preserve canonical IDs and merge existing labels, or retain an unfiltered mailbox view for deduplication metadata. Add a regression test covering a filtered rescan of an already-synced message.


Reviewers: 2 done | Synthesis: codex, 9s | Total: 7m46s

@roborev-ci

roborev-ci Bot commented Jul 27, 2026

Copy link
Copy Markdown

roborev: Combined Review (88a2932)

No issues found.


Reviewers: 2 done | Synthesis: codex | Total: 12m13s

@wesm
wesm marked this pull request as ready for review July 27, 2026 11:05
@roborev-ci

roborev-ci Bot commented Jul 27, 2026

Copy link
Copy Markdown

roborev: Combined Review (6abf45e)

Code is generally solid, but one medium-severity terminal injection issue should be fixed.

Medium

  • Terminal escape injection through IMAP mailbox namescmd/msgvault/cmd/listfolders.go:143

    The list-folders command prints server-provided mailbox names directly to the terminal. A malicious mailbox name containing ANSI/OSC control sequences could forge output, alter terminal state, or manipulate clipboard contents. This also remains unsanitized when streamed through the daemon.

    Sanitize f.Mailbox with the existing textutil.SanitizeTerminal helper before formatting it.


Reviewers: 2 done | Synthesis: codex, 8s | Total: 8m46s

@roborev-ci

roborev-ci Bot commented Jul 27, 2026

Copy link
Copy Markdown

roborev: Combined Review (821990a)

Changes need a fix for partial-snapshot label merging.

Medium

  • internal/sync/sync.go:764 — Partial-snapshot label merging occurs only during RFC822 deduplication. Messages already archived under the same composite ID are filtered out by processBatch before reaching this branch, so a filtered Gmail-style IMAP rescan through All Mail cannot add newly discovered folder labels.
    • Fix: Merge labels for exact-ID matches when snapshots are incomplete. Add a regression test where a filtered rescan returns an existing composite ID with an additional label.

Reviewers: 2 done | Synthesis: codex, 7s | Total: 9m5s

@wesm

wesm commented Jul 27, 2026

Copy link
Copy Markdown
Member

I can take over from here

@wesm
wesm force-pushed the feat/imap-folder-filter branch from 821990a to 841e9d8 Compare July 27, 2026 20:05
@roborev-ci

roborev-ci Bot commented Jul 27, 2026

Copy link
Copy Markdown

roborev: Combined Review (841e9d8)

The changes introduce two medium-severity sync correctness and efficiency issues.

Medium

  • internal/sync/sync.go:208 — Complete snapshots acknowledge existing exact IDs without refreshing their labels. Labels merged during a filtered scan can remain permanently stale after remote membership changes, even after an unfiltered forced rescan, if the canonical IMAP ID remains stable. For complete snapshots, replace exact-ID label associations using authoritative label metadata and add a regression test covering filtered label addition, remote removal, and an unfiltered rescan.

  • internal/sync/sync.go:212 — Existing messages in incomplete snapshots are added to fetchIDs, and the IMAP batch fetch requests BODY.PEEK[]. On servers with an \All mailbox, a folder change can redownload every selected message—including attachments—only to merge labels. Add a label-only metadata path for existing IDs and reserve raw MIME fetching for new messages.


Reviewers: 2 done | Synthesis: codex, 10s | Total: 7m16s

@wesm
wesm force-pushed the feat/imap-folder-filter branch from 841e9d8 to 7504da5 Compare July 27, 2026 20:23
@roborev-ci

roborev-ci Bot commented Jul 27, 2026

Copy link
Copy Markdown

roborev: Combined Review (7504da5)

One-line verdict: Two medium-severity IMAP synchronization issues could cause label loss or unnecessarily abort batch processing.

Medium

  • Incomplete label snapshots can remove valid labelsinternal/imap/client.go:134
    LabelsSnapshotComplete considers only configured filters and ignores labelMapComplete. If mailbox membership collection fails for any mailbox, the snapshot is still treated as authoritative, so ReplaceMessageLabels may remove labels missing only because of that failure. Track label-map/enumeration completeness on the client and return false when collection is incomplete, causing labels to be merged instead of replaced.

  • A vanished UID aborts the entire metadata batchinternal/imap/batch_fetch.go:405
    If an existing UID is expunged between listing and metadata fetching, the resulting error aborts the whole label batch and marks every message in it as failed. Preserve per-message missing/fetch errors, as the raw-message batch path does, so the syncer can skip or individually record vanished messages while continuing with the remaining results.


Reviewers: 2 done | Synthesis: codex, 8s | Total: 5m5s

@mattf

mattf commented Jul 27, 2026

Copy link
Copy Markdown
Contributor Author

I can take over from here

👍

@wesm
wesm force-pushed the feat/imap-folder-filter branch from 7504da5 to cec0d51 Compare July 28, 2026 01:13
@roborev-ci

roborev-ci Bot commented Jul 28, 2026

Copy link
Copy Markdown

roborev: Combined Review (cec0d51)

Medium-severity label snapshot completeness bugs could cause existing folder memberships to be removed during IMAP rescans.

Medium

  • internal/imap/client.go:614 — Servers that do not advertise a \All mailbox never build msgIDToLabels, yet the snapshot is marked complete. Consequently, GetMessageLabelsBatch returns only the current mailbox, and ReplaceMessageLabels may discard other folder memberships previously accumulated for the same message.

    • Fix: Build the membership map for these servers, or mark their snapshots incomplete and merge labels instead of replacing them.
  • internal/imap/client.go:139LabelsSnapshotComplete ignores active --after and --before filters even though those filters constrain membership collection. A date-bounded rescan can therefore treat a partial label view as authoritative and remove labels from existing messages.

    • Fix: Require since and before to be zero before declaring the snapshot complete, and add a regression test for a date-filtered rescan.

Reviewers: 2 done | Synthesis: codex, 12s | Total: 8m9s

@roborev-ci

roborev-ci Bot commented Jul 28, 2026

Copy link
Copy Markdown

roborev: Combined Review (51b9231)

One medium-severity correctness issue was found; no security vulnerabilities were identified.

Medium

  • internal/imap/batch_fetch.go:365 — Label-only fetches bypass cross-mailbox deduplication. The label-only path does not seed seenRFC822IDs. On non-Gmail servers where \All overlaps another mailbox, an existing All Mail|UID is refreshed, then fetched again under an alternate composite ID. The duplicate can overwrite the canonical source ID and replace complete labels with only the alternate mailbox, causing IDs and labels to oscillate across syncs.

    Suggested fix: Record successful \All label fetches in seenRFC822IDs (or otherwise preserve deduplication ordering across label-only and raw fetches), and add a rescan test where one RFC822 message appears in both \All and another mailbox.


Reviewers: 2 done | Synthesis: codex, 8s | Total: 5m43s

@roborev-ci

roborev-ci Bot commented Jul 29, 2026

Copy link
Copy Markdown

roborev: Combined Review (64be577)

The change has one medium-severity correctness issue; no security vulnerabilities were identified.

Medium

  • Incomplete reconciliation on servers without \Allinternal/imap/client.go:620, internal/sync/sync.go:861

    An unfiltered sync-full --noresume is always treated as incomplete when the server lacks a \All mailbox. If a message moves from INBOX|42 to Trash|99, deduplication adds Trash but does not remove INBOX or update the composite ID. The stored ID may reference a vanished UID, breaking later IMAP trash/delete operations.

    Suggested fix: Treat fully enumerated, unfiltered scans as authoritative by building a complete Message-ID membership map before ingestion or reconciling memberships and composite IDs afterward. Keep filtered and high-water incremental scans additive.


Reviewers: 2 done | Synthesis: codex, 9s | Total: 9m20s

@roborev-ci

roborev-ci Bot commented Jul 29, 2026

Copy link
Copy Markdown

roborev: Combined Review (e89b405)

Code review found two medium-severity sync reconciliation issues; no security issues were identified.

Medium

  • Stale labels and source IDs in cached viewsinternal/sync/sync.go:262, internal/sync/sync.go:870
    Successful label and canonical-ID reconciliation does not increment checkpoint.MessagesUpdated. Because cache invalidation relies on this counter, rescans that only modify existing messages can leave Parquet-backed TUI and web views stale indefinitely. Track reconciliation changes in batchResult, increment MessagesUpdated, and persist the count so affected messages are rewritten during cache rebuilding.

  • Invalid composite IDs retained after mailbox movesinternal/sync/sync.go:864
    Every incomplete snapshot preserves the old composite ID, including normal high-water scans. When a message moves between mailboxes, the old mailbox|uid may no longer exist; sync adds the new label but retains the vanished UID, causing later fetch or deletion operations to target the wrong remote location. Distinguish explicitly filtered snapshots from high-water scans, and update or validate the canonical composite ID when the stored copy no longer exists while continuing to merge labels conservatively.


Reviewers: 2 done | Synthesis: codex, 9s | Total: 8m35s

@roborev-ci

roborev-ci Bot commented Jul 29, 2026

Copy link
Copy Markdown

roborev: Combined Review (58dc188)

One medium-severity correctness issue remains.

Medium

  • Cross-mailbox deduplication can permanently omit a mailbox labelinternal/imap/client.go:644
    When buildLabelMap returns an incomplete membership map, cross-mailbox deduplication remains active. If a mailbox’s header fetch fails but its later raw fetch succeeds, an overlapping message can become an acknowledged dedup stub before its mailbox label is recorded. This may advance the folder high-water mark and prevent the missing label from being retried.

    Suggested fix: Enable seenRFC822IDs only when the membership map is complete, or ensure copies deduplicated under an incomplete map reconcile their current mailbox label without prematurely advancing folder state.


Reviewers: 2 done | Synthesis: codex, 8s | Total: 12m22s

@roborev-ci

roborev-ci Bot commented Jul 29, 2026

Copy link
Copy Markdown

roborev: Combined Review (e895b8f)

Medium-severity reconciliation bug found; no Critical or High issues identified.

Medium

  • internal/imap/batch_fetch.go:516SourceMessageExists treats a missing or renamed mailbox as an inconclusive SELECT error. During high-water reconciliation, this permanently prevents replacement of the stale canonical ID, leaves the new mailbox copy unacknowledged, and causes every subsequent sync to retry the same failure.
    • Fix: If the mailbox is absent from the current unfiltered LIST cache, return (false, nil). Preserve retryable errors only when a currently listed mailbox cannot be selected.

Reviewers: 2 done | Synthesis: codex, 8s | Total: 14m1s

@roborev-ci

roborev-ci Bot commented Jul 29, 2026

Copy link
Copy Markdown

roborev: Combined Review (ebca96a)

Code review found two medium-severity reconciliation risks; no material security regressions were identified.

Medium

  • internal/sync/sync.go:906 — A filtered scan preserves the old composite ID for a deduplicated message while acknowledging the newly observed ID and advancing that folder’s high-water state. After a move such as INBOX|1Trash|1, a later unfiltered scan may skip both unchanged folders, leaving the source ID permanently stale.

    • Fix: Do not advance the filtered folder’s state for unresolved alternate IDs, or persist a reconciliation marker that forces validation during the next unfiltered scan.
  • internal/imap/batch_fetch.go:506SourceMessageExists treats any successful fetch for mailbox|uid as proof that the original message still exists. Following a UIDVALIDITY change, that UID may identify an unrelated message, preserving a stale ID and potentially directing later IMAP operations at the wrong message.

    • Fix: Treat a UIDVALIDITY mismatch as definitive invalidation and/or compare the fetched RFC822 Message-ID with the expected stored Message-ID.

Reviewers: 2 done | Synthesis: codex, 11s | Total: 10m29s

@roborev-ci

roborev-ci Bot commented Jul 29, 2026

Copy link
Copy Markdown

roborev: Combined Review (76edc65)

The changes are generally sound, but one medium-severity sync correctness issue remains.

Medium

  • internal/sync/sync.go:222 — Existing composite IDs are trusted without validating the fetched RFC822MessageID. After an IMAP UIDVALIDITY change, a reused mailbox|uid may refer to an unrelated message; the message is classified as existing, receives only a label refresh, and is never archived.
    • Suggested fix: Compare exact-ID metadata with the stored RFC822 Message-ID, particularly across invalidated UID epochs. Preserve or rekey the previously archived row before ingesting the newly assigned message.

Reviewers: 2 done | Synthesis: codex, 10s | Total: 10m5s

@roborev-ci

roborev-ci Bot commented Jul 30, 2026

Copy link
Copy Markdown

roborev: Combined Review (cab7a6b)

High-severity correctness issue: UID reuse can suppress message archival; statistics can also report negative skipped counts.

High

  • internal/imap/batch_fetch.go:375 — Label-only fetches seed seenRFC822IDs before validating stored and fetched identities. If a UID is reused or UIDVALIDITY changes, the archived row is invalidated and a raw refetch is requested, but that refetch becomes a dedup stub because its Message-ID was already seeded. The replacement message remains unarchived even though folder state may advance, causing later syncs to skip it.
    • Fix: Seed dedup state only after identity validation succeeds, or make identity-mismatch refetches remove/bypass the premature dedup entry. Add coverage for UID reuse during a complete/--noresume scan.

Medium

  • internal/sync/sync.go:345 — UID reuse counts one listed item as both updated when the old row is invalidated and added when its replacement is archived. Consequently, MessagesSkipped = processed - added - updated can produce -1 for a one-message replacement and persist inconsistent statistics.
    • Fix: Count replacements in one mutually exclusive category, or track skipped items directly instead of deriving them from overlapping mutation counts.

Reviewers: 2 done | Synthesis: codex, 12s | Total: 6m50s

@roborev-ci

roborev-ci Bot commented Jul 30, 2026

Copy link
Copy Markdown

roborev: Combined Review (69e1ded)

Medium-severity identity validation flaw found; no High or Critical issues.

Medium

  • internal/imap/batch_fetch.go:591 — Exact-ID validation treats two missing RFC822 Message-ID values as an identity match. With --noresume, saved folder states are omitted, so a recreated mailbox can reuse a UID for a different message lacking a Message-ID; the replacement may be acknowledged without fetching its raw MIME and never archived.
    • Fix: Retain prior UIDVALIDITY metadata when forcing enumeration. Treat missing identities as inconclusive unless another stable check, such as raw-content comparison, confirms the match. Add coverage for UID reuse when both messages omit Message-ID.

Reviewers: 2 done | Synthesis: codex, 10s | Total: 10m47s

@roborev-ci

roborev-ci Bot commented Jul 30, 2026

Copy link
Copy Markdown

roborev: Combined Review (8fa4402)

The changes are generally sound with no security regressions identified, but two medium-severity correctness issues should be addressed.

Medium

  • internal/imap/batch_fetch.go:607 — Missing Message-ID can trigger repeated false UID-reuse detection. A Message-ID present on only one side is treated as conclusive evidence of UID reuse. Archived messages may lack this metadata after MIME parsing falls back to a placeholder, while the header-only parser still extracts it. Each rescan can then invalidate the live source ID and archive the same raw message again.

    • Fix: Treat either missing identity as inconclusive unless UIDVALIDITY or absence proves replacement. Fall back to the existing raw-MIME comparison and optionally backfill the Message-ID.
  • internal/sync/sync.go:1077 — Complete-snapshot dedup can unnecessarily replace a valid canonical source ID. If an overlapping mailbox copy appears on an earlier 100-message page than the archived exact ID, the exact ID has not yet seeded dedup, so the existing composite source ID is rekeyed even though it remains valid.

    • Fix: Verify whether the existing source ID still resolves to the same message before rekeying, including for complete snapshots. Preserve it and merge labels when valid, and add a cross-page overlap test.

Reviewers: 2 done | Synthesis: codex, 11s | Total: 12m1s

@wesm
wesm force-pushed the feat/imap-folder-filter branch from 8fa4402 to 8c60e35 Compare July 31, 2026 01:23
@roborev-ci

roborev-ci Bot commented Jul 31, 2026

Copy link
Copy Markdown

roborev: Combined Review (8c60e35)

The change is generally sound, but filtered syncs can incorrectly access excluded mailboxes during deduplication.

Medium

  • internal/sync/sync.go:1088 — Filtered syncs still validate deduplicated messages by fetching their previously stored mailbox, even when that mailbox is excluded. This can access deliberately skipped folders, fail messages when excluded mailboxes are inaccessible, and add an unnecessary FETCH per overlapping message.
    • Fix: Make source validation aware of the effective folder selection. When the stored mailbox is outside the current snapshot, preserve the canonical ID and merge labels, deferring validation until that mailbox is included.

Reviewers: 2 done | Synthesis: codex, 9s | Total: 13m6s

Add mailbox discovery and include/exclude filtering across the CLI, API, daemon client, documentation, and generated schemas.

- distinguish authoritative membership snapshots from filtered and high-water scans
- retain UIDVALIDITY evidence while forcing full enumeration
- preserve canonical mailbox UIDs across moves, renames, overlaps, and page boundaries
- compare raw MIME when Message-ID metadata cannot establish identity
- avoid accessing excluded canonical mailboxes during filtered deduplication
- leave inconclusive alternate IDs retryable for a later inclusive scan
- track reconciliation mutations for cache rebuilding and stable sync statistics
- cover complete, partial, filtered, UID-reuse, and cross-page rescan behavior

Co-authored-by: Wes McKinney <wesmckinn+git@gmail.com>
Generated with Codex
Co-authored-by: Codex <noreply@openai.com>
@wesm
wesm force-pushed the feat/imap-folder-filter branch from 8c60e35 to 5a955f2 Compare July 31, 2026 13:11
@roborev-ci

roborev-ci Bot commented Jul 31, 2026

Copy link
Copy Markdown

roborev: Combined Review (5a955f2)

Code review found two medium-severity IMAP synchronization issues; no security regressions were identified.

Medium

  • internal/sync/sync.go:556 — Deferred messages can be permanently skipped

    Deferred IMAP identities remain unacknowledged but do not increase the error count. runFullSync then saves every observed folder state, potentially advancing UIDNEXT beyond the deferred message. A later unfiltered sync may skip both folders, preventing identity reconciliation.

    Fix: Exclude folders containing pending unacknowledged messages from ObservedFolderStates, or otherwise prevent their states from being persisted during the final bulk save.

  • internal/sync/sync.go:1125 — Canonical \All source IDs are never adopted

    Reconciliation preserves an existing non-\All source ID whenever it still resolves. If an account is first synced with --folders INBOX and later without filters, the canonical \All identity is not adopted. Subsequent unfiltered syncs may therefore miss the exact-ID fast path and repeatedly download raw MIME when the account changes.

    Fix: During a complete snapshot, allow the IMAP client to identify the preferred canonical source ID and rekey valid non-\All IDs when their \All counterpart is observed.


Reviewers: 2 done | Synthesis: codex, 10s | Total: 16m41s

Deferred source identities must remain retryable until their exact mailbox can be validated, so folder high-water state cannot advance past them. Complete snapshots also need to converge older filtered-sync identities onto the mailbox advertised with \All, which keeps later rescans on the canonical fast path.

Generated with Codex
Co-authored-by: Codex <noreply@openai.com>
@roborev-ci

roborev-ci Bot commented Jul 31, 2026

Copy link
Copy Markdown

roborev: Combined Review (9ad43f5)

No issues found.


Reviewers: 2 done | Synthesis: codex | Total: 11m11s

@wesm

wesm commented Jul 31, 2026

Copy link
Copy Markdown
Member

Will merge pending CI. If I broke something in all my grinding please open another PR!

The IMAP regression coverage introduced by this branch crossed the repository's local testify-helper threshold and exposed additional static-analysis findings once that prerequisite passed. Keep the tests aligned with the enforced helper, resource-cleanup, and modern Go conventions so the CI lint gate can exercise the implementation.

Generated with Codex
Co-authored-by: Codex <noreply@openai.com>
@mattf

mattf commented Jul 31, 2026

Copy link
Copy Markdown
Contributor Author

@wesm that was a pretty epic back and forth

@roborev-ci

roborev-ci Bot commented Jul 31, 2026

Copy link
Copy Markdown

roborev: Combined Review (704fffa)

No Medium, High, or Critical findings; the reviewed changes are clean.


Reviewers: 2 done | Synthesis: codex, 7s | Total: 16m59s

@wesm
wesm merged commit 8f96df1 into kenn-io:main Jul 31, 2026
18 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

IMAP: add --folders flag for selective folder sync

3 participants