Skip to content

Added email provider adapter wiring - #29553

Open
wakqasahmed wants to merge 4 commits into
TryGhost:mainfrom
wakqasahmed:feature/issue-5-email-adapter-wiring
Open

Added email provider adapter wiring#29553
wakqasahmed wants to merge 4 commits into
TryGhost:mainfrom
wakqasahmed:feature/issue-5-email-adapter-wiring

Conversation

@wakqasahmed

@wakqasahmed wakqasahmed commented Jul 23, 2026

Copy link
Copy Markdown
Contributor

What does this PR do?

Adds the minimal email-provider adapter mechanism discussed in #25367. Ghost
continues to use Mailgun unless adapters.email is configured. When it is
configured, EmailServiceWrapper resolves the provider through
AdapterManager.

This unlocks community-maintained email providers without bundling a provider,
analytics integration, or Admin UI changes in Ghost core. The first intended
consumer is ghost-ses-email-adapter.

Why is this needed?

The existing AdapterManager can discover third-party adapters, but the email
service currently instantiates MailgunEmailProvider directly. In the
maintainer discussion on #25367, Ghost indicated that a generic adapter
mechanism is the preferred direction for self-hosted providers. This PR is
limited to that mechanism and leaves the default Mailgun behavior unchanged.

How to test

Ghost 6 / main (this PR)

  1. Check out this branch from a fork of TryGhost/Ghost, then run pnpm setup
    from the repository root.

  2. Install the adapter dependencies in the sibling checkout. The Compose
    overlay bind-mounts that checkout into the Ghost container, so a host-only
    symlink must not be used:

    cd ../ghost-ses-email-adapter
    npm ci
  3. From the Ghost repository root, start the Docker development stack with the
    adapter overlay. SES_ADAPTER_PATH is configurable and defaults to the
    sibling ../ghost-ses-email-adapter checkout:

    SES_ADAPTER_PATH=../ghost-ses-email-adapter \
    SES_FROM_EMAIL=news@example.com \
    DEV_COMPOSE_FILES='-f compose.dev.ses.yaml' \
    pnpm dev

    SES_FROM_EMAIL must be a verified SES sender. Set SES_REGION as needed
    and supply AWS credentials through the SDK default credential provider
    chain; do not commit credentials or local configuration.

  4. In a second terminal, verify the service is healthy rather than merely
    started:

    SES_ADAPTER_PATH=../ghost-ses-email-adapter \
    SES_FROM_EMAIL=news@example.com \
    docker compose -f compose.dev.yaml -f compose.dev.ses.yaml up -d --wait
    docker logs ghost-dev

    The container must remain running and pass its health check. The logs must
    not contain an Unable to find email adapter ses configuration error.

  5. Open
    http://localhost:2368/ghost, create a test newsletter, and send a test
    email. Confirm that the send is accepted by SES.

  6. Stop the overlay, then run Ghost without DEV_COMPOSE_FILES and repeat the
    newsletter test to confirm the default Mailgun provider is still used.

  7. Run the focused automated test:

    cd ghost/core
    pnpm test:single test/unit/server/services/email-service/email-service-wrapper.test.js

The test covers the default Mailgun path and configured-adapter resolution.

Ghost 5 (not covered by this PR)

This PR targets main and does not add Ghost 5 support.
For a browser smoke test of the existing Ghost 5 interim integration, use that
repository's ghost-5.x-email-adapter-wiring.patch against the exact Ghost 5
runtime, install the adapter as ses, configure adapters.email, restart
Ghost, and test a newsletter at /ghost. Before doing so, run its disposable
check:

cd ../ghost-ses-email-adapter
test/integration/ghost-5.sh

That check validates the interim patch and adapter wiring only; it is not
evidence that this Ghost 6/main PR applies to Ghost 5.

Checklist

  • I've read and followed the Contributor Guide.
  • I've explained my change.
  • I've written an automated test to prove the default and custom-provider paths.

@coderabbitai

coderabbitai Bot commented Jul 23, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: 3da0681d-e8d5-48ee-8ded-679463492d67

