fix(plugin-auth): a user import reports an invitation the email transport did not deliver

.changeset/23598-plugin-auth-import-invite-delivery-failure.md

@@ -0,0 +1,15 @@
+---
+'@objectstack/plugin-auth': patch
+---
+
+fix(plugin-auth): a user import reports an invitation that the email transport failed to deliver
+
+Clause-②: no
+
+Setup › Users › Import (`POST /api/v1/auth/admin/import-users`) used to report an email invitation as delivered when the email transport failed. The import invites by asking better-auth for a password reset, and better-auth catches whatever the send throws and still answers `{ status: true }`. So the import's `INVITE_EMAIL_FAILED` path was never reached by a failed delivery: the row read `delivery: 'email'`, and `summary.errors` stayed `0`.
+
+- **A failed invitation is now reported on its row.** This covers a transport that answers `failed` and a transport that throws. The row is `ok: false`, has no `delivery`, carries `code: 'INVITE_EMAIL_FAILED'` and an `error` naming the transport's reason, and is counted in `summary.errors`. Its `action` stays `'created'` and its `id` stays, because the account exists. The fix is to send the invitation again or to set the user's password as an admin. The import does not roll the row back and does not fall back to a temporary password.
+- **The SMS invitation reports a failure the same way.** A phone-only row whose invitation SMS fails is `ok: false` with no `delivery`, carries `code: 'INVITE_SMS_FAILED'`, and is counted in `summary.errors`. Before, it read `delivery: 'sms'` and was not counted.
+- **A delivery is reported only when the email service accepts it.** A row is reported delivered when the service answers `sent` or `queued`. If the reset request answers without the service accepting the message, the row is reported failed, never delivered.
+- **`summary.delivery` counts only deliveries that succeeded.** A row whose invitation failed is counted in `summary.errors` and in `summary.created`, so `created + updated + skipped + errors` exceeds `total` by exactly those rows.
+- **A successful invitation is reported as before.** A public `POST /api/v1/auth/request-password-reset` is unchanged: it answers the same thing whether the address's mail went out, failed, or has no account behind it.

packages/plugins/plugin-auth/src/account-invitation-marker.test.ts

@@ -10,21 +10,36 @@
  *
  * The end-to-end pins (the import sends the new key; a public reset never
  * does) are `admin-import-users-account-invitation.test.ts`.
+ *
+ * [#23598] The delivery receipt the same store carries, by the same legs: it
+ * settles only inside the import's store, for the address it names, on a call
+ * with no Request; nothing settled reads `unconfirmed`. The import-door pins
+ * are `admin-import-users-invite-delivery-failure.test.ts`.
  */
 
 import { describe, it, expect } from 'vitest';
 import { runWithRequestState } from '@better-auth/core/context';
-import { readAccountInvitation, runAsAccountInvitation, type AccountInvitationMarker } from './account-invitation-marker.js';
+import {
+  readAccountInvitation,
+  runImportPasswordReset,
+  settleImportResetDelivery,
+  type AccountInvitationMarker,
+} from './account-invitation-marker.js';
 
 const MARKER: AccountInvitationMarker = {
   email: 'Invitee@Acme.Example.com',
   inviter: { name: 'Ada Admin', email: 'ada@acme.example.com' },
 };
 const HTTP_REQUEST = () => new Request('https://acme.example.com/api/v1/auth/request-password-reset', { method: 'POST' });
 
+/** The import's opener, answering what `fn` returned. */
+async function insideImportReset<T>(marker: AccountInvitationMarker, fn: () => Promise<T>): Promise<T> {
+  return (await runImportPasswordReset(marker, fn)).result;
+}
+
 describe('[#23343] readAccountInvitation', () => {
   it('inside the marker store, for the invited address and no Request, answers the marker', async () => {
-    const read = await runAsAccountInvitation(MARKER, async () => {
+    const read = await insideImportReset(MARKER, async () => {
       await Promise.resolve();
       await new Promise((resolve) => setTimeout(resolve, 1));
       return readAccountInvitation('invitee@acme.example.com', undefined);
@@ -33,25 +48,25 @@ describe('[#23343] readAccountInvitation', () => {
   });
 
   it('leg 3: the same store, read for a call that carries a Request (any HTTP request), answers nothing', async () => {
-    const read = await runAsAccountInvitation(MARKER, () => readAccountInvitation('invitee@acme.example.com', HTTP_REQUEST()));
+    const read = await insideImportReset(MARKER, () => readAccountInvitation('invitee@acme.example.com', HTTP_REQUEST()));
     expect(read).toBeUndefined();
   });
 
   it('leg 4: the same store, read for another recipient, answers nothing', async () => {
-    const read = await runAsAccountInvitation(MARKER, () => readAccountInvitation('someone-else@acme.example.com', undefined));
+    const read = await insideImportReset(MARKER, () => readAccountInvitation('someone-else@acme.example.com', undefined));
     expect(read).toBeUndefined();
   });
 
   it('leg 2: a store opened for another request (as handleRequest opens one per request) carries no marker', async () => {
-    const read = await runAsAccountInvitation(MARKER, () =>
+    const read = await insideImportReset(MARKER, () =>
       runWithRequestState(new WeakMap(), () => readAccountInvitation('invitee@acme.example.com', undefined)),
     );
     expect(read).toBeUndefined();
   });
 
   it('outside any store, and after the marked call settled, answers nothing', async () => {
     expect(await readAccountInvitation('invitee@acme.example.com', undefined)).toBeUndefined();
-    await runAsAccountInvitation(MARKER, async () => undefined);
+    await insideImportReset(MARKER, async () => undefined);
     expect(await readAccountInvitation('invitee@acme.example.com', undefined)).toBeUndefined();
   });
 
@@ -62,7 +77,7 @@ describe('[#23343] readAccountInvitation', () => {
       await gate;
       return readAccountInvitation('invitee@acme.example.com', undefined);
     })();
-    const [inside, seenOutside] = await runAsAccountInvitation(MARKER, async () => {
+    const [inside, seenOutside] = await insideImportReset(MARKER, async () => {
       openGate();
       const seen = await outside;
       return [await readAccountInvitation('invitee@acme.example.com', undefined), seen] as const;
@@ -71,3 +86,73 @@ describe('[#23343] readAccountInvitation', () => {
     expect(seenOutside).toBeUndefined();
   });
 });
+
+describe('[#23598] the import delivery receipt', () => {
+  const INVITEE = 'invitee@acme.example.com';
+  const failingSend = async () => {
+    await new Promise((resolve) => setTimeout(resolve, 1));
+    throw new Error('550 mailbox unavailable');
+  };
+
+  it('a send that succeeds, inside the import\'s store, settles accepted', async () => {
+    const { delivery } = await runImportPasswordReset({ email: INVITEE }, () =>
+      settleImportResetDelivery(INVITEE, undefined, async () => {
+        await new Promise((resolve) => setTimeout(resolve, 1));
+      }),
+    );
+    expect(delivery).toEqual({ status: 'accepted' });
+  });
+
+  it('a send that throws settles failed with its reason, and the throw still reaches better-auth', async () => {
+    let rethrown: unknown;
+    const { delivery } = await runImportPasswordReset({ email: INVITEE }, async () => {
+      // better-auth's `runInBackgroundOrAwait`: catch and log.
+      try {
+        await settleImportResetDelivery('Invitee@Acme.Example.com', undefined, failingSend);
+      } catch (e) {
+        rethrown = e;
+      }
+    });
+    expect(delivery).toEqual({ status: 'failed', reason: '550 mailbox unavailable' });
+    expect((rethrown as Error)?.message).toBe('550 mailbox unavailable');
+  });
+
+  it('the receipt rides its own slot: no inviter, no marker, still a receipt', async () => {
+    const { result, delivery } = await runImportPasswordReset({ email: INVITEE }, async () => {
+      const marker = await readAccountInvitation(INVITEE, undefined);
+      await settleImportResetDelivery(INVITEE, undefined, async () => undefined);
+      return marker;
+    });
+    expect(result).toBeUndefined();
+    expect(delivery.status).toBe('accepted');
+  });
+
+  it('nothing settled is unconfirmed, never accepted', async () => {
+    const { delivery } = await runImportPasswordReset({ email: INVITEE }, async () => ({ status: true }));
+    expect(delivery).toEqual({ status: 'unconfirmed' });
+  });
+
+  it('a call that carries a Request gets the same promise back and settles nothing', async () => {
+    const pending = Promise.resolve('sent');
+    let handedBack: unknown;
+    const { delivery } = await runImportPasswordReset({ email: INVITEE }, async () => {
+      handedBack = settleImportResetDelivery(INVITEE, HTTP_REQUEST(), () => pending);
+      await handedBack;
+    });
+    expect(handedBack).toBe(pending);
+    expect(delivery).toEqual({ status: 'unconfirmed' });
+  });
+
+  it('a send for another recipient settles nothing, and its failure still propagates', async () => {
+    const { delivery } = await runImportPasswordReset({ email: INVITEE }, async () => {
+      await expect(settleImportResetDelivery('someone-else@acme.example.com', undefined, failingSend))
+        .rejects.toThrow('550 mailbox unavailable');
+    });
+    expect(delivery).toEqual({ status: 'unconfirmed' });
+  });
+
+  it('outside any store, a send runs and propagates as before', async () => {
+    await expect(settleImportResetDelivery(INVITEE, undefined, async () => 'ok')).resolves.toBe('ok');
+    await expect(settleImportResetDelivery(INVITEE, undefined, failingSend)).rejects.toThrow('550 mailbox unavailable');
+  });
+});

packages/plugins/plugin-auth/src/account-invitation-marker.ts

@@ -40,7 +40,7 @@
  *
  * ## Why a public reset request can never select the invitation
  *
- *  1. Nothing on the wire writes it. Only {@link runAsAccountInvitation} sets
+ *  1. Nothing on the wire writes it. Only {@link runImportPasswordReset} sets
  *     the slot, and its only caller is the import, behind the platform-admin
  *     gate. No body, header or query key of `POST /request-password-reset`
  *     reaches it.
@@ -60,6 +60,36 @@
  * Absent, unreadable or mismatched, the answer is "no marker" and the send is
  * today's `auth.password_reset`, byte for byte. Absence is never upgraded into
  * an invitation.
+ *
+ * ## [#23598] The delivery receipt: how the import learns its send failed
+ *
+ * better-auth runs `sendResetPassword` through `runInBackgroundOrAwait`, which
+ * CATCHES and logs whatever the send throws, and `requestPasswordReset` still
+ * answers `{ status: true }` (installed 1.7.3, `dist/context/create-context.mjs`
+ * and `dist/api/routes/password.mjs`). That is right for the public door — the
+ * answer must not say whether an address exists or whether its mail went out —
+ * and it means a throw inside the send can never reach the import. No
+ * better-auth option makes the call rethrow: `advanced.backgroundTasks.handler`
+ * only stops it being awaited at all, and this plugin does not set it.
+ *
+ * So the import's store carries a second slot beside the marker: a receipt the
+ * import creates, that `sendResetPassword` settles ({@link
+ * settleImportResetDelivery}), and that the import reads after its request
+ * returns ({@link runImportPasswordReset}). It is its own slot because it is
+ * not about the copy: a row whose inviter cannot be named gets the reset copy
+ * and no marker, and its delivery is still the import's to report. The legs
+ * that keep the marker server-only hold for it the same way — only the import
+ * creates one, each HTTP request runs in its own store, a call that carries a
+ * `Request` neither reads nor settles it, and it names its recipient — and on
+ * WebContainer it rides the same polyfilled request state, so it is read at the
+ * start of the send and settled on the object the import holds, never looked
+ * up again after an `await`.
+ *
+ * A receipt nobody settled is NOT a delivery. It is reported as unconfirmed,
+ * which the import reports as a failed invitation: a send better-auth never
+ * started (no account behind the address) or did not await (a background-task
+ * handler) has not been shown to reach anyone. Absence is never upgraded into
+ * "delivered".
  */
 
 import { defineRequestState, runWithRequestState } from '@better-auth/core/context';
@@ -75,20 +105,86 @@ export interface AccountInvitationMarker {
 const accountInvitationState = defineRequestState<AccountInvitationMarker | null>(() => null);
 
 /**
- * Run `fn` — the import's own `requestPasswordReset` call — inside a fresh
- * request-state store that carries `marker`. The store lives exactly as long
- * as that one call: a second row opens its own.
+ * [#23598] What `sendResetPassword` reports about the one send it made for the
+ * import. `accepted`: the email service answered `sent` or `queued`. `failed`:
+ * it answered `failed`, or the send threw. `unconfirmed`: nothing settled the
+ * receipt (see the header) — not a delivery.
+ */
+export type ImportResetDelivery =
+  | { status: 'accepted' }
+  | { status: 'failed'; reason: string }
+  | { status: 'unconfirmed' };
+
+interface ImportResetReceipt {
+  /** The address the import asked a reset for, lowercased. */
+  email: string;
+  /** Set once, by the send. */
+  settled?: Exclude<ImportResetDelivery, { status: 'unconfirmed' }>;
+}
+
+const importResetReceiptState = defineRequestState<ImportResetReceipt | null>(() => null);
+
+const normalise = (email: unknown) => String(email ?? '').trim().toLowerCase();
+
+/**
+ * Run `fn` — the import's own `requestPasswordReset` call for one row — inside
+ * a fresh request-state store, and answer how its send went. The store carries
+ * the delivery receipt always, and the invitation marker when `inviter` is
+ * given (an invitation names its inviter; without one the send is the reset
+ * copy). It lives exactly as long as that one call: a second row opens its own.
  */
-export async function runAsAccountInvitation<T>(
-  marker: AccountInvitationMarker,
+export async function runImportPasswordReset<T>(
+  scope: { email: string; inviter?: AccountInvitationMarker['inviter'] },
   fn: () => Promise<T>,
-): Promise<T> {
-  const store = new WeakMap<object, unknown>([
-    [accountInvitationState.ref, { email: marker.email.trim().toLowerCase(), inviter: { ...marker.inviter } }],
-  ]);
+): Promise<{ result: T; delivery: ImportResetDelivery }> {
+  const email = normalise(scope.email);
+  const receipt: ImportResetReceipt = { email };
+  const entries: Array<[object, unknown]> = [[importResetReceiptState.ref, receipt]];
+  if (scope.inviter) entries.push([accountInvitationState.ref, { email, inviter: { ...scope.inviter } }]);
   // `await`, not a bare return: `runWithRequestState` is generic over its
   // callback, so the composed call is typed `Promise<Promise<T>>`.
-  return await runWithRequestState(store, fn);
+  const result = await runWithRequestState(new WeakMap<object, unknown>(entries), fn);
+  return { result, delivery: receipt.settled ?? { status: 'unconfirmed' } };
+}
+
+/**
+ * [#23598] Run `send` — `sendResetPassword`'s own body — and, when it is the
+ * import's request for `recipientEmail`, settle that request's receipt with how
+ * it went. The outcome is re-raised unchanged either way, so better-auth logs a
+ * failure exactly as before.
+ *
+ * A call that carries a `Request` (every HTTP request; the public door) is
+ * handed straight to `send` — the same promise, not one extra tick — so what a
+ * public `POST /request-password-reset` caller observes cannot change.
+ */
+export function settleImportResetDelivery<T>(
+  recipientEmail: string,
+  request: unknown,
+  send: () => Promise<T>,
+): Promise<T> {
+  if (request !== undefined && request !== null) return send();
+  return settleInsideImport(recipientEmail, send);
+}
+
+async function settleInsideImport<T>(recipientEmail: string, send: () => Promise<T>): Promise<T> {
+  let receipt: ImportResetReceipt | null = null;
+  try {
+    // Read once, BEFORE the send's awaits, and settled on the object itself.
+    receipt = await importResetReceiptState.get();
+  } catch {
+    // No request state at all ("No request state found"): no receipt.
+  }
+  // Only the address the import asked for.
+  if (receipt && receipt.email !== normalise(recipientEmail)) receipt = null;
+  if (!receipt) return send();
+  try {
+    const result = await send();
+    receipt.settled ??= { status: 'accepted' };
+    return result;
+  } catch (e) {
+    receipt.settled ??= { status: 'failed', reason: (e as Error)?.message ?? String(e) };
+    throw e;
+  }
 }
 
 /**