PR Review: feat: add production SMTP email sender wrapping go-mail

Overall this is a well-structured addition — clean interface preservation, good fail-fast behavior, and solid test coverage. A few issues worth addressing before merge.

Bugs / Correctness

go-mail classified as // indirect in go.mod
github.com/wneessen/go-mail is used directly in pkg/email/smtp.go, so it should be a direct dependency:

- github.com/wneessen/go-mail v0.7.2 // indirect
+ github.com/wneessen/go-mail v0.7.2

Running go mod tidy should fix this automatically.

os.Unsetenv in tests doesn't restore original value
TestNewEmailSenderFromEnv_DefaultConsole and TestNewEmailSenderFromEnv_SMTP_DefaultPort use os.Unsetenv(...) which won't restore the env var if it was set before the test. Use t.Setenv consistently — it restores automatically after the test:

// Instead of:
os.Unsetenv("EMAIL_PROVIDER")

// Use:
t.Setenv("EMAIL_PROVIDER", "")

(or restructure so the unset is handled via t.Cleanup)

Performance

New SMTP client and TCP/TLS connection created per Send() call
smtp.go:Send() creates a new mail.Client and calls DialAndSend on every invocation. For transactional email this is usually fine, but if the app ever sends batches (e.g., bulk notifications), each email opens a fresh connection. Consider noting this limitation in the doc comment, or holding the client as a field and reconnecting only on failure.

Security

TLSOpportunistic silently falls back to plaintext
This is intentional for dev compatibility (Mailpit etc.), and the PR notes explain the reasoning. However, worth adding a warning comment in the code for operators who may configure this against a production server that they believe is TLS-only:

// TLSOpportunistic upgrades to TLS when available but allows plaintext fallback.
// For strict TLS enforcement in production, change to mail.TLSMandatory.
mail.WithTLSPortPolicy(mail.TLSOpportunistic),

Code Quality

Duplicated newEmailSender() wrapper in handler.go.tmpl vs direct call in auth.go
handler.go.tmpl introduces a private newEmailSender() helper that just wraps NewEmailSenderFromEnv() with log.Fatalf, while auth.go (for ProtectResources) calls NewEmailSenderFromEnv() directly with explicit error handling. The inconsistency is minor but could confuse readers of generated code. Either approach is fine — just worth being consistent.

Multiple email sender instances created at startup
Each of the 6 handler factories (Handler, LogoutHandler, MagicLinkHandler, etc.) independently calls newEmailSender() → NewEmailSenderFromEnv(). For SMTPEmailSender this is harmless (it doesn't hold a persistent connection), but it does re-read and re-validate env vars N times. Low priority, but initializing once and passing it in would be cleaner.

Import insertion ordering in auth.go
When both "log" and "email" need inserting, "log" gets \n\t (no blank line) and "email" gets \n\n\t. The result depends on what's already in the import block. Worth validating the output looks clean with goimports after injection.

Minor Nits

• TestSMTPEmailSender_ImplementsInterface is a compile-time check — it will only catch the error at go test time, not go build. That's fine but could alternatively be a var _ EmailSender = (*SMTPEmailSender)(nil) at package level.
• EMAIL_FROM_NAME appears in getVarReason but not in the required-vars list — which is correct since it's optional — but a comment noting it's optional alongside the required-vars loop would help future readers.

Summary

Good work overall — the fail-fast approach, env-based provider selection, and backwards-compatible default are all solid design choices.

🤖 Generated with Claude Code

PR Review: feat: add production SMTP email sender

Overall this is a clean, well-scoped implementation. Good use of the factory pattern, sensible defaults, and solid test coverage. A few issues worth addressing:

Bug: go-mail marked as indirect in go.mod

The line github.com/wneessen/go-mail v0.7.2 // indirect in go.mod is wrong. This is a direct dependency (used in pkg/email/smtp.go). Run go mod tidy to correct it.

Security concern: TLSOpportunistic in production

mail.WithTLSPortPolicy(mail.TLSOpportunistic) means emails will be sent in plaintext if the server does not advertise TLS support. This is acceptable for dev tools like Mailpit, but should be documented so users know to verify TLS is active in production. Consider adding a comment next to the option.

Fragile import detection in auth.go

hasLog := strings.Contains(mainContent, "\t\"log\"\n") || strings.Contains(mainContent, "\t\"log\"\r\n")

This produces a false negative (log import not detected, duplicate gets inserted) if:

• "log" is the last entry in the import block (no trailing newline before ))
• The file uses space indentation instead of tabs

