fix: harden routed preview recovery boundaries

Author: chubes4

.github/workflows/agent-task-contracts.yml

@@ -98,6 +98,7 @@ jobs:
       - run: npm run test:trusted-apply-artifact-channel
       - run: npm run test:runtime-command-artifact-bounds
       - run: npm run test:redaction
+      - run: npm run test:browser-preview-routing
       - run: npm run test:production-boundary-enforcement
       - run: npm run test:runtime-tool-policy
 

packages/runtime-playground/src/browser-metrics.ts

@@ -527,7 +527,7 @@ export async function serializeBrowserFinishedRequest(request: Request, timestam
   if (!response) {
     return {
       type: "response",
-      url: request.url(),
+      url: redactBrowserNetworkUrl(request.url()),
       method: request.method(),
       resourceType: request.resourceType(),
       timestamp,
@@ -545,7 +545,7 @@ export async function serializeBrowserResponse(response: Response, timestamp = n
   const responseTextPreview = await browserDocument5xxResponsePreview(response)
   return {
     type: "response",
-    url: response.url(),
+    url: redactBrowserNetworkUrl(response.url()),
     method: request.method(),
     resourceType: request.resourceType(),
     status: response.status(),
@@ -596,17 +596,22 @@ function redactBrowserResponseText(body: string): string {
 }
 
 export function serializeBrowserRequestFailure(request: Request, timestamp = now()): BrowserProbeNetworkRecord {
+  const failure = request.failure()
   return {
     type: "requestfailed",
-    url: request.url(),
+    url: redactBrowserNetworkUrl(request.url()),
     method: request.method(),
     resourceType: request.resourceType(),
     timing: browserRequestTiming(request),
-    failure: request.failure(),
+    failure: failure ? { errorText: redactString(failure.errorText, { redactAllUrlQueryValues: true, redactUrlHash: true, redactQueryAssignments: true }) } : null,
     timestamp,
   }
 }
 
+function redactBrowserNetworkUrl(url: string): string {
+  return redactString(url, { redactAllUrlQueryValues: true, redactUrlHash: true, redactQueryAssignments: true })
+}
+
 function browserRequestTiming(request: Request): Record<string, number> {
   return Object.fromEntries(
     Object.entries(request.timing()).filter((entry): entry is [string, number] => typeof entry[1] === "number" && Number.isFinite(entry[1])),

packages/runtime-playground/src/browser-preview-routing.ts

@@ -53,10 +53,11 @@ export interface BrowserPreviewTopology {
 export interface BrowserPreviewRouteTracker {
   pending: Set<Promise<void>>
   errors: unknown[]
+  registrations: number
 }
 
 export function createBrowserPreviewRouteTracker(): BrowserPreviewRouteTracker {
-  return { pending: new Set(), errors: [] }
+  return { pending: new Set(), errors: [], registrations: 0 }
 }
 
 export function browserPreviewRouting(args: string[], runtimeSpec: RuntimeCreateSpec | undefined, localPreviewOrigin: string): BrowserProbePreviewRouting {
@@ -271,19 +272,23 @@ export async function routeBrowserPreviewContextNetwork(context: import("playwri
 
 export async function drainBrowserPreviewRouteTracker(tracker: BrowserPreviewRouteTracker, timeoutMs = BROWSER_PREVIEW_ROUTE_DRAIN_TIMEOUT_MS): Promise<void> {
   const deadline = Date.now() + timeoutMs
-  while (tracker.pending.size > 0) {
+  let observedRegistrations = -1
+  while (tracker.pending.size > 0 || tracker.registrations !== observedRegistrations) {
+    observedRegistrations = tracker.registrations
     const remainingMs = deadline - Date.now()
     if (remainingMs <= 0) {
       throw new Error(`wordpress.browser-probe route-host timed out waiting for ${tracker.pending.size} routed request(s) to finish`)
     }
 
+    const pending = [...tracker.pending]
     const result = await Promise.race([
-      Promise.allSettled([...tracker.pending]).then(() => "drained" as const),
+      Promise.allSettled(pending).then(() => "drained" as const),
       wait(remainingMs).then(() => "timeout" as const),
     ])
     if (result === "timeout") {
       throw new Error(`wordpress.browser-probe route-host timed out waiting for ${tracker.pending.size} routed request(s) to finish`)
     }
+    await wait(0)
   }
 
   if (tracker.errors.length > 0) {
@@ -307,45 +312,67 @@ async function routeBrowserPreviewNetwork(routePattern: (url: string, handler: (
 
   const origin = new URL(previewOrigin)
   await routePattern("**/*", async (route) => {
-    const request = route.request()
-    let requestUrl: URL
-    try {
-      requestUrl = new URL(request.url())
-    } catch {
-      await route.continue()
-      return
-    }
-
-    const host = normalizeBrowserPreviewHost(requestUrl.hostname)
-    const stat = browserPreviewNetworkPolicyHostStat(policy, host)
-    stat.requests += 1
-    stat.external = !policy.firstPartyHosts.has(host)
-
-    if (policy.blockHosts.has(host) || (policy.mode === "block" && stat.external && !policy.allowHosts.has(host))) {
-      stat.blocked += 1
-      await route.abort("blockedbyclient")
-      return
-    }
-
-    if (!policy.routeHosts.has(host)) {
-      await route.continue()
-      return
+    let operation = "inspect-request"
+    const task = handleBrowserPreviewRoute(route, policy, origin, (nextOperation) => {
+      operation = nextOperation
+    })
+    if (tracker) {
+      tracker.registrations += 1
+      tracker.pending.add(task)
     }
-
-    stat.routed += 1
-    const task = fulfillBrowserPreviewRoutedHost(route, requestUrl, policy, origin)
-    tracker?.pending.add(task)
     try {
       await task
     } catch (error) {
-      tracker?.errors.push(sanitizeBrowserPreviewRouteError(error))
-      await route.abort("failed").catch(() => undefined)
+      if (!isBrowserPreviewRouteClosedError(error)) {
+        tracker?.errors.push(browserPreviewRouteCallbackError(route, operation, error))
+      }
+      try {
+        await route.abort("failed")
+      } catch (abortError) {
+        if (!isBrowserPreviewRouteClosedError(abortError)) {
+          tracker?.errors.push(browserPreviewRouteCallbackError(route, "abort-after-error", abortError))
+        }
+      }
     } finally {
       tracker?.pending.delete(task)
     }
   })
 }
 
+async function handleBrowserPreviewRoute(route: Route, policy: BrowserPreviewNetworkPolicy, origin: URL, setOperation: (operation: string) => void): Promise<void> {
+  const request = route.request()
+  let requestUrl: URL
+  try {
+    requestUrl = new URL(request.url())
+  } catch {
+    setOperation("continue-invalid-url")
+    await route.continue()
+    return
+  }
+
+  const host = normalizeBrowserPreviewHost(requestUrl.hostname)
+  const stat = browserPreviewNetworkPolicyHostStat(policy, host)
+  stat.requests += 1
+  stat.external = !policy.firstPartyHosts.has(host)
+
+  if (policy.blockHosts.has(host) || (policy.mode === "block" && stat.external && !policy.allowHosts.has(host))) {
+    stat.blocked += 1
+    setOperation("abort-policy-block")
+    await route.abort("blockedbyclient")
+    return
+  }
+
+  if (!policy.routeHosts.has(host)) {
+    setOperation("continue-unrouted")
+    await route.continue()
+    return
+  }
+
+  stat.routed += 1
+  setOperation("fulfill-routed-host")
+  await fulfillBrowserPreviewRoutedHost(route, requestUrl, policy, origin)
+}
+
 async function fulfillBrowserPreviewRoutedHost(route: Route, requestUrl: URL, policy: BrowserPreviewNetworkPolicy, localOrigin: URL): Promise<void> {
   const response = await fetchBrowserPreviewRoutedHost(route, requestUrl, policy, localOrigin)
   if (!response) {
@@ -430,7 +457,9 @@ async function fetchBrowserPreviewRoutedHost(route: Route, requestUrl: URL, poli
 
     let response: Awaited<ReturnType<Route["fetch"]>> | undefined
     const resourceType = route.request().resourceType()
-    const maxAttempts = resourceType === "document" ? BROWSER_PREVIEW_ROUTE_DOCUMENT_FETCH_ATTEMPTS : BROWSER_PREVIEW_ROUTE_SUBRESOURCE_FETCH_ATTEMPTS
+    const method = route.request().method().toUpperCase()
+    const methodCanRetry = browserPreviewRouteMethodCanRetry(method)
+    const maxAttempts = methodCanRetry ? (resourceType === "document" ? BROWSER_PREVIEW_ROUTE_DOCUMENT_FETCH_ATTEMPTS : BROWSER_PREVIEW_ROUTE_SUBRESOURCE_FETCH_ATTEMPTS) : 1
     for (let attempt = 1; attempt <= maxAttempts; attempt += 1) {
       try {
         response = await route.fetch({
@@ -466,6 +495,9 @@ async function fetchBrowserPreviewRoutedHost(route: Route, requestUrl: URL, poli
     if (!response) {
       return undefined
     }
+    if (!methodCanRetry) {
+      return response
+    }
 
     const location = response.headers().location
     if (!location || response.status() < 300 || response.status() >= 400) {
@@ -514,6 +546,35 @@ export function isBrowserPreviewRouteFetchTransientTransportError(error: unknown
   return error instanceof Error && /\b(?:ECONNRESET|ECONNREFUSED|EPIPE|ETIMEDOUT|UND_ERR_SOCKET|socket (?:hang up|closed|ended)|connection (?:reset|refused|closed)|other side closed)\b/i.test(error.message)
 }
 
+export function isBrowserPreviewRouteClosedError(error: unknown): boolean {
+  return error instanceof Error && /(?:Request context disposed|Target (?:page, context or browser|page|context|browser) has been closed|Browser has been closed|context closed|page closed)/i.test(error.message)
+}
+
+function browserPreviewRouteMethodCanRetry(method: string): boolean {
+  return method === "GET" || method === "HEAD" || method === "OPTIONS"
+}
+
+function browserPreviewRouteCallbackError(route: Route, operation: string, error: unknown): Error {
+  const request = browserPreviewRouteRequestSummary(route)
+  const cause = sanitizeBrowserPreviewRouteError(error).message.replace(/[\r\n]+/g, " ")
+  const diagnostic = new Error(`wordpress.browser-probe route callback failed: operation=${operation} method=${request.method} resourceType=${request.resourceType} url=${request.url} cause=${cause}`)
+  diagnostic.name = "BrowserPreviewRouteCallbackError"
+  return diagnostic
+}
+
+function browserPreviewRouteRequestSummary(route: Route): { method: string; resourceType: string; url: string } {
+  try {
+    const request = route.request()
+    return {
+      method: request.method(),
+      resourceType: request.resourceType(),
+      url: redactString(request.url(), { redactAllUrlQueryValues: true, redactUrlHash: true, redactQueryAssignments: true }),
+    }
+  } catch {
+    return { method: "unknown", resourceType: "unknown", url: "[unavailable]" }
+  }
+}
+
 function browserPreviewRouteFetchExhaustedError(route: Route, requestUrl: URL, attempts: number, error: unknown): Error {
   const method = route.request().method()
   const resourceType = route.request().resourceType()

packages/runtime-playground/src/browser-probe-runner.ts

@@ -451,19 +451,10 @@ export async function runSingleBrowserProbeCommand({
   } finally {
     if (abortSignal?.aborted) {
       await closeBrowserBestEffort(browser)
+      await drainBrowserPreviewRouteTracker(routeTracker).catch(() => undefined)
       abortSignal.removeEventListener("abort", abortHandler)
       throw pendingError ?? new Error("Browser command aborted during runtime cleanup")
     }
-    try {
-      await drainBrowserPreviewRouteTracker(routeTracker)
-    } catch (error) {
-      const routeError = redactError(error, { redactAllUrlQueryValues: true, redactUrlHash: true, redactQueryAssignments: true })
-      if (!pendingError && runPlan.routeHostDrain === "required") {
-        pendingError = routeError
-        progress.fail("probe-error", routeError)
-      }
-      errors.push(serializeBrowserError("probe-error", error))
-    }
     if (page) {
       finalUrl = page.url()
       windowLocationOrigin = windowLocationOrigin ?? await page.evaluate(() => window.location.origin).catch(() => undefined)
@@ -504,6 +495,16 @@ export async function runSingleBrowserProbeCommand({
     await settleBrowserNetworkTasks(networkTasks, livenessPolicy.networkSettleTimeoutMs)
     await geolocationPermissionCleanup?.()
     await browser.close()
+    try {
+      await drainBrowserPreviewRouteTracker(routeTracker)
+    } catch (error) {
+      const routeError = redactError(error, { redactAllUrlQueryValues: true, redactUrlHash: true, redactQueryAssignments: true })
+      if (!pendingError && runPlan.routeHostDrain === "required") {
+        pendingError = routeError
+        progress.fail("probe-error", routeError)
+      }
+      errors.push(serializeBrowserError("probe-error", error))
+    }
     if (captureSelection.console) {
       await artifactSession.writeJsonLines("console", "console.jsonl", consoleMessages)
     }

packages/runtime-playground/src/browser-probe-session-result-builder.ts

@@ -86,7 +86,8 @@ export interface BrowserProbeSessionResult {
 }
 
 export class BrowserProbeSessionResultBuilder {
-  compose(input: BrowserProbeSessionResultInput): BrowserProbeSessionResult {
+  compose(rawInput: BrowserProbeSessionResultInput): BrowserProbeSessionResult {
+    const input = sanitizeBrowserProbeSessionResultInput(rawInput)
     const assertionSummary = browserProbeAssertionSummary(input.assertions)
     const finishedAt = now()
     const files = browserProbeArtifactFileMap(input)
@@ -158,6 +159,27 @@ export class BrowserProbeSessionResultBuilder {
   }
 }
 
+function sanitizeBrowserProbeSessionResultInput(input: BrowserProbeSessionResultInput): BrowserProbeSessionResultInput {
+  const safeUrl = (url: string): string => safeBrowserProbeUrl(url) ?? "[unavailable]"
+  return {
+    ...input,
+    requestedUrl: safeUrl(input.requestedUrl),
+    finalUrl: safeUrl(input.finalUrl),
+    network: input.network.map((record) => ({ ...record, url: safeUrl(record.url) })),
+    preview: {
+      ...input.preview,
+      localOrigin: safeUrl(input.preview.localOrigin),
+      effectiveOrigin: safeUrl(input.preview.effectiveOrigin),
+      ...(input.preview.publicOrigin ? { publicOrigin: safeUrl(input.preview.publicOrigin) } : {}),
+    },
+    topologyOrigins: {
+      localPreviewOrigin: safeUrl(input.topologyOrigins.localPreviewOrigin),
+      effectivePreviewOrigin: safeUrl(input.topologyOrigins.effectivePreviewOrigin),
+      ...(input.topologyOrigins.requestedPreviewOrigin ? { requestedPreviewOrigin: safeUrl(input.topologyOrigins.requestedPreviewOrigin) } : {}),
+    },
+  }
+}
+
 function browserProbeArtifactFileMap(input: BrowserProbeSessionResultInput): BrowserProbeArtifact["files"] {
   return {
     ...(input.capture.has("console") || input.captureSelection?.consoleForAssertions ? { console: `${input.browserFilesDirectory}/console.jsonl` } : {}),

packages/runtime-playground/src/browser-probe-support.ts

@@ -287,7 +287,7 @@ export function safeBrowserProbeUrl(value: string | undefined): string | null {
   if (/^data:/i.test(value)) {
     return "data:[redacted]"
   }
-  return value
+  return browserRedirectSafeUrl(value)
 }
 
 export interface BrowserRedirectDiagnosticsArtifact {
@@ -400,7 +400,7 @@ export function browserRedirectSafeUrl(value: string): string {
   }
   const parsed = parseBrowserRedirectUrl(value)
   if (!parsed) {
-    return value
+    return redactString(value, { redactAllUrlQueryValues: true, redactUrlHash: true, redactQueryAssignments: true })
   }
   const search = parsed.queryKeys.length > 0
     ? `?${parsed.queryKeys.map((key) => `${encodeURIComponent(key)}=[redacted]`).join("&")}`

scripts/smoke-manifest.ts

@@ -47,6 +47,7 @@ export const smokeGroups = {
       npmScript("test:browser-runner-template"),
       npmScript("test:browser-runtime-file-ops"),
       npmScript("test:browser-provider-bridge-inheritance"),
+      npmScript("test:browser-preview-routing"),
       tsxSmoke("runtime-backend-registry-smoke"),
       tsxSmoke("backend-package-adapter-registry-smoke"),
       tsxSmoke("command-registry-smoke"),