COR-218 - A NOOP on a cold connection connects first

[dbezverkhnii]

Two pre-existing gaps found while fixing COR-211/217, both in IMAPSession.

noop() on a session with no mailimap returned success without sending anything — never connected, or torn down by unsetup(). The client uses NOOP as its connectivity/credentials probe (check(), checkAuthStatus()) and as the keep-alive that holds one render-pool connection warm; on a cold connection both got a success that checked nothing and connected nothing, and the first real command paid the handshake. NOOP now goes through loginIfNeeded() like every other command. Consequences to know about: the keep-alive re-logs in a connection that was torn down behind it within one tick (≤25 s), which on a LOGIN-rationed server (Yahoo) costs one LOGIN per deliberate teardown — that is what "warm" means; and check() on a cold pool can now return ErrorConnection where it used to succeed vacuously, which is the probe doing its job. A LOGIN that fails is retried by the keep-alive as before — unchanged, since a failed login never tore the socket down.

unsetup() published STATE_DISCONNECTED after closing and freeing the stream, outside the lock under which it nils mImap; needsReconnect() on another thread could call the session logged in while a TLS shutdown ran. The store moves next to the nil. Ordering-only; no deterministic test reaches inside unsetup().

Tests. testNoopConnectsAColdConnection: a NOOP on a never-connected lease connects (one client), then after a connection-scoped disconnect a NOOP reconnects (two clients). Against the old code: zero and one. testInterruptDoesNothingForAnOperationThatIsNotRunning adjusted: its queued NOOP now reconnects to the silent endpoint and blocks in turn, and is interrupted like the command before it. The cut test dropped its now-redundant explicit connect. 32 IMAP tests green, 30 under ASan.

Both source hunks are in upstream code: a 3-line deletion and a one-statement move. New Windows prebuilt needed.

🤖 Generated with Claude Code

---

Note

Medium Risk
Changes core IMAP connection lifecycle for NOOP, keep-alive, and cross-thread disconnect visibility; behavior shifts for cold pools and LOGIN-rationed servers.

Overview
IMAP NOOP no longer returns immediately when there is no active mailimap handle. It now goes through loginIfNeeded() like other commands, so connectivity checks, auth probes, and pool keep-alives actually open the socket and log in instead of reporting success without touching the server. After a deliberate teardown, the next NOOP reconnects within one keep-alive tick; check() on a cold pool can surface ErrorConnection where it used to succeed vacuously.

unsetup() now sets STATE_DISCONNECTED under the same lock as clearing mImap, before the stream is closed and freed, so needsReconnect() on another thread cannot treat the session as still logged in during teardown.

Tests add testNoopConnectsAColdConnection, drop a redundant explicit connect in the cut/reconnect lease test, and extend the interrupt queue test so a queued NOOP reconnects and blocks until interrupted.

^{Reviewed by Cursor Bugbot for commit b161dcb. Bugbot is set up for automated code reviews on this repo. Configure here.}

[dbezverkhnii]

bugbot run

[cursor]

✅ Bugbot reviewed your changes and found no new issues!

Comment @cursor review or bugbot run to trigger another review on this PR

^{Reviewed by Cursor Bugbot for commit b161dcb. Configure here.}

[Copilot]

🔵 Needs a closer look

The changes affect core IMAP connection lifecycle, reconnect behavior, and cross-thread teardown state, warranting final human review.

Pull request overview

Fixes IMAP lifecycle handling so NOOP connects cold sessions and teardown publishes disconnected state safely.

Changes:

• Routes NOOP through loginIfNeeded().
• Publishes disconnected state during teardown.
• Updates connection and interruption tests.

File summaries

File	Description
unittest/IMAPInterruptCurrentCommandTests.swift	Updates queued-operation interruption coverage.
unittest/IMAPConnectionLeaseTests.swift	Tests cold and reconnected NOOP behavior.
src/core/imap/MCIMAPSession.cpp	Fixes NOOP connection behavior and teardown state ordering.

Review details

• Files reviewed: 3/3 changed files
• Comments generated: 0
• Review effort level: Lite

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.