A safer approach: extract the import block first and search within it, or scope the search to just the import section.

New SMTP client created per Send() call

In smtp.go, mail.NewClient() is called inside Send(), establishing a new TCP connection for every email. For low-volume apps this is fine, but worth a comment explaining the trade-off for users who might hit this at scale.

Minor: newController() initialises a fresh email sender per handler factory

Each of Handler, LogoutHandler, MagicLinkHandler, etc. calls newController(queries) independently, so NewEmailSenderFromEnv() is called N times at startup. No correctness issue, but worth noting if consolidating to a single shared sender is preferred.

Missing Send() test for SMTPEmailSender

smtp_test.go covers config construction and interface assertion but not Send() itself. A lightweight test with an embedded SMTP server or mock would guard against MIME formatting or auth regressions.

Nit: EMAIL_FROM_NAME reason string does not indicate it is optional

Consider "sender display name (optional)" in getVarReason so lvt env output is unambiguous.

What is good

• Factory pattern with env-based dispatch is clean and correct
• Defaulting to console preserves full backwards compatibility
• log.Fatalf on misconfiguration at startup is the right call — silent production email failures are far worse
• Deleting email.go.tmpl (dead code never loaded by the generator) is the right housekeeping
• Deduplicating 6 handler factories via newController() is a clean reduction
• Factory error-case test coverage is thorough

The two items to address before merge: go-mail marked as indirect in go.mod (correctness) and fragile log import detection in auth.go (potential codegen bug that could produce uncompilable output). The TLS note is important for security-conscious users but less blocking.

Code Review: feat: add production SMTP email sender wrapping go-mail. Overall this is a well-structured PR. Full review text follows in replies.

Issues to Fix Before Merge

1. go-mail is marked indirect in go.mod

github.com/wneessen/go-mail v0.7.2 is directly imported by pkg/email/smtp.go and should not carry the // indirect marker. Running go mod tidy will correct this automatically.

2. SMTP port validation does not reject out-of-range values

In factory.go, strconv.Atoi accepts any integer including 0, -1, or 99999. A port outside 1-65535 will not be caught until the first dial attempt, producing a confusing send-time error instead of a startup error. A range check after parsing closes this gap.

Design and Performance Observations

3. Each handler factory independently constructs its own EmailSender

In handler.go.tmpl, each factory (Handler, LogoutHandler, MagicLinkHandler, etc.) independently calls newController(queries), which independently calls NewEmailSenderFromEnv() and reads os.Getenv. In practice this is fine at startup, but senders can diverge if env vars differ between calls (e.g. in parallel tests). Constructing the sender once in main and passing it as a parameter is the cleaner approach.

4. log.Fatalf in newController makes the error path untestable

newController calls log.Fatalf on error. The PR description justifies fail-fast at startup, which is acceptable. However this makes the error path impossible to test without process-level tricks. The generated ProtectResources code injects the same pattern. At minimum, add a comment that newController is only safe at program startup, not during request handling.

5. New SMTP client per Send call

SMTPEmailSender.Send creates mail.NewClient and calls DialAndSend on every invocation: a new TCP connection and TLS handshake per email. Fine for infrequent auth emails. A comment in smtp.go flagging the connection-reuse opportunity would help future contributors.

6. Import injection condition is subtly misleading

In auth.go (ProtectResources), the outer condition enters the block if either import is missing and the inner code handles each independently, so the logic is correct. But the outer OR may confuse future editors who expect AND. A brief comment or two separate guards would make the intent clearer.

Security Observations

7. TLSOpportunistic can silently fall back to plaintext in production

TLSOpportunistic is the right default for local dev tools like Mailpit, but in production it falls back to an unencrypted connection if TLS negotiation fails, potentially exposing SMTP credentials and magic-link tokens. Consider adding a SMTP_TLS env var (opportunistic / mandatory / none) so operators can enforce TLSMandatory. At minimum, add a warning comment in the generated .env file next to SMTP_HOST.

8. Plain-text only is an undocumented interface constraint

SMTPEmailSender hardcodes mail.TypeTextPlain. The EmailSender interface doc does not mention this. If a future template generates HTML, callers will silently deliver plain text. A one-line note on the EmailSender interface would prevent this confusion.

