Two guards were added while fixing review findings on #165 and shipped without tests. Both are small; both protect behaviour that a future refactor could silently remove.
1. verifySmtpConnection's outer timeout race
src/providers/adapters/smtp/verify.ts races transporter.verify() against a rejecting timer, because nodemailer exposes no cancellation token and its socket-level timeouts can fail to fire. This runs inside the connect/check HTTP request, so without the race a hang holds an operator's request open indefinitely.
No test exercises it. CodeRabbit's phrasing is exactly right: "a regression that removes the race would still pass this suite."
Acceptance: a transporter whose verify() never settles, a small timeoutMs, and an assertion that the call rejects with the timeout rather than hanging.
2. runImapFetch's missing-config-credential-or-cursor skip branch
src/mail/imap-fetch.ts skips a mailbox that has no config, no credential, or no baseline cursor. That state is reachable — a partially-completed connect, or a credential row removed out of band — and the skip is what keeps one broken mailbox from failing the whole sweep.
Acceptance: a mailbox in each of the three states is skipped, the sweep continues to the next mailbox, and skipped is counted (that counter now drives the log gate, so a miscount hides an intake outage).
Why these were not fixed in #165
Both are coverage gaps, not defects — the guards work. #165 had already been through six review rounds and a main merge that turned up a migration-id collision and a missing RLS lockdown; extending it further to add tests was the worse trade. Adjudicated on the PR before merge rather than dropped.
Related: #172 (web/ has no test infrastructure at all) is the larger version of this problem.
Two guards were added while fixing review findings on #165 and shipped without tests. Both are small; both protect behaviour that a future refactor could silently remove.
1.
verifySmtpConnection's outer timeout racesrc/providers/adapters/smtp/verify.tsracestransporter.verify()against a rejecting timer, because nodemailer exposes no cancellation token and its socket-level timeouts can fail to fire. This runs inside the connect/check HTTP request, so without the race a hang holds an operator's request open indefinitely.No test exercises it. CodeRabbit's phrasing is exactly right: "a regression that removes the race would still pass this suite."
Acceptance: a transporter whose
verify()never settles, a smalltimeoutMs, and an assertion that the call rejects with the timeout rather than hanging.2.
runImapFetch'smissing-config-credential-or-cursorskip branchsrc/mail/imap-fetch.tsskips a mailbox that has no config, no credential, or no baseline cursor. That state is reachable — a partially-completed connect, or a credential row removed out of band — and the skip is what keeps one broken mailbox from failing the whole sweep.Acceptance: a mailbox in each of the three states is skipped, the sweep continues to the next mailbox, and
skippedis counted (that counter now drives the log gate, so a miscount hides an intake outage).Why these were not fixed in #165
Both are coverage gaps, not defects — the guards work. #165 had already been through six review rounds and a
mainmerge that turned up a migration-id collision and a missing RLS lockdown; extending it further to add tests was the worse trade. Adjudicated on the PR before merge rather than dropped.Related: #172 (
web/has no test infrastructure at all) is the larger version of this problem.