[IAmJSD]

Closes #1433

Root cause

The SMTPSender send methods took ctx but never used it. dispatchSMTP had no context parameter. The STARTTLS path dialed with smtp.Dial and the plain path used smtp.SendMail, and neither sets a deadline. A mail server that accepted the TCP connection and then stopped answering could hold the caller until the OS gave up, past the 30s failureNotifyTimeout budget for the purchase-failed alert, and past a Lambda invocation's remaining time.

Fix

internal/email/smtp_sender.go:

• dispatchSMTP(ctx, rcpts, msg) now takes the caller's context. All three call sites pass it.
• sendMail dials with net.Dialer.DialContext(ctx, ...) and registers context.AfterFunc(ctx, ...). When ctx ends (deadline or cancel), that moves the connection's deadline into the past, so any read or write in flight fails, including the TLS handshake and every step of the SMTP conversation. When ctx is done, the returned error wraps ctx.Err(), so errors.Is(err, context.DeadlineExceeded) (or context.Canceled) holds. This covers the issue's "deadline from ctx.Deadline()" and "close on cancel" points in one mechanism. Using AfterFunc rather than also calling SetDeadline(dl) avoids a race where the I/O timeout fires before ctx.Err() is set.
• The STARTTLS path and the plain AllowInsecure path share one conversation, smtpConverse: smtp.NewClient on the dialed conn, STARTTLS, AUTH, MAIL/RCPT/DATA, QUIT. With UseTLS, STARTTLS is mandatory as before (TLS 1.2 minimum, fix(scheduler): single-flight cold-start collection through the marker #410). Without it, STARTTLS is used when the server offers it, as smtp.SendMail did. smtpAuthenticate now also refuses early with smtp: server doesn't support AUTH when the server doesn't advertise AUTH, which keeps SendMail's behavior for the plain path.
• A second commit addresses lint: the SetDeadline error is logged at debug level for errcheck's check-blank rule, and a test name uses "canceled".

Behavior change to note: callers that pass an already-canceled or expired context now get an error instead of a send. SES sends already behaved this way, because the AWS SDK honors ctx. The purchase-failed alert uses ctxutil.DetachedWithTimeout, which is the intended 30s budget.

Verification

All local runs, Linux, Go 1.26.9 toolchain:

• New test TestSMTPSender_SendHonorsContext (internal/email/smtp_server_test.go). A local stub accepts connections and then stalls, either with no 220 greeting or after answering EHLO, while holding the connection open until cleanup. Subtests cover the STARTTLS path (no credentials, useTLS) and the plain AllowInsecure path with credentials, at a 200ms deadline plus one cancellation case. Each asserts that SendToEmail returns within 2s with an error wrapping context.DeadlineExceeded (or context.Canceled). A 5s guard fails the test if the call stays blocked.
• Fail-before: with smtp_sender.go and smtp_server_test.go from origin/main plus only the new test appended, all 5 subtests fail with "SendToEmail ignored its context: still blocked 5s after a 200ms deadline" (25s total).
• Pass-after: all 5 pass in about 0.9s under -race.
• Existing SMTP tests pass unchanged except for signature updates (sendMailTLS became sendMail(ctx, ...) and dispatchSMTP(ctx, ...) in 6 call sites) and three stale comments.
• go build ./..., go vet ./..., go vet -tags integration ./..., (cd tests/e2e && go vet -tags e2e ./...): clean.
• golangci-lint v2.10.1 on ./internal/email/, with and without --build-tags integration: 0 issues. gocyclo -over 10 internal/email/smtp_sender.go: nothing reported.
• go test -race -short -count=1 ./internal/email/ and go test -race -tags integration -count=1 ./internal/email/: ok.
• secrets-scan.sh on the changed files: "clean; planted key flagged". scripts/test-git-secrets-allowlist.sh: 183 PASS, 0 FAIL.

Not verified locally: a send through real SendGrid or Azure ACS SMTP endpoints. The happy path is covered only by the existing local stub-server tests.

🤖 Generated with Claude Code

[coderabbitai]

Warning

Review limit reached

• Run on-demand review

_{This review includes 4 billable files and costs up to $1.00.}

Or wait 59 minutes for your next included review.

Check out review usage here.

View limit details

Limit details: You’ve used the included review currently available.

Learn how review limits work.

Review configuration:

Run configuration

• Source mode: Auto
• Repository: LeanerCloud/cloud-commitments-platform
• Branch: main
• File: .coderabbit.yaml
• Commit: c8bb869
• Layers: Repository YAML
• Review profile: CHILL
• Plan: Advanced
• Run ID: 6c78e459-7d39-451c-89d0-2baf772e516f

Commits

Reviewing files that changed from the base of the PR and between c8bb869 and ba43481.

Files selected for processing: 4

• internal/email/coverage_extra_test.go
• internal/email/smtp_sender.go
• internal/email/smtp_sender_test.go
• internal/email/smtp_server_test.go

• Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

_{Comment @coderabbitai help to get the list of available commands.}

[IAmJSD]

Opened at head 9762732: SMTP sends now dial with the caller's context and fail any in-flight I/O once it ends (STARTTLS and plain paths); fail-before/pass-after and local check results are in the PR body.

[IAmJSD]

Independent review (IAmJSD's agent, fresh context, given only the diff, the issue and the PR text): VERDICT: APPROVE ba4348161c53d87d42b0872bd54097c4cf325724

Blocking:

• none

Non-blocking:

• internal/email/smtp_sender.go:343 (smtpAuthenticate): the new "server doesn't support AUTH" check now runs on the STARTTLS path too. The old sendMailTLS called c.Auth without it. It is also stricter than smtp.SendMail, which skipped AUTH entirely when c.ext == nil (a HELO-only server). Both are fail-closed and harmless with SendGrid and ACS, which advertise AUTH after STARTTLS. Still, it's a production behavior change the PR body doesn't mention.
• A ctx with no deadline and no cancel (for example an HTTP request ctx on a server with no per-request timeout) still leaves the conversation unbounded, as before. The issue didn't ask for a default cap, so this is just a possible follow-up.
• Dial-time expiry isn't wrapped by sendMail (the DialContext error is returned directly). net's timeout and cancel errors do satisfy errors.Is for the context errors, but nothing tests that.

Attacked:

• Correctness of the AfterFunc approach: stop is deferred after conn.Close, so it runs first. A late AfterFunc only hits SetDeadline on a still-open conn. After STARTTLS, the TLS client reads through the raw conn, so the past deadline unblocks a stalled handshake too. Client.Mail/Rcpt still run validateLine (go/src/net/smtp/smtp.go:246,269), so dropping smtp.SendMail loses no CRLF guard. STARTTLS is still mandatory with useTLS, with MinVersion TLS 1.2. I grepped for email sends from goroutines on a request ctx, which would now fail on cancel: there are none. The purchase-failed alert uses DetachedWithTimeout.
• Fail-before: main's internal/email/ with only the new test (and its strconv import) appended. All 5 TestSMTPSender_SendHonorsContext subtests fail with "still blocked 5s after a 200ms deadline" (25s). With the change, they pass, and -race -count=5 on that test passes too.
• go build ./...; go vet ./internal/email/ (plain and -tags integration); go test -race -short -count=1 and go test -race -tags integration -count=1 on ./internal/email/: ok. golangci-lint, with and without integration tags: 0 issues. gocyclo -over 10 on smtp_sender.go: nothing.

Body-stale:

• none

_{Reviewer: rC3. Bar while the maintainers are away (set by IAmJSD as acting owner): one approving independent review on the head, two for money, auth and security paths.}

[IAmJSD]

Local verification at ba4348161c53d87d42b0872bd54097c4cf325724 merged onto main (d87b96ff), together with #1437, #1438, #1439, #1440, #1441 in that order (combined tree 37f4d0bb, Linux, Go 1.26.10): go build ./..., go vet with and without -tags integration, the full go test -race -short -count=1 ./..., and go test -race -tags integration -count=1 on every changed package, against Docker Postgres. All pass. CI Success and pre-commit are green on this head, with 1 approving independent review(s) on it. Nothing was run against any cloud.

[IAmJSD]

Merged by IAmJSD's agent at IAmJSD's request (IAmJSD is acting owner while the maintainers are away), under the bar IAmJSD set: 1 approving independent review(s) on ba434816, green CI, and the combined-tree verification above. Squash-merged at exactly ba4348161c53d87d42b0872bd54097c4cf325724.