9. SMTP password stored as plain string in SMTPConfig

Standard Go practice, so no action needed unless the project has a policy on in-memory secret handling.

Test Coverage

The factory tests are thorough: default/console/noop/smtp/missing-host/missing-from/invalid-port/unknown-provider paths are all covered.

Small gap: EMAIL_FROM_NAME propagation is never asserted. The field is read from env and assigned to SMTPConfig.FromName, but no test verifies it. Easy to add alongside TestNewEmailSenderFromEnv_SMTP.

smtp_test.go only tests construction, not Send. A round-trip test against a loopback SMTP mock would catch message-assembly bugs. Nice-to-have, not a blocker.

Generator test update is correct - the auth_test.go idempotency fixture is properly updated to use NewEmailSenderFromEnv.

Summary

Solid implementation. Two items most worth fixing before merge:

1. go mod tidy - go-mail should be a direct dependency, not indirect.
2. SMTP port range check in factory.go - catch invalid ports at startup rather than at send time.

Everything else is a documentation improvement or future-iteration concern. Backward compatibility (defaulting to console) is clean and the test suite is well-structured.

Generated with Claude Code

Code Review

Overall this is a clean, well-structured PR. The design decisions are sound and well-justified. A few things worth addressing.

go.mod: indirect vs direct dependency

go-mail is imported directly in pkg/email/smtp.go but is marked // indirect in go.mod. Run go mod tidy to correct this — it should be a direct dependency.

smtp.go: new TCP connection per Send() call

The SMTP client is created fresh on every Send() invocation (new TCP connection + TLS handshake per email). For the current use case (magic links, password resets — low frequency) this is fine, but worth a comment so future contributors understand the intentional trade-off and don't reach for connection pooling unnecessarily.

auth.go: import group formatting when both imports are missing

When ProtectResources injects both "log" and the email import, "log" ends up just before the closing ) rather than consolidated with other stdlib imports. The \n\n separator before the email import correctly creates a third-party group, but "log" adjacent to it produces non-idiomatic grouping. gofmt will reformat it at the block level but won't regroup imports — goimports would. Worth noting or fixing if generated code doesn't pass through goimports in the pipeline.

handler.go.tmpl: newController() called once per handler type

With 6 handler factories, NewEmailSenderFromEnv() is called 6 times at startup. For smtp this means 6x env var reads and 6x SMTPEmailSender allocations (no connections yet — those happen in DialAndSend). Functionally fine, but a single shared sender initialized once would be cleaner if you want to be precise.

Minor: port range validation missing

NewSMTPEmailSenderFromEnv parses SMTP_PORT but doesn't validate it's in range (1–65535). An out-of-range value passes config validation and fails later with a less clear error. Easy fix:

if port < 1 || port > 65535 {
    return nil, fmt.Errorf("email: SMTP_PORT %d out of valid range (1-65535)", port)
}

Security note: TLSOpportunistic in production

The PR description explains this choice well (Mailpit compat). Worth a brief code comment next to mail.WithTLSPortPolicy(mail.TLSOpportunistic) noting that users wanting strict TLS enforcement can switch to mail.TLSMandatory.

What's done well

• Clean newController() extraction eliminating 6x duplicate init code
• t.Setenv usage in tests (proper cleanup vs os.Setenv)
• log.Fatalf fail-fast at startup — correct choice over silently dropping production emails
• Deleting the dead email.go.tmpl is the right cleanup
• E2E replace directive for unreleased pkg/ changes is pragmatic
• Solid test coverage for all factory error paths

The // indirect in go.mod and the import group formatting in auth.go are the two worth fixing before merge. The rest are suggestions.

PR Review: feat: add production SMTP email sender wrapping go-mail

Overall this is a solid, well-structured addition. The factory pattern is clean, the go-mail wrapper is minimal, and the fail-fast startup behavior is the right call for misconfiguration. A few things worth addressing before merge:

Bug: go-mail marked as indirect in go.mod

github.com/wneessen/go-mail v0.7.2 // indirect

go-mail is directly imported in pkg/email/smtp.go — it should not have the // indirect comment. This would be caught by go mod tidy. Likely a tooling artifact, easy fix.

Concern: EMAIL_FROM missing from generated .env template

