From a8c59f3c58532e45a64d16261ad8bde7c43cdba0 Mon Sep 17 00:00:00 2001 From: zeroaltitude Date: Wed, 4 Mar 2026 15:45:36 -0700 Subject: [PATCH] refactor(browser): extract enrichTabResponseBody + add 10 unit tests Extracted the URL enrichment logic from withRouteTabContext into a pure, testable function. Tests cover: enrichment of ok responses, postRunUrl preference, tab.url fallback, existing field preservation, non-ok/null/ array/primitive rejection, and missing URL handling. --- .../routes/agent.shared.enrich-url.test.ts | 63 +++++++++++++++ src/browser/routes/agent.shared.ts | 80 +++++++++++-------- 2 files changed, 111 insertions(+), 32 deletions(-) create mode 100644 src/browser/routes/agent.shared.enrich-url.test.ts diff --git a/src/browser/routes/agent.shared.enrich-url.test.ts b/src/browser/routes/agent.shared.enrich-url.test.ts new file mode 100644 index 00000000000..2af5440e9c2 --- /dev/null +++ b/src/browser/routes/agent.shared.enrich-url.test.ts @@ -0,0 +1,63 @@ +import { describe, expect, it } from "vitest"; +import { enrichTabResponseBody } from "./agent.shared.js"; + +const TAB = { targetId: "tid-1", url: "https://example.com/page" }; + +describe("enrichTabResponseBody", () => { + it("adds targetId and url to successful response", () => { + const body: Record = { ok: true, data: "snapshot" }; + const result = enrichTabResponseBody(body, TAB); + expect(result).toBe(true); + expect(body.targetId).toBe("tid-1"); + expect(body.url).toBe("https://example.com/page"); + }); + + it("prefers postRunUrl over tab.url", () => { + const body: Record = { ok: true }; + enrichTabResponseBody(body, TAB, "https://example.com/after-navigate"); + expect(body.url).toBe("https://example.com/after-navigate"); + }); + + it("falls back to tab.url when postRunUrl is undefined", () => { + const body: Record = { ok: true }; + enrichTabResponseBody(body, TAB, undefined); + expect(body.url).toBe("https://example.com/page"); + }); + + it("does not overwrite existing targetId", () => { + const body: Record = { ok: true, targetId: "existing-id" }; + enrichTabResponseBody(body, TAB); + expect(body.targetId).toBe("existing-id"); + }); + + it("does not overwrite existing url", () => { + const body: Record = { ok: true, url: "https://existing.com" }; + enrichTabResponseBody(body, TAB, "https://new.com"); + expect(body.url).toBe("https://existing.com"); + }); + + it("returns false for non-ok responses", () => { + const body = { ok: false, error: "not found" }; + expect(enrichTabResponseBody(body, TAB)).toBe(false); + expect((body as Record).targetId).toBeUndefined(); + }); + + it("returns false for null body", () => { + expect(enrichTabResponseBody(null, TAB)).toBe(false); + }); + + it("returns false for array body", () => { + expect(enrichTabResponseBody([{ ok: true }], TAB)).toBe(false); + }); + + it("returns false for primitive body", () => { + expect(enrichTabResponseBody("ok", TAB)).toBe(false); + }); + + it("handles tab with no url and no postRunUrl", () => { + const body: Record = { ok: true }; + enrichTabResponseBody(body, { targetId: "tid-2" }); + expect(body.targetId).toBe("tid-2"); + expect(body.url).toBeUndefined(); + }); +}); diff --git a/src/browser/routes/agent.shared.ts b/src/browser/routes/agent.shared.ts index 4f4592c87a1..0d5ae82a38d 100644 --- a/src/browser/routes/agent.shared.ts +++ b/src/browser/routes/agent.shared.ts @@ -5,6 +5,38 @@ import type { BrowserRouteContext, ProfileContext } from "../server-context.js"; import type { BrowserRequest, BrowserResponse } from "./types.js"; import { getProfileContext, jsonError } from "./utils.js"; +/** + * Enrich a tab-targeting response body with targetId and URL. + * Pure function — no side effects, no I/O. Mutates `body` in place. + * + * @returns true if enrichment was applied, false if body was not eligible. + */ +export function enrichTabResponseBody( + body: unknown, + tab: { targetId: string; url?: string }, + postRunUrl?: string, +): boolean { + if ( + !body || + typeof body !== "object" || + Array.isArray(body) || + (body as Record).ok !== true + ) { + return false; + } + const record = body as Record; + if (record.targetId === undefined) { + record.targetId = tab.targetId; + } + if (record.url === undefined) { + const resolvedUrl = postRunUrl || tab.url; + if (resolvedUrl) { + record.url = resolvedUrl; + } + } + return true; +} + export const SELECTOR_UNSUPPORTED_MESSAGE = [ "Error: 'selector' is not supported. Use 'ref' from snapshot instead.", "", @@ -148,41 +180,25 @@ export async function withRouteTabContext( // Now enrich and flush the intercepted response body. if (jsonCalled) { - if ( - interceptedBody && - typeof interceptedBody === "object" && - !Array.isArray(interceptedBody) && - (interceptedBody as Record).ok === true - ) { - const record = interceptedBody as Record; - if (record.targetId === undefined) { - record.targetId = tab.targetId; - } - if (record.url === undefined) { - // Resolve live URL *after* the handler ran, so navigating actions - // report the post-action URL. Try Playwright first (actual page - // state), fall back to tab metadata URL. - let postRunUrl: string | undefined; - try { - const pwMod = await getPwAiModuleBase({ mode: "soft" }); - if (pwMod?.getPageForTargetId) { - const page = await pwMod.getPageForTargetId({ - cdpUrl: profileCtx.profile.cdpUrl, - targetId: tab.targetId, - }); - if (page) { - postRunUrl = page.url(); - } - } - } catch { - // Playwright unavailable — fall back to tab.url - } - const resolvedUrl = postRunUrl || tab.url; - if (resolvedUrl) { - record.url = resolvedUrl; + // Resolve live URL *after* the handler ran, so navigating actions + // report the post-action URL. Try Playwright first (actual page + // state), fall back to tab metadata URL. + let postRunUrl: string | undefined; + try { + const pwMod = await getPwAiModuleBase({ mode: "soft" }); + if (pwMod?.getPageForTargetId) { + const page = await pwMod.getPageForTargetId({ + cdpUrl: profileCtx.profile.cdpUrl, + targetId: tab.targetId, + }); + if (page) { + postRunUrl = page.url(); } } + } catch { + // Playwright unavailable — fall back to tab.url } + enrichTabResponseBody(interceptedBody, tab, postRunUrl); // Restore res.json before flushing so that if originalJson throws // (e.g. BigInt serialization), the outer catch can still send errors. params.res.json = originalJson;