Add fix for page.url() race when navigation happens as a result of act()

packages/core/lib/v3/handlers/handlerUtils/actHandlerUtils.ts

@@ -77,6 +77,12 @@ export async function performUnderstudyMethod(
     args: Array.from(args),
   });
 
+  // Arm the watcher before the action so we do not miss very fast URL changes,
+  // but start the timeout window only after the click returns.
+  const navWatcher = shouldWaitForPostClickSettle(method)
+    ? watchMainFrameUrlChangeStart(page, page.url())
+    : null;
+
   try {
     const handler = METHOD_HANDLER_MAP[method] ?? null;
 
@@ -106,6 +112,11 @@ export async function performUnderstudyMethod(
           );
       }
     }
+    if (navWatcher && (await navWatcher.wait(400))) {
+      await waitForDomNetworkQuiet(page.mainFrame(), domSettleTimeoutMs).catch(
+        () => {},
+      );
+    }
   } catch (e) {
     const msg = e instanceof Error ? e.message : String(e);
     const stack = e instanceof Error ? e.stack : undefined;
@@ -126,10 +137,111 @@ export async function performUnderstudyMethod(
     }
     throw new UnderstudyCommandException(msg, e);
   } finally {
+    navWatcher?.dispose();
     SessionFileLogger.logUnderstudyActionCompleted();
   }
 }
 
+function shouldWaitForPostClickSettle(method: string): boolean {
+  return method === "click" || method === "doubleClick";
+}
+
+function watchMainFrameUrlChangeStart(
+  page: Page,
+  initialUrl: string,
+): {
+  wait: (timeoutMs: number) => Promise<boolean>;
+  dispose: () => void;
+} {
+  const session = page.mainFrame().session;
+  const initialMainFrameId = page.mainFrameId();
+  let matched = false;
+  let disposed = false;
+  let waiter:
+    | {
+        resolve: (value: boolean) => void;
+        timer: ReturnType<typeof setTimeout>;
+      }
+    | undefined;
+
+  const finish = (value: boolean) => {
+    if (disposed) return;
+    // Latch a successful match so wait() can return immediately when a very
+    // fast navigation starts before the post-click timeout window begins.
+    if (value) matched = true;
+    if (waiter) {
+      clearTimeout(waiter.timer);
+      const current = waiter;
+      waiter = undefined;
+      current.resolve(value);
+    }
+  };
+
+  const dispose = () => {
+    // dispose() is called from cleanup paths that can overlap, so it must be
+    // safe to run more than once.
+    if (disposed) return;
+    disposed = true;
+    session.off("Page.frameNavigated", onFrameNavigated);
+    session.off("Page.navigatedWithinDocument", onNavigatedWithinDocument);
+    if (waiter) {
+      clearTimeout(waiter.timer);
+      waiter.resolve(false);
+      waiter = undefined;
+    }
+  };
+
+  // Main-frame identity can shift during navigation. Treat both the original
+  // root frame id and the current page.mainFrameId() as authoritative.
+  const isMainFrame = (frameId: string): boolean =>
+    frameId === initialMainFrameId || frameId === page.mainFrameId();
+
+  // We only treat main-frame events with an actual URL change as navigation
+  // start. Real pages can emit same-URL history events that should not trigger
+  // a post-click settle wait.
+  const onFrameNavigated = (event: Protocol.Page.FrameNavigatedEvent) => {
+    if (
+      isMainFrame(event.frame.id) &&
+      event.frame.url &&
+      event.frame.url !== initialUrl
+    ) {
+      finish(true);
+    }
+  };
+
+  const onNavigatedWithinDocument = (
+    event: Protocol.Page.NavigatedWithinDocumentEvent,
+  ) => {
+    if (isMainFrame(event.frameId) && event.url !== initialUrl) {
+      finish(true);
+    }
+  };
+
+  session.on("Page.frameNavigated", onFrameNavigated);
+  session.on("Page.navigatedWithinDocument", onNavigatedWithinDocument);
+
+  return {
+    wait: async (timeoutMs: number) => {
+      // If navigation started while the click was still in flight, surface that
+      // immediately instead of burning the full timeout budget after the click.
+      if (matched) return true;
+      if (disposed) return false;
+      return await new Promise<boolean>((resolve) => {
+        waiter = {
+          resolve,
+          timer: setTimeout(() => {
+            // Clear waiter before resolving so a late navigation event cannot
+            // race in and resolve the same wait() call twice.
+            waiter = undefined;
+            resolve(false);
+          }, timeoutMs),
+        };
+      });
+    },
+    dispose,
+  };
+}
+
 /* ===================== Handlers & Map ===================== */
 
 const METHOD_HANDLER_MAP: Record<

