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

* 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 #19744 (67bac62c2) 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>
This commit is contained in:
金炳
2026-03-01 01:23:24 +08:00
committed by GitHub
parent 26db298d3e
commit 28c80689d4
2 changed files with 159 additions and 2 deletions

View File

@@ -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");
});
});

View File

@@ -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 });
},
});
});