📥 Commits

Reviewing files that changed from the base of the PR and between 11f63f2 and 6ab86a8.

📒 Files selected for processing (1)
  • ghost/core/core/server/services/email-service/email-service-wrapper.js
🚧 Files skipped from review as they are similar to previous changes (1)
  • ghost/core/core/server/services/email-service/email-service-wrapper.js

Walkthrough

Adds an EmailProviderBase contract, registers it for email adapters, and updates EmailServiceWrapper to select a configured adapter or fall back to Mailgun. The selected provider is passed to SendingService. Tests cover both selection paths, and a Compose override configures local SES development.

Possibly related PRs

  • TryGhost/Ghost#29430: Updates adapter-manager import and export wiring in the same module that registers the email provider base class.
🚥 Pre-merge checks | ✅ 4
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title summarizes the new email adapter wiring and is clearly related to the main change.
Description check ✅ Passed The description matches the adapter wiring change and default Mailgun fallback behavior.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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.

❤️ Share

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

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@ghost/core/core/server/adapters/email/email-provider-base.js`:
- Around line 7-8: Update the email-provider contract in the constructor’s
requiredFns definition to include getMaximumRecipients and
getTargetDeliveryWindow alongside send, ensuring AdapterManager rejects
providers that omit methods used by SendingService.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: f00efc6d-1f90-46c3-86ac-4f91c991672f

📥 Commits

Reviewing files that changed from the base of the PR and between 90ea162 and f712a13.

📒 Files selected for processing (4)
  • ghost/core/core/server/adapters/email/email-provider-base.js
  • ghost/core/core/server/services/adapter-manager/index.ts
  • ghost/core/core/server/services/email-service/email-service-wrapper.js
  • ghost/core/test/unit/server/services/email-service/email-service-wrapper.test.js

Comment thread ghost/core/core/server/adapters/email/email-provider-base.js Outdated

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@compose.dev.ses.yaml`:
- Line 15: Update the adapters__email__ses__fromEmail configuration in
compose.dev.ses.yaml to remove the newsletter@example.test fallback; require
SES_FROM_EMAIL explicitly or replace the fallback with a verified development
sender address.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: 7f9c20ef-c64d-4393-8756-e95a3c84e148

📥 Commits

Reviewing files that changed from the base of the PR and between f712a13 and 7406034.

📒 Files selected for processing (1)
  • compose.dev.ses.yaml

Comment thread compose.dev.ses.yaml Outdated
Allows community email providers while preserving Mailgun as the default.
Mounts the local adapter checkout at Ghost's container discovery path.
Rejects adapters that omit methods required by newsletter sending.
@wakqasahmed

Copy link
Copy Markdown
Contributor Author

@acburdine, since you've been driving most of the recent adapter-manager work, tagging you for a look.

This PR adds a minimal email adapter type to AdapterManager, following the same pattern already used for storage/scheduling/sso/cache/redirects/route-settings. EmailServiceWrapper resolves the configured provider through the adapter manager when adapters.email is set, and falls back to MailgunEmailProvider unmodified otherwise. The default path is unchanged, and no core behavior is touched unless someone opts in.

Scope is intentionally small:

5 files, ~90 lines, one new required-fns contract in EmailProviderBase, and a unit test covering both the default and configured-adapter paths. No Admin UI, no bundled providers.

Why this is worth doing beyond just this one PR:

right now MailgunEmailProvider is the only bulk-email path, it's instantiated directly in EmailServiceWrapper rather than resolved through inversion of control, which is inconsistent with every other adapter-backed subsystem in core. That's a real ceiling for self-hosters: anyone who can't or won't use Mailgun (EU-data-residency requirements, existing SES/Postmark/Brevo relationships, or fully self-hosted setups on something like Postal) currently has no supported path at all.

Opening this one seam doesn't commit the core team to maintaining any of those providers, it lets the community carry that weight the same way it already does for storage and SSO adapters, while removing a real adoption blocker for self-hosters who'd otherwise have to fork core or skip newsletters entirely. First consumer is a standalone package ghost-ses-email-adapter; happy to help make sure the contract is generic enough for Postmark/Brevo/Postal to build on it too.

