Swap sendgrid by SMTP#4551
Conversation
dosubot
Bot
added
the
size:L
|
|
dosubot
Bot
added
Backend
documentation
|
@Shunmuka is attempting to deploy a commit to the agenta projects Team on Vercel. A member of the Team first needs to authorize it. |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (3)
🚧 Files skipped from review as they are similar to previous changes (3)
📝 WalkthroughSummary by CodeRabbit
WalkthroughAdds SMTP email delivery support (config, env wiring), refactors email_service to prefer SMTP then SendGrid, integrates SMTP with SuperTokens passwordless, updates service sender selection and entrypoint detection, and adds tests and docs. ChangesSMTP Email Delivery Feature
Sequence Diagram(s)sequenceDiagram
participant Client
participant EmailService
participant SMTPImpl
participant SendGridImpl
Client->>EmailService: send_email(to, subject, html, from)
alt env.smtp.enabled == true
EmailService->>SMTPImpl: _send_smtp_email(...)
SMTPImpl->>SMTPImpl: build EmailMessage
alt env.smtp.use_ssl == true
SMTPImpl->>SMTPImpl: connect via SMTP_SSL
else env.smtp.use_tls == true
SMTPImpl->>SMTPImpl: connect SMTP then starttls()
else
SMTPImpl->>SMTPImpl: connect plain SMTP
end
SMTPImpl->>SMTPImpl: optional login + send_message
SMTPImpl-->>EmailService: True / raises HTTPException(500)
else env.sendgrid.enabled == true
EmailService->>SendGridImpl: _send_sendgrid_email(...)
SendGridImpl-->>EmailService: bool
else
EmailService->>EmailService: log disabled -> return True
end
EmailService-->>Client: bool result
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
Adds first-class SMTP support for email delivery (preferred over SendGrid), updates env/docs, and ensures auth/email flows enable OTP when at least one email provider is fully configured.
Changes:
- Add SMTP configuration and provider selection logic (SMTP preferred, SendGrid fallback) across web entrypoint and API services.
- Update self-host docs and docker-compose example env files to include SMTP variables and updated migration guidance.
- Add unit tests covering entrypoint env inference, email service provider selection, and SuperTokens passwordless SMTP delivery.
Reviewed changes
Copilot reviewed 16 out of 16 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| web/entrypoint.sh | Infers SMTP + overall email-delivery enablement and uses it to select OTP vs password auth. |
| hosting/docker-compose/oss/env.oss.gh.example | Documents SMTP env vars and notes SendGrid fallback behavior. |
| hosting/docker-compose/oss/env.oss.dev.example | Documents SMTP env vars and notes SendGrid fallback behavior. |
| hosting/docker-compose/ee/env.ee.gh.example | Documents SMTP env vars and notes SendGrid fallback behavior. |
| hosting/docker-compose/ee/env.ee.dev.example | Documents SMTP env vars and notes SendGrid fallback behavior. |
| docs/docs/self-host/upgrades/v0.100.3-migration.mdx | Updates migration mapping to prefer SMTP_FROM_EMAIL with SendGrid fallback. |
| docs/docs/self-host/02-configuration.mdx | Adds SMTP configuration section and updates env-var mapping guidance accordingly. |
| api/oss/tests/pytest/unit/test_web_entrypoint_email_env.py | Tests entrypoint inference for OTP enablement with SMTP/SendGrid/incomplete SMTP. |
| api/oss/tests/pytest/unit/test_email_service.py | Tests SMTP sending behavior, provider preference, fallback to SendGrid, and auth method detection. |
| api/oss/tests/pytest/unit/auth/test_supertokens_config.py | Tests SuperTokens passwordless email delivery uses SMTP when enabled. |
| api/oss/src/utils/env.py | Introduces SmtpConfig and updates auth email method selection to include SMTP. |
| api/oss/src/services/user_service.py | Sends password reset emails when SMTP or SendGrid is configured; chooses correct sender. |
| api/oss/src/services/organization_service.py | Sends invitation emails when SMTP or SendGrid is configured; chooses correct sender. |
| api/oss/src/services/email_service.py | Implements SMTP sending + provider routing (SMTP preferred), adds provider init logging. |
| api/oss/src/core/auth/supertokens/config.py | Configures SuperTokens passwordless email delivery to use SMTP when enabled. |
| api/ee/src/services/organization_service.py | Aligns EE org invitation email gating with SMTP-or-SendGrid delivery availability. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| if (env.smtp.host or env.smtp.port) and not env.smtp.from_email: | ||
| log.warn("✗ SMTP disabled: missing sender email address") | ||
| else: | ||
| log.warn("✗ SMTP disabled") |
There was a problem hiding this comment.
Updated — replaced the deprecated log.warn calls with log.warning.
| async def _send_smtp_email( | ||
| to_email: str, subject: str, html_content: str, from_email: str | ||
| ) -> bool: |
There was a problem hiding this comment.
Updated — moved the blocking smtplib SMTP work into a synchronous helper and call it through asyncio.to_thread(...) from the async _send_smtp_email function, so SMTP connect/login/send no longer runs directly on the event loop.
| try: | ||
| if env.smtp.use_ssl: | ||
| with smtplib.SMTP_SSL( | ||
| env.smtp.host, | ||
| env.smtp.port, | ||
| context=context, | ||
| ) as smtp: | ||
| if env.smtp.username: | ||
| smtp.login(env.smtp.username, env.smtp.password) | ||
| smtp.send_message(message) | ||
| else: | ||
| with smtplib.SMTP(env.smtp.host, env.smtp.port) as smtp: | ||
| if env.smtp.use_tls: | ||
| smtp.starttls(context=context) | ||
| if env.smtp.username: | ||
| smtp.login(env.smtp.username, env.smtp.password) | ||
| smtp.send_message(message) | ||
| return True |
There was a problem hiding this comment.
Updated — moved the blocking smtplib connect/login/send logic into a synchronous helper and call it from _send_smtp_email using asyncio.to_thread(...). This keeps the async interface while avoiding direct blocking SMTP I/O on the event loop.
| except Exception as e: | ||
| raise HTTPException(status_code=500, detail=str(e)) |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
api/ee/src/services/organization_service.py (1)
107-127:⚠️ Potential issue | 🔴 Critical | 🏗️ Heavy liftCritical: Missing dynamic sender email selection logic.
The EE version updates the email delivery enablement guard (line 108) to check both SMTP and SendGrid, but line 122 still hardcodes
"account@hello.agenta.ai"as the sender. This is inconsistent with the OSS implementation and will break SMTP email delivery.When SMTP is enabled, the guard clause will pass, but
email_service.send_emailwill attempt to use the hardcoded sender address instead ofenv.smtp.from_email, causing SMTP authentication or sender validation failures.🔧 Proposed fix to add dynamic sender selection
Apply the same pattern used in the OSS version:
html_content = html_template.format( username_placeholder=user.username, action_placeholder="invited you to join", workspace_placeholder=organization.name, call_to_action=( "Click the link below to accept the invitation:</p><br>" f'<a href="{invite_link}">Accept Invitation</a>' ), ) + from_email = env.smtp.from_email if env.smtp.enabled else env.sendgrid.from_address + if not from_email: + raise ValueError("Email delivery requires a sender email address to work.") + await email_service.send_email( - from_email="account@hello.agenta.ai", + from_email=from_email, to_email=email, subject=f"{user.username} invited you to join {organization.name}", html_content=html_content, )
🧹 Nitpick comments (3)
api/oss/src/services/organization_service.py (1)
172-174: ⚡ Quick winConsider providing more specific error context.
The error message could be more actionable by specifying which environment variable is missing. When SMTP is enabled but
env.smtp.from_emailis missing, users would benefit from knowing to setSMTP_FROM_EMAIL. Similarly for SendGrid'sSENDGRID_FROM_ADDRESS.📝 Proposed enhancement for error message
- if not from_email: - raise ValueError("Email delivery requires a sender email address to work.") + if not from_email: + provider = "SMTP" if env.smtp.enabled else "SendGrid" + var_name = "SMTP_FROM_EMAIL" if env.smtp.enabled else "SENDGRID_FROM_ADDRESS" + raise ValueError( + f"Email delivery via {provider} requires a sender email address. " + f"Please set the {var_name} environment variable." + )api/oss/src/services/user_service.py (1)
162-164: ⚡ Quick winConsider providing more specific error context.
Same suggestion as in
organization_service.py: the error message could specify which environment variable (SMTP_FROM_EMAILorSENDGRID_FROM_ADDRESS) needs to be set based on the enabled provider.📝 Proposed enhancement for error message
- if not from_email: - raise ValueError("Email delivery requires a sender email address to work.") + if not from_email: + provider = "SMTP" if env.smtp.enabled else "SendGrid" + var_name = "SMTP_FROM_EMAIL" if env.smtp.enabled else "SENDGRID_FROM_ADDRESS" + raise ValueError( + f"Email delivery via {provider} requires a sender email address. " + f"Please set the {var_name} environment variable." + )api/oss/tests/pytest/unit/test_web_entrypoint_email_env.py (1)
57-68: ⚡ Quick winConsider adding test coverage for edge cases.
The current tests validate SMTP-only, SendGrid-only, and incomplete SMTP configurations. Consider adding test cases for:
- Both SMTP and SendGrid enabled: Verify priority and expected behavior when both providers are configured.
- Neither provider enabled: Verify fallback to password auth with OTP disabled.
- SMTP fallback chain: Test the
SMTP_FROM_EMAILfallback toAGENTA_AUTHN_EMAIL_FROMandAGENTA_SEND_EMAIL_FROM_ADDRESS.These additional cases would provide comprehensive coverage of the entrypoint's email delivery derivation logic.
📋 Example test cases
def test_entrypoint_both_smtp_and_sendgrid_enabled(tmp_path): """Verify behavior when both SMTP and SendGrid are configured.""" env_js = _run_entrypoint( tmp_path, REPO_ROOT, { "SMTP_HOST": "host.docker.internal", "SMTP_PORT": "1025", "SMTP_FROM_EMAIL": "smtp@example.com", "SENDGRID_API_KEY": "sg-key", "SENDGRID_FROM_ADDRESS": "sendgrid@example.com", }, ) assert 'NEXT_PUBLIC_AGENTA_SENDGRID_ENABLED: "true"' in env_js assert 'NEXT_PUBLIC_AGENTA_AUTHN_EMAIL: "otp"' in env_js assert 'NEXT_PUBLIC_AGENTA_AUTH_EMAIL_ENABLED: "true"' in env_js def test_entrypoint_no_email_delivery_configured(tmp_path): """Verify fallback when neither SMTP nor SendGrid is configured.""" env_js = _run_entrypoint( tmp_path, REPO_ROOT, {}, ) assert 'NEXT_PUBLIC_AGENTA_SENDGRID_ENABLED: "false"' in env_js assert 'NEXT_PUBLIC_AGENTA_AUTHN_EMAIL: "password"' in env_js assert 'NEXT_PUBLIC_AGENTA_AUTH_EMAIL_ENABLED: "true"' in env_js def test_entrypoint_smtp_from_email_fallback(tmp_path): """Verify SMTP_FROM_EMAIL fallback chain.""" env_js = _run_entrypoint( tmp_path, REPO_ROOT, { "SMTP_HOST": "host.docker.internal", "SMTP_PORT": "1025", "AGENTA_AUTHN_EMAIL_FROM": "fallback@example.com", }, ) assert 'NEXT_PUBLIC_AGENTA_AUTHN_EMAIL: "otp"' in env_js assert 'NEXT_PUBLIC_AGENTA_AUTH_EMAIL_ENABLED: "true"' in env_js
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 413a0711-287d-43b5-bae7-10a54dc02295
📒 Files selected for processing (16)
api/ee/src/services/organization_service.pyapi/oss/src/core/auth/supertokens/config.pyapi/oss/src/services/email_service.pyapi/oss/src/services/organization_service.pyapi/oss/src/services/user_service.pyapi/oss/src/utils/env.pyapi/oss/tests/pytest/unit/auth/test_supertokens_config.pyapi/oss/tests/pytest/unit/test_email_service.pyapi/oss/tests/pytest/unit/test_web_entrypoint_email_env.pydocs/docs/self-host/02-configuration.mdxdocs/docs/self-host/upgrades/v0.100.3-migration.mdxhosting/docker-compose/ee/env.ee.dev.examplehosting/docker-compose/ee/env.ee.gh.examplehosting/docker-compose/oss/env.oss.dev.examplehosting/docker-compose/oss/env.oss.gh.exampleweb/entrypoint.sh
|
Thanks @Shunmuka for the PR with clear description and the video. We'll look at it and review it soon. In the mean time, can you please look at the AI review and address it. |
|
Thank you for the response. @mmabrouk I finished looking over and addressing the AI review! |
Summary
This PR migrates email delivery toward SMTP while preserving SendGrid as a backward-compatible fallback for deployments that still use the existing SendGrid environment variables.
Previously, email delivery was tied to SendGrid-specific configuration. Since SendGrid also supports SMTP, and self-hosted users may want to use their own SMTP server, this change introduces SMTP as the preferred delivery path.
Changes included:
Added SMTP environment configuration in
api/oss/src/utils/env.py.Added strict SMTP enabled detection so SMTP is only considered configured when the required values are present:
Updated email provider selection so the order is:
Updated runtime/public environment derivation so SMTP-only deployments can enable email delivery/email auth without requiring SendGrid.
Updated passwordless/OTP email delivery to use SMTP configuration where needed.
Updated env example files and documentation with the new SMTP configuration values.
Preserved SendGrid fallback behavior for existing deployments using the old SendGrid variables.
Kept existing invitation and password-reset call sites unchanged.
This solves the issue by allowing self-hosted deployments to configure standard SMTP delivery while still supporting existing SendGrid-based setups.
Testing
Verified locally
I tested SMTP delivery locally using Mailpit as a local SMTP server.
Local SMTP configuration used:
Verified locally:
Added or updated tests
QA follow-up
Additional QA should verify:
Demo
Local demo video: https://youtu.be/LWlvUI6fNK8
The demo shows SMTP configured locally with SendGrid disabled, Mailpit running as the SMTP server, and an email successfully delivered to Mailpit.