Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
70 changes: 70 additions & 0 deletions .changeset/mail-provider-options-honest.md
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,70 @@
---
"@objectstack/service-settings": minor
"@objectstack/plugin-email": minor
---

fix(service-settings,plugin-email): the mail provider dropdown lists only providers that actually deliver (#5094)

**Settings → Mail → Provider** offered `SMTP | SendGrid | Amazon SES | Postmark`.
`@objectstack/plugin-email` has never carried a SendGrid or an SES transport —
`makeTransport` knows `log` / `resend` / `postmark` / `smtp` and nothing else. So
selecting either of the two validated, saved, showed a success toast, and then
delivered no mail at all: the same declared-but-not-delivered gap #5087 closed
for SMTP, one field to the left.

The same field broke the invariant in the other direction at the same time:
**`resend` has shipped a working transport all along and was not on the list**,
so nobody could pick the one HTTP provider that worked.

**The dropdown is now `SMTP | Resend | Postmark | None (log only — no real
delivery)` — exactly the set `makeTransport` can build.** No email capability was
removed with SendGrid and SES. Both publish SMTP endpoints, and #5087 shipped a
real `SmtpTransport`, so both are configured today as `smtp`:

| provider | host | port | credentials |
|:---------|:-----|:-----|:------------|
| SendGrid | `smtp.sendgrid.net` | 587 | username `apikey`, password = your API key |
| Amazon SES | `email-smtp.<region>.amazonaws.com` | 587 | SES **SMTP credentials** (generated in the SES console — not your AWS access keys) |

The provider field's own description says this, so the migration is in front of
whoever goes looking for the option that disappeared.

`log` is listed rather than hidden. It is the one option that does not deliver —
but it does not pretend to: the label says so, `LogTransport` still records every
message to `sys_email`, and "Send test email" answers `ok: false` for it. That
gives an operator the deliberate, visible opt-out AGENTS.md asks a degradation to
be, instead of expressing "no outbound mail" as a half-filled SMTP form. It is
also what makes *offered* and *deliverable* the same set rather than merely
overlapping — which is the property a test can hold.

**Already saved `sendgrid` or `ses`? Nothing breaks and nothing goes quiet.** The
stored value outlives the dropdown, so `applyMailSettings` now recognises it
explicitly: the previous transport is kept (a settings row written by an older
release must never fail a boot), and the server logs at `error` with both halves
AGENTS.md requires — the consequence (*no mail is delivered through it*) and the
fix (the SMTP settings above), not a bare "unknown provider". It is checked
*before* the API-key check, because "set an API key" is the wrong instruction for
a provider that has nothing to hand a key to. "Send test email" refuses the same
way and sends nothing. Switching the provider to `smtp` and saving recovers the
transport without a restart.

Two smaller corrections in the same field:

- `api_key` is now shown and required for exactly `resend` and `postmark`
(`provider === 'resend' || provider === 'postmark'`). It was `provider !==
'smtp'`, which only worked because every non-SMTP option happened to be an
HTTP API; `required` is enforced server-side wherever the field is visible, so
that expression would have refused to save "None (log only)" until an API key
it never reads had been typed in.
- The built-in `mail/test` fallback (the one that runs when no email plugin is
mounted) rejects any `provider` outside the manifest's own option list instead
of answering "the form is well-formed".

**Held by a test, in both directions.** `EMAIL_TRANSPORT_PROVIDERS` is now a
runtime array (the `EmailTransportProvider` union is derived from it), and
`plugin-email`'s `mail-manifest-providers.contract.test.ts` asserts set equality
between it and the manifest's option values, then builds a real transport for
each. Adding an option without a transport fails; adding a transport without an
option fails. `RETIRED_EMAIL_PROVIDERS` / `isEmailTransportProvider` /
`unsupportedProviderFix` are exported alongside it for hosts that surface the
same guidance.
1 change: 1 addition & 0 deletions packages/plugins/plugin-email/package.json
Original file line numberDiff line numberDiff line change
Expand Up@@ -25,6 +25,7 @@
"nodemailer": "^9.0.3"
},
"devDependencies": {
"@objectstack/service-settings": "workspace:*",
"@types/node": "^26.1.2",
"@types/nodemailer": "^8.0.1",
"typescript": "^6.0.3",
Expand Down
128 changes: 128 additions & 0 deletions packages/plugins/plugin-email/src/email-plugin.mail-settings.test.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -15,6 +15,7 @@ import { describe, it, expect, vi, beforeEach } from 'vitest';
import { EmailServicePlugin } from './email-plugin.js';
import { EmailService, LogTransport } from './email-service.js';
import { SmtpTransport } from './transports/smtp.js';
import { ResendTransport } from './transports/resend.js';

const nm = vi.hoisted(() => ({ createTransport: vi.fn(), sendMail: vi.fn() }));
vi.mock('nodemailer', () => ({
Expand DownExpand Up@@ -221,6 +222,110 @@ describe('applyMailSettings — provider=smtp that cannot be built', () => {
});
});

// ── stored provider values with no transport (#5094) ───────────────────────
//
// `sendgrid` and `ses` sat in the settings dropdown for several releases with
// no transport behind either. #5094 removed the options; it cannot remove the
// rows. A workspace that saved one still resolves `provider: 'sendgrid'` on
// every boot, so this is the one part of that change with live data behind it:
// the read must not throw, must not quietly look configured, and must say what
// to do — SendGrid and SES both publish SMTP endpoints, which is now the route.

describe('applyMailSettings — a stored provider this build cannot deliver with', () => {
const RETIRED: Array<[string, RegExp]> = [
['sendgrid', /smtp\.sendgrid\.net/],
['ses', /email-smtp\.<region>\.amazonaws\.com/],
];

for (const [provider, migration] of RETIRED) {
it(`keeps the transport, never throws, and names the SMTP migration for provider=${provider}`, async () => {
const { service, ctx } = await boot({
provider: { value: provider, source: 'global' },
api_key: { value: 'legacy-key', source: 'global' },
});

// A settings row written by an older release must never be able to kill
// a running server: previous transport kept, boot completed.
expect(transportOf(service)).toBeInstanceOf(LogTransport);

expect(ctx.logger.error).toHaveBeenCalledTimes(1);
const line = ctx.logger.error.mock.calls[0][0] as string;
expect(line).toContain(`provider='${provider}'`);
// Consequence…
expect(line).toMatch(/NO mail is delivered through it/);
// …and the fix, in the same line (AGENTS.md degradation-log-level).
expect(line).toMatch(/Fix:/);
expect(line).toMatch(migration);
});
}

it('reports the missing transport, not a missing api_key, when both are absent', async () => {
// "Set an API key" is the wrong instruction for a provider that has
// nothing to hand the key to — so the unsupported-provider check runs
// before the api_key check.
const { ctx } = await boot({ provider: { value: 'sendgrid', source: 'global' } });
const line = ctx.logger.error.mock.calls[0][0] as string;
expect(line).not.toMatch(/api_key is empty/);
expect(line).toMatch(/smtp\.sendgrid\.net/);
});

it('leaves a working boot-configured SMTP transport in place', async () => {
// Mail may well still be going out (OS_EMAIL_SMTP_* configured the
// transport at boot). The stored provider is still unusable and the
// operator still has to fix it, so it is reported — but nothing that
// currently delivers is torn down on the way.
const { service, ctx } = await boot(
{ provider: { value: 'ses', source: 'global' }, api_key: { value: 'k', source: 'global' } },
{ provider: 'smtp', providerOptions: { host: 'smtp.boot.test' } },
);
expect((transportOf(service) as SmtpTransport).describe()).toMatchObject({ host: 'smtp.boot.test' });
expect(ctx.logger.error).toHaveBeenCalledTimes(1);
});

it('treats a typo the same way, naming the providers that do work', async () => {
const { ctx } = await boot({
provider: { value: 'postmarkk', source: 'global' },
api_key: { value: 'k', source: 'global' },
});
const line = ctx.logger.error.mock.calls[0][0] as string;
expect(line).toMatch(/log \/ resend \/ postmark \/ smtp/);
});

it('recovers on the next save — the bad value is not sticky', async () => {
const { service, settings } = await boot({
provider: { value: 'sendgrid', source: 'global' },
api_key: { value: 'legacy-key', source: 'global' },
});
expect(transportOf(service)).toBeInstanceOf(LogTransport);

await settings.save({
provider: { value: 'smtp', source: 'global' },
smtp_host: { value: 'smtp.sendgrid.net', source: 'global' },
smtp_user: { value: 'apikey', source: 'global' },
smtp_password: { value: 'legacy-key', source: 'global' },
});

expect((transportOf(service) as SmtpTransport).describe()).toMatchObject({
host: 'smtp.sendgrid.net',
auth: { user: 'apikey' },
});
});
});

describe('applyMailSettings — provider=resend', () => {
it('builds the transport the settings page can finally select', async () => {
// The reverse half of the same invariant: `resend` had a working transport
// all along and was missing from the dropdown (#5094). Now that it can be
// picked, prove picking it does something.
const { service, ctx } = await boot({
provider: { value: 'resend', source: 'global' },
api_key: { value: 're_live_key', source: 'global' },
});
expect(transportOf(service)).toBeInstanceOf(ResendTransport);
expect(ctx.logger.error).not.toHaveBeenCalled();
});
});

describe('EmailServicePlugin constructor path (CLI / os serve)', () => {
it('THROWS when provider=smtp has no host — a boot that cannot deliver fails loudly', async () => {
const ctx = fakeCtx({ manifest: { register: () => {} } });
Expand DownExpand Up@@ -290,6 +395,29 @@ describe('mail/test action', () => {
expect(nm.sendMail).not.toHaveBeenCalled();
});

it.each(['sendgrid', 'ses'])(
'refuses to "test" a stored provider=%s and points at SMTP instead',
async (provider) => {
const { settings } = await boot({
provider: { value: provider, source: 'global' },
api_key: { value: 'legacy-key', source: 'global' },
});
const result = await settings.action('test')!({
values: { provider, api_key: 'legacy-key', from_email: 'no-reply@example.test' },
payload: { to: 'admin@example.test' },
});

expect(result).toMatchObject({ ok: false, severity: 'error' });
// Not "Failed to build sendgrid transport: unknown provider" — the
// operator needs the route that works, not the internal symptom.
expect(result.message).toMatch(provider === 'sendgrid' ? /smtp\.sendgrid\.net/ : /email-smtp/);
expect(result.message).toMatch(/NOTHING was sent/);
expect(nm.sendMail).not.toHaveBeenCalled();
// ...and it does not ask for an API key it cannot use.
expect(result.message).not.toMatch(/api_key is required/);
},
);

it('never reports success while only the LogTransport is active', async () => {
const { settings } = await boot({ provider: { value: 'log', source: 'global' } });
const result = await settings.action('test')!({
Expand Down
39 changes: 37 additions & 2 deletions packages/plugins/plugin-email/src/email-plugin.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -13,6 +13,8 @@ import {
makeTransport,
SmtpTransport,
smtpOptionsFromMailSettings,
isEmailTransportProvider,
unsupportedProviderFix,
type EmailTransportProvider,
} from './transports/index.js';
import { BUILTIN_AUTH_TEMPLATES } from './templates/auth-templates.js';
Expand DownExpand Up@@ -262,12 +264,24 @@ export class EmailServicePlugin implements Plugin {
return { ok: false, severity: 'error', message: `Failed to build SMTP transport: ${err?.message ?? String(err)}` };
}
} else if (provider !== 'log') {
// A provider with no transport behind it — a value stored while
// the settings page still offered SendGrid / Amazon SES (#5094).
// Refuse before asking for an API key: nothing here can use one.
if (!isEmailTransportProvider(provider)) {
return {
ok: false,
severity: 'error',
message: `provider='${provider}' is not a provider this server can deliver with, so NOTHING was `
+ 'sent (and nothing has been sent through it since it was saved). Fix: '
+ unsupportedProviderFix(provider),
};
}
if (!apiKey) {
return { ok: false, severity: 'error', message: `${provider}: api_key is required.` };
}
try {
const transport = makeTransport({
provider: provider as 'resend' | 'postmark',
provider,
apiKey,
logger: ctx.logger,
});
Expand DownExpand Up@@ -567,6 +581,12 @@ export class EmailServicePlugin implements Plugin {
* still applied.
* - `provider = 'resend' | 'postmark'` rebuilds the transport using
* `api_key` from settings.
* - anything else — including `sendgrid` / `ses`, which the settings page
* offered for several releases without a transport behind either (#5094)
* and which persisted workspaces still resolve — keeps the previous
* transport and reports at `error` with the SMTP migration that replaces
* it. A settings value written by an older release must not be able to
* stop a server from booting, and must not be able to look configured.
*
* **This path never throws.** A settings save must not be able to kill a
* running server, so a transport that cannot be built leaves the previous
Expand DownExpand Up@@ -649,6 +669,21 @@ export class EmailServicePlugin implements Plugin {
return;
}

// A stored provider this build cannot deliver with — checked BEFORE the
// api_key branch, because "set an API key" is the wrong instruction for a
// provider that has no transport to hand the key to. Same shape as every
// other failure here: previous transport kept, error naming the consequence
// and the fix, no throw. Workspaces that saved `sendgrid` / `ses` while the
// settings page still offered them arrive here on every boot (#5094).
if (!isEmailTransportProvider(provider)) {
ctx.logger.error(
`EmailServicePlugin: provider='${provider}' is not a provider this server can deliver with — the `
+ 'previous transport is kept and NO mail is delivered through it. Fix: '
+ unsupportedProviderFix(provider),
);
return;
}

const apiKey = typeof values.api_key === 'string' ? values.api_key : undefined;
if (!apiKey) {
ctx.logger.error(
Expand All@@ -660,7 +695,7 @@ export class EmailServicePlugin implements Plugin {

try {
const transport = makeTransport({
provider: provider as 'resend' | 'postmark',
provider,
apiKey,
logger: ctx.logger,
});
Expand Down
16 changes: 13 additions & 3 deletions packages/plugins/plugin-email/src/index.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -4,9 +4,14 @@
* @objectstack/plugin-email
*
* Outbound email delivery for ObjectStack. Registers an `IEmailService`
* implementation backed by a pluggable `IEmailTransport` (SMTP via
* nodemailer, SendGrid, Resend, SES, …) and persists each attempt to
* the `sys_email` system object for audit / activity-stream display.
* implementation backed by a pluggable `IEmailTransport` — SMTP via
* nodemailer, Resend, Postmark — and persists each attempt to the
* `sys_email` system object for audit / activity-stream display.
*
* The list above is exhaustive on purpose: it used to read "SendGrid, …",
* which no transport here has ever implemented (#5094). SendGrid and Amazon
* SES are delivered through `SmtpTransport` against their published SMTP
* endpoints. `EMAIL_TRANSPORT_PROVIDERS` is the machine-readable form.
*/

export { EmailServicePlugin } from './email-plugin.js';
Expand All@@ -20,6 +25,11 @@ export {
SmtpTransport,
makeTransport,
smtpOptionsFromMailSettings,
EMAIL_TRANSPORT_PROVIDERS,
RETIRED_EMAIL_PROVIDERS,
isEmailTransportProvider,
retiredProviderGuidance,
unsupportedProviderFix,
type ResendTransportOptions,
type PostmarkTransportOptions,
type SmtpTransportOptions,
Expand Down
Loading
Loading