From ac4ae9ed613472b5750441a8a8bd430c806bdc6f Mon Sep 17 00:00:00 2001 From: Peter Steinberger Date: Wed, 18 Feb 2026 22:18:38 +0000 Subject: [PATCH] refactor(browser): dedupe storage and download route parsing --- src/browser/routes/agent.act.ts | 63 +++++++++++++--------- src/browser/routes/agent.storage.test.ts | 27 +++++++++- src/browser/routes/agent.storage.ts | 66 ++++++++++++++++++------ 3 files changed, 113 insertions(+), 43 deletions(-) diff --git a/src/browser/routes/agent.act.ts b/src/browser/routes/agent.act.ts index 24c8be236df..b0c393eae6a 100644 --- a/src/browser/routes/agent.act.ts +++ b/src/browser/routes/agent.act.ts @@ -18,9 +18,34 @@ import { resolvePathWithinRoot, resolvePathsWithinRoot, } from "./path-output.js"; -import type { BrowserRouteRegistrar } from "./types.js"; +import type { BrowserResponse, BrowserRouteRegistrar } from "./types.js"; import { jsonError, toBoolean, toNumber, toStringArray, toStringOrEmpty } from "./utils.js"; +function resolveDownloadPathOrRespond(res: BrowserResponse, requestedPath: string): string | null { + const downloadPathResult = resolvePathWithinRoot({ + rootDir: DEFAULT_DOWNLOAD_DIR, + requestedPath, + scopeLabel: "downloads directory", + }); + if (!downloadPathResult.ok) { + res.status(400).json({ error: downloadPathResult.error }); + return null; + } + return downloadPathResult.path; +} + +function buildDownloadRequestBase(cdpUrl: string, targetId: string, timeoutMs: number | undefined) { + return { + cdpUrl, + targetId, + timeoutMs: timeoutMs ?? undefined, + }; +} + +function respondWithDownloadResult(res: BrowserResponse, targetId: string, result: unknown) { + res.json({ ok: true, targetId, download: result }); +} + export function registerBrowserAgentActRoutes( app: BrowserRouteRegistrar, ctx: BrowserRouteContext, @@ -443,24 +468,18 @@ export function registerBrowserAgentActRoutes( run: async ({ cdpUrl, tab, pw }) => { let downloadPath: string | undefined; if (out.trim()) { - const downloadPathResult = resolvePathWithinRoot({ - rootDir: DEFAULT_DOWNLOAD_DIR, - requestedPath: out, - scopeLabel: "downloads directory", - }); - if (!downloadPathResult.ok) { - res.status(400).json({ error: downloadPathResult.error }); + const resolvedDownloadPath = resolveDownloadPathOrRespond(res, out); + if (!resolvedDownloadPath) { return; } - downloadPath = downloadPathResult.path; + downloadPath = resolvedDownloadPath; } + const requestBase = buildDownloadRequestBase(cdpUrl, tab.targetId, timeoutMs); const result = await pw.waitForDownloadViaPlaywright({ - cdpUrl, - targetId: tab.targetId, + ...requestBase, path: downloadPath, - timeoutMs: timeoutMs ?? undefined, }); - res.json({ ok: true, targetId: tab.targetId, download: result }); + respondWithDownloadResult(res, tab.targetId, result); }, }); }); @@ -485,23 +504,17 @@ export function registerBrowserAgentActRoutes( targetId, feature: "download", run: async ({ cdpUrl, tab, pw }) => { - const downloadPathResult = resolvePathWithinRoot({ - rootDir: DEFAULT_DOWNLOAD_DIR, - requestedPath: out, - scopeLabel: "downloads directory", - }); - if (!downloadPathResult.ok) { - res.status(400).json({ error: downloadPathResult.error }); + const downloadPath = resolveDownloadPathOrRespond(res, out); + if (!downloadPath) { return; } + const requestBase = buildDownloadRequestBase(cdpUrl, tab.targetId, timeoutMs); const result = await pw.downloadViaPlaywright({ - cdpUrl, - targetId: tab.targetId, + ...requestBase, ref, - path: downloadPathResult.path, - timeoutMs: timeoutMs ?? undefined, + path: downloadPath, }); - res.json({ ok: true, targetId: tab.targetId, download: result }); + respondWithDownloadResult(res, tab.targetId, result); }, }); }); diff --git a/src/browser/routes/agent.storage.test.ts b/src/browser/routes/agent.storage.test.ts index 0990a160f4f..5705af5bd69 100644 --- a/src/browser/routes/agent.storage.test.ts +++ b/src/browser/routes/agent.storage.test.ts @@ -1,5 +1,9 @@ import { describe, expect, it } from "vitest"; -import { parseStorageKind, parseStorageMutationRequest } from "./agent.storage.js"; +import { + parseRequiredStorageMutationRequest, + parseStorageKind, + parseStorageMutationRequest, +} from "./agent.storage.js"; describe("browser storage route parsing", () => { describe("parseStorageKind", () => { @@ -37,4 +41,25 @@ describe("browser storage route parsing", () => { }); }); }); + + describe("parseRequiredStorageMutationRequest", () => { + it("returns parsed request for supported kinds", () => { + expect( + parseRequiredStorageMutationRequest("session", { + targetId: " tab-9 ", + }), + ).toEqual({ + kind: "session", + targetId: "tab-9", + }); + }); + + it("returns null for unsupported kind", () => { + expect( + parseRequiredStorageMutationRequest("cookie", { + targetId: "tab-1", + }), + ).toBeNull(); + }); + }); }); diff --git a/src/browser/routes/agent.storage.ts b/src/browser/routes/agent.storage.ts index f9d28c65fdf..86830ab27ce 100644 --- a/src/browser/routes/agent.storage.ts +++ b/src/browser/routes/agent.storage.ts @@ -5,7 +5,7 @@ import { resolveTargetIdFromQuery, withPlaywrightRouteContext, } from "./agent.shared.js"; -import type { BrowserRouteRegistrar } from "./types.js"; +import type { BrowserRequest, BrowserResponse, BrowserRouteRegistrar } from "./types.js"; import { jsonError, toBoolean, toNumber, toStringOrEmpty } from "./utils.js"; type StorageKind = "local" | "session"; @@ -27,6 +27,42 @@ export function parseStorageMutationRequest( }; } +export function parseRequiredStorageMutationRequest( + kindParam: unknown, + body: Record, +): { kind: StorageKind; targetId: string | undefined } | null { + const parsed = parseStorageMutationRequest(kindParam, body); + if (!parsed.kind) { + return null; + } + return { + kind: parsed.kind, + targetId: parsed.targetId, + }; +} + +function parseStorageMutationOrRespond( + res: BrowserResponse, + kindParam: unknown, + body: Record, +) { + const parsed = parseRequiredStorageMutationRequest(kindParam, body); + if (!parsed) { + jsonError(res, 400, "kind must be local|session"); + return null; + } + return parsed; +} + +function parseStorageMutationFromRequest(req: BrowserRequest, res: BrowserResponse) { + const body = readBody(req); + const parsed = parseStorageMutationOrRespond(res, req.params.kind, body); + if (!parsed) { + return null; + } + return { body, parsed }; +} + export function registerBrowserAgentStorageRoutes( app: BrowserRouteRegistrar, ctx: BrowserRouteContext, @@ -139,29 +175,27 @@ export function registerBrowserAgentStorageRoutes( }); app.post("/storage/:kind/set", async (req, res) => { - const body = readBody(req); - const parsed = parseStorageMutationRequest(req.params.kind, body); - if (!parsed.kind) { - return jsonError(res, 400, "kind must be local|session"); + const mutation = parseStorageMutationFromRequest(req, res); + if (!mutation) { + return; } - const kind = parsed.kind; - const key = toStringOrEmpty(body.key); + const key = toStringOrEmpty(mutation.body.key); if (!key) { return jsonError(res, 400, "key is required"); } - const value = typeof body.value === "string" ? body.value : ""; + const value = typeof mutation.body.value === "string" ? mutation.body.value : ""; await withPlaywrightRouteContext({ req, res, ctx, - targetId: parsed.targetId, + targetId: mutation.parsed.targetId, feature: "storage set", run: async ({ cdpUrl, tab, pw }) => { await pw.storageSetViaPlaywright({ cdpUrl, targetId: tab.targetId, - kind, + kind: mutation.parsed.kind, key, value, }); @@ -171,24 +205,22 @@ export function registerBrowserAgentStorageRoutes( }); app.post("/storage/:kind/clear", async (req, res) => { - const body = readBody(req); - const parsed = parseStorageMutationRequest(req.params.kind, body); - if (!parsed.kind) { - return jsonError(res, 400, "kind must be local|session"); + const mutation = parseStorageMutationFromRequest(req, res); + if (!mutation) { + return; } - const kind = parsed.kind; await withPlaywrightRouteContext({ req, res, ctx, - targetId: parsed.targetId, + targetId: mutation.parsed.targetId, feature: "storage clear", run: async ({ cdpUrl, tab, pw }) => { await pw.storageClearViaPlaywright({ cdpUrl, targetId: tab.targetId, - kind, + kind: mutation.parsed.kind, }); res.json({ ok: true, targetId: tab.targetId }); },