EMAIL_FROM is correctly added to the required-vars validation (good), but it doesn't appear to be added to generateEnvContent() in commands/env.go. If users generate an env file and switch to EMAIL_PROVIDER=smtp, they'll get a validation error for EMAIL_FROM with no hint of it in the generated template. It should appear as a commented-out example alongside SMTP_HOST/PORT/USER/PASS.

Each handler factory calls NewEmailSenderFromEnv() independently

func Handler(queries *models.Queries) http.Handler {
    controller := newController(queries) // calls NewEmailSenderFromEnv()
    ...
}

func LogoutHandler(queries *models.Queries) http.Handler {
    return http.HandlerFunc(newController(queries).HandleLogout) // another call
}

With 5–6 handler factories, NewEmailSenderFromEnv() is called 5–6 times at startup. For console/noop this is trivial, but for smtp it reads and validates env vars each time. Not a correctness issue, but worth sharing a single sender across handlers (e.g. create it once in main.go and pass it in, or accept that startup cost is negligible here).

Minor: SMTP port range not validated

port, err := strconv.Atoi(portStr)
if err != nil {
    return nil, fmt.Errorf("email: invalid SMTP_PORT %q: %w", portStr, err)
}

strconv.Atoi("99999") succeeds but produces an invalid port. A simple if port < 1 || port > 65535 check would give a better error before the connection attempt.

Nice things worth calling out

• Deleting the dead email.go.tmpl is the right call — it was duplicating pkg/email/ and was never loaded.
• The LogoutHandler cleanup correctly removes the dead controller variable that was being created but ignored (it was already calling newController(queries) on the return line in the old code).
• SMTPAuthAutoDiscover + TLSOpportunistic is the right combo for broad compatibility without sacrificing security where available.
• log.Fatalf on startup misconfiguration is correct — silent email drops in production are worse than a hard crash.
• Test coverage in factory_test.go is thorough and uses t.Setenv correctly for env isolation.

Verdict: Fix the // indirect in go.mod and check generateEnvContent for EMAIL_FROM. The rest are minor polish items. Good PR.

PR Review: feat: add production SMTP email sender wrapping go-mail

Overall this is a solid, well-scoped addition. Interface is preserved, test coverage is good, and the design decisions are reasonable. A few things worth addressing before merge.

Bug: go-mail marked as // indirect in go.mod

github.com/wneessen/go-mail is imported directly in pkg/email/smtp.go, so it must be a direct dependency:

- github.com/wneessen/go-mail v0.7.2 // indirect
+ github.com/wneessen/go-mail v0.7.2

Run go mod tidy — it should fix this automatically.

Bug: handler.go.tmpl's newController() bypasses env validation

pkg/email/factory.go provides NewEmailSenderFromEnv() which validates SMTP_HOST and EMAIL_FROM at startup and returns an error. But handler.go.tmpl's newController() does its own switch that calls email.NewSMTPEmailSender() directly with raw os.Getenv() values — no validation, no early failure:

case "smtp":
    sender = email.NewSMTPEmailSender(email.SMTPConfig{
        Host: os.Getenv("SMTP_HOST"),  // empty string if misconfigured
        From: os.Getenv("EMAIL_FROM"), // empty string if misconfigured
        ...
    })

The PR description says "fails fast with log.Fatalf on SMTP misconfiguration" — but in generated apps, misconfiguration won't be caught until the first Send() call at runtime. The template should use NewEmailSenderFromEnv() (with log.Fatalf on error) to match the intent:

sender, err := email.NewEmailSenderFromEnv()
if err != nil {
    log.Fatalf("email: %v", err)
}

Security: TLSOpportunistic silently falls back to plaintext

This is called out in the PR description and is intentional for Mailpit/dev compatibility — fair tradeoff. Worth a brief inline comment so operators don't assume TLS is enforced:

// TLSOpportunistic upgrades to TLS when available but allows plaintext fallback.
// Change to mail.TLSMandatory to enforce TLS in strict production environments.
mail.WithTLSPortPolicy(mail.TLSOpportunistic),

Performance: new TCP connection per Send() call

smtp.go:Send() creates a fresh mail.Client and calls DialAndSend on every invocation. For transactional email (one-at-a-time) this is fine. Worth a doc comment on the method noting the connection-per-send behavior, so future maintainers know there's no connection pooling if they ever add batch sending.

Nit: EMAIL_FROM_NAME in getVarReason looks like a required var

