Skip to content

Commit db04e5c

Browse files
fix(plugin-email,service-settings): a sender with no From name goes out under the product name (#30548)
<!-- claude-projects-attribution: {"github_login":"os-bill"} --> _Requested by **Bill** · [project thread](https://claude.ai/code/project/chan_016iMXWSZDzqWRWvWzdNo8wg?thread=cmsg_016iMXWSZDzqWRWvWzdNo8wgXUJ7mi7dh8n9KT6npD9Tvg)_ domain:server#5 · os-bill · session_01Utp6CYVV6fcAQrNTDQRvPb ## 维护者速读 - 改了什么:发件人名没设置时,邮件不再署名 ObjectStack,而是用产品名(Setup → 品牌 里设的产品名,没设就用部署名,再没有才是 ObjectStack);Setup → 邮件 的“发件人名称”去掉了默认值 ObjectStack。 - 为什么改:管理员只填了发件地址,收件人看到发件人是 ObjectStack,而主题和正文写的是产品名,显得像别家发的信(卡 #30212 第 3 行)。 - 风险与回滚:只影响“没设发件人名”的默认发件人;设过名字的照旧优先,调用方自带的发件人不改。回滚即 revert 本 PR。以前保存过的“ObjectStack”会被当作管理员显式设的名字继续生效,清空该栏即跟随产品名。 - 先看:packages/plugins/plugin-email/src/email-plugin.ts(namedSender / applyDefaultSender 与 branding 绑定)、packages/services/service-settings/src/manifests/mail.manifest.ts、packages/plugins/plugin-email/src/email-plugin.mail-settings.test.ts 末尾新 describe。 - 两分钟验证:`pnpm --filter @objectstack/plugin-email exec vitest run src/email-plugin.mail-settings.test.ts`,预期 34 条全过,其中“sender display name follows the product name”一组 10 条。 - 需要你做的:无,席位落地。第 2 行(导入账号设密码链接 60 分钟)按席位裁定不在本 PR,留在卡上待决。 Part of #30212 Clause-②: no Metadata-protocol: none — a server default changes (the sender's display name, and the Mail settings page's From name default); nothing a metadata author writes changes. Skills-docs: none — `git grep -n -i -E "from_name|mail\.from|from name|sender name|defaultFrom|resetPasswordTokenExpiresIn|set-password link|60 minutes" -- skills` names no sentence about the mail sender or the link lifetime (its one hit is `defaultFromRow` in `skills/objectstack-ui/rules/actions.md`, an action-param key). Row 3 of the card. Row 2 (the bulk-import set-password link lifetime) is not in this PR: the seat ruled it out after the measurement (a credential-setting link's lifetime, the security seat vacant), so the card stays open for it. Rows 1 and 4 landed earlier. ## What changes - **`@objectstack/plugin-email`.** `EmailServicePlugin` names a default sender that has no display name of its own with the product name the auth mails' subject and body already read: the Setup → Branding product name (`branding.workspace_name`) when an administrator SET one, else the deployment's name (`defaultTemplateContext.appName`, which `os serve` resolves from `OS_APP_NAME` and the config chain), else `ObjectStack`. This is the chain plugin-auth binds to the auth mails' `{{appName}}`. - The Branding value is read on `kernel:ready` and again on every Branding save, so a rename reaches the next mail with no restart. A manifest default is not taken for a set value. The namespace is bound only where the settings service lists it; a walled posture withholds it, and the deployment's name applies there. - Which senders: the Setup → Mail sender saved with the From name empty, and a boot sender declared without a name (`OS_EMAIL_FROM` holding an address alone, or `config.email.defaultFrom` with `address` alone). - A From name somebody set always wins: a Mail settings From name whose source is not `default`, or a boot-declared name. A sender a send passes as `input.from`, and a template's own sender, are never renamed. - "Send test email" sends under the same name real mail goes out with. - The plugin keeps the declared sender apart from the live one, so a later Branding save renames only a sender that has no name. `EmailService`'s public surface is unchanged: the plugin installs the named sender through the existing `setDefaultFrom`. - **`@objectstack/service-settings`.** `mail.from_name` drops its `default: 'ObjectStack'`: an unsaved field reads empty, and the resolver answers `value: null`, `source: 'default'`, as Branding's product name already does. The field gains help text in en, zh-CN, ja-JP and es-ES saying that an empty value uses the product name. The Branding manifest's header names plugin-email as a reader of `workspace_name`. ## Tests - `packages/plugins/plugin-email/src/email-plugin.mail-settings.test.ts`, new top-level describe "sender display name follows the product name when none is set" in the existing harness (the harness's fake settings service now lists namespaces and serves a `branding` snapshot): Branding name set; Branding unset falls back to the deployment name; nothing configured gives `ObjectStack`; an explicit From name wins; a From name that is only a manifest default does not; a Branding save and a Branding clear take effect without a restart; a boot sender without a name is named; a caller's own `from` is untouched; no Branding read under a walled posture; the test mail carries the name. - `packages/services/service-settings/src/manifests/mail.manifest.test.ts`: `from_name` declares no default, stays optional, and carries the help text. ## Acceptance notes - A From name stored before this change (for example `ObjectStack`, if a save sent the prefilled default) is an explicit value and keeps winning. Clearing the field makes the sender follow the product name. - The boot fallback sender that `resolveEmailCapabilityArg` builds when nothing declares one keeps its derived name (the deployment's name). Setup → Branding does not rename that one. It is the sender for a deployment with no From address configured anywhere. - Non-auth templates render `{{appName}}` from the boot-time template context (the deployment's name), not from Setup → Branding. This PR does not change that. Only the auth mails read the Branding product name today. ## Verification Head `a2519c047`, merged with `origin/main` `fa9c0aa2a` (up to date). <details><summary>Tests and typecheck (both touched packages)</summary> - Red first on `origin/main` `fa9c0aa2a`: the new describe ran 8 failed / 1 passed (the passing one is the control: a caller's own `from` is untouched); for example "expected 'no-reply@acme.test' to be 'Acme Portal (no-reply@acme.test)'", angle brackets spelled as parentheses here. - `pnpm --filter @objectstack/plugin-email test` then `typecheck` under the verify lock: VERDICT command-exit 0; Test Files 48 passed (48), Tests 795 passed (795); `check:test-typecheck` OK. - `pnpm --filter @objectstack/service-settings test` then `typecheck` under the verify lock: VERDICT command-exit 0; Test Files 68 passed (68), Tests 1157 passed (1157). - `pnpm exec eslint --no-inline-config` on the 9 changed source files: exit 0. The repo-wide `pnpm lint` is left to CI (`Lint & Repo Gates`). </details> <details><summary>Ablation (once, through scripts/ablation-replace.mjs)</summary> - Anchor in `email-plugin.ts` `namedSender`: the line that keeps a sender's own name, replaced by an unconditional `return sender;` (marker ABLATED-30212). Mutation landed (anchor 1 to 0, blob b493082 to 6572e816ed05). - `vitest run src/email-plugin.mail-settings.test.ts`: Tests 8 failed / 26 passed (the 7 naming pins plus the existing sys_email delivery pin that expects the named sender). - Restored: blob back to HEAD b493082, `git diff HEAD` empty, marker count 0. The tests import `src` directly, so no dist rebuild is involved. </details> <details><summary>Derived gates (dispatch-gates --commands, reconciled with --ran)</summary> - 60 derived, 59 run with exit 0, 1 NOT MEASURED: `pnpm check:dual-build-cjs-loads` exit 3, PREREQUISITE NOT MET (it reads every package's built output; only this diff's closure is built here). Left to CI. - `node scripts/check-plugin-teardown-shape.mjs --self-test` exited 3 on the shallow clone (its pinned fixture commit was missing); after `git fetch origin 621a487` it exited 0 (48 cases pass). - `--ran` verdict: "60 derived famil(ies) accounted for, 59 run, 1 NOT-MEASURED". Changed lines 366 (+344 / -22), under the 3000 threshold. </details> --- _Generated by [Claude Code](https://claude.ai/code/session_01Utp6CbJtwiuT8bs47ARmYb)_ --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent 48b3ae6 commit db04e5c

10 files changed

Lines changed: 344 additions & 22 deletions

File tree

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,14 @@
1+
---
2+
'@objectstack/plugin-email': patch
3+
'@objectstack/service-settings': patch
4+
---
5+
6+
fix(plugin-email,service-settings): mail whose sender has no From name goes out under the product name, not "ObjectStack"
7+
8+
Clause-②: no
9+
Metadata-protocol: none
10+
11+
- **Now.** A default sender with no display name of its own goes out under the product name the auth mails' subject and body already use: the Setup → Branding product name when an administrator set one, else the deployment's name (`OS_APP_NAME`, then `config.email.appName`, `config.email.defaultTemplateContext.appName` and the top-level `appName`), else `ObjectStack`. A Branding save renames the sender on the next mail, with no restart. The "Send test email" button sends under the same name.
12+
- **Which senders.** The sender saved in Setup → Mail with the From name left empty, and a sender declared at boot without a name (`OS_EMAIL_FROM=no-reply@acme.com`, or `config.email.defaultFrom` with an `address` alone). Before, the first went out as `ObjectStack <…>` and the second with no display name.
13+
- **Unchanged.** A From name somebody set always wins: one saved in Setup → Mail, or one declared at boot (`OS_EMAIL_FROM="Acme <no-reply@acme.com>"`, `config.email.defaultFrom.name`). A sender a send names itself, and a template's own sender, are never renamed. Under a walled tenancy posture, where Setup → Branding is not offered, the deployment's name applies.
14+
- **`@objectstack/service-settings`.** Setup → Mail → From name no longer has the default `ObjectStack`: an unsaved field reads empty, and `GET /api/settings/mail` answers `from_name` as `value: null`, `source: 'default'`. The field's help says that an empty value uses the product name. A From name saved earlier is kept and still wins; clear the field to follow the product name.

‎packages/plugins/plugin-email/src/email-plugin.mail-settings.test.ts‎

Lines changed: 161 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -27,22 +27,46 @@ vi.mock('nodemailer', () => ({
2727

2828
interface Resolved { value: unknown; source?: string }
2929

30-
function fakeSettings(values: Record<string, Resolved>) {
31-
const listeners: Array<() => void> = [];
30+
/**
31+
* `branding` is the namespace's snapshot when the settings service OFFERS it
32+
* (a single-posture deployment lists it); absent, the fake lists no
33+
* `branding` namespace, as a walled posture does.
34+
*/
35+
function fakeSettings(values: Record<string, Resolved>, branding?: Record<string, Resolved>) {
36+
const listeners: Array<{ ns: string | undefined; cb: () => void }> = [];
3237
const actions = new Map<string, (a: any) => Promise<any>>();
38+
const read: string[] = [];
39+
const emit = async (ns: string) => {
40+
for (const l of listeners) if (l.ns === undefined || l.ns === ns) l.cb();
41+
// let the async re-apply settle
42+
await new Promise((r) => setTimeout(r, 0));
43+
};
3344
return {
3445
createClient: () => ({}),
35-
getNamespace: async () => ({ values }),
36-
subscribe: (_ns: string, cb: () => void) => { listeners.push(cb); },
46+
/** Every namespace read, in order. */
47+
read,
48+
getNamespace: async (ns: string) => {
49+
read.push(ns);
50+
if (ns === 'branding') {
51+
if (!branding) throw new Error(`UNKNOWN_NAMESPACE: ${ns}`);
52+
return { values: branding };
53+
}
54+
return { values };
55+
},
56+
listManifests: () => [{ namespace: 'mail' }, ...(branding ? [{ namespace: 'branding' }] : [])],
57+
subscribe: (ns: string | undefined, cb: () => void) => { listeners.push({ ns, cb }); },
3758
registerAction: (ns: string, id: string, fn: (a: any) => Promise<any>) => {
3859
actions.set(`${ns}/${id}`, fn);
3960
},
4061
/** Simulate a save: patch the snapshot and emit settings:changed. */
4162
async save(patch: Record<string, Resolved>) {
4263
Object.assign(values, patch);
43-
for (const l of listeners) l();
44-
// let the async re-apply settle
45-
await new Promise((r) => setTimeout(r, 0));
64+
await emit('mail');
65+
},
66+
/** Simulate a save on Setup → Branding. */
67+
async saveBranding(patch: Record<string, Resolved>) {
68+
Object.assign(branding!, patch);
69+
await emit('branding');
4670
},
4771
action: (id: string) => actions.get(`mail/${id}`),
4872
};
@@ -84,9 +108,13 @@ const MAIL_DEFAULTS: Record<string, Resolved> = {
84108
};
85109

86110
/** Boot the plugin against fake services and run its kernel:ready hook. */
87-
async function boot(mailValues: Record<string, Resolved>, opts: any = {}) {
111+
async function boot(
112+
mailValues: Record<string, Resolved>,
113+
opts: any = {},
114+
branding?: Record<string, Resolved>,
115+
) {
88116
const engine = fakeEngine();
89-
const settings = fakeSettings({ ...MAIL_DEFAULTS, ...mailValues });
117+
const settings = fakeSettings({ ...MAIL_DEFAULTS, ...mailValues }, branding);
90118
const services: Record<string, unknown> = {
91119
manifest: { register: () => {} },
92120
objectql: engine,
@@ -467,3 +495,127 @@ describe('registered email service — isConfigured follows the mail settings',
467495
expect(service.isConfigured()).toBe(true);
468496
});
469497
});
498+
499+
// ── sender display name ────────────────────────────────────────────────────
500+
501+
// The Setup walkthrough's finding: an administrator who filled in only
502+
// Settings → Mail → From email got mail from "ObjectStack <…>" while the
503+
// subject and body named the product (`branding.workspace_name`, which the
504+
// auth mails read). A sender with no display name of its own now carries the
505+
// product name the auth mails read: the Branding product name when one is
506+
// SET, else the deployment's name, else 'ObjectStack'. A From name somebody
507+
// set always wins.
508+
describe('sender display name follows the product name when none is set', () => {
509+
const SMTP_LIVE: Record<string, Resolved> = {
510+
provider: { value: 'smtp', source: 'global' },
511+
smtp_host: { value: 'smtp.example.test', source: 'global' },
512+
from_email: { value: 'no-reply@acme.test', source: 'global' },
513+
// The manifest declares no From name default: an unset one resolves empty.
514+
from_name: { value: undefined, source: 'default' },
515+
};
516+
const fromOnWire = () => nm.sendMail.mock.calls[nm.sendMail.mock.calls.length - 1][0].from;
517+
const sendOne = (service: EmailService) =>
518+
service.send({ to: 'new.hire@example.test', subject: 'Welcome', text: 'hi' });
519+
520+
it('names the sender with the Branding product name an administrator set', async () => {
521+
const { service } = await boot(SMTP_LIVE, {}, {
522+
workspace_name: { value: 'Acme Portal', source: 'tenant' },
523+
});
524+
await sendOne(service);
525+
expect(fromOnWire()).toBe('Acme Portal <no-reply@acme.test>');
526+
expect(service.getDefaultFrom()).toEqual({ address: 'no-reply@acme.test', name: 'Acme Portal' });
527+
});
528+
529+
it('falls back to the deployment name when no Branding product name is set', async () => {
530+
const { service } = await boot(
531+
SMTP_LIVE,
532+
{ defaultTemplateContext: { appName: 'Acme CRM' } },
533+
{ workspace_name: { value: undefined, source: 'default' } },
534+
);
535+
await sendOne(service);
536+
expect(fromOnWire()).toBe('Acme CRM <no-reply@acme.test>');
537+
});
538+
539+
it("says 'ObjectStack' only when no product name is configured anywhere", async () => {
540+
const { service } = await boot(SMTP_LIVE, {}, {
541+
workspace_name: { value: undefined, source: 'default' },
542+
});
543+
await sendOne(service);
544+
expect(fromOnWire()).toBe('ObjectStack <no-reply@acme.test>');
545+
});
546+
547+
it('keeps a From name an administrator set, whatever the product name', async () => {
548+
const { service } = await boot(
549+
{ ...SMTP_LIVE, from_name: { value: 'Acme Support', source: 'global' } },
550+
{ defaultTemplateContext: { appName: 'Acme CRM' } },
551+
{ workspace_name: { value: 'Acme Portal', source: 'tenant' } },
552+
);
553+
await sendOne(service);
554+
expect(fromOnWire()).toBe('Acme Support <no-reply@acme.test>');
555+
});
556+
557+
it('does not take a From name that is only a manifest default for a set one', async () => {
558+
// A resolver answering a default is no decision anybody made, the rule the
559+
// auth mails apply to `workspace_name` itself.
560+
const { service } = await boot(
561+
{ ...SMTP_LIVE, from_name: { value: 'ObjectStack', source: 'default' } },
562+
{},
563+
{ workspace_name: { value: 'Acme Portal', source: 'tenant' } },
564+
);
565+
await sendOne(service);
566+
expect(fromOnWire()).toBe('Acme Portal <no-reply@acme.test>');
567+
});
568+
569+
it('follows a Branding save without a restart, and a cleared name falls back', async () => {
570+
const branding: Record<string, Resolved> = { workspace_name: { value: 'Acme Portal', source: 'tenant' } };
571+
const { service, settings } = await boot(SMTP_LIVE, { defaultTemplateContext: { appName: 'Acme CRM' } }, branding);
572+
573+
await settings.saveBranding({ workspace_name: { value: 'Acme Workspace', source: 'tenant' } });
574+
await sendOne(service);
575+
expect(fromOnWire()).toBe('Acme Workspace <no-reply@acme.test>');
576+
577+
await settings.saveBranding({ workspace_name: { value: undefined, source: 'default' } });
578+
await sendOne(service);
579+
expect(fromOnWire()).toBe('Acme CRM <no-reply@acme.test>');
580+
});
581+
582+
it('names a boot-declared sender that carries no display name', async () => {
583+
const { service } = await boot(
584+
{ from_email: { value: undefined, source: 'default' } },
585+
{ defaultFrom: { address: 'ops@acme.test' } },
586+
{ workspace_name: { value: 'Acme Portal', source: 'tenant' } },
587+
);
588+
expect(service.getDefaultFrom()).toEqual({ address: 'ops@acme.test', name: 'Acme Portal' });
589+
});
590+
591+
it('leaves a sender the caller names untouched', async () => {
592+
const { service } = await boot(SMTP_LIVE, {}, {
593+
workspace_name: { value: 'Acme Portal', source: 'tenant' },
594+
});
595+
await service.send({
596+
to: 'new.hire@example.test', subject: 'Welcome', text: 'hi', from: { address: 'no-reply@acme.test' },
597+
});
598+
expect(fromOnWire()).toBe('no-reply@acme.test');
599+
});
600+
601+
it('reads no Branding namespace where the settings service does not offer one (walled posture)', async () => {
602+
const { service, settings, ctx } = await boot(SMTP_LIVE, { defaultTemplateContext: { appName: 'Acme CRM' } });
603+
await sendOne(service);
604+
expect(fromOnWire()).toBe('Acme CRM <no-reply@acme.test>');
605+
expect(settings.read).not.toContain('branding');
606+
const warned = ctx.logger.warn.mock.calls.map((c: unknown[]) => String(c[0]));
607+
expect(warned.filter((line: string) => /branding/i.test(line))).toEqual([]);
608+
});
609+
610+
it('sends the test mail under the name real mail goes out with', async () => {
611+
const { settings } = await boot(SMTP_LIVE, {}, {
612+
workspace_name: { value: 'Acme Portal', source: 'tenant' },
613+
});
614+
const result = await settings.action('test')!({
615+
values: { provider: 'smtp', smtp_host: 'smtp.example.test', from_email: 'no-reply@acme.test' },
616+
payload: { to: 'admin@example.test' },
617+
});
618+
expect(result).toMatchObject({ ok: true });
619+
expect(fromOnWire()).toBe('Acme Portal <no-reply@acme.test>');
620+
});
621+
});

0 commit comments

Comments
 (0)