From 6aecf985c4d1627e18eaa407337b1c21629b8f45 Mon Sep 17 00:00:00 2001
From: important-new
Your ${appName} workspace has used 4 of your 5 free inspections. You have one free inspection left.
${cta}` : `Your ${appName} workspace has used your 5 free inspections — everything stays usable; subscribe to create new ones.
${cta}`; - await this.sendEmail([owner.email], subject, html); + // Hand-built HTML, not a registry template — so it names its class + // explicitly. This send was absent from the §5.0 raw-call-site audit + // because that census swept ROUTES, and this one lives inside the + // email service itself. Moving it onto a template is P3's job; making + // the boundary able to see it is this one's. + await this.sendEmail([owner.email], subject, html, undefined, { classId: 'usage-quota-warning' }); if (deps.kv) await deps.kv.put(dedupeKey, '1'); } @@ -199,7 +203,7 @@ export function TransactionalEmailMixinhi
', enabled: true, ...over }); + +describe('class-carrying send boundary', () => { + it('carries the class to the boundary, not just an address and a string', () => { + const p = new Probe(); + p.send(rendered('report-ready')); + expect(p.captured[0].classId).toBe('report-ready'); + }); + + it('takes the class from the render result, so it cannot name a template it did not render', () => { + // The trigger is not a parameter of sendRendered — it rides inside the + // RenderResult. A caller has no argument through which to declare a + // different class from the one that produced the body. + const p = new Probe(); + p.send(rendered('payment-request', { subject: 'Invoice' })); + expect(p.captured[0].classId).toBe('payment-request'); + expect(p.captured[0].subject).toBe('Invoice'); + }); + + it('keeps the class when a caller appends to the body', () => { + // booking-confirmation spreads its result to append the SMS opt-in block. + // Rebuilding the object instead of spreading would silently drop the + // trigger and turn a classified send into an unclassified one. + const p = new Probe(); + const base = rendered('booking-confirmation'); + p.send({ ...base, html: `${base.html}opt in
` }); + expect(p.captured[0].classId).toBe('booking-confirmation'); + expect(p.captured[0].html).toContain('opt in'); + }); + + it('still delivers subject, body and recipients unchanged', () => { + const p = new Probe(); + p.send(rendered('booking-confirmation', { subject: 'S', html: 'B' })); + expect(p.captured[0]).toMatchObject({ to: ['jane@x.com'], subject: 'S', html: 'B' }); + }); + + it('leaves the class absent for a caller that did not name one — unclassified, never silently muted', () => { + // An unclassified send must remain SENDABLE (it goes out) while being + // un-mutable, per `isSuppressible`'s fail-closed default. A boundary that + // dropped unclassified mail would turn a missing annotation into lost + // notifications. + const p = new Probe(); + p.sendEmail(['jane@x.com'], 'Ad-hoc', 'x
'); + expect(p.captured[0].classId).toBeUndefined(); + }); +}); diff --git a/tests/unit/notifications/classes.spec.ts b/tests/unit/notifications/classes.spec.ts index 3f3e07d5b..1633fc7e2 100644 --- a/tests/unit/notifications/classes.spec.ts +++ b/tests/unit/notifications/classes.spec.ts @@ -32,6 +32,9 @@ const NEVER_OFF = [ 'client-portal-login', 'agreement-request', 'agreement-signed', 'evidence-pack', 'payment-request', 'report-ready', 'report-ready-pdf', + // Not §2.0/§2.1 but the same harm: muting it means the workspace hits the + // free-tier wall with no warning. + 'usage-quota-warning', ]; /** Spec §2.2-§2.4 — the recipient's call. */ From e4f07708018f32e863c81f25c03d31cff6393454 Mon Sep 17 00:00:00 2001 From: important-new` and its
margin, so "optional" could only be expressed by the caller assembling
the block list — no template could declare it. And every `multiline`
block invites newlines that HTML then collapsed; they now survive as
` Client Portal
- Click the button below to access your inspections. This link expires in 15 minutes.
-
- Open my portal
- If you didn't request this, you can safely ignore this email. Repair Request
- A repair request list has been shared with you. Click the link below to review the items.
- Message ${safeMessage} A repair request list for ${address} has been shared with you. Click the link below to access your inspections. This link expires in 15 minutes. If you didn't request this, you can safely ignore this email. Link: ${loginUrl} Your ${appName} workspace has used 4 of your 5 free inspections. You have one free inspection left. Your ${appName} workspace has used your 5 free inspections — everything stays usable; subscribe to create new ones. Your ${appName} workspace has used 4 of your 5 free inspections. You have one free inspection left. Your ${appName} workspace has used your 5 free inspections — everything stays usable; subscribe to create new ones. ]*>\s*<\/p>/);
+ });
+
it('renders the CTA button with the primary color and url', () => {
const html = EmailLayout({ brand, heading: 'H', paragraphs: [], cta: { label: 'View Report', url: 'https://x/y' } });
expect(html).toContain('https://x/y');
diff --git a/tests/unit/email/email-registry.spec.ts b/tests/unit/email/email-registry.spec.ts
index cb8bf497c..db2208e83 100644
--- a/tests/unit/email/email-registry.spec.ts
+++ b/tests/unit/email/email-registry.spec.ts
@@ -1,17 +1,24 @@
import { describe, it, expect } from 'vitest';
import { REGISTRY, getDescriptor } from '../../../server/lib/email-templates/registry';
+import { sampleDataFor } from '../../../server/lib/email-templates/sample-data';
describe('email template registry', () => {
- it('has exactly 20 descriptors', () => {
- expect(REGISTRY.length).toBe(20);
+ // A hand-maintained count is the only tripwire for a template being DELETED
+ // by accident — nothing else in the suite notices a shrinking registry (an
+ // orphaned class is legal, since a class may exist before its template does).
+ it('has exactly 24 descriptors — bump deliberately when adding one', () => {
+ expect(REGISTRY.length).toBe(24);
});
it('every trigger is unique', () => {
const t = REGISTRY.map(d => d.trigger);
- expect(new Set(t).size).toBe(20);
+ expect(new Set(t).size).toBe(REGISTRY.length);
});
- it('marks exactly one non-editable (platform) trigger: password-reset', () => {
+ it('marks exactly the platform-owned triggers non-editable', () => {
+ // Non-editable == "this is OUR message, on OUR footing": account recovery
+ // and our own billing. A tenant rewriting either would be rewriting
+ // something they are not the author of.
const platform = REGISTRY.filter(d => !d.editable).map(d => d.trigger);
- expect(platform).toEqual(['password-reset']);
+ expect(platform).toEqual(['password-reset', 'usage-quota-warning', 'usage-quota-reached']);
});
// Which triggers are `required` is no longer asserted here. A hardcoded pair
// in this file was a snapshot of the answer, and the answer was wrong: only
@@ -40,6 +47,20 @@ describe('email template registry', () => {
}
}
});
+ it('every declared variable has a preview example', () => {
+ // `sampleDataFor` falls back to the literal `{name}` when a variable has no
+ // example, so the preview an admin uses to check their copy silently shows
+ // `{loginUrl}` where the button link should be — and the CTA renders with a
+ // junk href. Nothing failed; it just looked wrong to whoever opened it.
+ const missing: string[] = [];
+ for (const d of REGISTRY) {
+ const sample = sampleDataFor(d);
+ for (const v of d.variables) {
+ if (sample[v.name] === `{${v.name}}`) missing.push(`${d.trigger}.${v.name}`);
+ }
+ }
+ expect(missing).toEqual([]);
+ });
it('getDescriptor returns by trigger and undefined for unknown', () => {
expect(getDescriptor('report-ready')?.name).toBeTruthy();
expect(getDescriptor('nope')).toBeUndefined();
diff --git a/tests/unit/email/email-renderer.spec.ts b/tests/unit/email/email-renderer.spec.ts
index 6a5c6a817..3372de781 100644
--- a/tests/unit/email/email-renderer.spec.ts
+++ b/tests/unit/email/email-renderer.spec.ts
@@ -26,6 +26,25 @@ describe('EmailTemplateRenderer', () => {
expect(r.html).not.toContain('');
expect(r.html).toContain('<script>');
});
+ it('keeps line breaks a sender or template author typed', () => {
+ // Every `multiline: true` block invites newlines, and until now they were
+ // collapsed into one run-on paragraph. The break is inserted AFTER escaping,
+ // so it is layout chrome — an author still cannot smuggle HTML through.
+ const r = mk().render('repair-request-share', {
+ propertyAddress: '12 Elm',
+ message: 'Line one.\nLine two.',
+ shareUrl: 'https://x/s',
+ });
+ expect(r.html).toContain('Line one. ]*>\s*<\/p>/);
+ });
+
it('throws for an unknown trigger', () => {
expect(() => mk().render('nope', {})).toThrow();
});
diff --git a/tests/unit/email/email-service-rendered.spec.ts b/tests/unit/email/email-service-rendered.spec.ts
index 7c647a36b..61e692043 100644
--- a/tests/unit/email/email-service-rendered.spec.ts
+++ b/tests/unit/email/email-service-rendered.spec.ts
@@ -64,3 +64,67 @@ describe('EmailService rendered path', () => {
expect(body.html).not.toContain('inspection.ics');
});
});
+
+/**
+ * The two sends the ROUTES used to build by hand.
+ *
+ * Each one shipped a hardcoded slate button (`#0f172a`) that ignored the
+ * company's colour and logo, could not be edited or translated, and reached the
+ * send boundary with no notification class. Being a template is what fixes all
+ * four at once, so these assert all four.
+ */
+describe('sends converted off hand-built HTML', () => {
+ /** Captures what reached the boundary, including the class the routes lacked. */
+ class Probe extends EmailService {
+ captured: Array<{ to: string[]; subject: string; html: string; classId?: string }> = [];
+ override async sendEmail(
+ to: string[], subject: string, html: string,
+ _attachments?: Array<{ filename: string; content: ArrayBuffer | string; contentType?: string }>,
+ opts?: { classId?: string },
+ ): Promise<{ delivered: boolean }> {
+ this.captured.push({ to, subject, html, classId: opts?.classId });
+ return { delivered: true };
+ }
+ }
+ const probe = () => new Probe('re_test', 'reports@acme.com', 'Acme', undefined, renderer);
+
+ it('client portal sign-in: tenant-branded, carries the link and its class', async () => {
+ const p = probe();
+ await p.sendClientPortalLogin('a@x.com', 'https://x/portal/acme/auth?link=tok');
+ expect(p.captured[0].classId).toBe('client-portal-login');
+ expect(p.captured[0].subject).toBe('Sign in to your client portal');
+ expect(p.captured[0].html).toContain('https://x/portal/acme/auth?link=tok');
+ // Tenant brand, not the platform's — a client's portal belongs to one company.
+ expect(p.captured[0].html).toContain('Acme');
+ expect(p.captured[0].html).not.toContain('#0f172a;">Open my portal');
+ });
+
+ it('repair-request share: subject carries the address, body the note and the link', async () => {
+ const p = probe();
+ await p.sendRepairRequestShare('contractor@x.com', {
+ propertyAddress: '12 Elm St',
+ shareUrl: 'https://x/repair-request/tok',
+ message: 'Please quote items 2 and 3.\nThanks.',
+ });
+ expect(p.captured[0].classId).toBe('repair-request-share');
+ expect(p.captured[0].subject).toBe('Repair request — 12 Elm St');
+ expect(p.captured[0].html).toContain('https://x/repair-request/tok');
+ // The sender's newline survives — they typed it into a textarea.
+ expect(p.captured[0].html).toContain('Please quote items 2 and 3. ]*>\s*<\/p>/);
+ });
+
+ it('falls back to "your property" rather than a subject ending in a dash', async () => {
+ const p = probe();
+ await p.sendRepairRequestShare('contractor@x.com', { propertyAddress: '', shareUrl: 'https://x/s' });
+ expect(p.captured[0].subject).toBe('Repair request — your property');
+ });
+});
diff --git a/tests/unit/email/email-templates-api.spec.ts b/tests/unit/email/email-templates-api.spec.ts
index 44810d636..5516d1caa 100644
--- a/tests/unit/email/email-templates-api.spec.ts
+++ b/tests/unit/email/email-templates-api.spec.ts
@@ -16,6 +16,7 @@ vi.mock('drizzle-orm/d1', () => ({ drizzle: vi.fn() }));
import { drizzle as mockDrizzle } from 'drizzle-orm/d1';
import emailTemplateRoutes from '../../../server/api/email-templates';
+import { REGISTRY } from '../../../server/lib/email-templates/registry';
const TENANT_ID = '00000000-0000-0000-0000-000000000001';
@@ -62,14 +63,20 @@ describe('GET /api/admin/email-templates', () => {
vi.clearAllMocks();
});
- it('returns 200 with 19 items, no password-reset, correct fields', async () => {
+ it('lists every editable template and no platform-owned one, with correct fields', async () => {
const app = buildApp();
const res = await app.request('/api/admin/email-templates', {}, { DB: {} });
expect(res.status).toBe(200);
const body = await res.json() as { success: boolean; data: Array<{ trigger: string; name: string; required: boolean; enabled: boolean; isCustomized: boolean; subject: string; category: string }> };
expect(body.success).toBe(true);
- expect(body.data).toHaveLength(19);
- expect(body.data.every(t => t.trigger !== 'password-reset')).toBe(true);
+ expect(body.data).toHaveLength(21);
+ // The editor lists what a TENANT may rewrite. Account recovery and our
+ // own billing notices are ours, so they must never appear here — assert
+ // the rule, not just its first instance.
+ const listed = new Set(body.data.map(t => t.trigger));
+ for (const d of REGISTRY.filter(x => !x.editable)) {
+ expect(listed.has(d.trigger), `${d.trigger} is not editable and must not be listed`).toBe(false);
+ }
for (const item of body.data) {
expect(typeof item.trigger).toBe('string');
expect(typeof item.name).toBe('string');
diff --git a/tests/unit/helpers/repair-builder-routes-harness.ts b/tests/unit/helpers/repair-builder-routes-harness.ts
index eebb777af..cd9896d40 100644
--- a/tests/unit/helpers/repair-builder-routes-harness.ts
+++ b/tests/unit/helpers/repair-builder-routes-harness.ts
@@ -208,7 +208,7 @@ export function makeShareDb(inspResult: unknown) {
export function makeShareServices(overrides: {
getByShareToken?: ReturnType ` —
+ // the whole point of moving it onto a registry template.
+ expect(body.html).toContain('');
+ expect(body.html).toContain('https://billing.example.com');
});
it('emails the tenant owner with the 5/5 "cap reached" copy', async () => {
From fe60d47fa8d6ac287626c1086212857cb49255c5 Mon Sep 17 00:00:00 2001
From: important-new
+ {m.notif_prefs_always_reason()}
+ {m.notif_prefs_choose_empty()} {m.portal_notif_desc()} {error ?? saveError}
+ {m.settings_notifications_eyebrow()}
+ {m.settings_notifications_desc()} {error} {loadError} {m.settings_notifications_desc()} {error}
{m.agent_portal_settings_notifications_desc()}
{notifyError} {notifications.error} {m.notif_prefs_legend()}
+ {consent.state === "revoked" ? m.notif_prefs_sms_revoked({ date: day(consent.at) })
+ : consent.state === "none" ? m.notif_prefs_sms_none()
+ : consent.phone ? m.notif_prefs_sms_on({ phone: consent.phone })
+ : m.notif_prefs_sms_on_no_phone()}
+
+ {m.notif_prefs_sms_captured({
+ date: day(consent.at),
+ source: SOURCE[consent.capturedVia](),
+ })}
+ {m.notif_prefs_sms_implied()} {m.notif_prefs_sms_implied()}
+ {consent.disclosure!.text}
+ {loadError} {m.notif_prefs_sms_implied()}
- {consent.disclosure!.text}
- {data.disclosureText}
- {data.privacyUrl && (
- {m.sms_optin_privacy_link()}
- )}
- {data.privacyUrl && data.termsUrl && · }
- {data.termsUrl && (
- {m.sms_optin_terms_link()}
- )}
-
diff --git a/app/routes/settings-profile.tsx b/app/routes/settings-profile.tsx
index 160ed5ee3..2f4aaa6ee 100644
--- a/app/routes/settings-profile.tsx
+++ b/app/routes/settings-profile.tsx
@@ -18,7 +18,7 @@ import { LOCALE_OPTIONS } from "~/lib/locales";
import { SectionNav } from "~/components/settings/SectionNav";
import { CredentialsEditor, type EditorCredential } from "~/components/settings/CredentialsEditor";
import { NotificationPreferencesCard } from "~/components/settings/NotificationPreferencesCard";
-import { bulkNotificationChoice, loadNotificationScreen, saveNotificationChoice } from "~/lib/settings-notifications.server";
+import { bulkNotificationChoice, grantNotificationSms, loadNotificationScreen, saveNotificationChoice } from "~/lib/settings-notifications.server";
import { m } from "~/paraglide/messages";
/* ------------------------------------------------------------------ */
@@ -73,6 +73,10 @@ export async function action({ request, context }: Route.ActionArgs) {
return { ...(await bulkNotificationChoice(api, fd)), intent };
}
+ if (intent === "grant-notification-sms") {
+ return { ...(await grantNotificationSms(api, request)), intent };
+ }
+
// Handle save-signature intent from the SignaturePad fetcher
if (intent === "save-signature") {
const signatureBase64 = fd.get("signatureBase64") as string | null;
@@ -502,6 +506,7 @@ export default function SettingsProfilePage() {
alwaysSent={notifications.alwaysSent}
youChoose={notifications.youChoose}
loadError={notifications.error}
+ smsConsent={notifications.smsConsent}
/>
{m.auth_agent_signup_have_account()}{" "}
{m.auth_agent_signup_login_link()}
From 228edd3f77e0fb90106bb93e3f55d0d3ffbf2f43 Mon Sep 17 00:00:00 2001
From: important-new
+ {data.brand.companyName ?? m.portal_brand_eyebrow_fallback()}
+
+ {m.portal_landing_signed_in_as({ email: data.email })}
+
+ {data.brand.companyName ?? m.portal_brand_eyebrow_fallback()}
+ {m.portal_notif_signin_subtitle()} {m.portal_landing_sent_title()} {m.portal_landing_sent_body()} {m.portal_landing_sent_recovery()} {effectiveDate}
+ {m.public_legal_last_updated({ date: effectiveDate })}
+ {m.settings_profile_signature_subtitle()} {m.settings_profile_signature_empty()} {m.settings_profile_saved_signature_subtitle()} {m.settings_profile_photo_subtitle()} {m.settings_profile_photo_hint()}
- {m.settings_profile_signature_subtitle()}
- {m.settings_profile_signature_empty()}
- {m.settings_profile_saved_signature_subtitle()}
- {m.settings_profile_photo_subtitle()} {m.settings_profile_photo_hint()} {fields.phone.errors[0]} {fields.licenseNumber.errors[0]} {m.settings_profile_license_hint()} {m.media_logo_hint()} {m.media_logo_hint()} {m.settings_profile_signature_empty()} {uploadError.message} {m.settings_profile_saved_signature_subtitle()} {m.settings_profile_signature_empty_hint()} {uploadError} {m.settings_profile_credentials_empty()} {m.settings_profile_credentials_primary_badge()} {shownError.message}
`, inserted after escaping, so no author-supplied markup goes live.
The second one was already wrong for every editable template, not just
the new ones.
A new gate asserts every declared variable has a preview example. It
found two pre-existing holes: `agent-login-link.loginUrl` and
`concierge-cancelled-agent.reason` fell back to the literal `{loginUrl}`,
so the preview an admin used to check their copy rendered the CTA with a
junk href. Nothing failed; it just looked wrong to whoever opened it.
The class gate proved itself again before being trusted: with the four
new descriptors added and no classes, it went red naming exactly the
unclassified ones.
The registry outgrew the file-size cap, so it splits by audience into
`catalog/{system,client,agent,concierge}.ts`. Splitting rather than
bumping the baseline is safe here because the scannable "everything we
send" list is now NOTIFICATION_CLASSES; the registry is the copy store.
Two count assertions were stale and are now honest: `email-registry`'s
uniqueness check compared against a literal 20 instead of REGISTRY.length,
and the route's own OpenAPI prose still claimed 17 editable templates when
there were 19.
Co-Authored-By: Claude Opus 5 Sign in to your portal
-
')
- : '';
-
- const html = `
- ${safeAddress}
-
');
+}
+
/** Reverse the HTML-entity encoding interpolate() added, so the subject is plain text (not entity-encoded). */
function unescapeEntities(s: string): string {
return s.replace(/</g, '<').replace(/>/g, '>').replace(/"/g, '"').replace(/'/g, "'").replace(/&/g, '&');
diff --git a/server/lib/email-templates/sample-data.ts b/server/lib/email-templates/sample-data.ts
index 32a13ceeb..1ae7ad17f 100644
--- a/server/lib/email-templates/sample-data.ts
+++ b/server/lib/email-templates/sample-data.ts
@@ -8,11 +8,14 @@ const EXAMPLES: Record
Line two.');
+ expect(r.html).not.toContain('');
+ });
+
+ it('leaves no empty paragraph when an optional block resolves to nothing', () => {
+ const r = mk().render('repair-request-share', { propertyAddress: '12 Elm', shareUrl: 'https://x/s' });
+ expect(r.html).toContain('12 Elm');
+ expect(r.html).not.toMatch(/
Thanks.');
+ });
+
+ it('repair-request share with no note leaves no empty block behind', async () => {
+ const p = probe();
+ await p.sendRepairRequestShare('contractor@x.com', {
+ propertyAddress: '12 Elm St',
+ shareUrl: 'https://x/repair-request/tok',
+ });
+ expect(p.captured[0].html).not.toMatch(/
+ {m.notif_prefs_always_heading()}
+
+
+ {m.notif_prefs_always_show()}
+
+
+ {alwaysSent.map((item) => (
+
+
+ {m.notif_prefs_choose_heading()}
+
+
{m.notif_prefs_choose_heading()}
+ {/* aria-live so the confirmation reaches a reader who cannot
+ see the row they just changed. Same words either way —
+ the switch already said WHAT changed. */}
+
+ {status === "saving" ? m.notif_prefs_saving()
+ : status === "saved" ? m.notif_prefs_saved()
+ : ""}
+
{m.portal_notif_heading()}
+ {m.settings_notifications_heading()}
+ {m.settings_notifications_heading()}
+ {m.notif_prefs_sms_heading()}
+
+
+
+ {m.notif_prefs_sms_disclosure_show()}
+
+ {m.settings_profile_credentials_details_summary()}
{m.portal_notif_signin_heading()}
+
{heading}
- ` is one most
recipients never see.
`CredentialService.listRenderable` replaces three hand-written copies of the
same six-line mapping (booking's footer, the Profile preview, and this), which
is how the badge URL form comes to differ between the email a client receives
and the page they land on. Its spec covers the parts a re-implementation gets
wrong: url-encoding a key containing a space, the inspector's own sort order,
dropping inactive rows, and dropping a row that is neither badge nor label — a
credential row is created BLANK and filled in, so an abandoned one would
otherwise render as an empty chip.
The report cover strip and report signature block are still fed nothing; that
is the snapshot work, which is a larger piece.
Co-Authored-By: Claude Opus 5 (1M context)
]+src="https:\/\/app\.inspectorhub\.io\/api\/public\/brand-asset/);
+ // The negative half: no `src="/…"` anywhere in the signature, which is
+ // what shipping the stored value verbatim would produce.
+ expect(html).not.toMatch(/
]+src="\/api\/public/);
+ });
+
+ it('renders a text-only credential too, so a blocked image never loses it', async () => {
+ await svc.sendReportReady('client@example.com', '1 Main St', 'https://r.example/abc', WITH_CREDENTIALS, HOST);
+ const html = sent[0]?.html ?? '';
+ // Mail clients block remote images by default. A credential that exists
+ // only as an
is a credential most recipients never see.
+ expect(html).toContain('InterNACHI Certified #NACHI-22');
+ expect(html).toContain('Licensed home inspector #TX-9001');
+ });
+
+ it('still sends a clean signature for an inspector with no credentials', async () => {
+ await svc.sendReportReady('client@example.com', '1 Main St', 'https://r.example/abc',
+ { ...STUB_INSPECTOR, credentials: [] }, HOST);
+ const html = sent[0]?.html ?? '';
+ expect(html).toContain('Mike Reynolds');
+ expect(html).not.toContain('
Date: Sat, 1 Aug 2026 01:56:53 +0800
Subject: [PATCH 28/48] refactor(settings): a button means submit; its absence
means it saved
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
Settings → Profile had six sections and ONE save affordance, floating over all
of them and owning one. The other five saved on upload, on toggle, on sign, on
blur, on click. So the sticky "Save Profile" bar taught the wrong rule in both
directions: a reader who edited a credential saw it and assumed nothing had
been saved yet (it had), and a reader who edited their name watched it follow
them down the page with no sign of what it belonged to.
The button now lives INSIDE the profile card, and the form contains only that
card. Every other section saves itself — which was already true of four of
them, and is now true of the email-signature toggle as well. It was a checkbox
inside the profile form, saved by the page's button along with name and phone:
a control that looked self-contained and was not.
MOVING IT OUT SET A TRAP, which is the part worth reading. The old code did
`fd.getAll("signatureEnabled").at(-1) === "true"`. On a form that no longer
carries the field that evaluates `undefined === "true"` and writes FALSE — so
saving an unrelated profile field would quietly switch an inspector's email
signature off, with nothing on screen to say so; they would find out from a
recipient. `signatureEnabledFromForm` makes absence distinct from false, and
its spec turns red the moment the guard is removed.
The saves with no button now have to be confirmable, or "no button means it
saved" is a claim the page cannot back:
- credentials save on BLUR, the most invisible save here — nothing moved when
it worked and nothing moved when it did not;
- a FAILED photo upload said nothing at all (success reloads the page, and
failure left the old photo sitting there looking untouched);
- and all four credential handlers `await`ed the call and returned
`success: true` whatever came back, so a rejected write reported as a save.
Survivable while nothing rendered the result; not survivable now.
The two signature cards moved into their own module, because holding this rule
in the route meant the route also held two fetchers, a toast, the pad's state
and their markup on top of the one form it actually submits. Each card owning
its own is what makes the rule legible rather than asserted. Their specs pin
that neither grew a submit control.
Verified in the browser before the extraction: one submit button, inside
`#profile-details`; no sticky bar; the form contains only that section; the
toggle persists (`is_signature_enabled` 1 -> 0) and reports "Saved"; and a
profile save afterwards leaves the flag alone. Both themes checked by computed
style — card, divider and button tokens all resolve per theme. Chrome's
screenshot transport failed partway through (a CDP parameter error), so the
post-extraction pass is render specs and type-check rather than a picture.
File-size baseline: +15 after extracting 90 lines out.
Co-Authored-By: Claude Opus 5 (1M context)
{m.settings_profile_signature_heading()}
+ {m.settings_profile_saved_signature_heading()}
+ {m.settings_profile_photo_heading()}
-
- ) : (
- {m.settings_profile_photo_none()}
- )}
-
{m.settings_profile_signature_heading()}
- {m.settings_profile_saved_signature_heading()}
- {m.settings_profile_photo_heading()}
+
+ ) : (
+ {m.settings_profile_photo_none()}
+ )}
+
Licensed home inspector · TX-INSP-9001
📞 (303) 555-0142 ✉️ mike@acme.test
Book again: https://app.inspectorhub.io/book/acme
Licensed home inspector #TX-INSP-9001
📞 (303) 555-0142 ✉️ mike@acme.test
Book again: https://app.inspectorhub.io/book/acme
) : (
(equal
// height); text-only credentials -> "label #member". layout comes from the
// resolved profile's badgeLayout (Plan 1a): 'strip' = its own wrapping row,
@@ -24,7 +25,7 @@ export function CredentialBadges({
return (
+
))}
{texts.length > 0 && (
diff --git a/scripts/file-size-baseline.json b/scripts/file-size-baseline.json
index 74e5901a8..df56ae6d1 100644
--- a/scripts/file-size-baseline.json
+++ b/scripts/file-size-baseline.json
@@ -3,12 +3,12 @@
"app/routes/inspector-portal.tsx": 1180,
"server/services/inspection/inspection-core.service.ts": 1117,
"server/services/booking.service.ts": 966,
- "server/services/inspection/inspection-report.service.ts": 948,
+ "server/services/inspection/inspection-report.service.ts": 941,
"server/durable-objects/inspection-doc.ts": 912,
"server/lib/collab/results-doc.ts": 874,
"app/routes/inspections.tsx": 867,
"server/api/sms.ts": 843,
- "app/components/portal/sections/ReportView.tsx": 806,
+ "app/components/portal/sections/ReportView.tsx": 807,
"app/routes/settings-communication.tsx": 777,
"server/services/inspection.service.ts": 757,
"server/api/admin/admin-settings.ts": 742,
diff --git a/server/api/public/inspector-profile.ts b/server/api/public/inspector-profile.ts
index 753aa5e40..fe4b0e534 100644
--- a/server/api/public/inspector-profile.ts
+++ b/server/api/public/inspector-profile.ts
@@ -9,6 +9,9 @@ import { isServableBrandAsset } from '../../lib/report-style/brand-asset-key';
import { getDrizzle } from '../../lib/route-helpers';
import { r2Get } from '../../lib/r2/objects';
import { getBaseUrl } from '../../lib/url';
+import { imagesBinding } from '../../lib/media/serve-photo';
+import { resolveBadgeVariant, isVectorBadge } from '../../lib/media/badge-variant';
+import { logger } from '../../lib/logger';
const brandRoute = createRoute(withMcpMetadata({
method: 'get',
@@ -63,7 +66,20 @@ const brandAssetRoute = createRoute(withMcpMetadata({
path: '/brand-asset',
tags: ['public'],
summary: 'Public brand asset (tenant logo) bytes',
- request: { query: z.object({ key: z.string().describe('R2 object key under the branding/ prefix.') }) },
+ request: { query: z.object({
+ key: z.string().describe('R2 object key under the branding/ prefix.'),
+ // A STRING, not an enum, and that is deliberate. An enum 400s on an
+ // unrecognised value, which during a rolling deploy means a client
+ // holding older or newer JS asks for a variant this worker does not
+ // know and gets a BROKEN IMAGE. Unknown degrades to the original
+ // instead: bigger than it needed to be, which is a cost, rather than
+ // absent, which is a defect. `resolveBadgeVariant` owns the allowlist.
+ v: z.string().optional().describe(
+ 'Render variant: email | reportSignature | reportCover. Badges are drawn at 28-40px ' +
+ 'but stored at whatever was uploaded, so this serves a size the surface can use. ' +
+ 'Omitted or unrecognised serves the original, never an error.',
+ ),
+ }) },
responses: {
200: { content: { 'image/*': { schema: z.any() } }, description: 'Asset bytes' },
404: { description: 'Key outside branding/ or object missing' },
@@ -72,6 +88,16 @@ const brandAssetRoute = createRoute(withMcpMetadata({
description: 'Streams a tenant brand asset (logo) from R2. Only keys under the public `branding/` prefix are servable; everything else in the bucket stays scoped to its own routes.',
}, { scopes: [], tier: 'extended' }));
+/** The stored bytes, unchanged — the answer whenever a transform is not wanted
+ * or not possible. */
+function brandAssetResponse(obj: R2ObjectBody): Response {
+ const headers = new Headers();
+ headers.set('Content-Type', obj.httpMetadata?.contentType || 'application/octet-stream');
+ headers.set('Cache-Control', 'public, max-age=3600');
+ if (obj.httpEtag) headers.set('etag', obj.httpEtag);
+ return new Response(obj.body, { status: 200, headers });
+}
+
const publicInspectorProfileRoutes = createApiRouter()
.openapi(brandRoute, async (c) => {
const { tenant } = c.req.valid('param');
@@ -112,7 +138,7 @@ const publicInspectorProfileRoutes = createApiRouter()
} }, 200);
})
.openapi(brandAssetRoute, async (c) => {
- const { key } = c.req.valid('query');
+ const { key, v } = c.req.valid('query');
if (!c.env.PHOTOS) return c.notFound();
// Public brand-asset endpoint may ONLY serve branding logos (never arbitrary
// R2 objects). New layout: {tenantId}/branding/logo-{uuid}.{ext}; legacy:
@@ -121,11 +147,39 @@ const publicInspectorProfileRoutes = createApiRouter()
if (!isServableBrandAsset(key)) return c.notFound();
const obj = await r2Get(c.env.PHOTOS, key);
if (!obj) return c.notFound();
- const headers = new Headers();
- headers.set('Content-Type', obj.httpMetadata?.contentType || 'application/octet-stream');
- headers.set('Cache-Control', 'public, max-age=3600');
- if (obj.httpEtag) headers.set('etag', obj.httpEtag);
- return new Response(obj.body, { status: 200, headers });
+
+ // Serve-time downscale. This is what reaches the badges ALREADY in R2 —
+ // an upload-time rule only ever helps the next upload, and the 2 MB
+ // photograph somebody has already saved is the one costing every
+ // recipient of every email.
+ //
+ // FAILS OPEN IN EVERY DIRECTION: no variant asked for, no IMAGES
+ // binding, a vector, or a transform that throws — all serve the
+ // original. A badge that is larger than it needed to be is a cost; a
+ // badge that does not render is a broken document.
+ const variant = resolveBadgeVariant(v);
+ const images = imagesBinding(c.env);
+ if (variant && images && !isVectorBadge(obj.httpMetadata?.contentType)) {
+ try {
+ const out = await images.input(obj.body)
+ .transform({ width: variant.width })
+ .output({ format: variant.format });
+ const r = out.response();
+ const h = new Headers(r.headers);
+ // Immutable: the key carries a uuid, so a replaced badge is a
+ // new key. Longer than the original's hour for that reason.
+ h.set('Cache-Control', 'public, max-age=31536000, immutable');
+ return new Response(r.body, { status: 200, headers: h });
+ } catch (err) {
+ logger.warn('[brand-asset] badge transform failed — serving original', {
+ key, variant: v, error: String(err),
+ });
+ const orig = await r2Get(c.env.PHOTOS, key);
+ if (!orig) return c.notFound();
+ return brandAssetResponse(orig);
+ }
+ }
+ return brandAssetResponse(obj);
});
export default publicInspectorProfileRoutes;
diff --git a/server/lib/inspector-signature.ts b/server/lib/inspector-signature.ts
index a9c5adf20..306786000 100644
--- a/server/lib/inspector-signature.ts
+++ b/server/lib/inspector-signature.ts
@@ -16,6 +16,8 @@
* retired. SignatureUser.slug is still accepted but is no longer read.
*/
+import { badgeUrl } from './media/badge-variant';
+
export interface SignatureUser {
name?: string | null;
email?: string | null;
@@ -91,7 +93,12 @@ export function inspectorSignature(user: SignatureUser, host: string): Signature
const imgs = creds
.filter((c) => c.imageUrl)
.map((c) => {
- const abs = c.imageUrl!.startsWith('/') ? `https://${host}${c.imageUrl}` : c.imageUrl!;
+ // The EMAIL variant: PNG (Outlook cannot draw WebP) at twice the
+ // 28px it is about to be scaled to. Without this the recipient
+ // downloads whatever was uploaded — up to 2 MB — to render a
+ // chip the height of a line of text.
+ const sized = badgeUrl(c.imageUrl, 'email') ?? c.imageUrl!;
+ const abs = sized.startsWith('/') ? `https://${host}${sized}` : sized;
return `
`;
})
.join('');
diff --git a/server/lib/mcp/openapi-snapshot.json b/server/lib/mcp/openapi-snapshot.json
index dc2acedcc..49be3f5af 100644
--- a/server/lib/mcp/openapi-snapshot.json
+++ b/server/lib/mcp/openapi-snapshot.json
@@ -7620,6 +7620,16 @@
"type": "string",
"description": "R2 object key under the branding/ prefix."
}
+ },
+ {
+ "name": "v",
+ "in": "query",
+ "required": false,
+ "description": "Render variant: email | reportSignature | reportCover. Badges are drawn at 28-40px but stored at whatever was uploaded, so this serves a size the surface can use. Omitted or unrecognised serves the original, never an error.",
+ "schema": {
+ "type": "string",
+ "description": "Render variant: email | reportSignature | reportCover. Badges are drawn at 28-40px but stored at whatever was uploaded, so this serves a size the surface can use. Omitted or unrecognised serves the original, never an error."
+ }
}
],
"body": null
diff --git a/server/lib/media/badge-variant.ts b/server/lib/media/badge-variant.ts
new file mode 100644
index 000000000..9a74a1684
--- /dev/null
+++ b/server/lib/media/badge-variant.ts
@@ -0,0 +1,88 @@
+/**
+ * Credential badges are rendered TINY and stored as whatever was uploaded.
+ *
+ * Nothing crops, resizes or re-encodes a badge on the way in: the uploader keeps
+ * the original format on purpose (a transparent PNG or an SVG must survive
+ * intact), and the server writes the bytes straight to R2 behind a 2 MB cap and
+ * a mime allowlist. Meanwhile every surface that renders one scales it to
+ * between 28 and 40 CSS pixels.
+ *
+ * So a 2 MB photograph — which the allowlist permits, and which people do
+ * upload — is delivered in full and then drawn at 28px. In an inbox that is 2 MB
+ * per recipient per send, because mail clients have no `srcset` and fetch
+ * whatever the `src` names.
+ *
+ * Transforming at SERVE time rather than at upload time is what fixes the badges
+ * ALREADY in R2, which an upload-time rule cannot reach.
+ */
+
+/** The CSS height each surface draws a badge at, and the width we serve for it. */
+export const BADGE_VARIANTS = {
+ /** `inspector-signature.ts` renders `height:28px`. */
+ email: { width: 56, format: 'image/png' as const },
+ /** `ReportSignatureBlock` renders `h-8` (32px). */
+ reportSignature: { width: 64, format: 'image/webp' as const },
+ /** `CredentialBadges` renders `h-10` (40px). */
+ reportCover: { width: 80, format: 'image/webp' as const },
+} as const;
+
+export type BadgeVariant = keyof typeof BADGE_VARIANTS;
+
+/**
+ * EMAIL GETS PNG, NOT WEBP, and that is not a rounding error.
+ *
+ * Everything else on the web can take WebP. Mail is the one medium where the
+ * renderer is somebody else's decade-old client: Outlook on Windows draws with
+ * Word's engine and shows a broken-image box for WebP. A badge that fails to
+ * render in an inbox is worse than a badge that is 30% larger, so the email
+ * variant stays PNG — which also keeps transparency, the whole point of a badge.
+ */
+export function badgeFormat(variant: BadgeVariant): string {
+ return BADGE_VARIANTS[variant].format;
+}
+
+/**
+ * Append the variant to a `/api/public/brand-asset` URL.
+ *
+ * A no-op for anything that is not one of our brand-asset paths, so a caller
+ * holding an absolute or already-transformed URL passes it through unchanged.
+ */
+export function badgeUrl(imageUrl: string | null, variant: BadgeVariant): string | null {
+ if (!imageUrl || !imageUrl.startsWith('/api/public/brand-asset?')) return imageUrl;
+ return `${imageUrl}&v=${variant}`;
+}
+
+/**
+ * Resolve a `?v=` query value to a transform, or null.
+ *
+ * Null means "serve the original": an unknown variant, or none asked for. Never
+ * an error — a badge that fails to load is a worse outcome than a badge that is
+ * bigger than it needed to be, so every uncertain path here degrades to the
+ * bytes that were uploaded.
+ */
+export function resolveBadgeVariant(
+ raw: string | undefined,
+): { width: number; format: string } | null {
+ if (!raw) return null;
+ // `Object.hasOwn`, not a bare lookup: `BADGE_VARIANTS['__proto__']` resolves
+ // to `Object.prototype`, which is TRUTHY — so a bare index would take the
+ // transform branch with `{ width: undefined, format: undefined }` for a
+ // caller who asked for `?v=__proto__`. The route's zod enum already bars
+ // that, but this function is exported and reachable on its own.
+ if (!Object.hasOwn(BADGE_VARIANTS, raw)) return null;
+ const v = BADGE_VARIANTS[raw as BadgeVariant];
+ return { width: v.width, format: v.format };
+}
+
+/**
+ * SVG is left ALONE.
+ *
+ * It is already resolution-independent, so there is nothing to gain, and
+ * rasterising it would throw away the one property that makes it the format the
+ * uploader recommends. Detected on the stored content type rather than the key,
+ * because the key's extension is derived from that content type at upload and
+ * the content type is what the browser will act on.
+ */
+export function isVectorBadge(contentType: string | null | undefined): boolean {
+ return (contentType ?? '').includes('svg');
+}
diff --git a/tests/unit/credentials/badge-variant.spec.ts b/tests/unit/credentials/badge-variant.spec.ts
new file mode 100644
index 000000000..00644a172
--- /dev/null
+++ b/tests/unit/credentials/badge-variant.spec.ts
@@ -0,0 +1,88 @@
+/**
+ * Serving a credential badge at the size it is actually drawn.
+ *
+ * Nothing crops or compresses a badge on the way in — deliberately, so a
+ * transparent PNG or an SVG survives intact — and every surface then scales it
+ * to between 28 and 40 CSS pixels. A 2 MB photograph (which the mime allowlist
+ * permits, and which people upload) was therefore delivered whole and drawn at
+ * 28px. In an inbox that is 2 MB per recipient per send, because mail clients
+ * have no `srcset` and fetch whatever the `src` names.
+ */
+import { describe, it, expect } from 'vitest';
+import {
+ BADGE_VARIANTS, badgeFormat, badgeUrl, resolveBadgeVariant, isVectorBadge,
+} from '../../../server/lib/media/badge-variant';
+
+const BASE = '/api/public/brand-asset?key=t1%2Fcredentials%2Fa%2Flogo.png';
+
+describe('badge variants', () => {
+ it('serves at least twice the CSS height each surface draws', () => {
+ // The rendered sizes are literals in three different files
+ // (`height:28px` inline, `h-8`, `h-10`). If one of them grows, this is
+ // the spec that should make someone revisit the width.
+ expect(BADGE_VARIANTS.email.width).toBeGreaterThanOrEqual(28 * 2);
+ expect(BADGE_VARIANTS.reportSignature.width).toBeGreaterThanOrEqual(32 * 2);
+ expect(BADGE_VARIANTS.reportCover.width).toBeGreaterThanOrEqual(40 * 2);
+ });
+
+ it('gives EMAIL png and the web surfaces webp', () => {
+ // Outlook on Windows draws with Word's engine and shows a broken-image
+ // box for WebP. A badge that fails to render in an inbox is worse than
+ // one that is 30% larger — and PNG keeps the transparency that makes it
+ // a badge rather than a white rectangle.
+ expect(badgeFormat('email')).toBe('image/png');
+ expect(badgeFormat('reportSignature')).toBe('image/webp');
+ expect(badgeFormat('reportCover')).toBe('image/webp');
+ });
+});
+
+describe('badgeUrl', () => {
+ it('appends the variant to a brand-asset url', () => {
+ expect(badgeUrl(BASE, 'email')).toBe(`${BASE}&v=email`);
+ expect(badgeUrl(BASE, 'reportCover')).toBe(`${BASE}&v=reportCover`);
+ });
+
+ it('passes through anything that is not a brand-asset path', () => {
+ // A caller may hold an absolute url (the email signature absolutises
+ // against the deployment host) or an external one. Rewriting those
+ // would produce a query the other end does not understand.
+ expect(badgeUrl('https://cdn.example/logo.png', 'email')).toBe('https://cdn.example/logo.png');
+ expect(badgeUrl(null, 'email')).toBeNull();
+ });
+});
+
+describe('resolveBadgeVariant', () => {
+ it('resolves the three known variants', () => {
+ expect(resolveBadgeVariant('email')).toEqual({ width: 56, format: 'image/png' });
+ expect(resolveBadgeVariant('reportCover')).toEqual({ width: 80, format: 'image/webp' });
+ });
+
+ it('returns null for absent or unknown, so the original is served', () => {
+ // Fail OPEN. A badge larger than it needed to be is a cost; a badge
+ // that does not render is a broken document.
+ expect(resolveBadgeVariant(undefined)).toBeNull();
+ expect(resolveBadgeVariant('')).toBeNull();
+ expect(resolveBadgeVariant('enormous')).toBeNull();
+ expect(resolveBadgeVariant('__proto__')).toBeNull();
+ });
+});
+
+describe('isVectorBadge', () => {
+ it('leaves SVG alone', () => {
+ // Already resolution-independent, so there is nothing to gain — and
+ // rasterising it would throw away the property that makes it the format
+ // the uploader recommends in the first place.
+ expect(isVectorBadge('image/svg+xml')).toBe(true);
+ });
+
+ it('transforms raster formats', () => {
+ for (const t of ['image/png', 'image/jpeg', 'image/webp']) {
+ expect(isVectorBadge(t), t).toBe(false);
+ }
+ });
+
+ it('treats a missing content type as raster rather than crashing', () => {
+ expect(isVectorBadge(null)).toBe(false);
+ expect(isVectorBadge(undefined)).toBe(false);
+ });
+});
diff --git a/tests/unit/email/email-signature-integration.spec.ts b/tests/unit/email/email-signature-integration.spec.ts
index 2ed4d61e0..1ec289058 100644
--- a/tests/unit/email/email-signature-integration.spec.ts
+++ b/tests/unit/email/email-signature-integration.spec.ts
@@ -150,6 +150,21 @@ describe('EmailService — credential badges reach the recipient', () => {
expect(html).not.toMatch(/
]+src="\/api\/public/);
});
+ it('asks for the EMAIL badge variant, not the stored original', async () => {
+ await svc.sendReportReady('client@example.com', '1 Main St', 'https://r.example/abc', WITH_CREDENTIALS, HOST);
+ const html = sent[0]?.html ?? '';
+ // Badges are stored at whatever was uploaded — up to 2 MB — and drawn
+ // here at 28px. Without the variant the recipient downloads the whole
+ // thing to render a chip the height of a line of text, on every open.
+ // `&`, not `&` — the whole src goes through escapeHtml, which is the
+ // correct encoding for an attribute and what every mail client decodes.
+ // Asserting the raw ampersand here would fail against correct output.
+ expect(html).toContain('&v=email');
+ // PNG, because Outlook draws with Word's engine and shows a broken-image
+ // box for WebP. The variant name is what carries that decision.
+ expect(html).toMatch(/
]+src="https:\/\/app\.inspectorhub\.io\/api\/public\/brand-asset[^"]*&v=email"/);
+ });
+
it('renders a text-only credential too, so a blocked image never loses it', async () => {
await svc.sendReportReady('client@example.com', '1 Main St', 'https://r.example/abc', WITH_CREDENTIALS, HOST);
const html = sent[0]?.html ?? '';
From 6ad893a61b4ecf11d343b6b5351c140df4a67b7b Mon Sep 17 00:00:00 2001
From: important-new
+ )}
+
+
+
+ {onPreviewReport && (
+
+