Skip to content

Commit ede9e6b

Browse files
fix(service-messaging): the email channel answers unavailable while only the log transport is live (#30547)
<!-- claude-projects-attribution: {"github_login":"os-bill"} --> _Requested by **Bill** · [project thread](https://claude.ai/code/project/chan_016iMXWSZDzqWRWvWzdNo8wg?thread=cmsg_016iMXWSZDzqWRWvWzdNo8wgLiQTZYATct8Zv1iH2wZvyz)_ domain:server#6 · os-bill · session_01Utp6CXCS98uhsFQYk64epE ## 维护者速读 - 改了什么:没配发信方式(只有「写日志」的假通道)时,邮件渠道不再自称可用;通知不再为邮件写投递记录,而是记一条「未配置发信」的抑制原因;已写下的投递记录被拒发,不再标成「成功」。 - 为什么改:执行 cloud#2549 裁定 B 第 ③ 项。过去所有托管租户在管理员配好邮箱前,邮件通知都显示「已送达」,其实一封没发。 - 风险与回滚:只动 service-messaging 的邮件渠道一个文件;没有实现 isConfigured 的第三方邮件服务行为不变。回滚即还原本 PR 一次提交。 - 先看:`packages/services/service-messaging/src/email-channel.ts` 的 `emailTransportRefusal` 与 `isAvailable` / `send` 两处调用;其次是测试文件末尾新增的 describe。 - 两分钟验证:`pnpm --filter @objectstack/service-messaging exec vitest run --maxWorkers=2 src/email-channel.test.ts`,预期 47 个全绿,其中 `a LogTransport is no transport (#30510)` 下 5 个。 - 需要你做的:无,席位落地。 Fixes #30510 Clause-②: no Metadata-protocol: none Skills-docs: none — `git grep -n -i -c "logtransport|log transport|transport_not_configured|email channel|e-mail channel" -- 'skills/**'` has no hit (exit 1) ## What changes plugin-email registers the `email` service on every boot. With no `mail` settings saved, its live transport is the `LogTransport`: a send succeeds, writes `sys_email`, and no mail leaves the process. The `email` channel answered `isAvailable()` from "an email service is registered", so fan-out wrote a delivery row and the dispatcher marked it `success` while nobody was mailed. Every hosted tenant is in this state until its admin configures mail (objectstack-ai/cloud#2549, ruling B, item 3). - `email-channel.ts`: new `emailTransportRefusal(email)` asks the service's own deliverability answer, `IEmailService.isConfigured()` (already on the spec contract, already implemented by plugin-email from the live transport and already following the `mail` settings hot swap). An explicit `false` makes `isAvailable()` answer `{ available: false, reason: 'transport_not_configured' }` and makes `send()` refuse with that same reason, graded permanent, without calling the service. It is read on every call, never cached, so a transport the settings swap in after boot flips the channel back with no restart. - `EmailSenderSurface` (the channel's structural view of the service) gains the optional `isConfigured?(): boolean`, mirroring the contract member. - The existing reason token is reused. No new reason, no new error code, no spec change, no plugin-email change. ## Tests New top-level describe `email channel — a LogTransport is no transport (#30510)` in `email-channel.test.ts` (the package's shared harness for this channel). Its double mirrors plugin-email's `EmailService` on the one axis that matters: `isConfigured()` answers from a swappable live transport. - unavailable with `transport_not_configured` while the transport only logs; - `send()` refuses with the same reason, graded permanent, and the service's `send` is never called (no fake `sys_email` row); - swap to a real transport: available and delivers; swap back: unavailable again; - control: a service with no `isConfigured` stays available and still delivers; - end to end (MessagingService + MemoryNotificationOutbox + dispatcher): an emit on a log-only deployment writes no delivery row and reports the suppression; after the swap one row lands `success`. The producer half is already pinned in plugin-email (`email-plugin.mail-settings.test.ts`, "registered email service — isConfigured follows the mail settings": `false` on the out-of-the-box boot, `true` after a saved SMTP config, no restart). ## Acceptance notes - **Absent `isConfigured` is read as unknown and keeps today's behaviour (available).** The contract makes the member optional and says a consumer must not read absence as `true`; this channel keeps sending through such a service, as it did before, because refusing would mute every host-supplied email service that predates the member. This follows the same choice the fan-out ruling made for a channel with no `isAvailable`. Only an explicit `false` refuses. If the seat reads the contract the other way, flipping it is a one-line change in `emailTransportRefusal`. - The real composition (plugin-email + service-messaging in one kernel) is pinned as two halves joined by the spec contract: service-messaging cannot import plugin-email (no dependency), and a dogfood pin is outside this card's file surface. - Serial note: this PR does not touch plugin-email, `EmailService` or the `LogTransport`, so it does not overlap seat 5's #30212 row 3. ## Verification All on head `fbfbaf47` (origin/main `fa9c0aa2` merged; no new commits on main since the branch point). <details><summary>Red on main, green on the fix, ablation</summary> - Red (tests only, on `fa9c0aa2`): `Tests 4 failed | 43 passed (47)`; the four new pins red, the control green. - Green: `Test Files 1 passed (1)` · `Tests 47 passed (47)`. - Ablation via `scripts/ablation-replace.mjs` (anchor `if (typeof email.isConfigured !== 'function') return undefined;` replaced so the helper always returns `undefined`, the pre-fix behaviour): anchor 1 to 0, blob `deebc1c7be23` to `a81f47328d2c`; `Tests 4 failed | 43 passed (47)`; restored, blob equals HEAD, `git diff HEAD` empty. Tests import the channel from `src/` by relative path, so no build sits between the mutation and the run. </details> <details><summary>Package suite, typecheck, lint, gates</summary> - `pnpm --filter @objectstack/service-messaging test` under the verify lock: `Test Files 66 passed (66)` · `Tests 817 passed (817)` · `VERDICT command-exit 0`. - `pnpm --filter @objectstack/service-messaging typecheck`: exit 0 (`tsc --listFiles` includes `email-channel.test.ts`). - `eslint --no-inline-config` on the two changed source files: exit 0. The repo-wide `pnpm lint` is left to CI's Lint job. - `node scripts/pm/dispatch-gates.mjs --commands --repo objectstack-ai/objectstack`: 62 commands; each run with its exit code captured before any pipe. `--ran` reconciliation: `62 derived, 59 run, 3 NOT-MEASURED, 0 UNRUN`. The three exit-3 prerequisite refusals: `pnpm check:dual-build-cjs-loads` (needs the whole workspace built), `pnpm check:i18n` (needs its build closure; this diff touches no translation bundle), `node scripts/check-plugin-teardown-shape.mjs --self-test` (needs an unshallow clone; checker health only). CI runs all three. </details> --- _Generated by [Claude Code](https://claude.ai/code/session_01Utp6CaGmRyZ3XL5MuUgEZg)_ --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent db04e5c commit ede9e6b

3 files changed

Lines changed: 209 additions & 3 deletions

File tree

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
---
2+
'@objectstack/service-messaging': patch
3+
---
4+
5+
fix(notifications): the `email` channel answers unavailable while the email service only has the log transport, so a notification is no longer recorded as delivered when no mail left the server
6+
7+
Clause-②: no
8+
Metadata-protocol: none
9+
10+
- **What was wrong.** plugin-email registers the `email` service on every boot; with no `mail` settings saved its live transport is the log transport, which writes the message to the server log and to `sys_email` and sends nothing. The `email` channel treated "an email service is registered" as "available", so fan-out wrote a `sys_notification_delivery` row per recipient and the dispatcher marked it `success` while nobody was mailed.
11+
- **What changes.** The channel now asks the service's own deliverability answer, `IEmailService.isConfigured()`, on every call. While it answers `false`, `isAvailable()` answers `{ available: false, reason: 'transport_not_configured' }`: fan-out writes no delivery row and records the suppression on `sys_notification.suppressed_channels`, exactly as it does with no email service at all. `send()` refuses a row that was already written with the same `transport_not_configured` reason, graded permanent, and never hands it to the service. No new reason and no new error code.
12+
- **No restart needed.** The answer is read from the live transport, so saving a working provider under Setup → Email makes the channel available on the next notification, and switching back to the log transport makes it unavailable again.
13+
- **Unchanged.** An email service that does not implement `isConfigured` (it is optional on the contract) keeps today's behaviour: the channel stays available and sends through it. `EmailSenderSurface` gains the optional `isConfigured?(): boolean` member, mirroring the contract.

‎packages/services/service-messaging/src/email-channel.test.ts‎

Lines changed: 130 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -745,3 +745,133 @@ describe('email channel', () => {
745745
});
746746
});
747747
});
748+
749+
// [#30510, cloud#2549 ruling B item ③] An email service is not a transport.
750+
//
751+
// plugin-email registers its `email` service on EVERY boot. With no `mail`
752+
// settings saved it holds the LogTransport, which only logs: a send resolves,
753+
// writes `sys_email`, and no mail leaves the process. The channel used to read
754+
// "a service is registered" as "available", so every e-mail notification on
755+
// such a deployment got a delivery row that dispatched to `success` while
756+
// nobody was mailed. The deliverability fact belongs to the package that owns
757+
// the transport (`IEmailService.isConfigured()`, read at call time so it
758+
// follows a transport the `mail` settings swap in), and this channel asks it.
759+
//
760+
// The double mirrors plugin-email's `EmailService` on the one axis that
761+
// matters here: `isConfigured()` answers from the LIVE transport, and
762+
// `setTransport()` swaps it the way the `mail` settings change bus does.
763+
// plugin-email's own suite pins that the real service answers this way
764+
// (`email-plugin.mail-settings.test.ts`, "isConfigured follows the mail
765+
// settings"); this file pins that the channel listens.
766+
describe('email channel — a LogTransport is no transport (#30510)', () => {
767+
function liveSwappableEmail(initial: 'log' | 'smtp' = 'log') {
768+
const sent: any[] = [];
769+
let transport: 'log' | 'smtp' = initial;
770+
return {
771+
sent,
772+
setTransport(next: 'log' | 'smtp') { transport = next; },
773+
service: {
774+
isConfigured: () => transport !== 'log',
775+
async send(input: any) {
776+
sent.push(input);
777+
return { id: 'email_row_1' };
778+
},
779+
},
780+
};
781+
}
782+
783+
it('answers unavailable with the declared reason while the live transport only logs', () => {
784+
const data = fakeData();
785+
const email = liveSwappableEmail('log');
786+
const ch = channel(() => email.service, data);
787+
expect(ch.isAvailable?.(silentCtx(), { organizationId: 'org_1' }))
788+
.toEqual({ available: false, reason: 'transport_not_configured' });
789+
});
790+
791+
it('send() refuses with that SAME reason and never hands the message to the service — no fake sys_email row', async () => {
792+
const data = fakeData();
793+
const email = liveSwappableEmail('log');
794+
const ch = channel(() => email.service, data);
795+
796+
const r = await ch.send(silentCtx(), delivery());
797+
798+
expect(r.ok).toBe(false);
799+
expect(r.externalId).toBeUndefined();
800+
const probe = ch.isAvailable?.(silentCtx(), {}) as { reason: string };
801+
expect(r.error?.startsWith(`${probe.reason}:`)).toBe(true);
802+
expect(r.error).toContain('user_1');
803+
// The LogTransport would have "sent" it: the service is never called.
804+
expect(email.sent).toHaveLength(0);
805+
expect(ch.classifyError?.(r.error ?? '')).toBe('permanent');
806+
});
807+
808+
it('flips to available, with no restart, once the mail settings swap a real transport in — and back', async () => {
809+
const data = fakeData();
810+
const email = liveSwappableEmail('log');
811+
const ch = channel(() => email.service, data);
812+
expect(ch.isAvailable?.(silentCtx(), {})).toEqual({ available: false, reason: 'transport_not_configured' });
813+
814+
email.setTransport('smtp');
815+
expect(ch.isAvailable?.(silentCtx(), {})).toEqual({ available: true });
816+
const r = await ch.send(silentCtx(), delivery());
817+
expect(r.ok).toBe(true);
818+
expect(email.sent).toHaveLength(1);
819+
820+
email.setTransport('log');
821+
expect(ch.isAvailable?.(silentCtx(), {})).toEqual({ available: false, reason: 'transport_not_configured' });
822+
});
823+
824+
it('THE CONTROL: a service that does not answer isConfigured stays available (absent = unknown, not a LogTransport)', async () => {
825+
// `isConfigured` is OPTIONAL on `IEmailService`. Reading its absence as
826+
// "no transport" would mute every host-supplied email service that
827+
// predates the member, which is a different decision than the ruling's.
828+
const data = fakeData();
829+
const email = fakeEmail();
830+
const ch = channel(() => email.service, data);
831+
expect(ch.isAvailable?.(silentCtx(), {})).toEqual({ available: true });
832+
const r = await ch.send(silentCtx(), delivery());
833+
expect(r.ok).toBe(true);
834+
expect(email.sent).toHaveLength(1);
835+
});
836+
837+
it('END TO END: an emit on a log-only deployment writes no delivery row and records the suppression; after the swap it delivers', async () => {
838+
const data = fakeData();
839+
const email = liveSwappableEmail('log');
840+
const outbox = new MemoryNotificationOutbox(1);
841+
const service = new MessagingService({
842+
logger: { info: () => {}, warn: () => {}, error: () => {} },
843+
outbox,
844+
getData: () => data,
845+
});
846+
service.registerChannel(channel(() => email.service, data));
847+
const dispatcher = new NotificationDispatcher({
848+
nodeId: 'node-test',
849+
outbox,
850+
channels: service,
851+
channelContext: silentCtx(),
852+
intervalMs: 10_000,
853+
});
854+
const emit = () => service.emit({
855+
topic: 'deal.won',
856+
audience: ['user_1'],
857+
channels: ['email'],
858+
payload: { title: 'Deal closed', body: 'Acme signed' },
859+
});
860+
861+
const before = await emit();
862+
await dispatcher.tick();
863+
expect(await outbox.list()).toHaveLength(0);
864+
expect(before.enqueued).toBe(0);
865+
expect(before.suppressed).toEqual([{ channel: 'email', reason: 'transport_not_configured' }]);
866+
expect(email.sent).toHaveLength(0);
867+
868+
email.setTransport('smtp');
869+
const after = await emit();
870+
await dispatcher.tick();
871+
const rows = await outbox.list();
872+
expect(after.suppressed).toEqual([]);
873+
expect(rows).toHaveLength(1);
874+
expect(rows[0].status).toBe('success');
875+
expect(email.sent).toHaveLength(1);
876+
});
877+
});

