fix(browser): encapsulate targetId resolution logic after navigation

Introduced a new function `resolveTargetIdAfterNavigate` to handle the resolution of the correct targetId after a navigation event that may trigger a renderer swap. This refactor improves code clarity and reuses the logic for determining the current targetId, ensuring that the correct tab is identified even when multiple tabs share the same URL.

src/browser/routes/agent.snapshot.test.ts

@@ -0,0 +1,121 @@
+import { describe, expect, it, vi } from "vitest";
+import { resolveTargetIdAfterNavigate } from "./agent.snapshot.js";
+
+type Tab = { targetId: string; url: string };
+
+function staticListTabs(tabs: Tab[]): () => Promise<Tab[]> {
+  return async () => tabs;
+}
+
+describe("resolveTargetIdAfterNavigate", () => {
+  it("returns original targetId when old target still exists (no swap)", async () => {
+    const result = await resolveTargetIdAfterNavigate({
+      oldTargetId: "old-123",
+      navigatedUrl: "https://example.com",
+      listTabs: staticListTabs([
+        { targetId: "old-123", url: "https://example.com" },
+        { targetId: "other-456", url: "https://other.com" },
+      ]),
+    });
+    expect(result).toBe("old-123");
+  });
+
+  it("resolves new targetId when old target is gone (renderer swap)", async () => {
+    const result = await resolveTargetIdAfterNavigate({
+      oldTargetId: "old-123",
+      navigatedUrl: "https://example.com",
+      listTabs: staticListTabs([
+        { targetId: "new-456", url: "https://example.com" },
+      ]),
+    });
+    expect(result).toBe("new-456");
+  });
+
+  it("prefers non-stale targetId when multiple tabs share the URL", async () => {
+    const result = await resolveTargetIdAfterNavigate({
+      oldTargetId: "old-123",
+      navigatedUrl: "https://example.com",
+      listTabs: staticListTabs([
+        { targetId: "preexisting-000", url: "https://example.com" },
+        { targetId: "fresh-777", url: "https://example.com" },
+      ]),
+    });
+    // Both differ from old targetId; the first non-stale match wins.
+    expect(result).toBe("preexisting-000");
+  });
+
+  it("retries and resolves targetId when first listTabs has no URL match", async () => {
+    vi.useFakeTimers();
+    let calls = 0;
+
+    const result$ = resolveTargetIdAfterNavigate({
+      oldTargetId: "old-123",
+      navigatedUrl: "https://delayed.com",
+      listTabs: async () => {
+        calls++;
+        if (calls === 1) {
+          return [{ targetId: "unrelated-1", url: "https://unrelated.com" }];
+        }
+        return [{ targetId: "delayed-999", url: "https://delayed.com" }];
+      },
+    });
+
+    await vi.advanceTimersByTimeAsync(800);
+    const result = await result$;
+
+    expect(result).toBe("delayed-999");
+    expect(calls).toBe(2);
+
+    vi.useRealTimers();
+  });
+
+  it("falls back to original targetId when no match found after retry", async () => {
+    vi.useFakeTimers();
+
+    const result$ = resolveTargetIdAfterNavigate({
+      oldTargetId: "old-123",
+      navigatedUrl: "https://no-match.com",
+      listTabs: staticListTabs([
+        { targetId: "unrelated-1", url: "https://unrelated.com" },
+        { targetId: "unrelated-2", url: "https://unrelated2.com" },
+      ]),
+    });
+
+    await vi.advanceTimersByTimeAsync(800);
+    const result = await result$;
+
+    expect(result).toBe("old-123");
+
+    vi.useRealTimers();
+  });
+
+  it("falls back to single remaining tab when no URL match after retry", async () => {
+    vi.useFakeTimers();
+
+    const result$ = resolveTargetIdAfterNavigate({
+      oldTargetId: "old-123",
+      navigatedUrl: "https://single-tab.com",
+      listTabs: staticListTabs([
+        { targetId: "only-tab", url: "https://some-other.com" },
+      ]),
+    });
+
+    await vi.advanceTimersByTimeAsync(800);
+    const result = await result$;
+
+    expect(result).toBe("only-tab");
+
+    vi.useRealTimers();
+  });
+
+  it("falls back to original targetId when listTabs throws", async () => {
+    const result = await resolveTargetIdAfterNavigate({
+      oldTargetId: "old-123",
+      navigatedUrl: "https://error.com",
+      listTabs: async () => {
+        throw new Error("CDP connection lost");
+      },
+    });
+    expect(result).toBe("old-123");
+  });
+});

