diff --git a/src/browser/routes/agent.snapshot.test.ts b/src/browser/routes/agent.snapshot.test.ts new file mode 100644 index 00000000000..77b802bdf7d --- /dev/null +++ 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 { + 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 index 3fb076bc061..7739caa051e 100644 --- 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>; +}): Promise { + 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 }); }, }); });