refactor(browser): dedupe storage and download route parsing

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

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();
+    });
+  });
 });

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<string, unknown>,
+): { 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<string, unknown>,
+) {
+  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 });
       },