src/browser/routes/agent.snapshot.ts

@@ -48,6 +48,41 @@ async function saveBrowserMediaResponse(params: {
   });
 }
 
+/** Resolve the correct targetId after a navigation that may trigger a renderer swap. */
+export async function resolveTargetIdAfterNavigate(opts: {
+  oldTargetId: string;
+  navigatedUrl: string;
+  listTabs: () => Promise<Array<{ targetId: string; url: string }>>;
+}): Promise<string> {
+  let currentTargetId = opts.oldTargetId;
+  try {
+    const refreshed = await opts.listTabs();
+    if (!refreshed.some((t) => t.targetId === opts.oldTargetId)) {
+      // Renderer swap: old target gone, resolve the replacement.
+      // Prefer a URL match whose targetId differs from the old one
+      // to avoid picking a pre-existing tab when multiple share the URL.
+      const byUrl = refreshed.filter((t) => t.url === opts.navigatedUrl);
+      const replaced = byUrl.find((t) => t.targetId !== opts.oldTargetId) ?? byUrl[0];
+      if (replaced) {
+        currentTargetId = replaced.targetId;
+      } else {
+        await new Promise((r) => setTimeout(r, 800));
+        const retried = await opts.listTabs();
+        const match =
+          retried.find((t) => t.url === opts.navigatedUrl && t.targetId !== opts.oldTargetId) ??
+          retried.find((t) => t.url === opts.navigatedUrl) ??
+          (retried.length === 1 ? retried[0] : null);
+        if (match) {
+          currentTargetId = match.targetId;
+        }
+      }
+    }
+  } catch {
+    // Best-effort: fall back to pre-navigation targetId
+  }
+  return currentTargetId;
+}
+
 export function registerBrowserAgentSnapshotRoutes(
   app: BrowserRouteRegistrar,
   ctx: BrowserRouteContext,
@@ -72,32 +107,11 @@ export function registerBrowserAgentSnapshotRoutes(
           url,
           ...withBrowserNavigationPolicy(ctx.state().resolved.ssrfPolicy),
         });
-        let currentTargetId = tab.targetId;
-        try {
-          const refreshed = await profileCtx.listTabs();
-          if (!refreshed.some((t) => t.targetId === tab.targetId)) {
-            // Renderer swap: old target gone, resolve the replacement.
-            // Prefer a URL match whose targetId differs from the old one
-            // to avoid picking a pre-existing tab when multiple share the URL.
-            const byUrl = refreshed.filter((t) => t.url === result.url);
-            const replaced = byUrl.find((t) => t.targetId !== tab.targetId) ?? byUrl[0];
-            if (replaced) {
-              currentTargetId = replaced.targetId;
-            } else {
-              await new Promise((r) => setTimeout(r, 800));
-              const retried = await profileCtx.listTabs();
-              const match =
-                retried.find((t) => t.url === result.url && t.targetId !== tab.targetId) ??
-                retried.find((t) => t.url === result.url) ??
-                (retried.length === 1 ? retried[0] : null);
-              if (match) {
-                currentTargetId = match.targetId;
-              }
-            }
-          }
-        } catch {
-          // Best-effort: fall back to pre-navigation targetId
-        }
+        const currentTargetId = await resolveTargetIdAfterNavigate({
+          oldTargetId: tab.targetId,
+          navigatedUrl: result.url,
+          listTabs: () => profileCtx.listTabs(),
+        });
         res.json({ ok: true, targetId: currentTargetId, ...result });
       },
     });