In commands/env.go, EMAIL_FROM_NAME appears in the reasons map alongside required vars. Adding a comment noting it's optional (vs EMAIL_FROM which is required for smtp) would prevent future confusion when auditing the required-vars logic.

Minor: compile-time interface check style

// smtp_test.go
func TestSMTPEmailSender_ImplementsInterface(t *testing.T) {
    var _ EmailSender = &SMTPEmailSender{}
}

This is valid but only runs during go test. The conventional Go pattern is a package-level blank assignment so it's checked at go build too:

// smtp.go (package level)
var _ EmailSender = (*SMTPEmailSender)(nil)

Summary

The validation gap in handler.go.tmpl is the most important to fix — everything else is polish. Good work overall on the cleanup of the 6 duplicated handler factories.

PR Review

There's a critical bug that undermines the PR's stated goal, plus a few secondary issues.

🐛 Critical: Template still hardcodes ConsoleEmailSender (defeats the PR's purpose)

The PR summary says "Update all 6 auth handler factories to use env-based sender instead of hardcoded ConsoleEmailSender", but handler.go.tmpl was not updated — newController still hardcodes the console sender:

// handler.go.tmpl
func newController(queries *models.Queries) *{{.StructName}}Controller {
    baseURL := "http://localhost:8080"
    if envURL := os.Getenv("BASE_URL"); envURL != "" {
        baseURL = envURL
    }
    emailSender := email.NewConsoleEmailSender()  // ← still hardcoded!
    return New{{.StructName}}Controller(queries, emailSender, baseURL)
}

This means generated apps will always use the console sender regardless of EMAIL_PROVIDER=smtp. The fix should be:

func newController(queries *models.Queries) *{{.StructName}}Controller {
    baseURL := "http://localhost:8080"
    if envURL := os.Getenv("BASE_URL"); envURL != "" {
        baseURL = envURL
    }
    sender, err := email.NewEmailSenderFromEnv()
    if err != nil {
        log.Fatalf("email: failed to initialize sender: %v", err)
    }
    return New{{.StructName}}Controller(queries, sender, baseURL)
}

Note: ProtectResources injection does use NewEmailSenderFromEnv() correctly — only the template is missing the update.

🐛 go-mail marked as // indirect in go.mod

pkg/email/smtp.go directly imports github.com/wneessen/go-mail, so it should be a direct dependency. Run go mod tidy to fix.

Security: TLSOpportunistic silently falls back to plaintext

Intentional for dev tooling compatibility (Mailpit etc.), but worth adding a comment for operators who may assume TLS is enforced:

// TLSOpportunistic upgrades to TLS when available but allows plaintext fallback.
// Change to mail.TLSMandatory for strict enforcement in production.
mail.WithTLSPortPolicy(mail.TLSOpportunistic),

Performance: New connection per Send() call

smtp.go:Send() creates a new mail.Client + TCP/TLS handshake on every call. Fine for transactional email, but worth noting in the doc comment in case batch use comes up later.

Minor

• EMAIL_FROM_NAME appears in getVarReason but not the required-vars list — correct since it's optional, but a comment noting this would help future readers.
• TestSMTPEmailSender_ImplementsInterface works but a package-level var _ EmailSender = (*SMTPEmailSender)(nil) catches the error at compile time rather than go test time.

Summary

The refactoring is clean and the factory approach is well-designed, but the template oversight means the feature won't actually work for generated apps. Once the newController template is fixed, this looks good to merge.

🤖 Generated with Claude Code

PR Review: feat: add production SMTP email sender

Overall this is a clean, well-structured PR. The go-mail wrapper is a good choice, the newController() refactor eliminates real duplication, and the factory pattern is intuitive. A few things worth addressing:

Bug / Correctness

Port range not validated (pkg/email/factory.go:28-35)

strconv.Atoi will happily parse 0, -1, or 99999 as a valid port. These fail silently at dial time with a confusing OS-level error. Consider adding a bounds check after parsing (e.g., if port < 1 or port > 65535, return an error describing the out-of-range value).

A test case alongside TestNewEmailSenderFromEnv_SMTP_InvalidPort would be straightforward to add.

Performance

New SMTP client per Send() call (pkg/email/smtp.go)

Every Send() dials a fresh TCP connection, negotiates TLS, authenticates, sends, and tears down. For low-volume transactional email (magic links, password resets) this is perfectly fine. Worth a brief note in the struct doc so users with higher-volume needs know they would need a different approach.

Security

TLSOpportunistic trade-off (pkg/email/smtp.go)

The comment already explains this well and points to mail.TLSMandatory. No action needed. An optional future enhancement: a TLSPolicy field on SMTPConfig to let callers enforce mandatory TLS without forking Send().

Code Quality

newController() comment vs. reality (handler.go.tmpl)

The godoc says "Called once per handler factory at startup", but each handler function calls newController() independently, creating separate controller instances. Functionally fine, but the comment implies shared state that does not exist. Suggested wording:

// newController creates an auth controller from environment configuration.
// Each handler factory calls this independently at registration time.

EMAIL_FROM_NAME in getVarReason

getVarReason is used for required vars, but EMAIL_FROM_NAME is optional. Harmless as a lookup-map entry, but a clarifying comment would help future maintainers.

Test Coverage

• Factory tests are thorough and use t.Setenv correctly.
• smtp_test.go is appropriately scoped to config/construction, no live server needed.
• Gap: no test for port out-of-range (add alongside the validation suggested above).
• Nice to have: a test that Send() returns a wrapped error when dial fails targeting localhost:1. Validates error wrapping without a real SMTP server.

Minor Nit

generateEnvContent now validates EMAIL_FROM as required for SMTP, but the generated .env template does not include an EMAIL_FROM= placeholder in the SMTP section. Users will hit the validator without knowing what to add. A commented placeholder would improve onboarding.

The email.go.tmpl deletion is correct, good cleanup.

Summary: Solid implementation. The one actionable item is port range validation + a test for it. Everything else is a suggestion rather than a blocker.

Code Review

Overall this is a clean, well-structured PR. The newController() refactor alone is a solid improvement, and the SMTP implementation is sensibly wrapped with good test coverage. A few things worth addressing:

Bug / Inconsistency: validateValues vs NewSMTPEmailSenderFromEnv disagree on required SMTP vars

commands/env.go:780 treats SMTP_USER and SMTP_PASS as required when EMAIL_PROVIDER=smtp. But pkg/email/factory.go treats SMTP_USER/SMTP_PASS as optional (reads them without validation). This means lvt env validate will reject a valid SMTP config that uses IAM role auth or an unauthenticated relay. The factory and the validator should agree on what is actually required.

Dead / misleading entry in getVarReason

commands/env.go:739 adds EMAIL_FROM_NAME with reason "sender display name (optional, not required)". EMAIL_FROM_NAME never appears in getRequiredVars(), so this map entry is unreachable dead code. Having "optional, not required" in a map whose purpose is to explain why things are required is also contradictory. This entry should be removed.

Security: TLSOpportunistic as the default

The comment in pkg/email/smtp.go explains the tradeoff well, but production users need a way to enforce strict TLS without editing generated code. A TLSPolicy field in SMTPConfig or an SMTP_TLS_POLICY env var in NewSMTPEmailSenderFromEnv would give them that control. At minimum, a note in the godoc that TLS is opportunistic by default would help users know to review this before deploying to production.

Minor: getRequiredVars vs validateValues asymmetry

getRequiredVars lists only EMAIL_PROVIDER for email features, but validateValues conditionally enforces 5 more SMTP vars. A user with EMAIL_PROVIDER=smtp and no SMTP_HOST set would pass the required-vars check but fail validation — a confusing UX. Worth documenting the two-phase approach or unifying the logic.

Nit: newController template comment

The comment tells users to replace NewConsoleEmailSender() with email.NewSMTPEmailSender(). Since NewEmailSenderFromEnv() is the intended future default, pointing users there post-release would be more consistent with the stated roadmap.

What is good

• newController() cleanly eliminates 6x boilerplate
• SMTPEmailSender is well-designed: per-connection is appropriate for low-volume auth email, 10s timeout default is sensible, errors are consistently wrapped with the "email:" prefix
• NewSMTPEmailSenderFromEnv validates early with clear messages; the port range check is a nice touch
• TLSOpportunistic + SMTPAuthAutoDiscover is the right default for broad SMTP compat with dev tools like Mailpit and MailHog
• Deleting email.go.tmpl is the right call; pkg/email/ is the correct source of truth
• Test coverage is solid; TestSMTPEmailSender_Send_DialError is a useful real integration smoke test