packages/core/tests/integration/perform-understudy-navigation-race.spec.ts

@@ -0,0 +1,110 @@
+import { expect, test } from "@playwright/test";
+import { V3 } from "../../lib/v3/v3.js";
+import { performUnderstudyMethod } from "../../lib/v3/handlers/handlerUtils/actHandlerUtils.js";
+import { closeV3 } from "./testUtils.js";
+import { v3DynamicTestConfig } from "./v3.dynamic.config.js";
+
+async function loadDelayedDocumentNavigationFixture(
+  v3: V3,
+  delayMs: number,
+): Promise<{
+  page: ReturnType<V3["context"]["pages"]>[number];
+  sourceUrl: string;
+  targetUrl: string;
+}> {
+  const page = v3.context.pages()[0];
+  const sourceUrl = "https://example.com/";
+  const targetUrl = `https://example.com/?nav-race=${delayMs}`;
+
+  await page.goto(sourceUrl, { waitUntil: "load" });
+
+  await page.mainFrame().evaluate(
+    ({ delay, nextUrl }) => {
+      document.open();
+      document.write(`<!DOCTYPE html>
+        <html>
+          <body>
+            <button id="go" type="button">Go</button>
+          </body>
+        </html>`);
+      document.close();
+
+      (window as typeof window & { __navScheduled?: boolean }).__navScheduled =
+        false;
+      document.getElementById("go")?.addEventListener("click", () => {
+        (
+          window as typeof window & { __navScheduled?: boolean }
+        ).__navScheduled = true;
+        setTimeout(() => {
+          window.location.assign(nextUrl);
+        }, delay);
+      });
+    },
+    { delay: delayMs, nextUrl: targetUrl },
+  );
+
+  return {
+    page,
+    sourceUrl,
+    targetUrl,
+  };
+}
+
+test.describe("performUnderstudyMethod navigation race", () => {
+  let v3: V3;
+
+  test.beforeEach(async () => {
+    v3 = new V3(v3DynamicTestConfig);
+    await v3.init();
+  });
+
+  test.afterEach(async () => {
+    await closeV3(v3);
+  });
+
+  test("waits for navigation that starts within 400ms of click", async () => {
+    const { page, targetUrl } = await loadDelayedDocumentNavigationFixture(
+      v3,
+      250,
+    );
+
+    await performUnderstudyMethod(
+      page,
+      page.mainFrame(),
+      "click",
+      "xpath=/html/body/button",
+      [],
+      3_000,
+    );
+
+    expect(page.url()).toBe(targetUrl);
+  });
+
+  test("does not wait for navigation that starts after 400ms of click", async () => {
+    const { page, sourceUrl, targetUrl } =
+      await loadDelayedDocumentNavigationFixture(v3, 900);
+
+    await performUnderstudyMethod(
+      page,
+      page.mainFrame(),
+      "click",
+      "xpath=/html/body/button",
+      [],
+      3_000,
+    );
+
+    expect(page.url()).toBe(sourceUrl);
+    expect(
+      await page.evaluate(() => {
+        return (window as typeof window & { __navScheduled?: boolean })
+          .__navScheduled;
+      }),
+    ).toBe(true);
+
+    await expect
+      .poll(() => page.url(), {
+        timeout: 3_000,
+      })
+      .toBe(targetUrl);
+  });
+});