Fix E2E test reliability issues and stale data bugs

- Fix race conditions: Set workers: 1 since all tests share test user state
- Fix stale data: GET /api/user and /api/cycle/current now fetch fresh data
  from database instead of returning stale PocketBase auth store cache
- Fix timing: Replace waitForTimeout with retry-based Playwright assertions
- Fix mobile test: Use exact heading match to avoid strict mode violation
- Add test user setup: Include notificationTime and update rule for users

All 1014 unit tests and 190 E2E tests pass.

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>

IMPLEMENTATION_PLAN.md

@@ -147,6 +147,11 @@ These are optional enhancements to improve E2E coverage. Not required for featur
 
 ## Revision History
 
+- 2026-01-13: Fixed E2E test reliability issues:
+  - Race conditions: Changed to single worker execution (tests share test user state)
+  - Stale data: GET /api/user and GET /api/cycle/current now fetch fresh data from database instead of stale auth cache
+  - Timing: Replaced fixed waitForTimeout calls with retry-based Playwright assertions
+  - Mobile test: Fixed strict mode violation by using exact heading match
 - 2026-01-13: Marked notifications.spec.ts as redundant (notification preferences already covered in settings.spec.ts)
 - 2026-01-13: Added dark-mode.spec.ts with 2 E2E tests (system preference detection for light/dark mode)
 - 2026-01-13: Added 4 Garmin E2E tests (network error recovery for save, disconnect, status fetch, retry)

e2e/dashboard.spec.ts

