fix(browser): resolve correct targetId in navigate response after renderer swap (openclaw#25326)

stone-jin · cursoragent · web-flow · commit 28c80689d42b · 2026-02-28T09:23:24.000-08:00
* fix(browser): resolve correct targetId in navigate response after renderer swap When `navigateViaPlaywright` triggers a Chrome renderer-process swap (e.g. navigating from chrome-extension:// to https://), the old `tab.targetId` captured before navigation becomes stale. The `/navigate` route previously returned this stale targetId in its response. After navigation, re-resolve the current tab by matching against the final URL via `profileCtx.listTabs()`. If the old target is already gone but the new one is not yet visible (extension re-attach in progress), retry after 800ms. Follow-up to openclaw#19744 (67bac62) which fixed the extension-side stale session cleanup. * fix(browser): prefer non-stale targetId when multiple tabs share the same URL When multiple tabs have the same URL after navigation, find() could pick a pre-existing tab instead of the newly created one. Now only re-resolve when the old target is gone (renderer swap detected), and prefer the tab whose targetId differs from the old one. * 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. * refactor(tests): simplify listTabs initialization in agent snapshot tests Updated the initialization of listTabs in the agent snapshot tests for better readability by removing unnecessary line breaks. This change enhances code clarity without altering the test logic. * fix(ui): widen Set type to accept string tokens in external-link helper * chore: retrigger CI (unrelated Windows flaky test) Co-authored-by: Cursor <cursoragent@cursor.com> --------- Co-authored-by: Cursor <cursoragent@cursor.com>
diff --git a/src/browser/routes/agent.snapshot.test.ts b/src/browser/routes/agent.snapshot.test.ts
@@ -0,0 +1,117 @@
+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");
+  });
+});
diff --git a/src/browser/routes/agent.snapshot.ts b/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,
@@ -65,14 +100,19 @@ export function registerBrowserAgentSnapshotRoutes(
       ctx,
       targetId,
       feature: "navigate",
-      run: async ({ cdpUrl, tab, pw }) => {
+      run: async ({ cdpUrl, tab, pw, profileCtx }) => {
         const result = await pw.navigateViaPlaywright({
           cdpUrl,
           targetId: tab.targetId,
           url,
           ...withBrowserNavigationPolicy(ctx.state().resolved.ssrfPolicy),
         });
-        res.json({ ok: true, targetId: tab.targetId, ...result });
+        const currentTargetId = await resolveTargetIdAfterNavigate({
+          oldTargetId: tab.targetId,
+          navigatedUrl: result.url,
+          listTabs: () => profileCtx.listTabs(),
+        });
+        res.json({ ok: true, targetId: currentTargetId, ...result });
       },
     });
   });