Context:

this follows the maintainer discussion in #25367, where the direction landed on "adapter mechanism in core, providers maintained externally" rather than bundling SES/Postmark/etc. into Ghost.

One thing I can't do myself: CI (ci.yml, tinybird.yml) is sitting at action_required since I'm an external contributor on a fork, would appreciate an "Approve and run workflows" when you get a chance, so the required checks can actually run.

Happy to adjust the approach if there's a preferred shape for this.

@acburdine

Copy link
Copy Markdown
Member

hey @wakqasahmed, thanks for the effort here 😄

it's worth reading through this comment on a similar PR: #28247 (comment). At the moment, the main blocker to a standardized email adapter pattern isn't the email sending bits itself, it's the analytics/suppression bits that have proven to be more complex. At the moment, Ghost polls Mailgun's Events API for that analytics/suppression data, and we'd need to refactor some stuff in Ghost internally to allow for webhooks to be utilized instead in order to unblock full adapter support. We are looking into it, but we'd likely want to get that solved first before merging any adapter work 😕

@9larsons

Copy link
Copy Markdown
Contributor

Throwing the blocked label on this. This is an area of active discussion and we want to make sure we get this right, so the core team will likely take this over or adapt contributions.

@wakqasahmed

wakqasahmed commented Jul 28, 2026

Copy link
Copy Markdown
Contributor Author

@acburdine @9larsons @belatedly

I specifically hit the same blocker as #28247 (comment) and had to go to alternate like SES and found out there was a half baked adapter available in PRs, hence I completed it at https://www.github.com/wakqasahmed/ghost-ses-email-adapter and while doing so, I found out why so many are blocked at this gate.

I totally understand the limitation and would love to help getting this blocker out. Let me (and others) help get this through fast, please.

@wakqasahmed

Copy link
Copy Markdown
Contributor Author

@acburdine @9larsons @belatedly would appreciate a suggested way forward and how can I be of help in this. Thanks

@wakqasahmed

Copy link
Copy Markdown
Contributor Author

Filed #29828 to track the webhook-based analytics/suppression ingestion path @acburdine and @9larsons flagged above as the actual blocker here (and on #28247). Scoped it as additive to the existing Mailgun poll loop, per @9larsons' "send-only seam, edges documented, scheduler untouched" framing on #28247, so it's a standalone reviewable unit of work rather than something that has to land inside this PR. Happy to help prototype it against ghost-ses-email-adapter if useful.

@wakqasahmed

Copy link
Copy Markdown
Contributor Author

For visibility: opened a draft prototype of the webhook ingestion path from #29828 at #29829. It composes with this PR's adapter-manager/EmailProviderBase wiring rather than duplicating it.

wakqasahmed added a commit to wakqasahmed/Ghost that referenced this pull request Aug 7, 2026
ref TryGhost#29828

Additive to the existing Mailgun poll loop, per the send-only-seam /
scheduler-untouched framing from TryGhost#28247 and TryGhost#29553:

- adapters/email/email-provider-base.js gains optional
  verifyWebhookRequest()/parseWebhookEvents(), defaulting to
  unsupported rather than throwing, so Mailgun and any adapter that
  only implements send() are unaffected.
- email-analytics-webhook-controller.js normalizes an adapter's
  webhook events and forwards them to the same EmailEventProcessor
  the poll loop already writes through, so analytics and the
  suppression list converge on one write path regardless of which
  ingestion mechanism produced the event.
- Wired as POST /webhooks/email-analytics on the members app,
  mirroring the existing Stripe webhook route.
- adapter-manager now registers an 'email' base class (also proposed
  standalone in TryGhost#29553), so this can compose with that PR instead of
  duplicating it.

This is a prototype attached to the design discussion in TryGhost#29828, not
a claim on the final shape - posted for concrete review, not as a
finished implementation.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants