Gate fixes: stale endpoint responses, stale project target, false magic-link success (PR #100 findings)

e2e/stu386-magic-link-signin.spec.js

@@ -9,6 +9,7 @@ import {
   magicLinkAliasRejected,
   magicLinkSendFailure,
   magicLinkVerifyFailure,
+  ok,
   ENDPOINTS,
 } from "./fixtures/apiFixtures.js";
 
@@ -105,6 +106,63 @@ test.describe("Magic-link sign-in", () => {
     await expect(input).toHaveValue("user+tag@gmail.com");
   });
 
+  test("an HTTP-success response with an unexpected error code is surfaced as an error", async ({ page }) => {
+    await applyDefaultRoutes(page, {
+      [ENDPOINTS.identity]: guestIdentity(),
+      [ENDPOINTS.getSubscription]: freeSubscription(),
+      // HTTP 200, but the body carries an error code that is not the
+      // specifically handled plus-alias rejection: the send did not happen.
+      [ENDPOINTS.authSendMagicLink]: ok({
+        code: "SEND_FAILED",
+        message: "We could not send the link right now.",
+      }),
+    });
+
+    await page.goto("/#account");
+    await page.locator("#auth-signedout button", { hasText: "Sign In" }).click();
+
+    const input = page.locator("#signin-email-input");
+    const submit = page.locator("#signin-submit-btn");
+    await input.fill("person@example.com");
+    await submit.click();
+
+    // The error is styled as an error, the address is kept for retry, and the
+    // control never claims the link was sent.
+    await expect(page.locator("#signin-message")).toContainText(
+      "could not send the link",
+    );
+    await expect(page.locator("#signin-message")).toHaveClass(/signin-message-error/);
+    await expect(input).toHaveValue("person@example.com");
+    await expect(input).toHaveAttribute("aria-invalid", "true");
+    await expect(submit).toHaveText("Send Sign-in Link");
+  });
+
+  test("an HTTP-success response that declares success:false is surfaced as an error", async ({ page }) => {
+    await applyDefaultRoutes(page, {
+      [ENDPOINTS.identity]: guestIdentity(),
+      [ENDPOINTS.getSubscription]: freeSubscription(),
+      // HTTP 200 with an explicit failure and no code: this must never read as
+      // "Sent!" or clear the address.
+      [ENDPOINTS.authSendMagicLink]: ok({ success: false }),
+    });
+
+    await page.goto("/#account");
+    await page.locator("#auth-signedout button", { hasText: "Sign In" }).click();
+
+    const input = page.locator("#signin-email-input");
+    const submit = page.locator("#signin-submit-btn");
+    await input.fill("person@example.com");
+    await submit.click();
+
+    await expect(page.locator("#signin-message")).toContainText(
+      "Something went wrong",
+    );
+    await expect(page.locator("#signin-message")).toHaveClass(/signin-message-error/);
+    await expect(input).toHaveValue("person@example.com");
+    await expect(input).toHaveAttribute("aria-invalid", "true");
+    await expect(submit).toHaveText("Send Sign-in Link");
+  });
+
   test("recovers from a failed send, and retry succeeds", async ({ page }) => {
     let attempt = 0;
     await applyDefaultRoutes(page, {

e2e/stu387-account-access.spec.js

@@ -4,6 +4,7 @@ import {
   guestIdentity,
   professionalSubscription,
   magicLinkSent,
+  ok,
   err,
   ENDPOINTS,
 } from "./fixtures/apiFixtures.js";
@@ -162,6 +163,33 @@ test.describe("STU-387 account access & data", () => {
     expect(rateLimitText?.trim()).not.toBe(serverErrorText?.trim());
   });
 
+  test("a resolved 200 that reports a send failure renders an error, never the sent confirmation", async ({ page }) => {
+    await seedSession(page);
+    await applyDefaultRoutes(page, {
+      ...refreshOverride(),
+      [ENDPOINTS.identity]: identityWithUsage({ email: EMAIL, count: 12 }),
+      [ENDPOINTS.getSubscription]: professionalSubscription(),
+      // HTTP 200, but the body says the send failed: the request resolving is
+      // not proof a link was sent, so the row must surface the server's
+      // failure message instead of the sent confirmation.
+      [ENDPOINTS.authSendMagicLink]: ok({
+        code: "SEND_FAILED",
+        message: "We could not send the link right now.",
+      }),
+    });
+    await page.goto("/");
+    await openAccount(page);
+
+    const msg = page.locator("#send-new-link-msg");
+    const btn = page.locator("#send-new-link-btn");
+    await btn.click();
+
+    await expect(msg).toContainText("We could not send the link right now.");
+    await expect(msg).toHaveClass(/error/);
+    await expect(msg).not.toContainText("Check your email");
+    await expect(btn).toBeEnabled();
+  });
+
   test("a second send while one is in flight is refused (single request)", async ({ page }) => {
     let calls = 0;
     await seedSession(page);

src/accountAccess.js

@@ -12,6 +12,8 @@
 // the identity/metering cookie, and stored credentials are never cleared by
 // any control in this section.
 
+import { isMagicLinkSuccess } from "./authMagicLink.js";
+
 // ---------------------------------------------------------------------------
 // Key contract
 // ---------------------------------------------------------------------------
@@ -84,11 +86,26 @@ export function performSignOut({ storage, onIdentityChanged, onPlanChanged }) {
 // Send new link (reauth)
 // ---------------------------------------------------------------------------
 
+/**
+ * The most useful user-facing message an unsuccessful send response carries:
+ * its `message`, else its `error` string, else a generic failure. Never empty,
+ * so the row's error line always says something.
+ */
+function sendFailureMessage(data) {
+  const message = data?.message || data?.error;
+  if (typeof message === "string" && message.trim()) return message.trim();
+  return "The server could not send the link.";
+}
+
 /**
  * Controller for the "Send new link" control. It reflects the ACTUAL backing
  * response or failure via onState(), moves through pending -> sent|error, and
  * hard-blocks a duplicate request while one is still in flight (settle first,
- * then allow the next send).
+ * then allow the next send). A resolved request is not automatically a sent
+ * link: the response is classified with the shared magic-link contract first,
+ * so an HTTP-success body that reports a failure (an error code,
+ * success:false, or an error field) becomes the error state carrying the
+ * server's message instead of a false "sent" confirmation.
  */
 export function createSendLinkController({ sendLink, getEmail, onState }) {
   let inFlight = false;
@@ -99,6 +116,9 @@ export function createSendLinkController({ sendLink, getEmail, onState }) {
     try {
       onState({ status: "pending", error: null });
       const data = await sendLink(getEmail());
+      if (!isMagicLinkSuccess(data)) {
+        throw new Error(sendFailureMessage(data));
+      }
       result = { status: "sent", data };
       onState({ status: "sent", data });
       return result;

src/accountAccess.test.js

@@ -30,6 +30,7 @@ import {
   isDeleteAccountSupported,
   DELETE_ACCOUNT_MESSAGE,
 } from "./accountAccess.js";
+import { MAGIC_LINK_SUCCESS_CODE } from "./authMagicLink.js";
 
 // Minimal in-memory Storage stand-in so the logic is hermetically testable with
 // no browser, network, clock dependency, or host-timezone coupling.
@@ -99,7 +100,10 @@ test("criterion 1: sign out clears the session but never scrubs identity or usag
 test("criterion 2: a successful reauth reports pending then sent with the actual response", async () => {
   const states = [];
   const send = createSendLinkController({
-    sendLink: async () => ({ code: "sent", message: "Check your email" }),
+    sendLink: async () => ({
+      code: MAGIC_LINK_SUCCESS_CODE,
+      message: "Check your email",
+    }),
     getEmail: () => "a@b.com",
     onState: (state) => states.push(state),
   });
@@ -108,7 +112,56 @@ test("criterion 2: a successful reauth reports pending then sent with the actual
   assert.equal(result.status, "sent");
   assert.equal(result.data.message, "Check your email");
   assert.equal(states[states.length - 1].status, "sent");
-  assert.equal(states[states.length - 1].data.code, "sent");
+  assert.equal(states[states.length - 1].data.code, MAGIC_LINK_SUCCESS_CODE);
+});
+
+test("criterion 2: a resolved 200 that reports a send failure is an error, never 'sent'", async () => {
+  const states = [];
+  const send = createSendLinkController({
+    sendLink: async () => ({
+      code: "SEND_FAILED",
+      message: "We could not send the link right now.",
+    }),
+    getEmail: () => "a@b.com",
+    onState: (state) => states.push(state),
+  });
+  const result = await send();
+  assert.equal(states[0].status, "pending");
+  assert.equal(result.status, "error");
+  // The server's own message is preserved so the row can say why.
+  assert.equal(result.error.message, "We could not send the link right now.");
+  assert.equal(states[states.length - 1].status, "error");
+  // A resolved HTTP request is not proof of a sent link: no state may claim it.
+  assert.ok(
+    !states.some((state) => state.status === "sent"),
+    "a failed send must never render as sent",
+  );
+});
+
+test("criterion 2: a resolved 200 with success:false is an error even without a code", async () => {
+  const states = [];
+  const send = createSendLinkController({
+    sendLink: async () => ({ success: false, error: "Mail delivery failed" }),
+    getEmail: () => "a@b.com",
+    onState: (state) => states.push(state),
+  });
+  const result = await send();
+  assert.equal(result.status, "error");
+  assert.equal(result.error.message, "Mail delivery failed");
+  assert.equal(states[states.length - 1].status, "error");
+});
+
+test("criterion 2: a genuine success body still reports sent", async () => {
+  const states = [];
+  const send = createSendLinkController({
+    sendLink: async () => ({ success: true, message: "Magic link sent" }),
+    getEmail: () => "a@b.com",
+    onState: (state) => states.push(state),
+  });
+  const result = await send();
+  assert.equal(result.status, "sent");
+  assert.equal(result.data.message, "Magic link sent");
+  assert.equal(states[states.length - 1].status, "sent");
 });
 
 test("criterion 2: a failed reauth surfaces the real failure, not a canned success", async () => {
@@ -151,7 +204,7 @@ test("criterion 2: a duplicate reauth while one is in flight is refused and send
   } finally {
     // Always settle the in-flight request so a failing assertion cannot strand
     // a forever-pending promise and cancel the sibling tests.
-    release({ code: "sent" });
+    release({ code: MAGIC_LINK_SUCCESS_CODE });
   }
   firstResult = await first;
   assert.equal(firstResult.status, "sent");
@@ -163,7 +216,7 @@ test("criterion 2: after a settle the controller accepts the next send", async (
   const send = createSendLinkController({
     sendLink: async () => {
       sendLinkCalls += 1;
-      return { code: "sent" };
+      return { code: MAGIC_LINK_SUCCESS_CODE };
     },
     getEmail: () => "a@b.com",
     onState: () => {},

src/authMagicLink.js

@@ -36,12 +36,29 @@ export function explainPlusAliasRule(rawEmail) {
   return renderRejectionMessage(email, domain);
 }
 
+/**
+ * A magic-link send is only successful when the server says so: an explicit
+ * `success: true`, the success code, or a legacy response that carries no code
+ * and no failure field. An explicit `success: false` or a truthy `error` is
+ * never a success, with or without a code, and an empty or malformed body is
+ * not success-shaped either. Any other code — including the plus-alias
+ * rejection — is an error, even though the HTTP request itself succeeded.
+ */
+export function isMagicLinkSuccess(data) {
+  if (!data || typeof data !== 'object' || Array.isArray(data)) return false;
+  if (data.success === false || data.error) return false;
+  if (data.code) return data.code === MAGIC_LINK_SUCCESS_CODE;
+  // Legacy code-less response: only an explicit success or the legacy
+  // message-only shape counts.
+  return data.success === true || typeof data.message === 'string';
+}
+
 export function getMagicLinkResultMessage(data, email) {
   if (data?.code === PLUS_ALIAS_REJECTED_CODE) {
     return data.message || explainPlusAliasRule(email) || 'Please enter your primary email address.';
   }
-  if (data?.code === MAGIC_LINK_SUCCESS_CODE || !data?.code) {
+  if (isMagicLinkSuccess(data)) {
     return `Check your email — we sent a link to ${trimEmail(email)}`;
   }
-  return data.message || 'Something went wrong. Please try again.';
+  return data?.message || 'Something went wrong. Please try again.';
 }

src/authMagicLink.test.js

@@ -5,6 +5,7 @@ import {
   explainPlusAliasRule,
   getMagicLinkResultMessage,
   isKnownProviderPlusAlias,
+  isMagicLinkSuccess,
   trimEmail,
 } from "./authMagicLink.js";
 
@@ -78,3 +79,81 @@ test("getMagicLinkResultMessage treats legacy success responses as sent", () =>
     `Check your email — we sent a link to user@${firstAllowlistedDomain}`
   );
 });
+
+test("isMagicLinkSuccess accepts only the success code or a genuinely success-shaped legacy response", () => {
+  assert.equal(isMagicLinkSuccess({ code: fixtures.successCode }), true);
+  assert.equal(isMagicLinkSuccess({ success: true, message: "Magic link sent" }), true);
+  assert.equal(isMagicLinkSuccess({ message: "Check your email" }), true);
+
+  // The alias rejection is an error despite the HTTP success...
+  assert.equal(
+    isMagicLinkSuccess({ code: fixtures.validationCode, message: "Use primary email." }),
+    false
+  );
+  // ...and so is any other server error code, which must never read as sent.
+  assert.equal(
+    isMagicLinkSuccess({ code: "SEND_FAILED", message: "Could not send the link." }),
+    false
+  );
+});
+
+test("isMagicLinkSuccess rejects explicit failures, error fields and empty bodies", () => {
+  // A 200 that says it failed is a failure, code or no code — never "sent".
+  assert.equal(isMagicLinkSuccess({ success: false }), false);
+  assert.equal(
+    isMagicLinkSuccess({ success: false, message: "Could not send the link." }),
+    false
+  );
+  assert.equal(
+    isMagicLinkSuccess({ success: false, error: "Mail delivery failed" }),
+    false
+  );
+  assert.equal(
+    isMagicLinkSuccess({ success: false, code: fixtures.successCode }),
+    false
+  );
+
+  // A code-less error payload is an error shape, not a legacy success.
+  assert.equal(isMagicLinkSuccess({ error: "Mail delivery failed" }), false);
+
+  // Empty or malformed bodies are not success-shaped.
+  assert.equal(isMagicLinkSuccess({}), false);
+  assert.equal(isMagicLinkSuccess(null), false);
+  assert.equal(isMagicLinkSuccess(undefined), false);
+  assert.equal(isMagicLinkSuccess("sent"), false);
+  assert.equal(isMagicLinkSuccess([]), false);
+});
+
+test("getMagicLinkResultMessage stays aligned with the success predicate", () => {
+  // A code-less failure must never read as a sent link.
+  assert.equal(
+    getMagicLinkResultMessage({ success: false }, `user@${firstAllowlistedDomain}`),
+    "Something went wrong. Please try again."
+  );
+  assert.equal(
+    getMagicLinkResultMessage(
+      { success: false, message: "Could not send the link." },
+      `user@${firstAllowlistedDomain}`
+    ),
+    "Could not send the link."
+  );
+  assert.equal(
+    getMagicLinkResultMessage(
+      { error: "Mail delivery failed" },
+      `user@${firstAllowlistedDomain}`
+    ),
+    "Something went wrong. Please try again."
+  );
+  // Even the success code cannot rescue an explicit failure.
+  assert.equal(
+    getMagicLinkResultMessage(
+      { success: false, code: fixtures.successCode, message: "Delivery failed." },
+      `user@${firstAllowlistedDomain}`
+    ),
+    "Delivery failed."
+  );
+  assert.equal(
+    getMagicLinkResultMessage(null, `user@${firstAllowlistedDomain}`),
+    "Something went wrong. Please try again."
+  );
+});