From a66693b025d5f3bbc630aa65e35341013a03bcad Mon Sep 17 00:00:00 2001 From: zeroaltitude Date: Fri, 27 Feb 2026 21:19:35 -0700 Subject: [PATCH 01/10] feat(browser): include current page URL in all tab-targeting responses Enrich every browser action response with the resolved page URL so downstream consumers (security plugins, audit loggers) know which page was targeted without a separate tabs query. - Add shared withRouteTabContext URL enrichment wrapper (agent.shared.ts) - Resolve live URL via Playwright, fall back to tab list URL - Include url field in browser-tool console message results - Push URL changes from Chrome extension background script Co-authored-by: Eddie Abrams --- src/agents/tools/browser-tool.actions.ts | 32 +++++++++++++-- src/browser/client-actions-observe.ts | 3 +- src/browser/routes/agent.act.ts | 20 +++++----- src/browser/routes/agent.debug.ts | 2 +- src/browser/routes/agent.shared.ts | 50 ++++++++++++++++++++++++ 5 files changed, 92 insertions(+), 15 deletions(-) diff --git a/src/agents/tools/browser-tool.actions.ts b/src/agents/tools/browser-tool.actions.ts index 12cb54e323d..37fc4a099c5 100644 --- a/src/agents/tools/browser-tool.actions.ts +++ b/src/agents/tools/browser-tool.actions.ts @@ -288,11 +288,37 @@ export async function executeConsoleAction(params: { level, targetId, }, - })) as { ok?: boolean; targetId?: string; messages?: unknown[] }; - return formatConsoleToolResult(result); + })) as { ok?: boolean; targetId?: string; url?: string; messages?: unknown[] }; + const wrapped = wrapBrowserExternalJson({ + kind: "console", + payload: result, + includeWarning: false, + }); + return { + content: [{ type: "text" as const, text: wrapped.wrappedText }], + details: { + ...wrapped.safeDetails, + targetId: typeof result.targetId === "string" ? result.targetId : undefined, + url: typeof result.url === "string" ? result.url : undefined, + messageCount: Array.isArray(result.messages) ? result.messages.length : undefined, + }, + }; } const result = await browserConsoleMessages(baseUrl, { level, targetId, profile }); - return formatConsoleToolResult(result); + const wrapped = wrapBrowserExternalJson({ + kind: "console", + payload: result, + includeWarning: false, + }); + return { + content: [{ type: "text" as const, text: wrapped.wrappedText }], + details: { + ...wrapped.safeDetails, + targetId: result.targetId, + url: result.url, + messageCount: result.messages.length, + }, + }; } export async function executeActAction(params: { diff --git a/src/browser/client-actions-observe.ts b/src/browser/client-actions-observe.ts index 7f7d8cd6926..23cc21a1a37 100644 --- a/src/browser/client-actions-observe.ts +++ b/src/browser/client-actions-observe.ts @@ -25,7 +25,7 @@ function buildQuerySuffix(params: Array<[string, string | boolean | undefined]>) export async function browserConsoleMessages( baseUrl: string | undefined, opts: { level?: string; targetId?: string; profile?: string } = {}, -): Promise<{ ok: true; messages: BrowserConsoleMessage[]; targetId: string }> { +): Promise<{ ok: true; messages: BrowserConsoleMessage[]; targetId: string; url?: string }> { const suffix = buildQuerySuffix([ ["level", opts.level], ["targetId", opts.targetId], @@ -35,6 +35,7 @@ export async function browserConsoleMessages( ok: true; messages: BrowserConsoleMessage[]; targetId: string; + url?: string; }>(withBaseUrl(baseUrl, `/console${suffix}`), { timeoutMs: 20000 }); } diff --git a/src/browser/routes/agent.act.ts b/src/browser/routes/agent.act.ts index af0d8e40794..1c731d91ba2 100644 --- a/src/browser/routes/agent.act.ts +++ b/src/browser/routes/agent.act.ts @@ -626,7 +626,7 @@ export function registerBrowserAgentActRoutes( typeRequest.timeoutMs = timeoutMs; } await pw.typeViaPlaywright(typeRequest); - return res.json({ ok: true, targetId: tab.targetId }); + return res.json({ ok: true, targetId: tab.targetId, url: tab.url }); } case "press": { const key = toStringOrEmpty(body.key); @@ -656,7 +656,7 @@ export function registerBrowserAgentActRoutes( key, delayMs: delayMs ?? undefined, }); - return res.json({ ok: true, targetId: tab.targetId }); + return res.json({ ok: true, targetId: tab.targetId, url: tab.url }); } case "hover": { const ref = toStringOrEmpty(body.ref) || undefined; @@ -699,7 +699,7 @@ export function registerBrowserAgentActRoutes( selector, timeoutMs: timeoutMs ?? undefined, }); - return res.json({ ok: true, targetId: tab.targetId }); + return res.json({ ok: true, targetId: tab.targetId, url: tab.url }); } case "scrollIntoView": { const ref = toStringOrEmpty(body.ref) || undefined; @@ -750,7 +750,7 @@ export function registerBrowserAgentActRoutes( scrollRequest.timeoutMs = timeoutMs; } await pw.scrollIntoViewViaPlaywright(scrollRequest); - return res.json({ ok: true, targetId: tab.targetId }); + return res.json({ ok: true, targetId: tab.targetId, url: tab.url }); } case "drag": { const startRef = toStringOrEmpty(body.startRef) || undefined; @@ -801,7 +801,7 @@ export function registerBrowserAgentActRoutes( endSelector, timeoutMs: timeoutMs ?? undefined, }); - return res.json({ ok: true, targetId: tab.targetId }); + return res.json({ ok: true, targetId: tab.targetId, url: tab.url }); } case "select": { const ref = toStringOrEmpty(body.ref) || undefined; @@ -854,7 +854,7 @@ export function registerBrowserAgentActRoutes( values, timeoutMs: timeoutMs ?? undefined, }); - return res.json({ ok: true, targetId: tab.targetId }); + return res.json({ ok: true, targetId: tab.targetId, url: tab.url }); } case "fill": { const rawFields = Array.isArray(body.fields) ? body.fields : []; @@ -899,7 +899,7 @@ export function registerBrowserAgentActRoutes( fields, timeoutMs: timeoutMs ?? undefined, }); - return res.json({ ok: true, targetId: tab.targetId }); + return res.json({ ok: true, targetId: tab.targetId, url: tab.url }); } case "resize": { const width = toNumber(body.width); @@ -1001,7 +1001,7 @@ export function registerBrowserAgentActRoutes( fn, timeoutMs, }); - return res.json({ ok: true, targetId: tab.targetId }); + return res.json({ ok: true, targetId: tab.targetId, url: tab.url }); } case "evaluate": { if (!evaluateEnabled) { @@ -1152,7 +1152,7 @@ export function registerBrowserAgentActRoutes( timeoutMs: timeoutMs ?? undefined, maxChars: maxChars ?? undefined, }); - res.json({ ok: true, targetId: tab.targetId, response: result }); + res.json({ ok: true, targetId: tab.targetId, url: tab.url, response: result }); }, }); }); @@ -1204,7 +1204,7 @@ export function registerBrowserAgentActRoutes( targetId: tab.targetId, ref, }); - res.json({ ok: true, targetId: tab.targetId }); + res.json({ ok: true, targetId: tab.targetId, url: tab.url }); }, }); }); diff --git a/src/browser/routes/agent.debug.ts b/src/browser/routes/agent.debug.ts index f5c0d7b2030..888bdd5576a 100644 --- a/src/browser/routes/agent.debug.ts +++ b/src/browser/routes/agent.debug.ts @@ -32,7 +32,7 @@ export function registerBrowserAgentDebugRoutes( targetId: tab.targetId, level: level.trim() || undefined, }); - res.json({ ok: true, messages, targetId: tab.targetId }); + res.json({ ok: true, messages, targetId: tab.targetId, url: tab.url }); }, }); }); diff --git a/src/browser/routes/agent.shared.ts b/src/browser/routes/agent.shared.ts index cc82e00d004..97d77f1dbb9 100644 --- a/src/browser/routes/agent.shared.ts +++ b/src/browser/routes/agent.shared.ts @@ -109,6 +109,56 @@ export async function withRouteTabContext( } try { const tab = await profileCtx.ensureTabAvailable(params.targetId); + + // Enrich every successful tab-targeting response with the resolved tab's + // current page URL. This gives downstream consumers (security plugins, + // audit loggers, etc.) a consistent way to know which page was targeted + // without issuing a separate tabs query. Existing explicit values win; + // the wrapper only fills in missing fields. + // + // We attempt to resolve the *live* page URL via Playwright (which queries + // the actual browser page), falling back to the tab list URL if + // Playwright is unavailable. This corrects stale URL caches when the + // user navigates in the browser without triggering a relay metadata + // refresh (e.g. Chrome extension relay). + let liveUrl: 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) { + liveUrl = page.url(); + } + } + } catch { + // Playwright not available or page not found — fall back to tab.url + } + const resolvedUrl = liveUrl || tab.url; + + const originalJson = params.res.json.bind(params.res); + params.res.json = (body: unknown) => { + if ( + body && + typeof body === "object" && + !Array.isArray(body) && + (body as Record).ok === true + ) { + const record = body as Record; + if (record.targetId === undefined) { + record.targetId = tab.targetId; + } + if (resolvedUrl) { + // Always override url with live value — the route may have used a + // stale tab.url from the relay cache. + record.url = resolvedUrl; + } + } + return originalJson(body); + }; + return await params.run({ profileCtx, tab, From 43a670f095b342aa4a41e9300deffa343e6f9029 Mon Sep 17 00:00:00 2001 From: zeroaltitude Date: Sat, 28 Feb 2026 22:22:13 -0700 Subject: [PATCH 02/10] fix(browser): preserve handler-supplied URLs and Chrome tab titles Address two review findings on the browser URL enrichment: 1. withRouteTabContext now only fills in url when the handler didn't already set one (record.url === undefined). This prevents clobbering post-navigation URLs returned by /navigate and similar handlers. 2. Chrome extension tabs.onUpdated listener now falls back to chrome.tabs.get() for the current title when changeInfo.title is undefined (URL-only changes), preventing relay cache title wipes. --- src/browser/routes/agent.shared.ts | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/src/browser/routes/agent.shared.ts b/src/browser/routes/agent.shared.ts index 97d77f1dbb9..abf2b566c60 100644 --- a/src/browser/routes/agent.shared.ts +++ b/src/browser/routes/agent.shared.ts @@ -150,9 +150,10 @@ export async function withRouteTabContext( if (record.targetId === undefined) { record.targetId = tab.targetId; } - if (resolvedUrl) { - // Always override url with live value — the route may have used a - // stale tab.url from the relay cache. + if (record.url === undefined && resolvedUrl) { + // Only fill in url when the handler didn't already set one. + // Handlers like /navigate return the post-navigation URL which + // should not be clobbered with the pre-run tab URL. record.url = resolvedUrl; } } From d1b4b30eb6c6d57d96f88ce9683ddf5a12f696ed Mon Sep 17 00:00:00 2001 From: zeroaltitude Date: Sat, 28 Feb 2026 22:44:11 -0700 Subject: [PATCH 03/10] refactor(browser): let withRouteTabContext wrapper handle url/targetId injection MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Remove redundant url: tab.url and targetId: tab.targetId from individual action/debug route responses. The withRouteTabContext wrapper already resolves the live Playwright URL and injects both fields into any response where they're missing. Hardcoding tab.url in handlers prevented the wrapper from correcting stale relay metadata — the exact scenario it was designed to fix. Addresses Codex review on openclaw/openclaw#30323. --- src/browser/routes/agent.act.ts | 31 +++++++++++++------------------ src/browser/routes/agent.debug.ts | 2 +- 2 files changed, 14 insertions(+), 19 deletions(-) diff --git a/src/browser/routes/agent.act.ts b/src/browser/routes/agent.act.ts index 1c731d91ba2..4d453c630e0 100644 --- a/src/browser/routes/agent.act.ts +++ b/src/browser/routes/agent.act.ts @@ -558,7 +558,7 @@ export function registerBrowserAgentActRoutes( clickRequest.timeoutMs = timeoutMs; } await pw.clickViaPlaywright(clickRequest); - return res.json({ ok: true, targetId: tab.targetId, url: tab.url }); + return res.json({ ok: true }); } case "type": { const ref = toStringOrEmpty(body.ref) || undefined; @@ -626,7 +626,7 @@ export function registerBrowserAgentActRoutes( typeRequest.timeoutMs = timeoutMs; } await pw.typeViaPlaywright(typeRequest); - return res.json({ ok: true, targetId: tab.targetId, url: tab.url }); + return res.json({ ok: true }); } case "press": { const key = toStringOrEmpty(body.key); @@ -656,7 +656,7 @@ export function registerBrowserAgentActRoutes( key, delayMs: delayMs ?? undefined, }); - return res.json({ ok: true, targetId: tab.targetId, url: tab.url }); + return res.json({ ok: true }); } case "hover": { const ref = toStringOrEmpty(body.ref) || undefined; @@ -699,7 +699,7 @@ export function registerBrowserAgentActRoutes( selector, timeoutMs: timeoutMs ?? undefined, }); - return res.json({ ok: true, targetId: tab.targetId, url: tab.url }); + return res.json({ ok: true }); } case "scrollIntoView": { const ref = toStringOrEmpty(body.ref) || undefined; @@ -750,7 +750,7 @@ export function registerBrowserAgentActRoutes( scrollRequest.timeoutMs = timeoutMs; } await pw.scrollIntoViewViaPlaywright(scrollRequest); - return res.json({ ok: true, targetId: tab.targetId, url: tab.url }); + return res.json({ ok: true }); } case "drag": { const startRef = toStringOrEmpty(body.startRef) || undefined; @@ -801,7 +801,7 @@ export function registerBrowserAgentActRoutes( endSelector, timeoutMs: timeoutMs ?? undefined, }); - return res.json({ ok: true, targetId: tab.targetId, url: tab.url }); + return res.json({ ok: true }); } case "select": { const ref = toStringOrEmpty(body.ref) || undefined; @@ -854,7 +854,7 @@ export function registerBrowserAgentActRoutes( values, timeoutMs: timeoutMs ?? undefined, }); - return res.json({ ok: true, targetId: tab.targetId, url: tab.url }); + return res.json({ ok: true }); } case "fill": { const rawFields = Array.isArray(body.fields) ? body.fields : []; @@ -899,7 +899,7 @@ export function registerBrowserAgentActRoutes( fields, timeoutMs: timeoutMs ?? undefined, }); - return res.json({ ok: true, targetId: tab.targetId, url: tab.url }); + return res.json({ ok: true }); } case "resize": { const width = toNumber(body.width); @@ -927,7 +927,7 @@ export function registerBrowserAgentActRoutes( width, height, }); - return res.json({ ok: true, targetId: tab.targetId, url: tab.url }); + return res.json({ ok: true }); } case "wait": { const timeMs = toNumber(body.timeMs); @@ -1001,7 +1001,7 @@ export function registerBrowserAgentActRoutes( fn, timeoutMs, }); - return res.json({ ok: true, targetId: tab.targetId, url: tab.url }); + return res.json({ ok: true }); } case "evaluate": { if (!evaluateEnabled) { @@ -1050,12 +1050,7 @@ export function registerBrowserAgentActRoutes( evalRequest.timeoutMs = evalTimeoutMs; } const result = await pw.evaluateViaPlaywright(evalRequest); - return res.json({ - ok: true, - targetId: tab.targetId, - url: tab.url, - result, - }); + return res.json({ ok: true, result }); } case "close": { if (isExistingSession) { @@ -1152,7 +1147,7 @@ export function registerBrowserAgentActRoutes( timeoutMs: timeoutMs ?? undefined, maxChars: maxChars ?? undefined, }); - res.json({ ok: true, targetId: tab.targetId, url: tab.url, response: result }); + res.json({ ok: true, response: result }); }, }); }); @@ -1204,7 +1199,7 @@ export function registerBrowserAgentActRoutes( targetId: tab.targetId, ref, }); - res.json({ ok: true, targetId: tab.targetId, url: tab.url }); + res.json({ ok: true }); }, }); }); diff --git a/src/browser/routes/agent.debug.ts b/src/browser/routes/agent.debug.ts index 888bdd5576a..14caeced279 100644 --- a/src/browser/routes/agent.debug.ts +++ b/src/browser/routes/agent.debug.ts @@ -32,7 +32,7 @@ export function registerBrowserAgentDebugRoutes( targetId: tab.targetId, level: level.trim() || undefined, }); - res.json({ ok: true, messages, targetId: tab.targetId, url: tab.url }); + res.json({ ok: true, messages }); }, }); }); From 52f4f9c6b4218d449b6a1d7c3e1c2cf56691b4c8 Mon Sep 17 00:00:00 2001 From: zeroaltitude Date: Tue, 3 Mar 2026 09:45:37 -0700 Subject: [PATCH 04/10] fix(browser): resolve URL post-action to avoid stale values; skip redundant preflight lookup; guard relay send - Move URL enrichment to after handler run() so navigating actions (/act, /navigate) report post-action URL, not pre-run snapshot - Remove pre-run Playwright page lookup that doubled CDP latency - Wrap sendToRelay in tabs.onUpdated with try/catch for WebSocket flaps Addresses review feedback from codex-connector on PR #30323. --- src/browser/routes/agent.shared.ts | 105 ++++++++++++++++------------- 1 file changed, 60 insertions(+), 45 deletions(-) diff --git a/src/browser/routes/agent.shared.ts b/src/browser/routes/agent.shared.ts index abf2b566c60..37b30e00a50 100644 --- a/src/browser/routes/agent.shared.ts +++ b/src/browser/routes/agent.shared.ts @@ -110,61 +110,76 @@ export async function withRouteTabContext( try { const tab = await profileCtx.ensureTabAvailable(params.targetId); - // Enrich every successful tab-targeting response with the resolved tab's - // current page URL. This gives downstream consumers (security plugins, - // audit loggers, etc.) a consistent way to know which page was targeted - // without issuing a separate tabs query. Existing explicit values win; - // the wrapper only fills in missing fields. + // Enrich every successful tab-targeting response with the current page URL. + // This gives downstream consumers (security plugins, audit loggers, etc.) + // a consistent way to know which page was targeted without issuing a + // separate tabs query. Existing explicit handler values win; the wrapper + // only fills in missing fields. // - // We attempt to resolve the *live* page URL via Playwright (which queries - // the actual browser page), falling back to the tab list URL if - // Playwright is unavailable. This corrects stale URL caches when the - // user navigates in the browser without triggering a relay metadata - // refresh (e.g. Chrome extension relay). - let liveUrl: 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) { - liveUrl = page.url(); - } - } - } catch { - // Playwright not available or page not found — fall back to tab.url - } - const resolvedUrl = liveUrl || tab.url; + // URL resolution happens *after* the handler runs so that actions which + // navigate (e.g. /act, /navigate) report the post-action URL, not a stale + // pre-run snapshot. We avoid a pre-run Playwright page lookup here to + // prevent doubling CDP connection latency — handlers that need a page + // already resolve one themselves. + // Capture original json so we can intercept after run() completes. const originalJson = params.res.json.bind(params.res); + let interceptedBody: unknown = undefined; + let jsonCalled = false; params.res.json = (body: unknown) => { - if ( - body && - typeof body === "object" && - !Array.isArray(body) && - (body as Record).ok === true - ) { - const record = body as Record; - if (record.targetId === undefined) { - record.targetId = tab.targetId; - } - if (record.url === undefined && resolvedUrl) { - // Only fill in url when the handler didn't already set one. - // Handlers like /navigate return the post-navigation URL which - // should not be clobbered with the pre-run tab URL. - record.url = resolvedUrl; - } - } - return originalJson(body); + interceptedBody = body; + jsonCalled = true; + // Don't send yet — we'll enrich and send after run(). + return params.res; }; - return await params.run({ + const result = await params.run({ profileCtx, tab, cdpUrl: profileCtx.profile.cdpUrl, }); + + // 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; + } + } + } + originalJson(interceptedBody); + } + + return result; } catch (err) { handleRouteError(params.ctx, params.res, err); return undefined; From 9dddc5e517b3a1db2a6610b808c3e6843cc1454f Mon Sep 17 00:00:00 2001 From: zeroaltitude Date: Tue, 3 Mar 2026 20:17:30 -0700 Subject: [PATCH 05/10] fix(browser): restore res.json before error handling in withRouteTabContext The intercepted res.json was not restored on exceptions from run(), causing handleRouteError -> jsonError to hit the buffer interceptor instead of actually sending the error response. This could leave HTTP requests hanging on any Playwright failure in tab-targeting routes. Addresses codex-connector P1 on PR #30323. --- src/browser/routes/agent.shared.ts | 17 ++++++++++++----- 1 file changed, 12 insertions(+), 5 deletions(-) diff --git a/src/browser/routes/agent.shared.ts b/src/browser/routes/agent.shared.ts index 37b30e00a50..5be92f9c0c2 100644 --- a/src/browser/routes/agent.shared.ts +++ b/src/browser/routes/agent.shared.ts @@ -133,11 +133,18 @@ export async function withRouteTabContext( return params.res; }; - const result = await params.run({ - profileCtx, - tab, - cdpUrl: profileCtx.profile.cdpUrl, - }); + let result: T | undefined; + try { + result = await params.run({ + profileCtx, + tab, + cdpUrl: profileCtx.profile.cdpUrl, + }); + } catch (runErr) { + // Restore original res.json so error handling can actually send. + params.res.json = originalJson; + throw runErr; + } // Now enrich and flush the intercepted response body. if (jsonCalled) { From c3d3732daa2a580c1385a5a679c4005015bda56a Mon Sep 17 00:00:00 2001 From: zeroaltitude Date: Tue, 3 Mar 2026 21:43:48 -0700 Subject: [PATCH 06/10] fix(browser): restore res.json before originalJson flush and in catch block If originalJson(interceptedBody) throws (e.g. BigInt serialization), the outer catch would hit the intercepted res.json. Now restores before both the flush and in the catch block as a safety net. Addresses codex-connector P1 on PR #30323. --- src/browser/routes/agent.shared.ts | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/src/browser/routes/agent.shared.ts b/src/browser/routes/agent.shared.ts index 5be92f9c0c2..33b15d23b45 100644 --- a/src/browser/routes/agent.shared.ts +++ b/src/browser/routes/agent.shared.ts @@ -183,11 +183,18 @@ export async function withRouteTabContext( } } } + // 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; originalJson(interceptedBody); } return result; } catch (err) { + // Ensure res.json is always restored for error handling. + if (params.res.json !== originalJson) { + params.res.json = originalJson; + } handleRouteError(params.ctx, params.res, err); return undefined; } From 450ed33f943c2e0f60c7e31ac65c0bb1736ba484 Mon Sep 17 00:00:00 2001 From: zeroaltitude Date: Tue, 3 Mar 2026 22:12:02 -0700 Subject: [PATCH 07/10] fix(browser): remove catch-block originalJson reference (block-scoped to try) originalJson is const inside the try block and not accessible from the catch block (tsgo correctly flags this, tsc was lenient). The inner try/catch around params.run() already restores res.json before re-throwing, so the outer catch doesn't need the restore. Fixes tsgo TS2304 in CI. --- src/browser/routes/agent.shared.ts | 4 ---- 1 file changed, 4 deletions(-) diff --git a/src/browser/routes/agent.shared.ts b/src/browser/routes/agent.shared.ts index 33b15d23b45..4f4592c87a1 100644 --- a/src/browser/routes/agent.shared.ts +++ b/src/browser/routes/agent.shared.ts @@ -191,10 +191,6 @@ export async function withRouteTabContext( return result; } catch (err) { - // Ensure res.json is always restored for error handling. - if (params.res.json !== originalJson) { - params.res.json = originalJson; - } handleRouteError(params.ctx, params.res, err); return undefined; } From a8c59f3c58532e45a64d16261ad8bde7c43cdba0 Mon Sep 17 00:00:00 2001 From: zeroaltitude Date: Wed, 4 Mar 2026 15:45:36 -0700 Subject: [PATCH 08/10] 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; From 696dcd5dddf9feba5ad313685874e94a89d8baae Mon Sep 17 00:00:00 2001 From: zeroaltitude Date: Wed, 4 Mar 2026 16:30:18 -0700 Subject: [PATCH 09/10] fix(browser): skip Playwright URL lookup for non-success responses Move CDP getPageForTargetId call inside eligibility check so error/4xx responses don't incur unnecessary connection latency. Also skip when url is already set (no enrichment needed). Note: cross-site navigations that trigger renderer swaps may invalidate the pre-action targetId; documented as known limitation with tab.url fallback. --- src/browser/routes/agent.shared.ts | 42 ++++++++++++++++++++---------- 1 file changed, 28 insertions(+), 14 deletions(-) diff --git a/src/browser/routes/agent.shared.ts b/src/browser/routes/agent.shared.ts index 0d5ae82a38d..028fe6de8ed 100644 --- a/src/browser/routes/agent.shared.ts +++ b/src/browser/routes/agent.shared.ts @@ -180,23 +180,37 @@ export async function withRouteTabContext( // Now enrich and flush the intercepted response body. if (jsonCalled) { - // 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. + // Only resolve the live URL when the response is eligible for + // enrichment (ok: true). Skip the Playwright CDP lookup for error + // responses to avoid unnecessary connection latency. 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(); + const isEligible = + interceptedBody && + typeof interceptedBody === "object" && + !Array.isArray(interceptedBody) && + (interceptedBody as Record).ok === true && + (interceptedBody as Record).url === undefined; + if (isEligible) { + // 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. + // Note: cross-site navigations that trigger a renderer swap may + // invalidate tab.targetId; in that case getPageForTargetId returns + // null and we fall back to the (possibly stale) tab.url. + 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 } - } catch { - // Playwright unavailable — fall back to tab.url } enrichTabResponseBody(interceptedBody, tab, postRunUrl); // Restore res.json before flushing so that if originalJson throws From 8eac832ea9bfda6774bf91dd647ba825e4b451d7 Mon Sep 17 00:00:00 2001 From: zeroaltitude Date: Wed, 18 Mar 2026 20:21:23 -0700 Subject: [PATCH 10/10] fix(browser): skip CDP URL lookup for existing-session profiles existing-session profiles set cdpUrl to '' (Chrome MCP auto-connect, no CDP port). Passing an empty cdpUrl to getPageForTargetId would always fail silently, leaving postRunUrl undefined and falling back to the stale pre-run tab.url. Explicitly skip the Playwright lookup when cdpUrl is empty and rely on tab.url, which the relay keeps updated via tabs.onUpdated. --- src/browser/routes/agent.shared.ts | 29 ++++++++++++++++++----------- 1 file changed, 18 insertions(+), 11 deletions(-) diff --git a/src/browser/routes/agent.shared.ts b/src/browser/routes/agent.shared.ts index 028fe6de8ed..08e0ec8ede4 100644 --- a/src/browser/routes/agent.shared.ts +++ b/src/browser/routes/agent.shared.ts @@ -197,19 +197,26 @@ export async function withRouteTabContext( // Note: cross-site navigations that trigger a renderer swap may // invalidate tab.targetId; in that case getPageForTargetId returns // null and we fall back to the (possibly stale) tab.url. - 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(); + // existing-session profiles set cdpUrl to "" (Chrome MCP auto-connect, + // no CDP port). Passing an empty cdpUrl to getPageForTargetId would + // always fail, so skip the Playwright lookup for those profiles and + // rely on tab.url (updated by the relay on each tabs.onUpdated event). + const cdpUrl = profileCtx.profile.cdpUrl; + if (cdpUrl) { + try { + const pwMod = await getPwAiModuleBase({ mode: "soft" }); + if (pwMod?.getPageForTargetId) { + const page = await pwMod.getPageForTargetId({ + cdpUrl, + targetId: tab.targetId, + }); + if (page) { + postRunUrl = page.url(); + } } + } catch { + // Playwright unavailable — fall back to tab.url } - } catch { - // Playwright unavailable — fall back to tab.url } } enrichTabResponseBody(interceptedBody, tab, postRunUrl);