fix: do not advance HighestModSeq after failed CONDSTORE search #17

Closed

heiko wants to merge 77 commits from fix/10-highestmodseq-search-error into master

heiko commented

2026-05-11 21:28:40 +02:00

Owner

Closes #10

Moves st.HighestModSeq = mailboxInfo.HighestModSeq inside the success path so the old modseq is preserved when SearchSinceModSeq returns an error.

Closes #10 Moves `st.HighestModSeq = mailboxInfo.HighestModSeq` inside the success path so the old modseq is preserved when `SearchSinceModSeq` returns an error.

heiko added 2 commits

2026-05-11 21:28:40 +02:00

test: cover modseq state after failed search af4f6a1b8c

fix: preserve modseq after failed search f4b97c9759

heiko reviewed

2026-05-11 21:54:10 +02:00

heiko left a comment

Review: fix/10-highestmodseq-search-error

Correctness: The one-line move is correct. If SearchSinceModSeq returns an error, advancing HighestModSeq would silently skip all changes on the next run. Moving the assignment inside the else (success) block preserves the old modseq so the next iteration retries.

Same pattern in the else if st.LastUID > 0 branch: The non-CONDSTORE fallback also advances modseq unconditionally:

if condStore {
    st.HighestModSeq = mailboxInfo.HighestModSeq
}

This sits outside the success block, so when SearchUIDs fails the modseq cursor is still advanced. Consider applying the same fix there for consistency.

Test adequacy: The regression test is a source-text grep — it verifies structural placement rather than runtime behaviour. Brittle on refactoring. A behavioural test (inject a failing search via imapmemserver, assert st.HighestModSeq unchanged) would be more robust.

No correctness issues found in the change itself.

— reviewed by ai:claude

## Review: fix/10-highestmodseq-search-error **Correctness:** The one-line move is correct. If `SearchSinceModSeq` returns an error, advancing `HighestModSeq` would silently skip all changes on the next run. Moving the assignment inside the `else` (success) block preserves the old modseq so the next iteration retries. **Same pattern in the `else if st.LastUID > 0` branch:** The non-CONDSTORE fallback also advances modseq unconditionally: ```go if condStore { st.HighestModSeq = mailboxInfo.HighestModSeq } ``` This sits *outside* the success block, so when `SearchUIDs` fails the modseq cursor is still advanced. Consider applying the same fix there for consistency. **Test adequacy:** The regression test is a source-text grep — it verifies structural placement rather than runtime behaviour. Brittle on refactoring. A behavioural test (inject a failing search via `imapmemserver`, assert `st.HighestModSeq` unchanged) would be more robust. No correctness issues found in the change itself. — reviewed by ai:claude

heiko added 1 commit

2026-05-11 22:01:25 +02:00

fix: also guard HighestModSeq in UID-search fallback path 5995f84352

heiko closed this pull request

2026-05-11 22:25:43 +02:00