@@ -129,12 +129,13 @@ test.describe("dashboard", () => {
         // Click the toggle
         await toggleCheckbox.click();
 
-        // Wait a moment for the API call
-        await page.waitForTimeout(500);
-
-        // Toggle should change state
-        const afterChecked = await toggleCheckbox.isChecked();
-        expect(afterChecked).not.toBe(initialChecked);
+        // Wait for the checkbox state to change using retry-based assertion
+        // The API call completes and React re-renders asynchronously
+        if (initialChecked) {
+          await expect(toggleCheckbox).not.toBeChecked({ timeout: 5000 });
+        } else {
+          await expect(toggleCheckbox).toBeChecked({ timeout: 5000 });
+        }
       } else {
         test.skip();
       }
@@ -244,59 +245,39 @@ test.describe("dashboard", () => {
     });
 
     test("displays cycle day in 'Day X' format", async ({ page }) => {
+      // Wait for dashboard to finish loading
       await page.waitForLoadState("networkidle");
 
-      // Look for "Day" followed by a number
-      const cycleDayText = page.getByText(/Day \d+/i);
-      const hasCycleDay = await cycleDayText
-        .first()
-        .isVisible()
-        .catch(() => false);
+      // Wait for either cycle day or onboarding - both are valid states
+      // Use Playwright's expect with retry for reliable detection
+      const cycleDayText = page.getByText(/Day \d+/i).first();
+      const onboarding = page.getByText(/set.*period|log.*period/i).first();
 
-      // Either has cycle day or onboarding (both valid states)
-      if (!hasCycleDay) {
-        const onboarding = page.getByText(/set.*period|log.*period/i);
-        const hasOnboarding = await onboarding
-          .first()
-          .isVisible()
-          .catch(() => false);
-        expect(hasCycleDay || hasOnboarding).toBe(true);
+      try {
+        // First try waiting for cycle day with a short timeout
+        await expect(cycleDayText).toBeVisible({ timeout: 5000 });
+      } catch {
+        // If no cycle day, expect onboarding banner
+        await expect(onboarding).toBeVisible({ timeout: 5000 });
       }
     });
 
     test("displays current phase name", async ({ page }) => {
+      // Wait for dashboard to finish loading
       await page.waitForLoadState("networkidle");
 
-      // Look for phase names
-      const phaseNames = [
-        "MENSTRUAL",
-        "FOLLICULAR",
-        "OVULATION",
-        "EARLY_LUTEAL",
-        "LATE_LUTEAL",
-      ];
-      let foundPhase = false;
+      // Wait for either a phase name or onboarding - both are valid states
+      const phaseRegex =
+        /MENSTRUAL|FOLLICULAR|OVULATION|EARLY_LUTEAL|LATE_LUTEAL/i;
+      const phaseText = page.getByText(phaseRegex).first();
+      const onboarding = page.getByText(/set.*period|log.*period/i).first();
 
-      for (const phase of phaseNames) {
-        const phaseText = page.getByText(new RegExp(phase, "i"));
-        const isVisible = await phaseText
-          .first()
-          .isVisible()
-          .catch(() => false);
-        if (isVisible) {
-          foundPhase = true;
-          break;
-        }
-      }
-
-      // Either has phase or shows onboarding
-      if (!foundPhase) {
-        const onboarding = page.getByText(/set.*period|log.*period/i);
-        const hasOnboarding = await onboarding
-          .first()
-          .isVisible()
-          .catch(() => false);
-        expect(foundPhase || hasOnboarding).toBe(true);
+      try {
+        // First try waiting for a phase name with a short timeout
+        await expect(phaseText).toBeVisible({ timeout: 5000 });
+      } catch {
+        // If no phase, expect onboarding banner
+        await expect(onboarding).toBeVisible({ timeout: 5000 });
       }
     });
 
@@ -381,81 +362,62 @@ test.describe("dashboard", () => {
     });
 
     test("displays seed cycling recommendation", async ({ page }) => {
+      // Wait for dashboard to finish loading
       await page.waitForLoadState("networkidle");
 
-      // Look for seed names (flax, pumpkin, sesame, sunflower)
-      const seedText = page.getByText(/flax|pumpkin|sesame|sunflower/i);
-      const hasSeeds = await seedText
-        .first()
-        .isVisible()
-        .catch(() => false);
+      // Wait for either seed info or onboarding - both are valid states
+      const seedText = page.getByText(/flax|pumpkin|sesame|sunflower/i).first();
+      const onboarding = page.getByText(/set.*period|log.*period/i).first();
 
-      // Either has seeds info or onboarding
-      if (!hasSeeds) {
-        const onboarding = page.getByText(/set.*period|log.*period/i);
-        const hasOnboarding = await onboarding
-          .first()
-          .isVisible()
-          .catch(() => false);
-        expect(hasSeeds || hasOnboarding).toBe(true);
+      try {
+        await expect(seedText).toBeVisible({ timeout: 5000 });
+      } catch {
+        await expect(onboarding).toBeVisible({ timeout: 5000 });
       }
     });
 
     test("displays carbohydrate range", async ({ page }) => {
+      // Wait for dashboard to finish loading
       await page.waitForLoadState("networkidle");
 
-      // Look for carb-related text
-      const carbText = page.getByText(/carb|carbohydrate/i);
-      const hasCarbs = await carbText
-        .first()
-        .isVisible()
-        .catch(() => false);
+      // Wait for either carb info or onboarding - both are valid states
+      const carbText = page.getByText(/carb|carbohydrate/i).first();
+      const onboarding = page.getByText(/set.*period|log.*period/i).first();
 
-      if (!hasCarbs) {
-        const onboarding = page.getByText(/set.*period|log.*period/i);
-        const hasOnboarding = await onboarding
-          .first()
-          .isVisible()
-          .catch(() => false);
-        expect(hasCarbs || hasOnboarding).toBe(true);
+      try {
+        await expect(carbText).toBeVisible({ timeout: 5000 });
+      } catch {
+        await expect(onboarding).toBeVisible({ timeout: 5000 });
       }
     });
 
     test("displays keto guidance", async ({ page }) => {
+      // Wait for dashboard to finish loading
       await page.waitForLoadState("networkidle");
 
-      // Look for keto-related text
-      const ketoText = page.getByText(/keto/i);
-      const hasKeto = await ketoText
-        .first()
-        .isVisible()
-        .catch(() => false);
+      // Wait for either keto info or onboarding - both are valid states
+      const ketoText = page.getByText(/keto/i).first();
+      const onboarding = page.getByText(/set.*period|log.*period/i).first();
 
-      if (!hasKeto) {
-        const onboarding = page.getByText(/set.*period|log.*period/i);
-        const hasOnboarding = await onboarding
-          .first()
-          .isVisible()
-          .catch(() => false);
-        expect(hasKeto || hasOnboarding).toBe(true);
+      try {
+        await expect(ketoText).toBeVisible({ timeout: 5000 });
+      } catch {
+        await expect(onboarding).toBeVisible({ timeout: 5000 });
       }
     });
 
     test("displays nutrition section header", async ({ page }) => {
+      // Wait for dashboard to finish loading
       await page.waitForLoadState("networkidle");
 
-      // Nutrition panel should have a header
+      // Wait for nutrition header or text
       const nutritionHeader = page.getByRole("heading", { name: /nutrition/i });
-      const hasHeader = await nutritionHeader.isVisible().catch(() => false);
+      const nutritionText = page.getByText(/nutrition/i).first();
 
-      if (!hasHeader) {
-        // May be text label instead of heading
-        const nutritionText = page.getByText(/nutrition/i);
-        const hasText = await nutritionText
-          .first()
-          .isVisible()
-          .catch(() => false);
-        expect(hasHeader || hasText).toBe(true);
+      try {
+        await expect(nutritionHeader).toBeVisible({ timeout: 5000 });
+      } catch {
+        await expect(nutritionText).toBeVisible({ timeout: 5000 });
       }
     });
   });

e2e/garmin.spec.ts

@@ -40,6 +40,19 @@ test.describe("garmin connection", () => {
       await page.waitForURL("/", { timeout: 10000 });
       await page.goto("/settings/garmin");
       await page.waitForLoadState("networkidle");
+
+      // Clean up: Disconnect if already connected to ensure clean state
+      const disconnectButton = page.getByRole("button", {
+        name: /disconnect/i,
+      });
+      const isConnected = await disconnectButton.isVisible().catch(() => false);
+
+      if (isConnected) {
+        await disconnectButton.click();
+        await page.waitForTimeout(1000);
+        // Wait for disconnect to complete
+        await page.waitForLoadState("networkidle");
+      }
     });
 
     test("shows not connected initially for new user", async ({ page }) => {

e2e/mobile.spec.ts

@@ -140,15 +140,18 @@ test.describe("mobile viewport", () => {
       const viewportSize = page.viewportSize();
       expect(viewportSize?.width).toBe(375);
 
-      // Calendar heading should be visible
-      const heading = page.getByRole("heading", { name: /calendar/i });
-      await expect(heading).toBeVisible();
+      // Calendar page title heading should be visible (exact match to avoid "Calendar Subscription")
+      const heading = page.getByRole("heading", {
+        name: "Calendar",
+        exact: true,
+      });
+      await expect(heading).toBeVisible({ timeout: 10000 });
 
       // Calendar grid should be visible
       const calendarGrid = page
         .getByRole("grid")
         .or(page.locator('[data-testid="month-view"]'));
-      await expect(calendarGrid).toBeVisible();
+      await expect(calendarGrid).toBeVisible({ timeout: 5000 });
 
       // Month navigation should be visible
       const monthYear = page.getByText(

e2e/plan.spec.ts

@@ -53,31 +53,32 @@ test.describe("plan page", () => {
     test("shows current cycle status section", async ({ page }) => {
       await page.waitForLoadState("networkidle");
 
-      // Look for Current Status section
+      // Wait for page to finish loading - look for Current Status or error state
       const statusSection = page.getByRole("heading", {
         name: "Current Status",
       });
-      const hasStatus = await statusSection.isVisible().catch(() => false);
+      // Use text content to find error alert (avoid Next.js route announcer)
+      const errorAlert = page.getByText(/error:/i);
 
-      if (hasStatus) {
-        await expect(statusSection).toBeVisible();
+      try {
+        // Wait for Current Status section to be visible (data loaded successfully)
+        await expect(statusSection).toBeVisible({ timeout: 10000 });
 
         // Should show day number
-        await expect(page.getByText(/day \d+/i)).toBeVisible();
+        await expect(page.getByText(/day \d+/i)).toBeVisible({ timeout: 5000 });
 
         // Should show training type
-        await expect(page.getByText(/training type:/i)).toBeVisible();
+        await expect(page.getByText(/training type:/i)).toBeVisible({
+          timeout: 5000,
+        });
 
         // Should show weekly limit
-        await expect(page.getByText(/weekly limit:/i)).toBeVisible();
-      } else {
-        // If no status, should see loading or error state
-        const loading = page.getByText(/loading/i);
-        const error = page.getByRole("alert");
-        const hasLoading = await loading.isVisible().catch(() => false);
-        const hasError = await error.isVisible().catch(() => false);
-
-        expect(hasLoading || hasError).toBe(true);
+        await expect(page.getByText(/weekly limit:/i)).toBeVisible({
+          timeout: 5000,
+        });
+      } catch {
+        // If status section not visible, check for error alert
+        await expect(errorAlert).toBeVisible({ timeout: 5000 });
       }
     });
 

e2e/pocketbase-harness.ts

@@ -140,6 +140,12 @@ async function addUserFields(pb: PocketBase): Promise<void> {
  * Sets up API rules for collections to allow user access.
  */
 async function setupApiRules(pb: PocketBase): Promise<void> {
+  // Allow users to update their own user record
+  const usersCollection = await pb.collections.getOne("users");
+  await pb.collections.update(usersCollection.id, {
+    updateRule: "id = @request.auth.id",
+  });
+
   // Allow users to read/write their own period_logs
   const periodLogs = await pb.collections.getOne("period_logs");
   await pb.collections.update(periodLogs.id, {
@@ -202,6 +208,7 @@ async function createTestUser(
     verified: true,
     lastPeriodDate,
     cycleLength: 28,
+    notificationTime: "07:00",
     timezone: "UTC",
   });
 

e2e/settings.spec.ts

@@ -435,10 +435,12 @@ test.describe("settings", () => {
       const newValue = originalValue === "08:00" ? "09:00" : "08:00";
       await notificationTimeInput.fill(newValue);
 
-      // Save
+      // Save and wait for success toast
       const saveButton = page.getByRole("button", { name: /save/i });
       await saveButton.click();
-      await page.waitForTimeout(1500);
+      await expect(page.getByText(/settings saved successfully/i)).toBeVisible({
+        timeout: 10000,
+      });
 
       // Reload the page
       await page.reload();
@@ -453,7 +455,9 @@ test.describe("settings", () => {
       // Restore original value
       await notificationTimeAfter.fill(originalValue);
       await saveButton.click();
-      await page.waitForTimeout(500);
+      await expect(page.getByText(/settings saved successfully/i)).toBeVisible({
+        timeout: 10000,
+      });
     });
 
     test("timezone changes persist after page reload", async ({ page }) => {
@@ -475,10 +479,12 @@ test.describe("settings", () => {
           : "America/New_York";
       await timezoneInput.fill(newValue);
 
-      // Save
+      // Save and wait for success toast
       const saveButton = page.getByRole("button", { name: /save/i });
       await saveButton.click();
-      await page.waitForTimeout(1500);
+      await expect(page.getByText(/settings saved successfully/i)).toBeVisible({
+        timeout: 10000,
+      });
 
       // Reload the page
       await page.reload();
@@ -493,7 +499,9 @@ test.describe("settings", () => {
       // Restore original value
       await timezoneAfter.fill(originalValue);
       await saveButton.click();
-      await page.waitForTimeout(500);
+      await expect(page.getByText(/settings saved successfully/i)).toBeVisible({
+        timeout: 10000,
+      });
     });
 
     test("multiple settings changes persist after page reload", async ({
@@ -536,10 +544,12 @@ test.describe("settings", () => {
       await notificationTimeInput.fill(newNotificationTime);
       await timezoneInput.fill(newTimezone);
 
-      // Save all changes at once
+      // Save all changes at once and wait for success toast
       const saveButton = page.getByRole("button", { name: /save/i });
       await saveButton.click();
-      await page.waitForTimeout(1500);
+      await expect(page.getByText(/settings saved successfully/i)).toBeVisible({
+        timeout: 10000,
+      });
 
       // Reload the page
       await page.reload();
@@ -561,7 +571,9 @@ test.describe("settings", () => {
       await notificationTimeAfter.fill(originalNotificationTime);
       await timezoneAfter.fill(originalTimezone);
       await saveButton.click();
-      await page.waitForTimeout(500);
+      await expect(page.getByText(/settings saved successfully/i)).toBeVisible({
+        timeout: 10000,
+      });
     });
 
     test("cycle length persistence verifies exact saved value", async ({
@@ -582,10 +594,12 @@ test.describe("settings", () => {
       const newValue = originalValue === "28" ? "31" : "28";
       await cycleLengthInput.fill(newValue);
 
-      // Save
+      // Save and wait for success toast
       const saveButton = page.getByRole("button", { name: /save/i });
       await saveButton.click();
-      await page.waitForTimeout(1500);
+      await expect(page.getByText(/settings saved successfully/i)).toBeVisible({
+        timeout: 10000,
+      });
 
       // Reload the page
       await page.reload();
@@ -600,7 +614,9 @@ test.describe("settings", () => {
       // Restore original value
       await cycleLengthAfter.fill(originalValue);
       await saveButton.click();
-      await page.waitForTimeout(500);
+      await expect(page.getByText(/settings saved successfully/i)).toBeVisible({
+        timeout: 10000,
+      });
     });
 
     test("settings form shows correct values after save without reload", async ({

playwright.config.ts

@@ -22,8 +22,9 @@ export default defineConfig({
   // Retry failed tests on CI only
   retries: process.env.CI ? 2 : 0,
 
-  // Limit parallel workers on CI to avoid resource issues
-  workers: process.env.CI ? 1 : undefined,
+  // Run tests sequentially since all tests share the same test user
+  // Parallel execution causes race conditions when tests modify user state
+  workers: 1,
 
   // Reporter configuration
   reporter: [["html", { open: "never" }], ["list"]],

src/app/api/cycle/current/route.test.ts

@@ -9,9 +9,29 @@ import type { User } from "@/types";
 // Module-level variable to control mock user in tests
 let currentMockUser: User | null = null;
 
+// Create mock PocketBase getOne function that returns fresh user data
+const mockPbGetOne = vi.fn().mockImplementation(() => {
+  if (!currentMockUser) {
+    throw new Error("User not found");
+  }
+  return Promise.resolve({
+    id: currentMockUser.id,
+    email: currentMockUser.email,
+    lastPeriodDate: currentMockUser.lastPeriodDate?.toISOString(),
+    cycleLength: currentMockUser.cycleLength,
+  });
+});
+
+// Create mock PocketBase client
+const mockPb = {
+  collection: vi.fn(() => ({
+    getOne: mockPbGetOne,
+  })),
+};
+
 // Mock PocketBase client for database operations
 vi.mock("@/lib/pocketbase", () => ({
-  createPocketBaseClient: vi.fn(() => ({})),
+  createPocketBaseClient: vi.fn(() => mockPb),
   loadAuthFromCookies: vi.fn(),
   isAuthenticated: vi.fn(() => currentMockUser !== null),
   getCurrentUser: vi.fn(() => currentMockUser),
@@ -24,7 +44,7 @@ vi.mock("@/lib/auth-middleware", () => ({
       if (!currentMockUser) {
         return NextResponse.json({ error: "Unauthorized" }, { status: 401 });
       }
-      return handler(request, currentMockUser);
+      return handler(request, currentMockUser, mockPb);
     };
   }),
 }));

src/app/api/cycle/current/route.ts

@@ -40,9 +40,18 @@ function getDaysUntilNextPhase(cycleDay: number, cycleLength: number): number {
   return nextPhaseStart - cycleDay;
 }
 
-export const GET = withAuth(async (_request, user) => {
+export const GET = withAuth(async (_request, user, pb) => {
+  // Fetch fresh user data from database to get latest values
+  // The user param from withAuth is from auth store cache which may be stale
+  const freshUser = await pb.collection("users").getOne(user.id);
+
   // Validate user has required cycle data
-  if (!user.lastPeriodDate) {
+  const lastPeriodDate = freshUser.lastPeriodDate
+    ? new Date(freshUser.lastPeriodDate as string)
+    : null;
+  const cycleLength = (freshUser.cycleLength as number) || 28;
+
+  if (!lastPeriodDate) {
     return NextResponse.json(
       {
         error:
@@ -53,20 +62,16 @@ export const GET = withAuth(async (_request, user) => {
   }
 
   // Calculate current cycle position
-  const cycleDay = getCycleDay(
-    user.lastPeriodDate,
-    user.cycleLength,
-    new Date(),
-  );
-  const phase = getPhase(cycleDay, user.cycleLength);
+  const cycleDay = getCycleDay(lastPeriodDate, cycleLength, new Date());
+  const phase = getPhase(cycleDay, cycleLength);
   const phaseConfig = getPhaseConfig(phase);
-  const daysUntilNextPhase = getDaysUntilNextPhase(cycleDay, user.cycleLength);
+  const daysUntilNextPhase = getDaysUntilNextPhase(cycleDay, cycleLength);
 
   return NextResponse.json({
     cycleDay,
     phase,
     phaseConfig,
     daysUntilNextPhase,
-    cycleLength: user.cycleLength,
+    cycleLength,
   });
 });

src/app/api/user/route.test.ts

@@ -12,10 +12,28 @@ let currentMockUser: User | null = null;
 // Track PocketBase update calls
 const mockPbUpdate = vi.fn().mockResolvedValue({});
 
+// Track PocketBase getOne calls - returns the current mock user data
+const mockPbGetOne = vi.fn().mockImplementation(() => {
+  if (!currentMockUser) {
+    throw new Error("User not found");
+  }
+  return Promise.resolve({
+    id: currentMockUser.id,
+    email: currentMockUser.email,
+    garminConnected: currentMockUser.garminConnected,
+    lastPeriodDate: currentMockUser.lastPeriodDate?.toISOString(),
+    cycleLength: currentMockUser.cycleLength,
+    notificationTime: currentMockUser.notificationTime,
+    timezone: currentMockUser.timezone,
+    activeOverrides: currentMockUser.activeOverrides,
+  });
+});
+
 // Create mock PocketBase client
 const mockPb = {
   collection: vi.fn(() => ({
     update: mockPbUpdate,
+    getOne: mockPbGetOne,
   })),
 };
 
@@ -55,6 +73,7 @@ describe("GET /api/user", () => {
     vi.clearAllMocks();
     currentMockUser = null;
     mockPbUpdate.mockClear();
+    mockPbGetOne.mockClear();
   });
 
   it("returns user profile when authenticated", async () => {
@@ -133,6 +152,7 @@ describe("PATCH /api/user", () => {
     vi.clearAllMocks();
     currentMockUser = null;
     mockPbUpdate.mockClear();
+    mockPbGetOne.mockClear();
   });
 
   // Helper to create mock request with JSON body

src/app/api/user/route.ts

@@ -13,23 +13,28 @@ const TIME_FORMAT_REGEX = /^([01]\d|2[0-3]):([0-5]\d)$/;
 /**
  * GET /api/user
  * Returns the authenticated user's profile.
+ * Fetches fresh data from database to ensure updates are reflected.
  * Excludes sensitive fields like encrypted tokens.
  */
-export const GET = withAuth(async (_request, user, _pb) => {
+export const GET = withAuth(async (_request, user, pb) => {
+  // Fetch fresh user data from database to get latest values
+  // The user param from withAuth is from auth store cache which may be stale
+  const freshUser = await pb.collection("users").getOne(user.id);
+
   // Format date for consistent API response
-  const lastPeriodDate = user.lastPeriodDate
-    ? user.lastPeriodDate.toISOString().split("T")[0]
+  const lastPeriodDate = freshUser.lastPeriodDate
+    ? new Date(freshUser.lastPeriodDate as string).toISOString().split("T")[0]
     : null;
 
   return NextResponse.json({
-    id: user.id,
-    email: user.email,
-    garminConnected: user.garminConnected,
-    cycleLength: user.cycleLength,
+    id: freshUser.id,
+    email: freshUser.email,
+    garminConnected: freshUser.garminConnected ?? false,
+    cycleLength: freshUser.cycleLength,
     lastPeriodDate,
-    notificationTime: user.notificationTime,
-    timezone: user.timezone,
-    activeOverrides: user.activeOverrides,
+    notificationTime: freshUser.notificationTime,
+    timezone: freshUser.timezone,
+    activeOverrides: freshUser.activeOverrides ?? [],
   });
 });