‎packages/services/service-messaging/src/email-channel.ts‎

Lines changed: 66 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,21 @@ export interface EmailSenderSurface {
4343
*/
4444
organizationId?: string;
4545
}): Promise<{ id?: string } | unknown>;
46+
/**
47+
* Structural mirror of `IEmailService.isConfigured` — whether mail sent
48+
* through the service is actually delivered, answered by the package that
49+
* owns the transport. plugin-email answers `false` while its live
50+
* transport is the LogTransport (no `mail` settings saved: every send
51+
* resolves and writes `sys_email`, and no mail leaves the process), and
52+
* reads the live transport on every call, so the answer follows a
53+
* transport the `mail` settings swap in without a restart.
54+
*
55+
* OPTIONAL, as on the contract. An implementation without it is UNKNOWN,
56+
* and this channel keeps treating it as it always has (available): only an
57+
* explicit `false` is evidence that nothing would be delivered
58+
* ({@link emailTransportRefusal}).
59+
*/
60+
isConfigured?(): boolean;
4661
/**
4762
* Structural mirror of `IEmailService.sendTemplate` (#9205) — resolves a
4863
* `sys_email_template` bundle by `(template, locale)` with the documented
@@ -96,7 +111,9 @@ export interface EmailChannelOptions {
96111
* {@link MessagingChannel.isAvailable} reports as
97112
* `transport_not_configured` and {@link MessagingChannel.send} REFUSES with
98113
* the same reason (commit 879b51270). ⛔ Not a no-op success: a delivery nothing was
99-
* sent for is never reported as delivered.
114+
* sent for is never reported as delivered. A resolved service whose
115+
* `isConfigured()` answers `false` (only the log transport is live) is
116+
* answered the same way (#30510).
100117
*/
101118
getEmail(): EmailSenderSurface | undefined;
102119
/** Resolve the data engine (recipient address lookup). */
@@ -198,6 +215,33 @@ export async function templateRefusalAfterCurrencyReadFault(
198215
*/
199216
const TRANSPORT_NOT_CONFIGURED: ChannelUnavailableReason = 'transport_not_configured';
200217

218+
/**
219+
* [#30510, cloud#2549 ruling B item ③] Why a resolved email service cannot
220+
* deliver, or `undefined` when nothing says it cannot.
221+
*
222+
* A registered `email` service is not a transport. plugin-email registers one
223+
* on every boot, and with no `mail` settings saved it holds the LogTransport:
224+
* a send resolves, writes `sys_email`, and no mail leaves the process. Before
225+
* this, the channel read "a service is registered" as "available", so every
226+
* e-mail notification on such a deployment dispatched to `success` while
227+
* nobody was mailed.
228+
*
229+
* Asked of the service, ⛔ never of the options it was built from: the answer
230+
* must follow a transport the `mail` settings swap in after boot, and only the
231+
* service sees its live transport. Read on every call, never cached.
232+
*
233+
* Only an explicit `false` refuses. `isConfigured` is optional on the
234+
* contract, and an implementation without it is unknown, not a log transport:
235+
* reading its absence as "no transport" would mute every host-supplied email
236+
* service that predates the member, which is not what the ruling decided.
237+
*/
238+
function emailTransportRefusal(email: EmailSenderSurface): string | undefined {
239+
if (typeof email.isConfigured !== 'function') return undefined;
240+
return email.isConfigured() === false
241+
? "the 'email' service has no delivering transport (only the log transport is active; configure the mail settings)"
242+
: undefined;
243+
}
244+
201245
const EMAIL_SHAPE = (s: string): boolean => {
202246
// Linear, non-backtracking "looks like an email" — same shape as the
203247
// recipient resolver's check (avoids the ReDoS-prone regex).
@@ -343,9 +387,13 @@ export function createEmailChannel(opts: EmailChannelOptions): MessagingChannel
343387
// it causes is announced ONCE per emit by the service and recorded
344388
// durably on `sys_notification.suppressed_channels` — a line per
345389
// emit here would be the noisy half of an answer already written down.
346-
return opts.getEmail()
390+
//
391+
// [#30510] A registered service whose live transport only logs is
392+
// the same condition as no service: nothing would be delivered.
393+
const email = opts.getEmail();
394+
return email && emailTransportRefusal(email) === undefined
347395
? { available: true }
348-
: { available: false, reason: 'transport_not_configured' };
396+
: { available: false, reason: TRANSPORT_NOT_CONFIGURED };
349397
},
350398

351399
async send(ctx: MessagingChannelContext, delivery: Delivery): Promise<SendResult> {
@@ -378,6 +426,21 @@ export function createEmailChannel(opts: EmailChannelOptions): MessagingChannel
378426
error: `${TRANSPORT_NOT_CONFIGURED}: no 'email' service is registered; nothing was sent to '${delivery.recipient}'`,
379427
};
380428
}
429+
// [#30510] The same answer for a service whose live transport only
430+
// logs, which `isAvailable()` above gives too. This is the residue
431+
// fan-out's suppression cannot cover: a row written while a real
432+
// transport was live and dispatched after the settings dropped back
433+
// to the log transport. ⛔ Never handed to the service: its
434+
// LogTransport would "send" it, write `sys_email`, and the row
435+
// would read `success` with nobody mailed.
436+
const refusal = emailTransportRefusal(email);
437+
if (refusal !== undefined) {
438+
ctx.logger.warn(`[email] ${refusal}; '${delivery.recipient}' was NOT emailed`);
439+
return {
440+
ok: false,
441+
error: `${TRANSPORT_NOT_CONFIGURED}: ${refusal}; nothing was sent to '${delivery.recipient}'`,
442+
};
443+
}
381444

382445
const n = delivery.notification;
383446
const resolved = await resolveRecipient(ctx, opts.getData(), delivery.recipient);

0 commit comments

Comments
 (0)