[codex] fix email cursor replay on first sync

[furukama]

What changed

Fix the email IMAP cursor bootstrap logic so a missing or incompatible persisted cursor seeds from the current mailbox head instead of replaying the entire inbox.

Add regression coverage for both startup modes:

• first start with no saved cursor does not ingest old inbox mail
• restart with a saved cursor still processes messages that arrived while HybridClaw was offline

Why

After the April 11, 2026 email cursor change, the first gateway start without an existing cursor file initialized lastProcessedUid to 0 and searched all mailbox UIDs. That caused historical inbox mail to be treated as new and replied to again.

Impact

Email users no longer get duplicate replies after upgrading into the persisted-cursor implementation.

Root cause

The persisted cursor path did not preserve the old channel behavior of treating the current mailbox head as the startup baseline when no cursor existed yet.

Validation

• ./node_modules/.bin/vitest run --configLoader runner --config vitest.unit.config.ts tests/email.connection.test.ts

[furukama]

@claude Do a thorough review of this PR in terms of: KISS, YAGNI, DRY, Fail Fast, Secure by Default and performance/speed

[claude]

Claude finished @furukama's task in 2m 27s —— View job

---

PR Review: KISS · YAGNI · DRY · Fail Fast · Secure by Default · Performance

• Read changed files
• Analyze src/channels/email/connection.ts
• Analyze tests/email.connection.test.ts
• Post full review

---

The fix is correct and the logic is sound. A few targeted observations below, ordered by principle.

---

KISS

Dead 0 in the lastProcessedUid initialization (connection.ts:253–255)

let lastProcessedUid = compatibleStoredCursor
  ? compatibleStoredCursor.lastProcessedUid
  : 0;  // ← this value is never read

When compatibleStoredCursor is null, the code falls through to the !compatibleStoredCursor guard on line 258 and returns immediately. The : 0 arm is dead. Readers have to trace the control flow to figure this out. Moving the lastProcessedUid declaration to after the guard makes the intent unambiguous:

const compatibleStoredCursor = ...;

const maxKnownUid = resolveMailboxUidNext(activeClient) - 1;
if (!compatibleStoredCursor) {
  persistedCursorState.set(folder, { uidValidity, lastProcessedUid: maxKnownUid });
  await savePersistedFolderCursorState(config.address, persistedCursorState);
  return;
}
let lastProcessedUid = compatibleStoredCursor.lastProcessedUid; // always defined here

Fix this →

---

DRY

Duplicated ImapFlow mock class (tests/email.connection.test.ts)

Both tests define a nearly identical ImapFlow mock (~25 lines each). The only structural difference is that the first test closes over mutable let uidNext while the second hardcodes uidNext: 4. This duplication will drift silently when the client interface changes (e.g., adding a new required method). Extract a shared mock factory:

function makeImapFlowMock(opts: { uidNext: () => number; search: Mock; fetch: Mock; messageFlagsAdd: Mock }) { ... }

Fix this →

Cursor path construction duplicated in tests (tests/email.connection.test.ts:133–137)

The second test manually reconstructs the cursor file path using the same base64url + strip-padding logic that lives in resolveCursorStatePath. If the encoding scheme changes, the test silently diverges.

Consider exporting resolveCursorStatePath for test use, or writing the cursor fixture via a helper that calls the real function.

Pre-existing: persistedCursorState.set(...) + savePersistedFolderCursorState(...) appears in 5 places across pollFolder. This PR adds one more instance (line 259–265), which is the minimum needed. A private updateCursor(folder, uidValidity, uid) helper would contain the repetition, but that's out of scope for this fix.

---

Fail Fast

No guard on maxKnownUid <= 0 (connection.ts:258–267)

If resolveMailboxUidNext returns 1 (its minimum due to Math.max(1, ...)), then maxKnownUid = 0. The bootstrap saves lastProcessedUid: 0, which causes the next poll to process UID 1 — a pre-existing message — defeating the fix. The scenario is unusual (empty or broken mailbox response) but worth a guard:

if (!compatibleStoredCursor) {
  if (maxKnownUid > 0) {  // only seed if the mailbox reported a real head
    persistedCursorState.set(folder, { uidValidity, lastProcessedUid: maxKnownUid });
    await savePersistedFolderCursorState(config.address, persistedCursorState);
  }
  return;
}

Fix this →

---

YAGNI

Nothing to flag. The change is minimal and precisely scoped to the bug. No speculative abstractions introduced.

---

Secure by Default

Nothing to flag in the new code. resolveCursorStatePath continues to base64url-encode the address before embedding it in the file path, preventing traversal. The cursor file stores no secrets.

Minor pre-existing note: imapSecure: false in BASE_EMAIL_CONFIG (test line 9) is fine for a test fixture, but SMTP smtpSecure: false on port 587 (STARTTLS) is the correct real-world default for SMTP submission — that's fine.

---

Performance

Pre-existing — SEARCH ALL instead of SEARCH UID lastUid:* (connection.ts:288)

await activeClient.search({ all: true }, { uid: true })

This returns every UID in the folder and filters client-side. For a large inbox (thousands of messages), this transfers the full UID list on every poll. An IMAP UID SEARCH UID <lastProcessedUid+1>:* would return only new UIDs. With imapflow this would be:

await activeClient.search({ uid: `${lastProcessedUid + 1}:*` }, { uid: true })

This is pre-existing and out of scope here, but worth a follow-up ticket.

Pre-existing — per-message file writes: savePersistedFolderCursorState is called after every single message (lines 314, 327). For a catchup batch of N messages this produces N sequential fsync-level writes. Batching the save to end-of-folder-poll would reduce I/O by N×. Again pre-existing, but the new bootstrap write makes this the sixth call site — a good forcing function to extract the helper and reconsider the write strategy.

---

Summary

Principle	Status
KISS	Minor: dead 0 in lastProcessedUid init
YAGNI	✅ Clean
DRY	Two test duplication issues (mock class, path construction)
Fail Fast	maxKnownUid <= 0 edge case unguarded
Secure by Default	✅ Clean
Performance	Pre-existing SEARCH ALL and per-message writes; no regressions introduced

The core fix is correct and well-tested. The two actionable items for this PR are the dead-code simplification and the maxKnownUid > 0 guard; the rest can reasonably be addressed as follow-ups.

[Copilot]

Pull request overview

Fixes the email IMAP cursor bootstrap behavior so that when there is no compatible persisted cursor (missing file, wrong uidValidity, etc.), the connection manager seeds lastProcessedUid from the current mailbox head rather than replaying the whole inbox. This restores the pre-persisted-cursor startup semantics and adds regression coverage for both fresh-start and restart flows.

Changes:

• Seed and persist the cursor to uidNext - 1 when no compatible stored cursor exists, and skip processing on that first poll.
• Add/adjust unit tests to validate (1) first start does not process historical UIDs, and (2) restart with a saved cursor processes only messages that arrived while offline.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File	Description
src/channels/email/connection.ts	Updates cursor bootstrap logic to persist a baseline cursor at mailbox head when no compatible cursor exists, preventing historical replay.
tests/email.connection.test.ts	Reworks existing test to assert no replay on missing cursor, and adds a new test ensuring saved-cursor resumes correctly.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.