From a813c8339ca061aabafae212eca2fa61493598d7 Mon Sep 17 00:00:00 2001 From: Randy Date: Wed, 7 Oct 2026 21:08:24 +0800 Subject: [PATCH 1/4] feat(extension): keep framework state out of the page main world Framework views (read_markdown normalization, smart-goto settle probes) now evaluate in a dedicated CDP isolated world via repl/isolated_world.py instead of page.evaluate, so page scripts can neither observe nor intercept them. Agent-authored page.evaluate keeps Playwright's main-world semantics. Exact Page <-> target matching (bind_current_page, page_for_target, switch_tab(page)) reads the top-frame CDP identity over an auxiliary CDP session instead of writing a temporary global marker into the page. The extension facade now serves an explicit attachToTarget on an already-announced tab as an independent auxiliary session, so context.new_cdp_session(page) and its detach no longer collide with Playwright's primary session. The extension no longer rewrites document.title with the attach marker (and no longer injects title-stripping code into utility worlds). The attachment indicator moves to extension UI (per-tab action badge); tab groups remain the visible "agent is driving this tab" signal. cleanupMarkersBeforeReload is renamed to cleanupAttachmentIndicatorsBeforeReload and only clears badges. Co-Authored-By: Claude --- TESTING.md | 2 +- chrome-extension/background.js | 555 +-------- .../daemon/server/facade_extension.py | 56 +- src/browserwright/repl/_md_normalize.py | 7 +- src/browserwright/repl/_smart_goto.py | 8 +- src/browserwright/repl/isolated_world.py | 63 + src/browserwright/repl/markdown.py | 5 +- src/browserwright/repl/playwright_handle.py | 145 +-- src/browserwright/tab_surface.py | 44 +- .../test_issue59_read_markdown_executor.py | 40 + .../e2e/test_l1_title_marker_no_freeze.py | 239 +--- .../e2e/test_l2_extension_tabs_live_url.py | 4 +- .../e2e/test_l2_issue21_page_binding.py | 5 +- tests/daemon/e2e/test_l2_recovery.py | 3 +- .../e2e/test_repro_extension_reload_orphan.py | 1 - .../test_extension_debugger_timeout_unit.py | 6 +- .../test_extension_title_marker_unit.py | 1060 ----------------- ...issue106_extension_applies_store_update.py | 2 +- tests/skill/test_tab_surface.py | 30 +- 19 files changed, 270 insertions(+), 2005 deletions(-) create mode 100644 src/browserwright/repl/isolated_world.py delete mode 100644 tests/daemon/test_extension_title_marker_unit.py diff --git a/TESTING.md b/TESTING.md index 34075ca..d1ab8b0 100644 --- a/TESTING.md +++ b/TESTING.md @@ -103,7 +103,7 @@ tests/daemon/test_adr0011_*.py the one TCP endpoint: surfaces, Origi tests/daemon/test_adr0012_isolation.py dev/prod isolation: own-port stale-detect, stop guard, plist carry-forward, activity verb tests/daemon/test_adr0013_*.py recovery state, bounded recover, executor adoption/handoff tests/skill/test_adr0013_diagnose_before_blame.py diagnose-before-blame errors, banned words, --reuse, attributed log -tests/daemon/test_extension_*.py extension upstream, title marker, version reload +tests/daemon/test_extension_*.py extension upstream, debugger timeouts, version reload tests/daemon/test_launch_chrome.py Chrome launch/discovery (fake binaries; no real Chrome) tests/daemon/test_doctor.py doctor schema + backend recommendation tests/daemon/test_stale_daemon*.py stale/half-alive daemon recovery diff --git a/chrome-extension/background.js b/chrome-extension/background.js index 5301f68..532a10f 100644 --- a/chrome-extension/background.js +++ b/chrome-extension/background.js @@ -367,7 +367,7 @@ async function applyPendingUpdateIfIdle() { const { version } = pendingUpdate; pendingUpdate = null; console.info("[bd-relay] reloading into store update", version); - await cleanupMarkersBeforeReload(); + await cleanupAttachmentIndicatorsBeforeReload(); chrome.runtime.reload(); } @@ -637,7 +637,7 @@ async function handleDaemonMessage(msg) { msg.reason || "manual", msg.expectedVersion || daemonVersion || "", ); - await cleanupMarkersBeforeReload(); + await cleanupAttachmentIndicatorsBeforeReload(); chrome.runtime.reload(); return; case "attach": @@ -721,13 +721,7 @@ async function handleDaemonMessage(msg) { // "completes after detach" window, `tabEpochs` below guards the steps in // between. // -// The title-marker path predates this and stays separate on purpose: -// `markerCommandBefore` shares ONE deadline across the phases of a marker -// install/remove (so hung calls cannot multiply the delay) and is guarded -// by its own per-tab tokens. Do not merge the two without re-running the -// marker unit tests. - -// The extension's OWN chrome.debugger commands (title marker, keep-rendered, +// The extension's OWN chrome.debugger commands (keep-rendered, // reload cleanup), and a relayed command from a daemon too old to send // `timeoutMs` (whose send_cdp waits 10.0s). const DEBUGGER_INTERNAL_COMMAND_TIMEOUT_MS = 9000; @@ -865,18 +859,17 @@ async function attachTab(tabId, { announce = true, skipIfAttached = false } = {} // Post-attach niceties, deliberately NOT awaited by callers. Called at each // site's original position (after/before the RPC response varies per site). function postAttachCosmetics(tabId) { - markTabAttached(tabId); // fire-and-forget; cosmetic + updateTabAttachmentIndicator(tabId); // fire-and-forget; extension UI only keepTabRendered(tabId); // fire-and-forget; keep off-screen tab rendering } -// Shared detach cleanup: strip title marker → chrome.debugger.detach → +// Shared detach cleanup: chrome.debugger.detach → // attachedTabs.delete → [announce `detached`]. Options: // announceReason — when set, emit {type:"detached", reason} to the daemon. // ignoreDetachError (default false) — swallow chrome.debugger.detach errors // (already detached / tab gone) instead of throwing. async function detachTab(tabId, { announceReason = null, ignoreDetachError = false } = {}) { const epoch = bumpTabEpoch(tabId); - await unmarkTabBeforeDetach(tabId); try { await debuggerDetach(tabId); } catch (e) { @@ -887,6 +880,7 @@ async function detachTab(tabId, { announceReason = null, ignoreDetachError = fal // daemon would drop a ghost that just re-joined). if (!isTabEpochCurrent(tabId, epoch)) return; attachedTabs.delete(tabId); + updateTabAttachmentIndicator(tabId); if (announceReason) { safeSend({ type: "detached", tabId, reason: announceReason }); } @@ -904,7 +898,7 @@ async function doAttach(id, tabId) { result: { targetInfo: { url: tab.url || "", - title: stripMarker(tab.title), + title: String(tab.title ?? ""), }, }, }); @@ -970,7 +964,7 @@ async function doAttachActive(id, groupName) { result: { tabId: tab.id, url: tab.url || "", - title: stripMarker(tab.title), + title: String(tab.title ?? ""), groupId: finalGroupId, }, }); @@ -1042,7 +1036,7 @@ async function doCreateTab( safeSend({ type: "response", id, - result: { tabId: tab.id, url: actualUrl, title: stripMarker(title), groupId }, + result: { tabId: tab.id, url: actualUrl, title: String(title ?? ""), groupId }, }); if (!skipPostAttachCommands && attached) { postAttachCosmetics(tab.id); @@ -1096,11 +1090,8 @@ async function doCloseTab(id, tabId) { await chrome.tabs.remove(tabId); // Chrome removed the tab and tears down its debugger session as part of // that operation. Commit our bookkeeping only after that confirmation; - // otherwise a failed remove would leave a visible marker on a tab we had - // already forgotten and detached. + // otherwise a failed remove would forget a tab that Chrome still owns. bumpTabEpoch(tabId); - invalidateMarkerInstall(tabId); - markedTabs.delete(tabId); attachedTabs.delete(tabId); safeSend({ type: "response", id, result: { ok: true, tabId } }); } catch (e) { @@ -1108,10 +1099,8 @@ async function doCloseTab(id, tabId) { if (msg.includes("no tab with id")) { // Already gone — caller wanted it closed, success-equivalent. bumpTabEpoch(tabId); - invalidateMarkerInstall(tabId); - markedTabs.delete(tabId); attachedTabs.delete(tabId); - safeSend({ type: "response", id, result: { ok: true, tabId } }); + safeSend({ type: "response", id, result: { ok: true, tabId } }); return; } safeSend({ @@ -1249,7 +1238,7 @@ async function doQueryGroup(id, groupName) { const out = (Array.isArray(tabs) ? tabs : []).map((tab) => ({ tabId: tab.id, url: tab.url || "", - title: stripMarker(tab.title), + title: String(tab.title ?? ""), active: !!tab.active, lastAccessed: tab.lastAccessed || 0, })); @@ -1279,7 +1268,7 @@ async function announceAttached(tabId) { safeSend({ type: "attached", tabId, - targetInfo: { url: tab.url || "", title: stripMarker(tab.title) }, + targetInfo: { url: tab.url || "", title: String(tab.title ?? "") }, }); } catch (e) { // Tab was closed before we could read it — silently drop. @@ -1292,373 +1281,37 @@ function errMessage(e) { return e.message || String(e); } -// ---- title marker: 👀 prefix on AI-attached tabs -------------------------- -// -// Prepend 👀 to document.title on every attached tab so the user can see in -// their browser tab strip which tab the agent is driving. Survives same-doc -// title mutations (React/Next.js routes, jQuery, etc.) via MutationObserver -// on document.head, and survives navigations via -// Page.addScriptToEvaluateOnNewDocument. On graceful detach we strip the -// prefix and disconnect the observer; on unexpected detach (DevTools steals -// the session) the prefix persists until the user reloads — acceptable. - -// The marker written in front of a real title: eye + separating space. +// ---- extension-owned attachment indicator -------------------------------- // -// There are TWO marker forms on purpose, and the injected side (PREFIX / -// PREFIX_BARE in MARKER_STRIP_PREFIX_SRC) is what decides between them. An -// empty title gets the *bare* eye, because HTML's `document.title` getter -// strips trailing ASCII whitespace: `"👀 "` can never be read back as `"👀 "`. -// The injected observer re-asserts the marker on every mutation, so a -// value that never reads back as what was written is rewritten forever — a -// microtask loop that pins the renderer's main thread. `chrome.debugger` -// commands then never resolve and the daemon reports -// `relay send failed: TimeoutError()`. Only empty-titled pages (about:blank, -// data: URLs, pages caught before their title is set) could reach it, which is -// what made the freeze look random. -// -// This constant is the read side: `stripMarker()` below must keep accepting -// both forms. There is no bare twin here because the SW never writes titles. -const TITLE_PREFIX = "\u{1F440} "; // 👀 + space - -// ---- shared injected-source fragments -------------------------------------- -// -// Both MARKER_INSTALL_SCRIPT and MARKER_REMOVE_SCRIPT are assembled from -// these canonical fragments so the prefix-stripping / title-reading logic -// can't drift between the two. The SW-side stripMarker() below mirrors -// MARKER_STRIP_PREFIX_SRC but can't be generated from it (MV3 extension CSP -// forbids eval/new Function) — keep them in sync by hand. -// -// Footgun: everything below lives inside template literals, so a backtick -// anywhere in them — including in a // comment — ends the string early and -// turns background.js into a syntax error. Quote identifiers in these comments -// with plain words, not backticks. - -// Defines PREFIX / PREFIX_BARE + stripPrefix(value) + markedTitle(value) in the -// injected scope. Mirrors TITLE_PREFIX / TITLE_PREFIX_BARE / stripMarker() on -// the SW side. -const MARKER_STRIP_PREFIX_SRC = ` - const PREFIX = '\u{1F440} '; - const PREFIX_BARE = '\u{1F440}'; - - function stripPrefix(value) { - let title = String(value ?? ''); - while (title.startsWith(PREFIX)) { - title = title.slice(PREFIX.length); - } - while (title.length > 0 && title.codePointAt(0) === 0x1F440) { - title = title.slice(2); - if (title.length > 0 && /\\s/.test(title[0])) { - title = title.slice(1); - } - } - return title; - } - - // The one place that decides what a marked title looks like. Used by BOTH - // writers (the observer's re-assert and the document.title setter) so they - // cannot disagree about the empty case — the case that used to loop. - // - // Invariant: markedTitle(x) must survive a DOM round-trip unchanged, i.e. - // reading back what it wrote yields the same string. With a trailing space - // and an empty clean title it does not, and the observer never settles. - // stripPrefix() accepts both the spaced and the bare form, so titles marked - // by an older build are still cleaned up correctly on detach. - function markedTitle(value) { - const clean = stripPrefix(value); - return clean ? PREFIX + clean : PREFIX_BARE; - } -`; - -// Defines rawTitle(doc) in the injected scope. Reads the real document title -// via a `titleDescriptor` that must already be in scope (each script derives -// its own), falling back to the element. writeRawTitle is NOT shared: -// the install and remove scripts intentionally differ in which descriptor -// they trust for writing. -const MARKER_RAW_TITLE_SRC = ` - function rawTitle(doc) { - doc = doc || document; - if (titleDescriptor && titleDescriptor.get) { - return titleDescriptor.get.call(doc) || ''; - } - const el = doc.querySelector && doc.querySelector('title'); - return el ? (el.textContent || '') : ''; - } -`; - -// Playwright reads titles -- `page.title()`, `expect(page).to_have_title()` -- -// in its own isolated "utility" world, which has its own Document.prototype, so -// the main-world accessor in MARKER_INSTALL_SCRIPT never reaches it and agents -// read "👀 <title>". Give every utility world a read-side strip too. Only the -// getter changes: the main-world observer re-marks whatever gets written. -const PLAYWRIGHT_UTILITY_WORLD_PREFIX = "__playwright_utility_world_"; -const UTILITY_WORLD_TITLE_SCRIPT = ` -(function() { -` + MARKER_STRIP_PREFIX_SRC + ` - const native = Object.getOwnPropertyDescriptor(Document.prototype, 'title'); - if (!native || !native.get || !native.set) return; - Object.defineProperty(Document.prototype, 'title', { - configurable: true, - enumerable: true, - get: function() { return stripPrefix(native.get.call(this)); }, - set: function(value) { native.set.call(this, value); }, - }); -})(); -`; - -function stripMarkerInUtilityWorld(source, params) { - const ctx = params?.context; - const name = ctx?.name || ctx?.auxData?.name || ""; - if (typeof ctx?.id !== "number" - || !name.startsWith(PLAYWRIGHT_UTILITY_WORLD_PREFIX)) { - return; - } - // Issued before the event is forwarded: Chrome runs one debuggee's commands - // in order, and Playwright cannot address this context until it has seen - // the event, so no title read in it can overtake the install. - debuggerCommand(source, "Runtime.evaluate", { - expression: UTILITY_WORLD_TITLE_SCRIPT, - contextId: ctx.id, - }).catch((e) => - console.warn("[bd-relay] utility-world title strip failed:", e)); -} - -const MARKER_INSTALL_SCRIPT = ` -(function() { -` + MARKER_STRIP_PREFIX_SRC + ` - const previousMarker = window.__bdTitleMarker || null; - try { - previousMarker && previousMarker.obs && previousMarker.obs.disconnect(); - } catch (e) {} - try { - previousMarker && previousMarker.restoreTitleAccessor && - previousMarker.restoreTitleAccessor(); - } catch (e) {} - - function findTitleOwner() { - if (typeof Document !== 'undefined' && - Object.getOwnPropertyDescriptor(Document.prototype, 'title')) { - return Document.prototype; - } - if (typeof HTMLDocument !== 'undefined' && - Object.getOwnPropertyDescriptor(HTMLDocument.prototype, 'title')) { - return HTMLDocument.prototype; - } - return typeof Document !== 'undefined' ? Document.prototype : null; - } - - const titleOwner = findTitleOwner(); - const titleDescriptor = titleOwner - ? Object.getOwnPropertyDescriptor(titleOwner, 'title') - : null; -` + MARKER_RAW_TITLE_SRC + ` - function writeRawTitle(doc, title) { - doc = doc || document; - if (titleDescriptor && titleDescriptor.set) { - titleDescriptor.set.call(doc, title); - return; - } - let el = doc.querySelector && doc.querySelector('title'); - if (!el && doc.head && doc.createElement) { - el = doc.createElement('title'); - doc.head.appendChild(el); - } - if (el) el.textContent = title; - } - - let normalizing = false; - // The value we last asked the DOM to store, cleared once it sticks. - // - // The normalizing flag is not a guard against self-feeding writes: - // ensurePrefix runs from a MutationObserver, whose callbacks are delivered - // asynchronously, so the finally below has always released the flag before - // the next one arrives. markedTitle() is a fixpoint, so this latch never - // fires in practice — but if a write ever fails to round-trip again (a future - // DOM normalization rule, another script fighting us for the title) it caps - // the argument at one wasted write instead of an unbounded microtask loop - // that freezes the tab. - let lastWritten = null; - function ensurePrefix() { - if (normalizing) return; - normalizing = true; - try { - const current = rawTitle(); - const marked = markedTitle(current); - if (current === marked) { - lastWritten = null; // settled; a later title change may re-mark - return; - } - if (marked === lastWritten) return; // already written, it didn't stick - lastWritten = marked; - writeRawTitle(document, marked); - } finally { - normalizing = false; - } - } - - function installTitleAccessor(target) { - if (!target) return; - Object.defineProperty(target, 'title', { - configurable: true, - enumerable: true, - get: function() { - return stripPrefix(rawTitle(this)); - }, - set: function(value) { - // Same fixpoint rule as ensurePrefix — a page clearing its own title - // must not store a value the getter will normalize into a mismatch. - writeRawTitle(this, markedTitle(value)); - }, - }); - } - - function restoreTitleAccessor() { - try { delete document.title; } catch (e) {} - if (!titleOwner) return; - try { - if (titleDescriptor) { - Object.defineProperty(titleOwner, 'title', titleDescriptor); - } else { - delete titleOwner.title; - } - } catch (e) {} - } - +// Attached state belongs to Chrome's extension UI, not the website's DOM. +// Per-tab action badges and the existing session group / popup identify the +// driven tab without changing document.title, DOM prototypes, or page globals. +// No renderer script is needed on attachment, navigation, or detach. +async function updateTabAttachmentIndicator(tabId, attached = attachedTabs.has(tabId)) { try { - installTitleAccessor(titleOwner); - installTitleAccessor(document); - } catch (e) {} - - const obs = new MutationObserver(ensurePrefix); - function attachObs() { - if (!document.head) return; - obs.observe(document.head, { childList: true, characterData: true, subtree: true }); - ensurePrefix(); - } - if (document.head) { - attachObs(); - } else { - document.addEventListener('DOMContentLoaded', attachObs, { once: true }); - } - window.__bdTitleMarker = { - obs, - ensurePrefix, - restoreTitleAccessor, - nativeTitleDescriptor: titleDescriptor, - nativeTitleOwner: titleOwner, - }; -})(); -`; - -const MARKER_REMOVE_SCRIPT = ` -(function() { -` + MARKER_STRIP_PREFIX_SRC + ` - const marker = window.__bdTitleMarker || null; - const nativeTitleDescriptor = - marker && marker.nativeTitleDescriptor; - const titleDescriptor = - nativeTitleDescriptor || - Object.getOwnPropertyDescriptor(Document.prototype, 'title') || - (typeof HTMLDocument !== 'undefined' - ? Object.getOwnPropertyDescriptor(HTMLDocument.prototype, 'title') - : null); -` + MARKER_RAW_TITLE_SRC + ` - function writeRawTitle(doc, title) { - doc = doc || document; - if (nativeTitleDescriptor && nativeTitleDescriptor.set) { - nativeTitleDescriptor.set.call(doc, title); - return; - } - if (titleDescriptor && titleDescriptor.set && - !(marker && marker.restoreTitleAccessor)) { - titleDescriptor.set.call(doc, title); - return; - } - let el = doc.querySelector && doc.querySelector('title'); - if (!el && doc.head && doc.createElement) { - el = doc.createElement('title'); - doc.head.appendChild(el); - } - if (el) el.textContent = title; - } - try { window.__bdTitleMarker && window.__bdTitleMarker.obs.disconnect(); } catch (e) {} - const clean = stripPrefix(rawTitle()); - try { - marker && marker.restoreTitleAccessor && marker.restoreTitleAccessor(); - } catch (e) { - try { delete document.title; } catch (_e) {} - } - delete window.__bdTitleMarker; - writeRawTitle(document, clean); -})(); -`; - -// tabId → scriptIdentifier returned by Page.addScriptToEvaluateOnNewDocument -// (needed to remove the per-document hook on detach). -const markedTabs = new Map(); -// tabId → {token, promise} for the in-flight installation. The unique token is -// the cancellation/ABA guard: detach invalidates it, and a later re-attach gets -// a different token that an old catch/finally cannot erase. -const markingTabs = new Map(); -// tabId → current opaque generation token. Object identity (not a resettable -// integer) stays safe if Chrome later recycles a numeric tab id. -const markerTokens = new Map(); -const MARKER_RELOAD_CLEANUP_TIMEOUT_MS = 1500; -// Marker CDP is cosmetic and must never hold debugger detach hostage. Each -// phase gets one absolute budget shared by all of its sendCommand calls, so a -// sequence of hung calls cannot multiply the delay. -const MARKER_INSTALL_TIMEOUT_MS = 1500; -const MARKER_REMOVE_TIMEOUT_MS = 1000; - -function invalidateMarkerInstall(tabId) { - const pending = markingTabs.get(tabId)?.promise; - markerTokens.set(tabId, {}); - markingTabs.delete(tabId); - return pending; -} - -async function markerCommandBefore(deadline, tabId, method, params) { - const remaining = deadline - Date.now(); - if (remaining <= 0) { - throw new Error("marker " + method + " exceeded its deadline"); - } - let timer = null; - try { - return await Promise.race([ - chrome.debugger.sendCommand({ tabId }, method, params), - new Promise((_, reject) => { - timer = setTimeout(() => reject(new Error( - "marker " + method + " timed out")), remaining); + await Promise.all([ + chrome.action.setBadgeText({ tabId, text: attached ? "AI" : "" }), + chrome.action.setBadgeBackgroundColor({ tabId, color: "#2563eb" }), + chrome.action.setTitle({ + tabId, + title: attached ? "browserwright: agent attached" : "browserwright-daemon relay", }), ]); - } finally { - if (timer !== null) clearTimeout(timer); + } catch (e) { + // Tab closure and extension UI failures must never affect browser driving. + console.warn("[bd-relay] attachment indicator(" + tabId + ") failed:", e); } } -// SW-side twin of the injected stripPrefix — see MARKER_STRIP_PREFIX_SRC -// above. Can't be generated from that fragment (MV3 CSP bans eval); if you -// change one, change both. -// -// Handles BOTH marker forms, and must keep doing so: the first loop eats -// TITLE_PREFIX (`"👀 Foo"`), the second eats a bare TITLE_PREFIX_BARE plus one -// optional following whitespace char (`"👀"`, `"👀Foo"`). That is not just -// tidiness — tabs marked by an older build carry the spaced form, and a user's -// long-lived Chrome still has those tabs open across an extension update, so a -// stripper that only knew the new form would leave 👀 stuck in their tab strip. -// There is deliberately no markedTitle() twin here: the SW only ever *reads* -// titles (to report them upstream), it never writes one. -function stripMarker(title) { - let clean = String(title ?? ""); - while (clean.startsWith(TITLE_PREFIX)) { - clean = clean.slice(TITLE_PREFIX.length); - } - while (clean.length > 0 && clean.codePointAt(0) === 0x1F440) { - clean = clean.slice(2); - if (clean.length > 0 && /\s/.test(clean[0])) { - clean = clean.slice(1); - } - } - return clean; +async function cleanupAttachmentIndicatorsBeforeReload() { + // Action state can outlive a service-worker realm. Clear it while we still + // know the attached tabs; debugger attachment itself is torn down by reload. + await Promise.race([ + Promise.allSettled( + [...attachedTabs].map((tabId) => updateTabAttachmentIndicator(tabId, false)), + ), + sleep(1500), // Cosmetic browser UI must never hold extension reload hostage. + ]); } async function keepTabRendered(tabId) { @@ -1667,7 +1320,7 @@ async function keepTabRendered(tabId) { // unthrottles requestAnimationFrame, keeps document.hasFocus() true, and // stops Chrome from freezing/discarding the tab. fire-and-forget; each // command is independently guarded so an unsupported one doesn't sink the - // other, and a failure never fails the attach (cosmetic-ish, like markTab). + // other, and a failure never fails the attach (independent of the browser UI indicator). try { await debuggerCommand( { tabId }, "Emulation.setFocusEmulationEnabled", { enabled: true }); @@ -1682,126 +1335,6 @@ async function keepTabRendered(tabId) { } } -async function markTabAttached(tabId) { - const pending = markingTabs.get(tabId); - if (pending && markerTokens.get(tabId) === pending.token) { - await pending.promise; - return; - } - if (markedTabs.has(tabId)) return; - const token = {}; - markerTokens.set(tabId, token); - const isCurrent = () => markerTokens.get(tabId) === token; - const install = (async () => { - const deadline = Date.now() + MARKER_INSTALL_TIMEOUT_MS; - // Reserve the slot up-front so concurrent markTabAttached(tabId) calls - // (e.g. popup-attach racing daemon attach-active) coalesce. - markedTabs.set(tabId, ""); - try { - // Page domain may not be enabled yet on a fresh chrome.debugger session; - // enabling is idempotent so this is safe to call repeatedly. - await markerCommandBefore(deadline, tabId, "Page.enable", {}); - if (!isCurrent()) return; - const reg = await markerCommandBefore( - deadline, - tabId, - "Page.addScriptToEvaluateOnNewDocument", - { source: MARKER_INSTALL_SCRIPT }, - ); - if (!isCurrent()) { - // Best effort for a registration that completed as detach invalidated - // us. Detach itself clears debugger-session registrations; this covers - // implementations where the completion won that race. Bounded like - // every other chrome.debugger call here. - if (reg?.identifier) { - debuggerCommand( - { tabId }, - "Page.removeScriptToEvaluateOnNewDocument", - { identifier: reg.identifier }, - ).catch(() => {}); - } - return; - } - markedTabs.set(tabId, reg?.identifier || ""); - // The above fires only on new documents; inject into the current one too. - await markerCommandBefore( - deadline, - tabId, - "Runtime.evaluate", - { expression: MARKER_INSTALL_SCRIPT }, - ); - if (!isCurrent()) return; - } catch (e) { - // Tab might have closed mid-attach, or chrome.debugger session is gone — - // not worth failing the whole attach over a cosmetic marker. - console.warn("[bd-relay] markTabAttached(" + tabId + ") failed:", e); - if (isCurrent()) markedTabs.delete(tabId); - } - })(); - const record = { token, promise: install }; - markingTabs.set(tabId, record); - try { - await install; - } finally { - if (markingTabs.get(tabId) === record) markingTabs.delete(tabId); - } -} - -async function unmarkTabBeforeDetach(tabId) { - const deadline = Date.now() + MARKER_REMOVE_TIMEOUT_MS; - const pending = invalidateMarkerInstall(tabId); - // Invalidate before any wait. Every continuation of the old installation - // checks its opaque token before issuing the next marker command. - const identifier = markedTabs.get(tabId); - markedTabs.delete(tabId); - // Dispatch both cleanup commands immediately. They queue behind any marker - // command Chrome is already processing; notably, current-page removal then - // runs after a late install Runtime.evaluate instead of detach racing ahead - // and leaving the visible marker behind. Awaiting one cleanup before sending - // the other would let it consume the whole shared deadline. - const removals = []; - if (identifier) { - removals.push(markerCommandBefore( - deadline, - tabId, - "Page.removeScriptToEvaluateOnNewDocument", - { identifier }, - ).catch(() => {})); - } - removals.push(markerCommandBefore( - deadline, - tabId, - "Runtime.evaluate", - { expression: MARKER_REMOVE_SCRIPT }, - ).catch(() => {})); - await Promise.allSettled(removals); - if (pending) { - // Still bounded: a wedged renderer must never hold debugger detach hostage. - await Promise.race([ - pending.catch(() => {}), - sleep(Math.max(0, deadline - Date.now())), - ]); - } -} - -async function cleanupMarkersBeforeReload() { - // A controlled extension reload destroys this service-worker realm and both - // in-memory sets. Strip markers while chrome.debugger is still usable. The - // union also covers a mark still in flight (attached, but no identifier yet) - // and any bookkeeping drift between the two collections. - const knownTabs = new Set([...attachedTabs, ...markedTabs.keys()]); - const cleanup = Promise.allSettled( - [...knownTabs].map((tabId) => unmarkTabBeforeDetach(tabId)), - ); - const completed = await Promise.race([ - cleanup.then(() => true), - sleep(MARKER_RELOAD_CLEANUP_TIMEOUT_MS).then(() => false), - ]); - if (!completed) { - console.warn("[bd-relay] marker cleanup timed out; reloading anyway"); - } -} - // ---- chrome.debugger event fan-out ---------------------------------------- chrome.debugger.onEvent.addListener((source, method, params) => { @@ -1832,9 +1365,6 @@ chrome.debugger.onEvent.addListener((source, method, params) => { if (typeof source.sessionId === "string" && source.sessionId) { return; } - if (method === "Runtime.executionContextCreated") { - stripMarkerInUtilityWorld(source, params); - } safeSend({ type: "event", tabId: source.tabId, @@ -1848,13 +1378,8 @@ chrome.debugger.onDetach.addListener((source, reason) => { // A detach invalidates every in-flight attach/arm continuation for this // tab: whatever Chrome settles from here on is stale by construction. bumpTabEpoch(source.tabId); - invalidateMarkerInstall(source.tabId); attachedTabs.delete(source.tabId); - // Unexpected detach (DevTools steals the session, tab crashes, etc.) — - // we can no longer run CDP commands, so the page-side observer keeps the - // 👀 prefix on the current document. It clears naturally on next - // navigation (addScriptToEvaluateOnNewDocument is no longer registered). - markedTabs.delete(source.tabId); + updateTabAttachmentIndicator(source.tabId); safeSend({ type: "detached", tabId: source.tabId, reason }); } }); @@ -1996,9 +1521,7 @@ chrome.tabGroups.onUpdated.addListener((group) => { chrome.tabs.onRemoved.addListener((tabId) => { if (!attachedTabs.has(tabId)) return; bumpTabEpoch(tabId); - invalidateMarkerInstall(tabId); attachedTabs.delete(tabId); - markedTabs.delete(tabId); safeSend({ type: "detached", tabId, reason: "tab_closed" }); }); diff --git a/src/browserwright/daemon/server/facade_extension.py b/src/browserwright/daemon/server/facade_extension.py index db3f7a7..881bc5c 100644 --- a/src/browserwright/daemon/server/facade_extension.py +++ b/src/browserwright/daemon/server/facade_extension.py @@ -201,6 +201,9 @@ def __init__( # `targetDestroyed`/`detachedFromTarget` references the same session and # forwarded extension events can be tagged with the right sessionId. self._tab_sessions: dict[int, str] = {} + # Explicit CDP attachments are independent handles over the shared + # debugger. Reusing a primary session crashes Playwright's CDP router. + self._aux_sessions: dict[str, int] = {} self._closed = False # Guard concurrent synthesis (autoAttach replay vs fan-out attach) so we # never announce the same tab twice. @@ -423,6 +426,9 @@ async def _handle_client_frame(self, frame: str) -> None: # --- A4: Runtime.enable barrier (session-scoped) --- if method == "Runtime.enable" and session_id is not None: + if session_id in self._aux_sessions: + await self._forward_session_command(req_id, session_id, method, params) + return await self._handle_runtime_enable(req_id, session_id, params) return @@ -431,6 +437,15 @@ async def _handle_client_frame(self, frame: str) -> None: # Browser.getVersion). Its responses carry no sessionId, which is # correct for these browser-level methods. --- if session_id is None: + if method == "Target.detachFromTarget" and params.get("sessionId") in self._aux_sessions: + detached_sid = params["sessionId"] + self._aux_sessions.pop(detached_sid) + await self._respond(req_id, {}) + await self._send_to_client(json.dumps({ + "method": "Target.detachedFromTarget", + "params": {"sessionId": detached_sid}, + })) + return if method == "Target.getTargets" and self._session_id is not None: try: envelope = await self._ext.get_targets( @@ -533,24 +548,12 @@ async def _handle_attach_to_target(self, req_id: int | None, if (isinstance(target_id, str) and not await self._authorize_target(req_id, target_id)): return - # Reuse an already-announced session for this tab if we have one, so - # auto-attach replay + an explicit attachToTarget agree on one session. + # An explicit attachment to an announced Page is an auxiliary handle; + # never reuse or re-announce its primary auto-attachment session. if tab_id is not None and tab_id in self._tab_sessions: - sid = self._tab_sessions[tab_id] - try: - # Shared attach core: relay attach + (re-)registering the sid - # in the upstream's table (it may differ from ours if this is - # the first explicit attach) so session-scoped commands - # resolve the tab. - await self._ext.attach_target(tab_id, sid=sid, timeout=10.0) - except _CommandError as e: - await self._error(req_id, e.code, e.message) - return - except Exception as e: # noqa: BLE001 - await self._error(req_id, -32603, f"attach failed: {e!r}") - return + sid = f"ext-aux-{secrets.token_hex(12)}" + self._aux_sessions[sid] = tab_id await self._respond(req_id, {"sessionId": sid}) - await self._announce_target(tab_id, sid=sid, send_created=False) return # Unknown tab → let ExtensionUpstream do the attach + sid fabrication, # then snoop its table to learn the sid it handed back. @@ -643,6 +646,7 @@ async def _handle_close_target(self, req_id: int | None, await self._respond(req_id, {"success": False}) return # Evict the facade-local per-tab state too. + await self._detach_aux_sessions(tab_id) self._evict_tab(tab_id) await self._respond(req_id, {"success": True}) # Real Chrome ALWAYS emits detachedFromTarget + targetDestroyed after a @@ -737,6 +741,8 @@ def _tab_id_for_session(self, session_id: str) -> int | None: authorization boundary: a facade client could otherwise invent ``ext-sid-<foreign-tab>-anything`` and bypass attachToTarget entirely. """ + if session_id in self._aux_sessions: + return self._aux_sessions[session_id] return next( (tab_id for tab_id, sid in self._tab_sessions.items() if sid == session_id), @@ -981,6 +987,7 @@ async def _on_relay_event(self, ext_msg: dict) -> None: sid = self._tab_sessions.get(tab_id) if self._session_id is not None and sid is None: return + await self._detach_aux_sessions(tab_id) self._evict_tab(tab_id) await self._emit_target_detached(tab_id, sid) return @@ -1021,10 +1028,25 @@ async def _on_relay_event(self, ext_msg: dict) -> None: if sid is not None: out["sessionId"] = sid await self._send_to_client(json.dumps(out)) + for aux_sid, target_tab in list(self._aux_sessions.items()): + if target_tab == tab_id: + await self._send_to_client(json.dumps({ + "method": method, "params": params, "sessionId": aux_sid, + })) return # ---- helpers --------------------------------------------------------- + async def _detach_aux_sessions(self, tab_id: int) -> None: + for aux_sid, target_tab in list(self._aux_sessions.items()): + if target_tab == tab_id: + self._aux_sessions.pop(aux_sid, None) + await self._send_to_client(json.dumps({ + "method": "Target.detachedFromTarget", + "params": {"sessionId": aux_sid, + "targetId": f"ext-tab-{tab_id}"}, + })) + async def _emit_target_detached(self, tab_id: int, sid: str | None) -> None: """Synthesize the CDP teardown pair for a gone tab — @@ -1087,6 +1109,8 @@ def _evict_tab(self, tab_id: int) -> None: """Drop all per-tab state for a closed/detached tab and wake any outstanding Runtime.enable barrier so it doesn't hang on a dead tab.""" self._tab_sessions.pop(tab_id, None) + for sid in [sid for sid, target in self._aux_sessions.items() if target == tab_id]: + self._aux_sessions.pop(sid, None) self._ext.evict_tab_sessions(tab_id) self._tab_url.pop(tab_id, None) self._fresh_blank_tabs.discard(tab_id) diff --git a/src/browserwright/repl/_md_normalize.py b/src/browserwright/repl/_md_normalize.py index 57f1574..1a92eee 100644 --- a/src/browserwright/repl/_md_normalize.py +++ b/src/browserwright/repl/_md_normalize.py @@ -13,7 +13,7 @@ So this script runs where the DOM is, rebuilds a detached copy with those two problems fixed, and hands back plain HTML strings. -Why ``page.evaluate`` and not ``add_script_tag`` +Why isolated protocol evaluation and not ``add_script_tag`` ------------------------------------------------ Verified in this repo against ``default-src 'none'; script-src 'none'`` with ``bypass_csp=False``: the page's own inline ``<script>`` does NOT run (CSP is @@ -28,8 +28,9 @@ the protocol call; a continuation scheduled from inside it (``setTimeout``, ``await``) loses it. Everything here is one synchronous pass. - **Never assign ``innerHTML``, never use ``DOMParser``.** Those are gated by - Trusted Types, which ``page.evaluate`` does NOT bypass — it runs in the main - world. *Reading* ``innerHTML``/``outerHTML`` and calling ``cloneNode`` / + Trusted Types when the script is reused through ``page.evaluate``. The + production view runs in an isolated world through ``isolated_evaluate``. + *Reading* ``innerHTML``/``outerHTML`` and calling ``cloneNode`` / ``importNode`` are unaffected, which is why the copy is built with node APIs and HTML is only ever read out. - **Never mutate the live page.** Everything is built detached, and Readability diff --git a/src/browserwright/repl/_smart_goto.py b/src/browserwright/repl/_smart_goto.py index 11b5f7e..0ac8e27 100644 --- a/src/browserwright/repl/_smart_goto.py +++ b/src/browserwright/repl/_smart_goto.py @@ -221,15 +221,17 @@ def _looks_loaded(page: Any, before_url: str | None = None) -> bool: return False if before_url is not None and url == before_url: return False - ready_state = page.evaluate("() => document.readyState") + from .isolated_world import isolated_evaluate + ready_state = isolated_evaluate(page, "() => document.readyState") return ready_state != "loading" except Exception: return False def _smart_wait_settled(page: Any, deadline: float | None, network: "_NetworkMonitor") -> None: + from .isolated_world import isolated_evaluate try: - page.evaluate(_INSTALL_MONITOR_JS) + isolated_evaluate(page, _INSTALL_MONITOR_JS) except Exception: return @@ -237,7 +239,7 @@ def _smart_wait_settled(page: Any, deadline: float | None, network: "_NetworkMon remaining_ms = None if deadline is None else max(1, int((deadline - time.monotonic()) * 1000)) poll_ms = 250 if remaining_ms is None else min(250, remaining_ms) try: - settled = page.evaluate(_SETTLED_JS, _STABLE_WINDOW_MS) + settled = isolated_evaluate(page, _SETTLED_JS, _STABLE_WINDOW_MS) except Exception: return if settled or network.is_idle(_STABLE_WINDOW_MS): diff --git a/src/browserwright/repl/isolated_world.py b/src/browserwright/repl/isolated_world.py new file mode 100644 index 0000000..51afa7f --- /dev/null +++ b/src/browserwright/repl/isolated_world.py @@ -0,0 +1,63 @@ +"""Internal document inspection in a CDP isolated world. + +Agent-authored ``page.evaluate`` keeps Playwright's main-world semantics. +Framework views use this helper so their globals and native DOM methods are +separate from the page's JavaScript. The CDP handle belongs to the Page; worlds +are resolved for every call because navigation destroys execution contexts. +""" +from __future__ import annotations + +import json +from typing import Any + +_WORLD_NAME = "browserwright-internal" +_SESSION_ATTR = "_browserwright_inspection_cdp" + + +def page_cdp_session(page: Any) -> Any: + session = getattr(page, _SESSION_ATTR, None) + if session is None: + session = page.context.new_cdp_session(page) + setattr(page, _SESSION_ATTR, session) + def clear_session(*_args: Any) -> None: + if getattr(page, _SESSION_ATTR, None) is session: + setattr(page, _SESSION_ATTR, None) + session.on("close", clear_session) + return session + + +def target_id_for_page(page: Any) -> str: + """Exact target identity without putting a marker in the document.""" + tree = page_cdp_session(page).send("Page.getFrameTree") + return tree["frameTree"]["frame"]["id"] + + +def isolated_evaluate(page: Any, expression: str, arg: Any = None) -> Any: + """Evaluate an internal JavaScript function with a JSON argument. + + Lightweight test doubles keep their existing ``evaluate`` surface. Real + Playwright Pages fail closed if isolated evaluation fails; there is no + fallback into the page's main world. + """ + if not hasattr(page, "_impl_obj"): + return page.evaluate(expression) if arg is None else page.evaluate(expression, arg) + session = page_cdp_session(page) + tree = session.send("Page.getFrameTree") + frame_id = tree["frameTree"]["frame"]["id"] + world = session.send("Page.createIsolatedWorld", { + "frameId": frame_id, + "worldName": _WORLD_NAME, + "grantUniveralAccess": False, + }) + result = session.send("Runtime.evaluate", { + "contextId": world["executionContextId"], + "expression": f"({expression})({json.dumps(arg, ensure_ascii=False)})", + "returnByValue": True, + "awaitPromise": True, + }) + if result.get("exceptionDetails"): + details = result["exceptionDetails"] + raise RuntimeError((details.get("exception") or {}).get("description") + or details.get("text") or "isolated evaluation failed") + remote = result.get("result") or {} + return remote.get("value") diff --git a/src/browserwright/repl/markdown.py b/src/browserwright/repl/markdown.py index ecb2d1a..6d95472 100644 --- a/src/browserwright/repl/markdown.py +++ b/src/browserwright/repl/markdown.py @@ -9,7 +9,7 @@ The pipeline, and why it has the shape it has (all of it is ADR-0007): live DOM - │ page.evaluate(_md_normalize) absolutize URLs, flatten open shadow + │ isolated_evaluate(_md_normalize) absolutize URLs, flatten open shadow │ roots, inline same-origin iframes ▼ normalized HTML ─── optional ───► Readability (in the same pass, on a @@ -42,6 +42,7 @@ from .._text import PRODUCER_BUDGET, truncate_lines as _truncate_lines from . import _md_normalize from ._md_convert import convert_html +from .isolated_world import isolated_evaluate MODES = ("auto", "article", "full") @@ -99,7 +100,7 @@ def render_page_markdown(page: Any, *, mode: str = "auto") -> RenderedPage: raise ValueError(f"mode must be one of {MODES!r}, got {mode!r}") want_article = mode in ("auto", "article") - raw = page.evaluate( + raw = isolated_evaluate(page, _md_normalize.build_script(extract=want_article), {"extract": want_article}, ) diff --git a/src/browserwright/repl/playwright_handle.py b/src/browserwright/repl/playwright_handle.py index 9091c34..f7fc947 100644 --- a/src/browserwright/repl/playwright_handle.py +++ b/src/browserwright/repl/playwright_handle.py @@ -35,7 +35,6 @@ import logging from typing import Any from urllib.parse import parse_qsl, urlencode, urlsplit, urlunsplit -from uuid import uuid4 from ..errors import BrowserwrightError, PageBindTimeout, TabRebindFailed @@ -160,15 +159,10 @@ def _agent_page_targets(sess: Any) -> list[dict]: (``tab_surface``) use, so the ids always line up with the ledger's ``current_target_id``. - Why the agent path and not a Playwright CDP session? Two Playwright CDP - sessions for target enumeration are FATAL over the extension facade: - `context.new_cdp_session(page)` collides with the page's primary session, - and even `browser.new_browser_cdp_session()` reuses the facade's single - synthetic browser sessionId — both trip a Playwright-driver assert that - kills the connection. The agent path is the daemon's own, fully-tested - channel; its targetIds are exactly the daemon/ledger ids (extension - `ext-tab-N`, cdp real ids), so they line up with the ledger's - `current_target_id` and with `connect_over_cdp`'s synthesized targetIds.""" + Enumeration stays on the agent connection, whose target ids match the + daemon/ledger ids. Page identity probes use independent auxiliary CDP + attachments; their detach never affects Playwright's primary attachment. + """ try: res = sess.cdp.send("Target.getTargets") except Exception: @@ -192,8 +186,8 @@ def _agent_page_targets(sess: Any) -> list[dict]: # Phase B: the persistent per-session executor (``browserwright._executor``) # runs the SAME connect+bind dance, just ONCE at cold-start instead of per # heredoc. These free functions are the single source of truth so the executor -# never re-implements (and never drifts from) the FATAL "no Playwright CDP -# session over the extension facade" constraint. ``PlaywrightHandle`` below is +# never re-implements (and never drifts from) exact CDP identity mapping. +# ``PlaywrightHandle`` below is # the per-heredoc Phase C consumer; the executor is the Phase B consumer. @@ -264,15 +258,10 @@ def bind_current_page(context: Any, sess: Any) -> Any: tab. ``context.new_page()`` over the facade would open an un-grouped tab the agent path can't track → ledger drift → tab explosion. - We then attach Playwright to that exact tab by writing a short-lived random - marker through the already-attached AGENT target session and accepting only - the ``context.pages`` entry that reads it. The facade replays ``attached`` - events for every open tab, so a session-group tab is enumerable once - materialized. Mapping creates NO Playwright CDP session — a per-page - (``context.new_cdp_session``) or even a second browser-level - (``new_browser_cdp_session``) session is fatal over the extension facade - because it reuses one synthetic sessionId and trips a Playwright-driver - assertion. + We match the resolved target with a Page's exact top-frame CDP identity. + The facade replays attachment events for open tabs, and each identity + probe uses an independent auxiliary CDP handle. Closing that handle leaves + Playwright's primary attachment intact and does not mutate the document. """ from ..session_runtime import resolve_current_target from ._smart_goto import patch_context_pages, patch_page_goto @@ -553,33 +542,25 @@ def page_for_target(context: Any, sess: Any, target_id: str, hint_url: str | None = None) -> Any | None: """Find the live Playwright Page for the session's ``target_id``. - Mapping uses NO *Playwright-created* CDP session (fatal over the extension - facade — see ``_agent_page_targets``). Instead, the already-attached agent - path writes a short-lived random marker into the exact target's main world; - only the Playwright Page that can read that marker is accepted. This avoids - guessing from page count or URL, both of which are ambiguous for launcher - tabs, duplicate URLs, and several ``about:blank`` pages. - - Lightweight unit-test doubles without an agent CDP surface retain a strict - unique-URL fallback. A real Session whose marker command fails returns no - match and lets the outer wait retry; it never degrades to a guess.""" + Mapping reads each Page's exact top-frame CDP identity. Auxiliary CDP + attachments are independent of Playwright's primary session and never + place state in the document. Lightweight test doubles without an agent + CDP surface retain the strict unique-URL fallback. + """ pages = list(context.pages) if not pages: return None - marker_attempted, marker = _install_target_marker(sess, target_id) - if marker_attempted: - if marker is None: - return None - key, value, cdp, session_id = marker - try: - matches = [ - page for page in pages - if _page_has_target_marker(page, key, value) - ] - return matches[0] if len(matches) == 1 else None - finally: - _clear_target_marker(cdp, session_id, key) + try: + sess.cdp + except AttributeError: + pass + except Exception: + return None + else: + matches = [page for page in pages + if _target_id_for_page(page) == target_id] + return matches[0] if len(matches) == 1 else None # Test-double compatibility only: without an agent CDP path, require a # unique URL match. Never use the old singleton/last-match heuristics. @@ -594,77 +575,14 @@ def page_for_target(context: Any, sess: Any, target_id: str, return matches[0] if len(matches) == 1 else None -def _install_target_marker( - sess: Any, - target_id: str, -) -> tuple[bool, tuple[str, str, Any, str] | None]: - """Mark ``target_id`` through the existing agent CDP attachment. +def _target_id_for_page(page: Any) -> str | None: + """Read exact CDP target identity without document state or URL guesses.""" + from .isolated_world import target_id_for_page - Returns ``(False, None)`` only for lightweight objects with no ``cdp`` - attribute (unit-test doubles). For a real Session, ``True`` means exact - matching was attempted; a failed attach/evaluate returns ``(True, None)`` - so callers retry rather than fall back to an inexact URL guess. - """ try: - cdp = sess.cdp - except AttributeError: - return False, None + return target_id_for_page(page) except Exception: - return True, None - - key = f"__browserwright_bind_{uuid4().hex}" - value = uuid4().hex - try: - session_id = cdp.attach(target_id) - import json - - expression = ( - "(() => {" - f"Object.defineProperty(globalThis, {json.dumps(key)}, " - f"{{value: {json.dumps(value)}, configurable: true}});" - "return true;" - "})()" - ) - result = cdp.send( - "Runtime.evaluate", - session=session_id, - expression=expression, - returnByValue=True, - ) - if result.get("exceptionDetails"): - return True, None - except Exception: - return True, None - return True, (key, value, cdp, session_id) - - -def _page_has_target_marker(page: Any, key: str, value: str) -> bool: - try: - return bool(page.evaluate( - "([key, value]) => globalThis[key] === value", - [key, value], - )) - except Exception: - return False - - -def _clear_target_marker( - cdp: Any, - session_id: str, - key: str, -) -> None: - """Best-effort cleanup of the temporary page-global marker.""" - try: - import json - - cdp.send( - "Runtime.evaluate", - session=session_id, - expression=f"delete globalThis[{json.dumps(key)}]", - returnByValue=True, - ) - except Exception: - pass + return None class PlaywrightHandle: @@ -701,8 +619,7 @@ def _ensure_connected(self) -> None: # Enter sync_playwright() and connect_over_cdp. Keep the context manager # so close() can __exit__ it (stops the bundled driver process). The # connect + bind logic is the shared free functions (also used by the - # Phase B executor), so the FATAL "no Playwright CDP session over the - # extension facade" constraint lives in exactly one place. + # Phase B executor), so exact CDP identity mapping lives in one place. from ..session import current_session self._pw_cm = sync_playwright() diff --git a/src/browserwright/tab_surface.py b/src/browserwright/tab_surface.py index 5bf4219..11349b0 100644 --- a/src/browserwright/tab_surface.py +++ b/src/browserwright/tab_surface.py @@ -15,9 +15,9 @@ it (same call and across calls), so the agent can keep several pages open and move between them without ``page.goto`` ping-pong. -Both go through the AGENT CDP path (``sess.cdp``) — never Playwright-created -CDP sessions (the fatal-over-the-facade constraint, see -``repl/playwright_handle``). ``switch_tab`` delegates to +Tab lifecycle operations go through the AGENT CDP path (``sess.cdp``). Exact +Playwright Page matching reads its top-frame identity over an independent CDP +attachment (see ``repl/playwright_handle``). ``switch_tab`` delegates to ``session_runtime.bind_target`` — the existing internal switch_tab — which attaches, persists the ledger binding, and fires the target-changed hook; it does NOT steal the user's focus (its best-effort ``Target.activateTarget`` is @@ -114,39 +114,15 @@ def _resolve_target_id(sess: Any, infos: list[dict], url_or_page: Any) -> str: def _match_page_object(sess: Any, infos: list[dict], page: Any) -> str: - """Find which session target a Playwright Page is, via the same short-lived - marker the binding glue uses (``repl/playwright_handle``). No guessing - from URL — duplicate URLs are legal.""" - from .repl.playwright_handle import ( - _clear_target_marker, - _install_target_marker, - _page_has_target_marker, - ) - - for t in infos: - marker_attempted, marker = _install_target_marker(sess, t["targetId"]) - if not marker_attempted or marker is None: - continue - key, value, cdp, session_id = marker - try: - if _page_has_target_marker(page, key, value): - return t["targetId"] - finally: - _clear_target_marker(cdp, session_id, key) - # A Page can legitimately sit on an internal URL (a fresh about:blank - # tab) that the filtered display list hides — probe everything. + """Match a Playwright Page by exact CDP identity; duplicate URLs are legal.""" + from .repl.playwright_handle import _target_id_for_page from .session_runtime import session_tabs - for t in session_tabs(sess, include_internal=True): - marker_attempted, marker = _install_target_marker(sess, t["targetId"]) - if not marker_attempted or marker is None: - continue - key, value, cdp, session_id = marker - try: - if _page_has_target_marker(page, key, value): - return t["targetId"] - finally: - _clear_target_marker(cdp, session_id, key) + target_id = _target_id_for_page(page) + # Internal URLs are hidden from display, but may still be matched exactly. + targets = infos + session_tabs(sess, include_internal=True) + if target_id is not None and any(t["targetId"] == target_id for t in targets): + return target_id raise TabMatchError( "switch_tab: the given Page does not belong to this session's tabs", fix="pass a Page from context.pages() / context.new_page() " diff --git a/tests/daemon/e2e/test_issue59_read_markdown_executor.py b/tests/daemon/e2e/test_issue59_read_markdown_executor.py index ed0356c..1f3378c 100644 --- a/tests/daemon/e2e/test_issue59_read_markdown_executor.py +++ b/tests/daemon/e2e/test_issue59_read_markdown_executor.py @@ -17,13 +17,27 @@ from __future__ import annotations import http.server +import json import threading +from pathlib import Path import pytest from .helpers import run_skill _PAGE = b"""<!doctype html><html><head><title>Issue 59 +

Executor Content View

Body paragraph with RELLINK inside it.

@@ -108,3 +122,29 @@ def test_read_markdown_notes_ride_the_executor_warning_channel( assert "output truncated" in result.stderr # ...and never into the markdown itself. assert "output truncated" not in result.stdout + + +def test_internal_views_use_an_isolated_world( + ext_ready, e2e_daemon, page_url, tmp_path, +): + """Page hooks cannot intercept internal views, and auxiliary CDP detach + leaves the executor's primary Playwright page usable.""" + result = run_skill( + f'page.goto({page_url!r})\n' + 'print(read_markdown(mode="full"))\n' + 'print(snapshot())\n' + 'aux = context.new_cdp_session(page)\n' + 'aux.detach()\n' + 'print(snapshot())\n' + 'import json\n' + 'print("ISOLATION:" + json.dumps(page.evaluate("() => ({reads: window.mainWorldDomReads, globals: Object.keys(window).filter(k => k.startsWith(\\\"__bw\\\") || k.startsWith(\\\"__browserwright\\\"))})")))', + backend="extension", runtime_dir=e2e_daemon.runtime_dir, timeout=120.0, + ) + artifact = Path(tmp_path) / "internal-views-isolation.json" + artifact.write_text(json.dumps({"stdout": result.stdout, + "stderr": result.stderr, + "returncode": result.returncode}, indent=2)) + assert result.returncode == 0, f"{artifact}: {result.stderr}" + evidence = json.loads(result.stdout.split("ISOLATION:", 1)[1].splitlines()[0]) + assert evidence == {"reads": 0, "globals": []}, f"{artifact}: {evidence}" + assert "Executor Content View" in result.stdout diff --git a/tests/daemon/e2e/test_l1_title_marker_no_freeze.py b/tests/daemon/e2e/test_l1_title_marker_no_freeze.py index edeb05d..6dd7575 100644 --- a/tests/daemon/e2e/test_l1_title_marker_no_freeze.py +++ b/tests/daemon/e2e/test_l1_title_marker_no_freeze.py @@ -1,219 +1,18 @@ -"""L1 -- the 👀 tab marker must reach a fixpoint, not spin the renderer. +"""Extension attachment must leave page title/accessors intact and responsive. -The reported symptom was a *random* freeze: an agent command goes out and -nothing ever comes back (`relay send failed: TimeoutError()`). The cause was -not in the daemon at all -- it was in the extension's cosmetic tab marker. - -`MARKER_INSTALL_SCRIPT` puts a `MutationObserver` on `document.head` whose -callback re-asserts the `"👀 "` prefix on `document.title`. The prefix carries a -trailing space, and the HTML spec's `document.title` getter *strips* trailing -ASCII whitespace -- so on a page whose title is empty the script writes `"👀 "`, -reads back `"👀"`, decides the title is still wrong, and writes again. Each -write mutates ``, which re-fires the observer as a microtask, which -writes again: an unbounded microtask loop that pins the renderer's main thread. -A pinned renderer never answers `chrome.debugger.sendCommand`, so the extension -never sends its `{type:"response"}`, so the relay's `asyncio.wait_for(fut, 10)` -expires. Only *empty-titled* pages trip it -- `about:blank`, `data:` URLs, pages -caught before their title is set -- which is exactly what made it look random. - -Two tests, deliberately at different levels: - -1. `test_marker_script_reaches_a_fixpoint` runs the **real** script text pulled - out of `chrome-extension/background.js` in a real renderer, with no daemon - and no extension in the way. It is the sharp one: it fails in seconds, it - names the fixpoint invariant directly, and it cannot pass by accident. -2. `test_empty_title_tab_answers_a_renderer_command` drives the whole stack the - way the user did -- session tab on `about:blank`, then a renderer-scoped CDP - command -- and gives it a wall-clock budget, in the spirit of - `tests/daemon/test_hang_budget.py`: *a result or an error, either is fine, - but within N seconds.* +The former title marker rewrote the DOM and could spin an empty-title renderer. +The browser UI indicator now belongs to the extension, so test the actual +extension-backed path rather than executing the retired marker snippets. """ from __future__ import annotations -import asyncio import json import time -from pathlib import Path - -import pytest from .helpers import run_skill -#: The marker converges in one or two writes. Anything past this is the loop. -FIXPOINT_BUDGET_S = 10.0 - -#: A renderer-scoped eval on a freshly-attached blank tab. The relay's own -#: per-command deadline is 10s; the budget covers tab creation + attach + eval -#: with room to spare, while still being far below "hung". RENDERER_BUDGET_S = 45.0 -EXT_SOURCE_DIR = Path(__file__).resolve().parents[3] / "chrome-extension" - -#: The marker section of background.js is pure JS -- constants and template -#: literals, no `chrome.*` -- so it can be evaluated verbatim in a page. Slicing -#: it by these anchors keeps the test pinned to the shipped source instead of a -#: copy that can silently drift. If either anchor moves, this test fails loudly -#: rather than testing a stale duplicate. -_REGION_START = "const TITLE_PREFIX" -_REGION_END = "const markedTabs" - - -def _marker_region() -> str: - src = (EXT_SOURCE_DIR / "background.js").read_text(encoding="utf-8") - try: - start = src.index(_REGION_START) - end = src.index(_REGION_END) - except ValueError as e: # pragma: no cover - structural drift - raise AssertionError( - f"could not slice the marker section out of background.js " - f"({_REGION_START!r} .. {_REGION_END!r}); the anchors moved" - ) from e - assert start < end - region = src[start:end] - assert "MARKER_INSTALL_SCRIPT" in region - # Comments in this region legitimately *discuss* chrome.debugger; only real - # code would stop it evaluating standalone. - code = "\n".join(ln for ln in region.splitlines() - if not ln.strip().startswith("//")) - assert "chrome." not in code, ( - "the marker region gained a chrome.* reference; it can no longer be " - "evaluated standalone in a page" - ) - return region - - -async def _fixpoint_probe(browser_binary: Path) -> dict: - """Install the real marker script on pages with hostile titles. - - Returns a report per scenario. Every step is wrapped in `asyncio.wait_for` - because the failure being pinned is a *wedged renderer*: without a deadline - `page.evaluate` simply never returns and the test hangs instead of failing. - - Driven with Chrome for Testing (the binary this suite already requires) - rather than Playwright's own bundled build, so the test needs no extra - download and exercises the same engine the rest of the e2e suite does. No - extension and no daemon are involved — only the script text. - """ - from playwright.async_api import async_playwright - - report: dict[str, dict] = {} - async with async_playwright() as p: - browser = await p.chromium.launch( - executable_path=str(browser_binary), headless=True) - try: - page = await browser.new_page() - await page.goto("about:blank") - scripts = await page.evaluate( - "(region) => new Function(region + '\\n" - "return {install: MARKER_INSTALL_SCRIPT," - " remove: MARKER_REMOVE_SCRIPT};')()", - _marker_region(), - ) - - scenarios = [ - # (label, page title before the marker is installed, expected - # title the page itself should still observe afterwards) - ("empty", "", ""), - ("normal", "Foo", "Foo"), - ] - for label, initial, expected_visible in scenarios: - page = await browser.new_page() - await page.goto("about:blank") - if initial: - await asyncio.wait_for( - page.evaluate("t => { document.title = t; }", initial), - FIXPOINT_BUDGET_S, - ) - # THE step that hangs today when `initial` is empty. - await asyncio.wait_for( - page.evaluate(scripts["install"]), FIXPOINT_BUDGET_S) - # The renderer must still be answering at all. - alive = await asyncio.wait_for( - page.evaluate("1 + 1"), FIXPOINT_BUDGET_S) - # Let any pending observer callbacks drain, then confirm the - # title has actually settled rather than merely being slow. - await asyncio.sleep(0.25) - first = await asyncio.wait_for( - page.evaluate( - "document.querySelector('title')" - " ? document.querySelector('title').textContent : null"), - FIXPOINT_BUDGET_S) - await asyncio.sleep(0.25) - second = await asyncio.wait_for( - page.evaluate( - "document.querySelector('title')" - " ? document.querySelector('title').textContent : null"), - FIXPOINT_BUDGET_S) - visible = await asyncio.wait_for( - page.evaluate("document.title"), FIXPOINT_BUDGET_S) - # The page setting its own empty title must not restart the loop. - await asyncio.wait_for( - page.evaluate("document.title = ''"), FIXPOINT_BUDGET_S) - await asyncio.sleep(0.25) - after_self_clear = await asyncio.wait_for( - page.evaluate("1 + 1"), FIXPOINT_BUDGET_S) - report[label] = { - "alive": alive, - "raw_first": first, - "raw_second": second, - "visible": visible, - "expected_visible": expected_visible, - "after_self_clear": after_self_clear, - } - await page.close() - finally: - await browser.close() - return report - - -def test_marker_script_reaches_a_fixpoint(cft_binary): - """The marker must be a fixpoint under the DOM's own title normalization. - - Write the marked title, read it back through the native getter, and the two - must agree -- otherwise the observer rewrites forever. Asserted on the real - script text from `chrome-extension/background.js`, in a real renderer. - """ - try: - import playwright.async_api # noqa: F401 - except ImportError: # pragma: no cover - pytest.skip("playwright not installed") - - try: - report = asyncio.run(_fixpoint_probe(cft_binary)) - except asyncio.TimeoutError as e: # pragma: no cover - this IS the bug - raise AssertionError( - "the renderer stopped answering after the tab marker was " - "installed — this is the title-rewrite loop, i.e. the freeze" - ) from e - - for label, r in report.items(): - assert r["alive"] == 2, f"[{label}] renderer not responsive: {r}" - assert r["after_self_clear"] == 2, ( - f"[{label}] renderer wedged after the page set an empty title " - f"through the marker's own setter: {r}" - ) - assert r["raw_first"] == r["raw_second"], ( - f"[{label}] the raw is still changing 250ms apart " - f"({r['raw_first']!r} -> {r['raw_second']!r}) — the marker never " - f"reached a fixpoint" - ) - assert r["raw_first"] is not None and "\U0001F440" in r["raw_first"], ( - f"[{label}] the marker is gone; the tab strip no longer shows the " - f"agent is driving this tab: {r}" - ) - assert r["raw_first"] == r["raw_first"].strip(), ( - f"[{label}] the marked title {r['raw_first']!r} carries padding " - f"whitespace the DOM getter will strip back off — that asymmetry " - f"is precisely what loops" - ) - assert r["visible"] == r["expected_visible"], ( - f"[{label}] the page's own document.title should read as if " - f"unmarked: {r}" - ) - - # The marker still reads the way a user expects on a page with a title. - assert report["normal"]["raw_first"] == "\U0001F440 Foo", report["normal"] - def _last_json(stdout: str) -> dict: line = next( @@ -222,13 +21,10 @@ def _last_json(stdout: str) -> dict: return json.loads(line) -def test_empty_title_tab_answers_a_renderer_command(ext_ready, e2e_daemon): - """End-to-end shape of the user's report: attach a blank tab, then ask the - renderer something. It must answer, and within a budget. - - `about:blank` is the trigger because its title is empty. Before the fix this - fails as `relay send failed: TimeoutError()` roughly 10s in -- the renderer - is spinning, so `chrome.debugger.sendCommand` never resolves. +def test_empty_title_tab_answers_a_renderer_command(ext_ready, e2e_daemon, tmp_path): + """A real extension attach preserves native title behavior, including empty + titles and titles that legitimately start with the former marker glyph. + Save the observed page state so the renderer proof can be inspected. """ script = ( "import json, time\n" @@ -241,21 +37,26 @@ def test_empty_title_tab_answers_a_renderer_command(ext_ready, e2e_daemon): "wait_for_ready(sess)\n" "t0 = time.monotonic()\n" "value = eval_js(sess, '1 + 1')\n" - "print(json.dumps({'value': value, 'eval_s': time.monotonic() - t0}))\n" + "probe = \"(() => { const samples = ['', 'Example', '👀 Example'].map(title => { document.title = title; return {expected: title, title: document.title, raw: document.querySelector('title').textContent}; }); const descriptor = Object.getOwnPropertyDescriptor(Document.prototype, 'title'); return {samples, ownTitle: Object.hasOwn(document, 'title'), nativeGetter: descriptor.get.toString().includes('[native code]'), nativeSetter: descriptor.set.toString().includes('[native code]'), markerGlobal: Object.hasOwn(window, '__bdTitleMarker')}; })()\"\n" + "integrity = eval_js(sess, probe)\n" + "print(json.dumps({'value': value, 'eval_s': time.monotonic() - t0, 'integrity': integrity}))\n" ) started = time.monotonic() result = run_skill(script=script, backend="extension", timeout=90, runtime_dir=e2e_daemon.runtime_dir) elapsed = time.monotonic() - started - assert result.returncode == 0, ( f"skill exited {result.returncode} after {elapsed:.1f}s; " f"stderr={result.stderr!r}" ) payload = _last_json(result.stdout) + (tmp_path / "extension-page-integrity.json").write_text( + json.dumps(payload, indent=2, ensure_ascii=False), encoding="utf-8") assert payload["value"] == 2, payload - assert elapsed < RENDERER_BUDGET_S, ( - f"blank-tab attach + one renderer eval took {elapsed:.1f}s " - f"(budget {RENDERER_BUDGET_S}s) — this is the shape of a hang, not a " - f"slow test" - ) + assert elapsed < RENDERER_BUDGET_S, payload + integrity = payload["integrity"] + assert not integrity["ownTitle"], integrity + assert integrity["nativeGetter"] and integrity["nativeSetter"], integrity + assert not integrity["markerGlobal"], integrity + for sample in integrity["samples"]: + assert sample["title"] == sample["raw"] == sample["expected"], sample diff --git a/tests/daemon/e2e/test_l2_extension_tabs_live_url.py b/tests/daemon/e2e/test_l2_extension_tabs_live_url.py index 8e2258e..fab9409 100644 --- a/tests/daemon/e2e/test_l2_extension_tabs_live_url.py +++ b/tests/daemon/e2e/test_l2_extension_tabs_live_url.py @@ -117,8 +117,8 @@ def test_tabs_reports_live_urls_for_session_tabs( assert any(r["current"] for r in rows), ( "tabs() must still mark the session's current tab") - # The live title comes from the same group query as the url. The - # extension strips its 👀 marker, so the server's <title> is intact. + # The live title comes from the same group query as the url; the + # extension never rewrites document.title, so the server's <title> is intact. titles = sorted(r["title"] for r in rows) assert titles == sorted( f"bw116 {name}" for name in ("one", "two", "three", "four") diff --git a/tests/daemon/e2e/test_l2_issue21_page_binding.py b/tests/daemon/e2e/test_l2_issue21_page_binding.py index 2d98d82..1a398c7 100644 --- a/tests/daemon/e2e/test_l2_issue21_page_binding.py +++ b/tests/daemon/e2e/test_l2_issue21_page_binding.py @@ -128,9 +128,8 @@ def test_page_binding_follows_switch_tab_across_close_of_old_tab( # ...and `page` must have followed the switch: it is C's page, NOT the # stale about:blank the old binding left behind. - # `page.title` may or may not carry the extension's "👀 " attach - # prefix depending on injection timing (pre-existing race, unrelated - # to the switch binding under test) — containment tolerates both. + # Containment keeps this independent of title formatting, which is + # unrelated to the switch binding under test. page_title = _grep(r.stdout, "PAGE_TITLE") assert "C-target" in page_title, ( f"page did not follow switch_tab: page.title={page_title!r} " diff --git a/tests/daemon/e2e/test_l2_recovery.py b/tests/daemon/e2e/test_l2_recovery.py index 9fc71c8..18d199c 100644 --- a/tests/daemon/e2e/test_l2_recovery.py +++ b/tests/daemon/e2e/test_l2_recovery.py @@ -97,8 +97,7 @@ def test_recovery_fast_path_across_processes(ext_ready, e2e_daemon): runtime_dir=rd, extra_env={"BD_SESSION": sid}) assert b.returncode == 0, ( f"transparent recovery failed; stdout={b.stdout!r} stderr={b.stderr!r}") - # The extension prepends a "👀 " attach-marker to the live DOM title; - # containment (not ==) correctly tolerates it. + # Containment (not ==) keeps this independent of title formatting. assert "RecoverFast" in _payload(b)["title"] finally: drop_ledger_session(_HOME, sid) diff --git a/tests/daemon/e2e/test_repro_extension_reload_orphan.py b/tests/daemon/e2e/test_repro_extension_reload_orphan.py index 121ac9e..98db4c3 100644 --- a/tests/daemon/e2e/test_repro_extension_reload_orphan.py +++ b/tests/daemon/e2e/test_repro_extension_reload_orphan.py @@ -64,7 +64,6 @@ "(async () => {" " const tabs = [...attachedTabs];" " attachedTabs.clear();" - " markedTabs.clear();" " await Promise.allSettled(tabs.map(t => chrome.debugger.detach({tabId: t})));" " forceReconnect('simulating fresh SW after reload');" " return {detached: tabs};" diff --git a/tests/daemon/test_extension_debugger_timeout_unit.py b/tests/daemon/test_extension_debugger_timeout_unit.py index 8b6d84f..d9b7731 100644 --- a/tests/daemon/test_extension_debugger_timeout_unit.py +++ b/tests/daemon/test_extension_debugger_timeout_unit.py @@ -216,7 +216,7 @@ def _run_attach_timeout_probe(payload: dict[str, str], *, const PROTOCOL_VERSION = "1.3"; const attachedTabs = new Set(); function stripMarker(t) { return t || ""; } - async function unmarkTabBeforeDetach() {} + async function updateTabAttachmentIndicator() {} async function postAttachCosmetics() {} async function announceAttached() {} ${wrapper} @@ -297,7 +297,7 @@ def _run_detach_timeout_probe(payload: dict[str, str]) -> dict[str, object]: vm.runInContext(` const PROTOCOL_VERSION = "1.3"; const attachedTabs = new Set([17]); - async function unmarkTabBeforeDetach() {} + async function updateTabAttachmentIndicator() {} ${wrapper} ${input.detachTab} ${input.doDetach} @@ -433,7 +433,7 @@ def _run_epoch_abandon_probe(payload: dict[str, str]) -> dict[str, object]: vm.runInContext(` const PROTOCOL_VERSION = "1.3"; const attachedTabs = new Set(); - async function unmarkTabBeforeDetach() {} + async function updateTabAttachmentIndicator() {} async function announceAttached(tabId) { announced.push(tabId); } ${wrapper} ${input.attachTab} diff --git a/tests/daemon/test_extension_title_marker_unit.py b/tests/daemon/test_extension_title_marker_unit.py deleted file mode 100644 index 47c9ada..0000000 --- a/tests/daemon/test_extension_title_marker_unit.py +++ /dev/null @@ -1,1060 +0,0 @@ -"""Unit checks for the extension title marker snippets. - -The tests extract only the marker helpers/scripts from -``chrome-extension/background.js`` and run them in Node with a tiny DOM stub. -They do not load the extension runtime or launch Chrome. -""" -from __future__ import annotations - -import json -import re -import subprocess -from pathlib import Path - - -ROOT = Path(__file__).resolve().parents[2] -BACKGROUND_JS = ROOT / "chrome-extension" / "background.js" -PREFIX = "\U0001f440 " - - -def _template_const(source: str, name: str) -> str: - # The marker scripts are assembled from shared source fragments - # (MARKER_STRIP_PREFIX_SRC / MARKER_RAW_TITLE_SRC in background.js), so - # evaluate the whole const region in Node instead of regex-slicing a - # single template literal. - start = source.index("const MARKER_STRIP_PREFIX_SRC") - end = source.index("const markedTabs") - snippet = source[start:end] - proc = subprocess.run( - ["node"], - input=f"{snippet}\nprocess.stdout.write(JSON.stringify({name}));", - text=True, - capture_output=True, - check=True, - timeout=5, - cwd=ROOT, - ) - return json.loads(proc.stdout) - - -def _function_decl(source: str, name: str) -> str: - match = re.search( - rf"function {name}\([^)]*\) \{{(?P<body>.*?)\n\}}", - source, - re.DOTALL, - ) - assert match is not None, f"{name} function not found" - return match.group(0) - - -def _marker_sources() -> dict[str, str]: - source = BACKGROUND_JS.read_text(encoding="utf-8") - return { - "installScript": _template_const(source, "MARKER_INSTALL_SCRIPT"), - "removeScript": _template_const(source, "MARKER_REMOVE_SCRIPT"), - "stripMarker": _function_decl(source, "stripMarker"), - } - - -def _service_worker_marker_sources() -> dict[str, str]: - """Extract the real SW marker lifecycle without loading the extension.""" - source = BACKGROUND_JS.read_text(encoding="utf-8") - marker_start = source.index("const markedTabs") - marker_end = source.index("// ---- chrome.debugger event fan-out", marker_start) - detach_start = source.index("async function detachTab(") - detach_end = source.index("\n\nasync function doAttach(", detach_start) - handler_start = source.index("async function handleDaemonMessage(") - handler_end = source.index("// Shared attach sequence", handler_start) - close_start = source.index("async function doCloseTab(") - close_end = source.index("\n\nasync function doDetach(", close_start) - # The bounded chrome.debugger wrapper region (issue #31): detachTab and - # doCloseTab route their chrome.debugger calls through it, so the probes - # that run those handlers need it injected too. - wrapper_start = source.index("// ---- bounded chrome.debugger calls") - wrapper_end = source.index("// Shared attach sequence:", wrapper_start) - return { - **_marker_sources(), - "wrapperRegion": source[wrapper_start:wrapper_end], - "workerMarkerRegion": source[marker_start:marker_end], - "detachTab": source[detach_start:detach_end], - "handleDaemonMessage": source[handler_start:handler_end], - "doCloseTab": source[close_start:close_end], - } - - -def _run_marker_detach_budget_probe( - payload: dict[str, str], hang_stage: str, -) -> dict[str, object]: - probe = r""" -const vm = require("node:vm"); -const input = JSON.parse(require("node:fs").readFileSync(0, "utf8")); -const calls = []; -const chrome = { - debugger: { - async sendCommand(target, method, params) { - calls.push({ kind: "command", method, params }); - const stage = method === "Runtime.evaluate" - ? (params.expression === input.installScript ? "installEvaluate" : "removeEvaluate") - : method; - if (stage === input.hangStage) return await new Promise(() => {}); - if (method === "Page.addScriptToEvaluateOnNewDocument") { - return { identifier: "marker-script" }; - } - return {}; - }, - async detach(target) { - calls.push({ kind: "detach", tabId: target.tabId }); - }, - }, -}; -const realm = vm.createContext({ - chrome, - sleep: (ms) => new Promise((resolve) => setTimeout(resolve, ms)), - setTimeout, - clearTimeout, - console: { info() {}, warn() {}, error: console.error }, -}); -const markerRegion = input.workerMarkerRegion - .replace("const MARKER_INSTALL_TIMEOUT_MS = 1500;", - "const MARKER_INSTALL_TIMEOUT_MS = 10;") - .replace("const MARKER_REMOVE_TIMEOUT_MS = 1000;", - "const MARKER_REMOVE_TIMEOUT_MS = 10;"); -vm.runInContext(` - const TITLE_PREFIX = "\\u{1F440} "; - const MARKER_INSTALL_SCRIPT = ${JSON.stringify(input.installScript)}; - const MARKER_REMOVE_SCRIPT = ${JSON.stringify(input.removeScript)}; - const PROTOCOL_VERSION = "1.3"; - const attachedTabs = new Set([17]); - function safeSend() {} - ${input.wrapperRegion} - ${markerRegion} - ${input.detachTab} -`, realm); - -(async () => { - if (["Page.enable", "Page.addScriptToEvaluateOnNewDocument", "installEvaluate"] - .includes(input.hangStage)) { - vm.runInContext("markTabAttached(17)", realm); - while (calls.length === 0 || - (input.hangStage === "Page.addScriptToEvaluateOnNewDocument" && calls.length < 2) || - (input.hangStage === "installEvaluate" && calls.length < 3)) { - await new Promise((resolve) => setTimeout(resolve, 0)); - } - } else { - vm.runInContext('markedTabs.set(17, "marker-script")', realm); - } - const completed = await Promise.race([ - vm.runInContext("detachTab(17)", realm).then(() => true), - new Promise((resolve) => setTimeout(() => resolve(false), 100)), - ]); - process.stdout.write(JSON.stringify({ - completed, - attached: vm.runInContext("attachedTabs.has(17)", realm), - calls, - })); -})().catch((error) => { - console.error(error); - process.exitCode = 1; -}); -""" - proc = subprocess.run( - ["node", "-e", probe], - input=json.dumps({**payload, "hangStage": hang_stage}), - text=True, - capture_output=True, - check=True, - timeout=5, - cwd=ROOT, - ) - return json.loads(proc.stdout) - - -def _run_marker_late_completion_probe( - payload: dict[str, str], late_stage: str, -) -> dict[str, object]: - probe = r""" -const vm = require("node:vm"); -const input = JSON.parse(require("node:fs").readFileSync(0, "utf8")); -const calls = []; -let releaseLate; -const lateGate = new Promise((resolve) => { releaseLate = resolve; }); -let commandQueue = Promise.resolve(); -let pageMarked = false; - -function stageFor(method, params) { - if (method !== "Runtime.evaluate") return method; - return params.expression === input.installScript - ? "installEvaluate" : "removeEvaluate"; -} - -const chrome = { - debugger: { - sendCommand(target, method, params) { - const stage = stageFor(method, params); - calls.push({ kind: "command", stage, method }); - const run = async () => { - if (stage === input.lateStage) await lateGate; - if (stage === "installEvaluate") pageMarked = true; - if (stage === "removeEvaluate") pageMarked = false; - if (method === "Page.addScriptToEvaluateOnNewDocument") { - return { identifier: "marker-script" }; - } - return {}; - }; - const result = commandQueue.then(run); - commandQueue = result.catch(() => {}); - return result; - }, - async detach(target) { - calls.push({ kind: "detach", tabId: target.tabId }); - }, - }, -}; -const realm = vm.createContext({ - chrome, - sleep: (ms) => new Promise((resolve) => setTimeout(resolve, ms)), - setTimeout, - clearTimeout, - console: { info() {}, warn() {}, error: console.error }, -}); -const markerRegion = input.workerMarkerRegion - .replace("const MARKER_INSTALL_TIMEOUT_MS = 1500;", - "const MARKER_INSTALL_TIMEOUT_MS = 100;") - .replace("const MARKER_REMOVE_TIMEOUT_MS = 1000;", - "const MARKER_REMOVE_TIMEOUT_MS = 10;"); -vm.runInContext(` - const TITLE_PREFIX = "\\u{1F440} "; - const MARKER_INSTALL_SCRIPT = ${JSON.stringify(input.installScript)}; - const MARKER_REMOVE_SCRIPT = ${JSON.stringify(input.removeScript)}; - const PROTOCOL_VERSION = "1.3"; - const attachedTabs = new Set([17]); - function safeSend() {} - ${input.wrapperRegion} - ${markerRegion} - ${input.detachTab} -`, realm); - -(async () => { - const marking = vm.runInContext("markTabAttached(17)", realm); - while (!calls.some((call) => call.stage === input.lateStage)) { - await new Promise((resolve) => setTimeout(resolve, 0)); - } - const detached = await vm.runInContext("detachTab(17)", realm); - const detachIndex = calls.findIndex((call) => call.kind === "detach"); - releaseLate(); - await Promise.allSettled([marking, commandQueue]); - await new Promise((resolve) => setTimeout(resolve, 0)); - await commandQueue; - process.stdout.write(JSON.stringify({ - calls, - detachIndex, - pageMarked, - marked: vm.runInContext("markedTabs.has(17)", realm), - marking: vm.runInContext("markingTabs.has(17)", realm), - })); -})().catch((error) => { - console.error(error); - process.exitCode = 1; -}); -""" - proc = subprocess.run( - ["node", "-e", probe], - input=json.dumps({**payload, "lateStage": late_stage}), - text=True, - capture_output=True, - check=True, - timeout=5, - cwd=ROOT, - ) - return json.loads(proc.stdout) - - -def _run_failed_close_marker_probe(payload: dict[str, str]) -> dict[str, object]: - probe = r""" -const vm = require("node:vm"); -const input = JSON.parse(require("node:fs").readFileSync(0, "utf8")); -const calls = []; -const attachedTabs = new Set([17]); -const markedTabs = new Map([[17, "marker-script"]]); -const chrome = { - debugger: { - async detach() { calls.push({ kind: "detach" }); }, - }, - tabs: { - async remove() { calls.push({ kind: "remove" }); throw new Error("close denied"); }, - }, - storage: { - session: { - async get() { return {}; }, - async set(value) { calls.push({ kind: "storageSet", value }); }, - }, - }, -}; -function errMessage(error) { return String(error.message || error); } -function safeSend(message) { calls.push({ kind: "response", message }); } -const realm = vm.createContext({ - chrome, console, errMessage, safeSend, - attachedTabs, markedTabs, -}); -vm.runInContext(input.wrapperRegion, realm); -vm.runInContext(input.doCloseTab, realm); -(async () => { - await vm.runInContext("doCloseTab(9, 17)", realm); - process.stdout.write(JSON.stringify({ - attached: attachedTabs.has(17), - marked: markedTabs.has(17), - calls, - })); -})().catch((error) => { - console.error(error); - process.exitCode = 1; -}); -""" - proc = subprocess.run( - ["node", "-e", probe], - input=json.dumps(payload), - text=True, - capture_output=True, - check=True, - timeout=5, - cwd=ROOT, - ) - return json.loads(proc.stdout) - - -def _run_service_worker_realm_rebuild_probe(payload: dict[str, str]) -> dict: - """Mark in one SW realm, then detach through a fresh realm. - - The page sandbox deliberately survives the service-worker sandbox. This is - the upgrade/reload shape: extension globals are rebuilt while the old tab's - injected accessor, observer, and visible title marker remain live. - """ - probe = r""" -const vm = require("node:vm"); -const input = JSON.parse(require("node:fs").readFileSync(0, "utf8")); -const EYE = "\u{1F440}"; - -class Document {} -Object.defineProperty(Document.prototype, "title", { - configurable: true, - enumerable: true, - get() { return this._rawTitle || ""; }, - set(value) { this._rawTitle = String(value); }, -}); -class MutationObserver { - constructor(callback) { - this.callback = callback; - this.disconnected = false; - } - observe() { this.callback(); } - disconnect() { this.disconnected = true; } -} - -const document = new Document(); -document._rawTitle = `${EYE} Legacy`; -document.head = { appendChild() {} }; -document.querySelector = () => null; -document.createElement = () => ({ textContent: "" }); -document.addEventListener = () => {}; -const page = vm.createContext({ - window: {}, document, Document, HTMLDocument: Document, MutationObserver, -}); - -const calls = []; -const chrome = { - debugger: { - async sendCommand(target, method, params) { - calls.push({ kind: "command", method, params }); - if (method === "Page.addScriptToEvaluateOnNewDocument") { - return { identifier: "old-realm-script" }; - } - if (method === "Runtime.evaluate") { - vm.runInContext(params.expression, page); - } - return {}; - }, - async detach(target) { - calls.push({ kind: "detach", tabId: target.tabId }); - }, - }, -}; - -function newWorkerRealm(attached) { - const realm = vm.createContext({ - chrome, console, setTimeout, clearTimeout, sleep: async () => {}, - }); - vm.runInContext(` - const TITLE_PREFIX = "\\u{1F440} "; - const MARKER_INSTALL_SCRIPT = ${JSON.stringify(input.installScript)}; - const MARKER_REMOVE_SCRIPT = ${JSON.stringify(input.removeScript)}; - const PROTOCOL_VERSION = "1.3"; - ${input.wrapperRegion} - ${input.workerMarkerRegion} - const attachedTabs = new Set(${attached ? "[17]" : "[]"}); - function safeSend() {} - ${input.detachTab} - `, realm); - return realm; -} - -(async () => { - const firstRealm = newWorkerRealm(true); - await vm.runInContext("markTabAttached(17)", firstRealm); - const titleAfterMark = document._rawTitle; - - // A service-worker update/reload rebuilds all extension globals. The page - // and its injected marker survive, but markedTabs is now an empty Map. - const secondRealm = newWorkerRealm(false); - await vm.runInContext("detachTab(17)", secondRealm); - - process.stdout.write(JSON.stringify({ - titleAfterMark, - titleAfterDetach: document._rawTitle, - markerAfterDetach: !!page.window.__bdTitleMarker, - callsAfterRealmRebuild: calls.slice(3), - })); -})().catch((error) => { - console.error(error); - process.exitCode = 1; -}); -""" - proc = subprocess.run( - ["node", "-e", probe], - input=json.dumps(payload), - text=True, - capture_output=True, - check=True, - timeout=5, - cwd=ROOT, - ) - return json.loads(proc.stdout) - - -def _run_controlled_reload_probe(payload: dict[str, object]) -> list[dict]: - probe = r""" -const vm = require("node:vm"); -const input = JSON.parse(require("node:fs").readFileSync(0, "utf8")); -const calls = []; -const chrome = { - debugger: { - async sendCommand(target, method, params) { - calls.push({ kind: "command", tabId: target.tabId, method, params }); - if (method === "Page.addScriptToEvaluateOnNewDocument") { - if (input.markInFlight) { - await new Promise((resolve) => setTimeout(resolve, 0)); - } - return { identifier: "known-script" }; - } - if (input.failRemove && - method === "Page.removeScriptToEvaluateOnNewDocument") { - throw new Error("registration already gone"); - } - if (input.hangRemove && method === "Runtime.evaluate" && - params.expression === input.removeScript) { - return await new Promise(() => {}); - } - return {}; - }, - }, - runtime: { - reload() { calls.push({ kind: "reload" }); }, - }, -}; -const realm = vm.createContext({ - chrome, - sleep: (ms) => new Promise((resolve) => setTimeout(resolve, ms)), - setTimeout, - clearTimeout, - console: { info() {}, warn() {}, error: console.error }, -}); -vm.runInContext(` - const daemonVersion = "test"; - const TITLE_PREFIX = "\\u{1F440} "; - const MARKER_INSTALL_SCRIPT = ${JSON.stringify(input.installScript)}; - const MARKER_REMOVE_SCRIPT = ${JSON.stringify(input.removeScript)}; - const attachedTabs = new Set([17]); - ${input.workerMarkerRegion.replace( - "const MARKER_RELOAD_CLEANUP_TIMEOUT_MS = 1500;", - "const MARKER_RELOAD_CLEANUP_TIMEOUT_MS = 10;", - )} - ${input.handleDaemonMessage} -`, realm); -(async () => { - let marking = null; - if (input.markInFlight) { - marking = vm.runInContext("markTabAttached(17)", realm); - } else { - vm.runInContext('markedTabs.set(17, "known-script")', realm); - } - await vm.runInContext( - 'handleDaemonMessage({type: "reloadExtension", reason: "test"})', - realm, - ); - if (marking) await marking; - process.stdout.write(JSON.stringify(calls)); -})().catch((error) => { - console.error(error); - process.exitCode = 1; -}); -""" - proc = subprocess.run( - ["node", "-e", probe], - input=json.dumps(payload), - text=True, - capture_output=True, - check=True, - timeout=5, - cwd=ROOT, - ) - return json.loads(proc.stdout) - - -def _run_node_probe(payload: dict[str, str]) -> dict[str, object]: - probe = r""" -const vm = require("node:vm"); -const input = JSON.parse(require("node:fs").readFileSync(0, "utf8")); -const PREFIX = "\u{1F440} "; -const EYE = "\u{1F440}"; -const NBSP = "\u00A0"; -const NARROW_NBSP = "\u202F"; - -function runMarkerScript(script, initialTitle) { - class Document {} - Object.defineProperty(Document.prototype, "title", { - configurable: true, - enumerable: true, - get() { - return this._rawTitle || ""; - }, - set(value) { - this._rawTitle = String(value); - }, - }); - - class MutationObserver { - constructor(callback) { - this.callback = callback; - this.disconnected = false; - } - observe() { - this.callback(); - } - disconnect() { - this.disconnected = true; - } - } - - const document = new Document(); - document._rawTitle = initialTitle; - document.head = { appendChild() {} }; - document.querySelector = () => null; - document.createElement = () => ({ textContent: "" }); - document.addEventListener = () => {}; - - const sandbox = { - window: {}, - document, - Document, - HTMLDocument: Document, - MutationObserver, - }; - vm.runInNewContext(script, sandbox); - return { sandbox, document }; -} - -vm.runInThisContext( - `const TITLE_PREFIX = ${JSON.stringify(PREFIX)};\n${input.stripMarker}`, -); - -const duplicateInstall = runMarkerScript( - input.installScript, - `${PREFIX}${PREFIX}Example`, -); -const cleanInstall = runMarkerScript(input.installScript, "Example"); -const bareEyeInstall = runMarkerScript(input.installScript, `${EYE}Hello`); -const nbspEyeInstall = runMarkerScript(input.installScript, `${EYE}${NBSP}Hello`); -const narrowEyeInstall = runMarkerScript( - input.installScript, - `${EYE}${NARROW_NBSP}Hello`, -); -const setterProbe = runMarkerScript(input.installScript, "Example"); -setterProbe.document.title = `${PREFIX}${PREFIX}Next`; -const bareEyeSetter = runMarkerScript(input.installScript, "Example"); -bareEyeSetter.document.title = `${EYE}World`; -const nonStringSetter = runMarkerScript(input.installScript, "Example"); -nonStringSetter.document.title = 42; -const duplicateRemove = runMarkerScript( - input.removeScript, - `${PREFIX}${PREFIX}${PREFIX}Example`, -); -const bareEyeRemove = runMarkerScript(input.removeScript, `${EYE}Hello`); -const nbspEyeRemove = runMarkerScript(input.removeScript, `${EYE}${NBSP}Hello`); -const lifecycle = runMarkerScript(input.installScript, `${PREFIX}${PREFIX}Example`); -lifecycle.document.title = `${EYE}${NBSP}Mid`; -vm.runInNewContext(input.removeScript, lifecycle.sandbox); -lifecycle.document.title = "After"; - -process.stdout.write(JSON.stringify({ - stripSamples: [ - stripMarker(`${PREFIX}${PREFIX}Example`), - stripMarker(`${PREFIX}${PREFIX}${PREFIX}`), - stripMarker("Plain"), - stripMarker(null), - stripMarker(`${EYE}Hello`), - stripMarker(`${EYE} Hello`), - stripMarker(`${EYE}${NBSP}Hello`), - stripMarker(`${EYE}${NARROW_NBSP}Hello`), - stripMarker(`${EYE}${EYE}Hello`), - stripMarker(`${PREFIX}${EYE}Hello`), - stripMarker(42), - stripMarker(undefined), - stripMarker(true), - ], - duplicateInstallTitle: { - rawTitle: duplicateInstall.document._rawTitle, - pageTitle: duplicateInstall.document.title, - }, - cleanInstallTitle: { - rawTitle: cleanInstall.document._rawTitle, - pageTitle: cleanInstall.document.title, - }, - bareEyeInstallTitle: { - rawTitle: bareEyeInstall.document._rawTitle, - pageTitle: bareEyeInstall.document.title, - }, - nbspEyeInstallTitle: { - rawTitle: nbspEyeInstall.document._rawTitle, - pageTitle: nbspEyeInstall.document.title, - }, - narrowEyeInstallTitle: { - rawTitle: narrowEyeInstall.document._rawTitle, - pageTitle: narrowEyeInstall.document.title, - }, - setterTitle: { - rawTitle: setterProbe.document._rawTitle, - pageTitle: setterProbe.document.title, - }, - bareEyeSetterTitle: { - rawTitle: bareEyeSetter.document._rawTitle, - pageTitle: bareEyeSetter.document.title, - }, - nonStringSetterTitle: { - rawTitle: nonStringSetter.document._rawTitle, - pageTitle: nonStringSetter.document.title, - stripped: stripMarker(nonStringSetter.document._rawTitle), - }, - duplicateRemoveTitle: { - rawTitle: duplicateRemove.document._rawTitle, - pageTitle: duplicateRemove.document.title, - }, - bareEyeRemoveTitle: { - rawTitle: bareEyeRemove.document._rawTitle, - pageTitle: bareEyeRemove.document.title, - }, - nbspEyeRemoveTitle: { - rawTitle: nbspEyeRemove.document._rawTitle, - pageTitle: nbspEyeRemove.document.title, - }, - lifecycleTitle: { - rawTitle: lifecycle.document._rawTitle, - pageTitle: lifecycle.document.title, - }, -})); -""" - proc = subprocess.run( - ["node", "-e", probe], - input=json.dumps(payload), - text=True, - capture_output=True, - check=True, - timeout=5, - cwd=ROOT, - ) - return json.loads(proc.stdout) - - -#: How many raw title writes the marker is allowed before we call it a loop. -#: Settling costs one write; a second is slack for a normalization round-trip. -WRITE_BUDGET = 4 - -#: How many observer callbacks may be delivered before we call it a loop. -CALLBACK_BUDGET = 20 - - -def _run_fixpoint_probe(payload: dict[str, str]) -> dict[str, object]: - """Re-run the install script against a *spec-accurate* `document.title`. - - The DOM stub used by the probe above returns whatever was written, which is - not what a browser does: per HTML, the `document.title` getter strips - leading/trailing ASCII whitespace and collapses internal runs. That - difference is the entire bug. The marker's prefix ends in a space, so on a - page with an empty title it wrote `"<eye> "`, read back `"<eye>"`, judged the - title wrong, and wrote again — forever. Each write mutates `<head>`, which - re-fires the MutationObserver, whose callbacks are delivered *asynchronously* - so the script's `normalizing` re-entry flag has already been released by the - time one arrives. The result is an unbounded microtask loop that pins the - renderer's main thread, which is why `chrome.debugger.sendCommand` stopped - answering and the daemon reported `relay send failed: TimeoutError()`. - - So this stub models the two things that matter and the other stub does not: - a normalizing getter, and observer callbacks queued rather than invoked - inline. It then asserts the property the script must have — *the marked - title is a fixpoint*: write it, read it back, and get the same string. - """ - probe = r""" -const vm = require("node:vm"); -const input = JSON.parse(require("node:fs").readFileSync(0, "utf8")); -const WRITE_BUDGET = input.writeBudget; -const CALLBACK_BUDGET = input.callbackBudget; - -// HTML's `document.title` getter: strip and collapse ASCII whitespace. -function normalize(value) { - return String(value) - .replace(/[\t\n\f\r ]+/g, " ") - .replace(/^ /, "") - .replace(/ $/, ""); -} - -function runMarkerScript(script, initialTitle, mutate) { - const observers = []; - const queue = []; - let writes = 0; - let overBudget = false; - - class Document {} - Object.defineProperty(Document.prototype, "title", { - configurable: true, - enumerable: true, - get() { - return normalize(this._rawTitle || ""); - }, - set(value) { - this._rawTitle = String(value); - if (++writes > WRITE_BUDGET) { - overBudget = true; - throw new Error("title write budget exceeded"); - } - // Writing the title mutates <head>. Real MutationObserver callbacks are - // delivered as microtasks, i.e. AFTER the writer's stack (and its - // `normalizing` guard) has unwound — queue, never call inline. - for (const obs of observers) { - if (!obs.disconnected) queue.push(() => obs.callback()); - } - }, - }); - - class MutationObserver { - constructor(callback) { - this.callback = callback; - this.disconnected = false; - } - observe() { - observers.push(this); - } - disconnect() { - this.disconnected = true; - } - } - - const document = new Document(); - document._rawTitle = initialTitle; - document.head = { appendChild() {} }; - document.querySelector = () => null; - document.createElement = () => ({ textContent: "" }); - document.addEventListener = () => {}; - - const sandbox = { window: {}, document, Document, HTMLDocument: Document, - MutationObserver }; - - let converged = true; - try { - vm.runInNewContext(script, sandbox); - if (mutate) mutate(document); - let steps = 0; - while (queue.length) { - if (++steps > CALLBACK_BUDGET) { converged = false; break; } - queue.shift()(); - } - } catch (e) { - converged = false; - } - return { - converged: converged && !overBudget, - writes, - raw: document._rawTitle, - // What the page itself sees through the marker's accessor. - pageTitle: (() => { try { return document.title; } catch (e) { return null; } })(), - // What the DOM would hand back for the value we actually stored — the - // fixpoint check: these two must agree or the observer rewrites forever. - readBack: normalize(document._rawTitle), - }; -} - -const scenarios = { - // about:blank / data: URLs / a page caught before it sets a title. - emptyTitle: runMarkerScript(input.installScript, ""), - // The ordinary case, which always worked. - normalTitle: runMarkerScript(input.installScript, "Example"), - // Whitespace-only is empty once the DOM normalizes it. - whitespaceTitle: runMarkerScript(input.installScript, " "), - // A stale marker with nothing behind it. - bareMarkerTitle: runMarkerScript(input.installScript, "\u{1F440}"), - // The other write path: the page clearing its own title through the - // accessor the marker installed. - pageClearsTitle: runMarkerScript(input.installScript, "Example", (doc) => { - doc.title = ""; - }), - // ...and setting a real one. - pageSetsTitle: runMarkerScript(input.installScript, "", (doc) => { - doc.title = "Later"; - }), -}; - -process.stdout.write(JSON.stringify(scenarios)); -""" - proc = subprocess.run( - ["node", "-e", probe], - input=json.dumps({**payload, "writeBudget": WRITE_BUDGET, - "callbackBudget": CALLBACK_BUDGET}), - text=True, - capture_output=True, - check=True, - timeout=15, - cwd=ROOT, - ) - return json.loads(proc.stdout) - - -def test_title_marker_is_a_fixpoint_under_html_title_normalization() -> None: - """The marked title must survive a DOM round-trip unchanged. - - If `read(write(x)) != write(x)` the MutationObserver that re-asserts the - marker never stops firing, and the renderer's main thread never gets back to - the event loop — the "random freeze" this file's other test could not see, - because its DOM stub echoes writes back verbatim. - """ - results = _run_fixpoint_probe(_marker_sources()) - - for label, r in results.items(): - assert r["converged"], ( - f"[{label}] the title marker never settled (writes={r['writes']}, " - f"raw={r['raw']!r}) — this is the rewrite loop that wedges the " - f"renderer, i.e. the freeze" - ) - assert r["raw"] == r["readBack"], ( - f"[{label}] the marker wrote {r['raw']!r} but the DOM hands back " - f"{r['readBack']!r}; that asymmetry is what loops" - ) - assert r["writes"] <= 2, ( - f"[{label}] settling took {r['writes']} title writes; one is " - f"expected, more means the marker is arguing with the DOM" - ) - assert "\U0001f440" in (r["raw"] or ""), ( - f"[{label}] the marker is missing — the user can no longer see " - f"which tab the agent is driving: {r}" - ) - - # The visible outcome, unchanged for pages that have a title. - assert results["normalTitle"]["raw"] == f"{PREFIX}Example" - assert results["normalTitle"]["pageTitle"] == "Example" - assert results["pageSetsTitle"]["raw"] == f"{PREFIX}Later" - assert results["pageSetsTitle"]["pageTitle"] == "Later" - # ...and for empty ones the marker is the bare eye, with no trailing space - # for the DOM to strip back off. - assert results["emptyTitle"]["raw"] == "\U0001f440" - assert results["emptyTitle"]["pageTitle"] == "" - assert results["pageClearsTitle"]["raw"] == "\U0001f440" - assert results["pageClearsTitle"]["pageTitle"] == "" - - -def test_title_marker_helpers_normalize_and_remove_all_leading_markers() -> None: - results = _run_node_probe(_marker_sources()) - - assert results["stripSamples"] == [ - "Example", - "", - "Plain", - "", - "Hello", - "Hello", - "Hello", - "Hello", - "Hello", - "Hello", - "42", - "", - "true", - ] - assert results["duplicateInstallTitle"] == { - "rawTitle": f"{PREFIX}Example", - "pageTitle": "Example", - } - assert results["cleanInstallTitle"] == { - "rawTitle": f"{PREFIX}Example", - "pageTitle": "Example", - } - assert results["bareEyeInstallTitle"] == { - "rawTitle": f"{PREFIX}Hello", - "pageTitle": "Hello", - } - assert results["nbspEyeInstallTitle"] == { - "rawTitle": f"{PREFIX}Hello", - "pageTitle": "Hello", - } - assert results["narrowEyeInstallTitle"] == { - "rawTitle": f"{PREFIX}Hello", - "pageTitle": "Hello", - } - assert results["setterTitle"] == { - "rawTitle": f"{PREFIX}Next", - "pageTitle": "Next", - } - assert results["bareEyeSetterTitle"] == { - "rawTitle": f"{PREFIX}World", - "pageTitle": "World", - } - assert results["nonStringSetterTitle"] == { - "rawTitle": f"{PREFIX}42", - "pageTitle": "42", - "stripped": "42", - } - assert results["duplicateRemoveTitle"] == { - "rawTitle": "Example", - "pageTitle": "Example", - } - assert results["bareEyeRemoveTitle"] == { - "rawTitle": "Hello", - "pageTitle": "Hello", - } - assert results["nbspEyeRemoveTitle"] == { - "rawTitle": "Hello", - "pageTitle": "Hello", - } - assert results["lifecycleTitle"] == { - "rawTitle": "After", - "pageTitle": "After", - } - - -def test_detach_cleans_marker_after_service_worker_realm_rebuild() -> None: - results = _run_service_worker_realm_rebuild_probe( - _service_worker_marker_sources()) - - assert results["titleAfterMark"] == f"{PREFIX}Legacy" - assert results["titleAfterDetach"] == "Legacy" - assert results["markerAfterDetach"] is False - assert results["callsAfterRealmRebuild"] == [ - { - "kind": "command", - "method": "Runtime.evaluate", - "params": {"expression": _marker_sources()["removeScript"]}, - }, - {"kind": "detach", "tabId": 17}, - ] - - -def test_controlled_reload_cleans_known_tabs_before_reloading() -> None: - sources = _service_worker_marker_sources() - assert _run_controlled_reload_probe(sources) == [ - { - "kind": "command", - "tabId": 17, - "method": "Page.removeScriptToEvaluateOnNewDocument", - "params": {"identifier": "known-script"}, - }, - { - "kind": "command", - "tabId": 17, - "method": "Runtime.evaluate", - "params": {"expression": sources["removeScript"]}, - }, - {"kind": "reload"}, - ] - - -def test_marker_cleanup_is_ordered_and_reload_remains_best_effort() -> None: - sources: dict[str, object] = _service_worker_marker_sources() - - remove_failed = _run_controlled_reload_probe({ - **sources, - "failRemove": True, - }) - assert [call.get("method") or call["kind"] for call in remove_failed] == [ - "Page.removeScriptToEvaluateOnNewDocument", - "Runtime.evaluate", - "reload", - ] - - remove_hung = _run_controlled_reload_probe({ - **sources, - "hangRemove": True, - }) - assert [call.get("method") or call["kind"] for call in remove_hung] == [ - "Page.removeScriptToEvaluateOnNewDocument", - "Runtime.evaluate", - "reload", - ] - - mark_in_flight = _run_controlled_reload_probe({ - **sources, - "markInFlight": True, - }) - # Cleanup now invalidates the in-flight generation immediately. Waiting - # for the full install before sending removal was the bug: the removal - # deadline could expire, reload/detach would proceed, and the old install - # continuation could then restore the marker. Page.enable may finish, but - # no addScript/install-evaluate is allowed after invalidation; current-page - # removal is queued immediately. - assert [call.get("method") or call["kind"] for call in mark_in_flight] == [ - "Page.enable", - "Runtime.evaluate", - "reload", - ] - assert mark_in_flight[1]["params"]["expression"] == sources["removeScript"] - - -def test_marker_commands_cannot_block_debugger_detach() -> None: - sources = _service_worker_marker_sources() - stages = [ - "Page.enable", - "Page.addScriptToEvaluateOnNewDocument", - "installEvaluate", - "Page.removeScriptToEvaluateOnNewDocument", - "removeEvaluate", - ] - for stage in stages: - result = _run_marker_detach_budget_probe(sources, stage) - assert result["completed"] is True, stage - assert result["attached"] is False, stage - assert result["calls"][-1] == {"kind": "detach", "tabId": 17}, stage - - -def test_late_marker_install_cannot_resume_after_debugger_detach() -> None: - sources = _service_worker_marker_sources() - for stage in ( - "Page.enable", - "Page.addScriptToEvaluateOnNewDocument", - "installEvaluate", - ): - result = _run_marker_late_completion_probe(sources, stage) - calls = result["calls"] - detach_index = result["detachIndex"] - assert detach_index >= 0, stage - assert not any( - call.get("stage") in { - "Page.addScriptToEvaluateOnNewDocument", "installEvaluate" - } - for call in calls[detach_index + 1:] - ), stage - assert result["pageMarked"] is False, stage - assert result["marked"] is False, stage - assert result["marking"] is False, stage - - -def test_failed_tab_close_preserves_marker_and_attachment_state() -> None: - result = _run_failed_close_marker_probe(_service_worker_marker_sources()) - - assert result["attached"] is True - assert result["marked"] is True - assert [call["kind"] for call in result["calls"]] == ["remove", "response"] - assert result["calls"][-1]["message"]["error"]["message"] == "close denied" diff --git a/tests/daemon/test_issue106_extension_applies_store_update.py b/tests/daemon/test_issue106_extension_applies_store_update.py index 0b48b65..1b25e06 100644 --- a/tests/daemon/test_issue106_extension_applies_store_update.py +++ b/tests/daemon/test_issue106_extension_applies_store_update.py @@ -37,7 +37,7 @@ def _run(scenario: str) -> dict: Date.now = () => now; let reloads = 0, cleanups = 0; const chrome = {{ runtime: {{ reload: () => {{ reloads += 1; }} }} }}; -async function cleanupMarkersBeforeReload() {{ cleanups += 1; }} +async function cleanupAttachmentIndicatorsBeforeReload() {{ cleanups += 1; }} const attachedTabs = new Set(); let inflightCommands = 0; let pendingUpdate = null; diff --git a/tests/skill/test_tab_surface.py b/tests/skill/test_tab_surface.py index 69f5291..332be8b 100644 --- a/tests/skill/test_tab_surface.py +++ b/tests/skill/test_tab_surface.py @@ -181,40 +181,22 @@ def __init__(self, url: str): self.evaluate = lambda *a, **k: None # callable — duck-type Page -def test_switch_tab_page_object_match(fake_session): +def test_switch_tab_page_object_match(fake_session, monkeypatch): from browserwright import switch_tab sess, fake = fake_session fake.responses["Runtime.evaluate"] = {"result": {"value": True}} - # A page that reads the marker installed on target-docs is resolved to it. - hits: list[str] = [] - - def marker_reads(marker_target): - # pretend the given Page is the docs tab - return marker_target == "target-docs" - + # Exact CDP identity resolves the Page independently of URL similarity. import browserwright.repl.playwright_handle as ph - - # Monkeypatch the marker helpers so the fake page "reads" the marker of - # exactly one target. - def fake_install(sess_, target_id): - hits.append(target_id) - return True, ("k", "v", None, "sid") - - def fake_read(page, key, value): - return marker_reads(hits[-1]) - - ph._install_target_marker = fake_install - ph._page_has_target_marker = fake_read - ph._clear_target_marker = lambda *a: None + monkeypatch.setattr(ph, "_target_id_for_page", lambda page: "target-docs") result = switch_tab(_FakePage("https://docs.example.com/api-keys")) assert result["targetId"] == "target-docs" assert sess.current_target_id == "target-docs" -def test_switch_tab_page_object_no_match(fake_session): +def test_switch_tab_page_object_no_match(fake_session, monkeypatch): from browserwright import switch_tab sess, fake = fake_session @@ -222,9 +204,7 @@ def test_switch_tab_page_object_no_match(fake_session): import browserwright.repl.playwright_handle as ph - ph._install_target_marker = lambda sess_, tid: (True, ("k", "v", None, "s")) - ph._page_has_target_marker = lambda page, k, v: False - ph._clear_target_marker = lambda *a: None + monkeypatch.setattr(ph, "_target_id_for_page", lambda page: None) with pytest.raises(TabMatchError, match="does not belong"): switch_tab(_FakePage("https://elsewhere.example.com/")) From 33470a3d177271ffb6b0168be9ced27a82b70eb7 Mon Sep 17 00:00:00 2001 From: Randy <zycode277@gmail.com> Date: Wed, 7 Oct 2026 21:08:40 +0800 Subject: [PATCH 2/4] feat(extension): keep Runtime quiet and discover contexts without Debugger The extension facade no longer keeps Runtime.enable on for Playwright's page sessions. Runtime event subscription makes Chrome build console object previews synchronously (large console.table calls slow the page even when nobody reads them), and it is observable page-side. Instead, Runtime.enable on a page session: - discovers real default execution contexts through native Document handles (DOM.getDocument / getFrameOwner / describeNode / resolveNode, new daemon/server/extension_contexts.py) and publishes synthetic executionContextCreated/Destroyed events; Debugger stays disabled, so `debugger` statements never pause or slow the page; - re-creates named utility worlds per frame on navigation; - enables the legacy Console domain and translates console-api messages into text-only Runtime.consoleAPICalled events, buffering startup text until the new realm is published. Full Runtime capture (object previews, exception events) is opt-in with BW_CAPTURE_CONSOLE=1 in the daemon environment; an explicit CDP session's Runtime.enable still subscribes natively. See docs/runtime-console.md. The e2e harness gains an extension_only_chrome fixture (no remote debugging port) used by the new console-cost and debugger-pause real-Chrome tests. Co-Authored-By: Claude <noreply@anthropic.com> --- docs/runtime-console.md | 44 +++ .../daemon/server/extension_contexts.py | 81 +++++ .../daemon/server/facade_extension.py | 334 +++++++++++++++++- tests/daemon/e2e/_real_browser.py | 20 +- tests/daemon/e2e/conftest.py | 19 + .../e2e/test_debugger_pause_extension.py | 220 ++++++++++++ tests/daemon/e2e/test_runtime_console_cost.py | 123 +++++++ tests/daemon/test_facade_extension_unit.py | 6 +- 8 files changed, 837 insertions(+), 10 deletions(-) create mode 100644 docs/runtime-console.md create mode 100644 src/browserwright/daemon/server/extension_contexts.py create mode 100644 tests/daemon/e2e/test_debugger_pause_extension.py create mode 100644 tests/daemon/e2e/test_runtime_console_cost.py diff --git a/docs/runtime-console.md b/docs/runtime-console.md new file mode 100644 index 0000000..25ee776 --- /dev/null +++ b/docs/runtime-console.md @@ -0,0 +1,44 @@ +# Console capture on the extension backend + +The extension CDP surface keeps Chrome's Runtime event subscription off by +default. Subscribing makes Chrome synchronously build console object previews; +large `console.table()` calls can slow down ordinary page execution even when +the agent never asks for console messages. + +`ExtensionFacadeBridge` supplies Playwright with real execution-context IDs +through `extension_contexts.py`. It resolves native Document handles with +`DOM.getDocument` / `DOM.getFrameOwner` / `DOM.describeNode` / `DOM.resolveNode`, +validates the context component of Chrome's V8 object ID, and releases each +handle. This works for scriptless local frames without a page preload. An +unfamiliar object ID format fails explicitly; discovery never enables Runtime +events as a fallback. Explicit isolated-world creation supplies each utility +world's name and context ID. + +Frame navigation and removal retire old contexts. Discovery retries provisional +documents and discards results from superseded navigations. Native Runtime +evaluation commands remain available, so `page.evaluate()` reads the page's +own globals as usual. Debugger stays disabled during ordinary automation: +even skipped pauses can make repeated `debugger` statements expensive. +The legacy Console domain supplies text messages without Runtime's object +previews. Startup text waits for its default realm to be published, so navigation +does not drop messages while context discovery is in flight. Translating those messages preserves Playwright's internal debug +marker used by `page.set_content()` to coordinate document lifecycle events. +Full console arguments and exception events require the capture opt-in below. + +Set `BW_CAPTURE_CONSOLE=1` in the daemon environment before starting the daemon +to retain native Runtime events, including Playwright console and exception +events. An explicit CDP session's `Runtime.enable` also subscribes normally; +its `Runtime.disable` turns the subscription off again. Console capture carries +Chrome's normal serialization cost. + +Repeatable verification: `uv run pytest +tests/daemon/e2e/test_runtime_console_cost.py -v`. Evidence is saved in +`tests/daemon/e2e/_artifacts/runtime-console-cost.json`: console timing with +capture off/on, main-world globals across navigation, and scriptless iframe +evaluation. + +Extension-only verification without a Chrome debugging port: `uv run pytest +tests/daemon/e2e/test_debugger_pause_extension.py -v`. Its artifact +`tests/daemon/e2e/_artifacts/debugger-pause-extension.json` records native pause +events, repeated debugger-statement timing, renderer swaps, scriptless child +realms, startup console text and levels, and executor rebind. diff --git a/src/browserwright/daemon/server/extension_contexts.py b/src/browserwright/daemon/server/extension_contexts.py new file mode 100644 index 0000000..1be3829 --- /dev/null +++ b/src/browserwright/daemon/server/extension_contexts.py @@ -0,0 +1,81 @@ +"""Discover extension execution contexts without Runtime or Debugger events. + +Blink resolves a native Document node in its frame's main world. V8's remote +object handle contains that world's actual context ID, so no page scripts, +globals, console previews, or debugger machinery are needed for discovery. +""" +from __future__ import annotations + +import re +from collections.abc import Awaitable, Callable + +SendCommand = Callable[[str, dict], Awaitable[dict]] +_REMOTE_OBJECT_ID = re.compile(r"(-?[0-9]+)\.([0-9]+)\.([0-9]+)\Z") + + +class ContextDiscoveryError(ValueError): + """Chrome could not expose a supported local document context.""" + + +class UnsupportedContextHandleError(ContextDiscoveryError): + """Chrome's remote object format cannot supply a validated context ID.""" + + +def _context_id(object_id: object) -> int: + # V8 15.4 remote-object-id.cc serializes isolate.context.boundObject. + # CDP treats the string as opaque: reject unfamiliar formats explicitly + # rather than guessing an ID or enabling expensive Runtime events. + match = (_REMOTE_OBJECT_ID.fullmatch(object_id) + if isinstance(object_id, str) and len(object_id) <= 128 else None) + if match is None: + raise UnsupportedContextHandleError("unsupported Chrome remote-object ID format") + _, context_id, bound_id = map(int, match.groups()) + if context_id <= 0 or bound_id <= 0: + raise UnsupportedContextHandleError("invalid Chrome remote-object ID") + return context_id + + +async def discover_default_context( + send: SendCommand, frame_id: str, *, main: bool, +) -> dict: + """Return the real default realm for a native Chrome frame ID. + + Child documents must belong to this CDP target; OOPIFs require their own + target routing. Internal discovery uses backend node IDs because requesting + the root document resets the DOM domain's frontend node ID bindings. + """ + if main: + document = (await send("DOM.getDocument", {"depth": 0}))["root"] + else: + owner = await send("DOM.getFrameOwner", {"frameId": frame_id}) + described = await send("DOM.describeNode", { + "backendNodeId": owner["backendNodeId"], "depth": 0, + }) + document = described["node"].get("contentDocument") + if not isinstance(document, dict): + raise ContextDiscoveryError("frame has no local document in this CDP target") + remote = (await send("DOM.resolveNode", { + "backendNodeId": document["backendNodeId"], + })).get("object", {}) + object_id = remote.get("objectId") + try: + if remote.get("subtype") != "node": + raise ContextDiscoveryError("document did not resolve to a native node") + context_id = _context_id(object_id) + checked = await send("Runtime.evaluate", { + "expression": "void 0", "contextId": context_id, + "silent": True, "returnByValue": True, "disableBreaks": True, + "throwOnSideEffect": True, + }) + if checked.get("exceptionDetails"): + raise ContextDiscoveryError("Chrome rejected the document execution context") + return { + "id": context_id, "origin": "", "name": "", + # Context numbers can repeat in a replacement renderer; the native + # isolate component distinguishes those realms during navigation. + "_native_identity": f"{int(object_id.split('.')[0])}.{context_id}", + "auxData": {"isDefault": True, "type": "default", "frameId": frame_id}, + } + finally: + if isinstance(object_id, str): + await send("Runtime.releaseObject", {"objectId": object_id}) diff --git a/src/browserwright/daemon/server/facade_extension.py b/src/browserwright/daemon/server/facade_extension.py index 881bc5c..7c3bbc2 100644 --- a/src/browserwright/daemon/server/facade_extension.py +++ b/src/browserwright/daemon/server/facade_extension.py @@ -37,9 +37,9 @@ * A3 — `Target.createTarget` → `RelayServer.create_background_tab` (the extension can't open browser-level targets), then synthesizes the created/attached events for the new tab. - * A4 — `Runtime.enable` execution-context barrier: forward, then wait - (bounded) for `Runtime.executionContextCreated` so Playwright doesn't - race ahead of the main-frame default context. + * A4 — `Runtime.enable` execution-context barrier: discover real contexts + from native document handles without subscribing to console previews; + native Runtime events remain available via `BW_CAPTURE_CONSOLE=1`. Page-session `Target.setAutoAttach` is forwarded because Playwright expects the page-session auto-attach command to reach Chrome. The extension service worker @@ -57,13 +57,16 @@ import contextlib import json import logging +import os import secrets +from collections import deque from typing import Any from websockets.asyncio.server import ServerConnection from ... import session_registry from . import call_deadline +from .extension_contexts import UnsupportedContextHandleError, discover_default_context from .extension_upstream import ( ExtensionUpstream, _tab_id_from_target_id, @@ -245,6 +248,19 @@ def __init__( # so `_handle_runtime_enable` can gate its response on the real event # rather than a blind sleep. self._ctx_waiters: dict[int, list[asyncio.Future]] = {} + # Runtime.enable subscribes Chrome to console previews, including + # synchronous table serialization. Native Document handles expose real + # context IDs without enabling either Runtime events or Debugger. + self._capture_console = os.environ.get("BW_CAPTURE_CONSOLE", "0") == "1" + self._quiet_runtime_tabs: set[int] = set() + self._native_runtime_tabs: set[int] = set() + self._execution_contexts: dict[int, dict[int, dict]] = {} + self._pending_console_text: dict[int, deque[dict]] = {} + self._world_names: dict[int, set[str]] = {} + self._frame_generations: dict[tuple[int, str], int] = {} + self._frame_discovery_serial = 0 + self._context_refresh_tasks: dict[tuple[int, str], asyncio.Task] = {} + self._context_refresh_pending: set[asyncio.Task] = set() @property def auto_title(self) -> str | None: @@ -298,6 +314,14 @@ async def aclose(self) -> None: return self._closed = True self._relay.remove_event_listener(self._on_relay_event) + # Native commands already on the shared relay must finish; subsequent + # refresh steps observe _closed and stop without issuing more work. + if self._context_refresh_pending: + await asyncio.gather(*self._context_refresh_pending, return_exceptions=True) + self._context_refresh_tasks.clear() + self._context_refresh_pending.clear() + self._frame_generations.clear() + self._pending_console_text.clear() # NOTE: we deliberately do NOT call self._ext.close() — ExtensionUpstream # .close() calls relay.set_event_handler(None), which would clobber the # AGENT path's primary event handler. Our dedicated ExtensionUpstream @@ -500,9 +524,27 @@ async def _forward_session_command(self, req_id: int | None, # PR3: a command scoped to the synthetic main-frame id (which equals the # targetId we handed Playwright) must target the REAL Chrome frame id. self._rewrite_command_frame_id(tab_id, params) + if method in ("Page.createIsolatedWorld", "Page.addScriptToEvaluateOnNewDocument"): + name = params.get("worldName") + if isinstance(name, str) and name: + self._world_names.setdefault(tab_id, set()).add(name) try: result = await self._relay.send_cdp( tab_id, method or "", params, timeout=self._command_wait_s()) + if method == "Runtime.enable": + self._native_runtime_tabs.add(tab_id) + elif method == "Runtime.disable": + self._native_runtime_tabs.discard(tab_id) + if (method == "Page.createIsolatedWorld" + and tab_id in self._quiet_runtime_tabs + and isinstance(result, dict) + and isinstance(result.get("executionContextId"), int)): + await self._publish_execution_context(tab_id, { + "id": result["executionContextId"], "origin": "", + "name": params.get("worldName", ""), + "auxData": {"isDefault": False, "type": "isolated", + "frameId": params.get("frameId")}, + }) # PR3: real Chrome makes a page's TOP frame id === its targetId, and # CRPage keys its frame→session map on the targetId # (`_sessions.set(targetId, mainFrameSession)`) then resolves the @@ -530,6 +572,16 @@ async def _forward_session_command(self, req_id: int | None, if isinstance(real_id, str) and real_id: self._tab_main_frame[tab_id] = real_id frame["id"] = f"ext-tab-{tab_id}" + # Existing child frames are replayed at cold bind too. + # Their own ids stay native; only the root parent id + # must agree with Playwright's synthetic main frame. + pending = list((result.get("frameTree") or {}).get("childFrames") or []) + while pending: + child = pending.pop() + child_frame = child.get("frame") or {} + if child_frame.get("parentId") == real_id: + child_frame["parentId"] = f"ext-tab-{tab_id}" + pending.extend(child.get("childFrames") or []) await self._respond(req_id, result, session_id=session_id) except _CommandError as e: await self._error(req_id, e.code, e.message, session_id=session_id) @@ -694,6 +746,9 @@ async def _handle_runtime_enable(self, req_id: int | None, f"unknown sessionId {session_id!r}", session_id=session_id) return + if not self._capture_console: + await self._enable_quiet_runtime(req_id, session_id, tab_id) + return # Arm the waiter BEFORE issuing enable so we can't miss the event # between the enable round-trip and registering the future. waiter = self._arm_context_waiter(tab_id) @@ -726,6 +781,229 @@ async def _handle_runtime_enable(self, req_id: int | None, self._disarm_context_waiter(tab_id, waiter) await self._respond(req_id, result, session_id=session_id) + async def _enable_quiet_runtime(self, req_id: int | None, + session_id: str, tab_id: int) -> None: + """Supply real context IDs without previews or debugger machinery.""" + self._quiet_runtime_tabs.add(tab_id) + self._native_runtime_tabs.discard(tab_id) + self._forget_tab_discoveries(tab_id) + self._execution_contexts.pop(tab_id, None) + waiter = self._arm_context_waiter(tab_id) + try: + await self._relay.send_cdp(tab_id, "Runtime.disable", {}, + timeout=self._command_wait_s()) + await self._relay.send_cdp(tab_id, "Console.enable", {}, + timeout=self._command_wait_s()) + await self._relay.send_cdp(tab_id, "Debugger.disable", {}, + timeout=self._command_wait_s()) + tree = await self._relay.send_cdp(tab_id, "Page.getFrameTree", {}, + timeout=self._command_wait_s()) + root = tree.get("frameTree") or {} + main_id = (root.get("frame") or {}).get("id") + if not isinstance(main_id, str): + raise ValueError("Chrome returned no main frame for context discovery") + self._tab_main_frame[tab_id] = main_id + # A rebind discovers every existing local frame; it does not rely + # on an event subscription replay or a page-script preload. + pending = [root] + while pending: + node = pending.pop() + frame_id = (node.get("frame") or {}).get("id") + if isinstance(frame_id, str): + generation = self._invalidate_frame_discovery(tab_id, frame_id) + await self._refresh_frame_worlds(tab_id, frame_id, generation) + pending.extend(node.get("childFrames") or []) + with contextlib.suppress(asyncio.TimeoutError): + await asyncio.wait_for(waiter, _RUNTIME_ENABLE_BARRIER_TIMEOUT) + await self._respond(req_id, {}, session_id=session_id) + except _CommandError as exc: + await self._error(req_id, exc.code, exc.message, session_id=session_id) + except Exception as exc: # noqa: BLE001 + await self._error(req_id, -32603, f"context discovery failed: {exc!r}", + session_id=session_id) + finally: + self._disarm_context_waiter(tab_id, waiter) + + def _invalidate_frame_discovery(self, tab_id: int, frame_id: str) -> int: + key = (tab_id, frame_id) + self._frame_discovery_serial += 1 + generation = self._frame_discovery_serial + self._frame_generations[key] = generation + # Let in-flight native commands finish. Cancelling a relay command can + # invalidate the shared extension transport; generation checks discard + # the result after it completes instead. + self._context_refresh_tasks.pop(key, None) + return generation + + def _schedule_frame_refresh(self, tab_id: int, frame_id: str, + *, rediscover: bool = False) -> None: + key = (tab_id, frame_id) + aliases = {frame_id} + if frame_id == self._tab_main_frame.get(tab_id): + aliases.add(f"ext-tab-{tab_id}") + if not rediscover and any( + (ctx.get("auxData") or {}).get("isDefault") + and (ctx.get("auxData") or {}).get("frameId") in aliases + for ctx in self._execution_contexts.get(tab_id, {}).values()): + return + pending = self._context_refresh_tasks.get(key) + if pending is not None and not pending.done(): + return + generation = self._frame_generations.get(key) + if generation is None: + generation = self._invalidate_frame_discovery(tab_id, frame_id) + task = asyncio.create_task(self._refresh_frame_worlds(tab_id, frame_id, generation)) + self._context_refresh_tasks[key] = task + self._context_refresh_pending.add(task) + task.add_done_callback(self._frame_refresh_done) + + def _frame_refresh_done(self, task: asyncio.Task) -> None: + self._context_refresh_pending.discard(task) + if not task.cancelled() and task.exception() is not None: + logger.warning("facade(ext): context discovery failed: %r", task.exception()) + + def _frame_discovery_current(self, tab_id: int, frame_id: str, + generation: int) -> bool: + return (not self._closed and tab_id in self._quiet_runtime_tabs + and self._frame_generations.get((tab_id, frame_id)) == generation) + + async def _publish_execution_context(self, tab_id: int, ctx: dict) -> None: + context_id = ctx.get("id") + if not isinstance(context_id, int): + return + contexts = self._execution_contexts.setdefault(tab_id, {}) + previous = contexts.get(context_id) + if previous is not None: + old_aux = previous.get("auxData") or {} + new_aux = ctx.get("auxData") or {} + if (old_aux.get("frameId") == new_aux.get("frameId") + and old_aux.get("isDefault") == new_aux.get("isDefault") + and previous.get("name") == ctx.get("name") + and previous.get("_native_identity") == ctx.get("_native_identity")): + return + contexts.pop(context_id) + await self._on_relay_event({ + "type": "event", "tabId": tab_id, + "method": "Runtime.executionContextDestroyed", + "params": {"executionContextId": context_id}, + }) + ctx['_console_ready'] = False + contexts[context_id] = ctx + payload = {key: value for key, value in ctx.items() if not key.startswith('_')} + if isinstance(payload.get("auxData"), dict): + payload["auxData"] = dict(payload["auxData"]) + await self._on_relay_event({ + "type": "event", "tabId": tab_id, + "method": "Runtime.executionContextCreated", "params": {"context": payload}, + }) + main_ids = {self._tab_main_frame.get(tab_id), f"ext-tab-{tab_id}"} + aux = ctx.get('auxData') or {} + if aux.get('isDefault') and aux.get('frameId') in main_ids: + # Publish the realm before text events, and preserve their order + # while more native messages arrive during the flush. + while (self._execution_contexts.get(tab_id, {}).get(context_id) is ctx + and self._pending_console_text.get(tab_id)): + message = self._pending_console_text[tab_id].popleft() + if tab_id not in self._native_runtime_tabs: + await self._emit_console_text(tab_id, context_id, message) + if not self._pending_console_text.get(tab_id): + self._pending_console_text.pop(tab_id, None) + ctx['_console_ready'] = True + + def _console_context_id(self, tab_id: int) -> int | None: + main_ids = {self._tab_main_frame.get(tab_id), f"ext-tab-{tab_id}"} + return next((context_id for context_id, context in + self._execution_contexts.get(tab_id, {}).items() + if (context.get("auxData") or {}).get("isDefault") + and context.get('_console_ready') + and (context.get("auxData") or {}).get("frameId") in main_ids), None) + + async def _emit_console_text(self, tab_id: int, context_id: int, message: dict) -> None: + await self._on_relay_event({ + "type": "event", "tabId": tab_id, + "method": "Runtime.consoleAPICalled", "params": { + "type": message.get("level", "log"), + "args": [{"type": "string", "value": message.get("text", "")}], + "executionContextId": context_id, "timestamp": 0, + }, + }) + + async def _refresh_frame_worlds(self, tab_id: int, frame_id: str, + generation: int) -> None: + async def send(method: str, params: dict) -> dict: + return await self._relay.send_cdp( + tab_id, method, params, timeout=self._command_wait_s()) + + # frameNavigated arrives at WillCommitLoad, before the new Document + # necessarily exists. Retry only while this exact document generation + # remains current, and let later lifecycle events retry an early miss. + for attempt in range(6): + if not self._frame_discovery_current(tab_id, frame_id, generation): + return + try: + context = await discover_default_context( + send, frame_id, main=frame_id == self._tab_main_frame.get(tab_id)) + if not self._frame_discovery_current(tab_id, frame_id, generation): + return + aliases = {frame_id} + if frame_id == self._tab_main_frame.get(tab_id): + aliases.add(f"ext-tab-{tab_id}") + if any((ctx.get("auxData") or {}).get("isDefault") + and (ctx.get("auxData") or {}).get("frameId") in aliases + and ctx.get("_native_identity") != context["_native_identity"] + for ctx in self._execution_contexts.get(tab_id, {}).values()): + await self._retire_frame_contexts(tab_id, frame_id) + await self._publish_execution_context(tab_id, context) + break + except UnsupportedContextHandleError: + raise + except Exception as exc: # noqa: BLE001 - a document may be provisional + if attempt == 5: + logger.debug("facade(ext): context discovery for frame %s: %r", + frame_id, exc) + return + await asyncio.sleep(0.025 * 2 ** attempt) + for name in self._world_names.get(tab_id, set()).copy(): + if not self._frame_discovery_current(tab_id, frame_id, generation): + return + with contextlib.suppress(_CommandError, Exception): + result = await send("Page.createIsolatedWorld", { + "frameId": frame_id, "worldName": name, + "grantUniveralAccess": True, + }) + if not self._frame_discovery_current(tab_id, frame_id, generation): + return + await self._publish_execution_context(tab_id, { + "id": result.get("executionContextId"), "origin": "", "name": name, + "auxData": {"isDefault": False, "type": "isolated", "frameId": frame_id}, + }) + + async def _retire_frame_contexts(self, tab_id: int, frame_id: str) -> None: + contexts = self._execution_contexts.get(tab_id, {}) + for context_id, ctx in list(contexts.items()): + frame = (ctx.get("auxData") or {}).get("frameId") + if frame in (frame_id, f"ext-tab-{tab_id}" if frame_id == self._tab_main_frame.get(tab_id) else frame_id): + contexts.pop(context_id, None) + await self._on_relay_event({ + "type": "event", "tabId": tab_id, + "method": "Runtime.executionContextDestroyed", + "params": {"executionContextId": context_id}, + }) + + def _forget_tab_discoveries(self, tab_id: int) -> None: + for key in [key for key in self._frame_generations if key[0] == tab_id]: + self._frame_generations.pop(key, None) + self._context_refresh_tasks.pop(key, None) + + async def _retire_tab_contexts(self, tab_id: int) -> None: + contexts = self._execution_contexts.pop(tab_id, {}) + for context_id in contexts: + await self._on_relay_event({ + "type": "event", "tabId": tab_id, + "method": "Runtime.executionContextDestroyed", + "params": {"executionContextId": context_id}, + }) + def _arm_context_waiter(self, tab_id: int) -> asyncio.Future: """Register a future resolved when the next default `Runtime.executionContextCreated` for `tab_id` is observed.""" @@ -999,17 +1277,59 @@ async def _on_relay_event(self, ext_msg: dict) -> None: return if method in ("Target.attachedToTarget", "Target.detachedFromTarget"): return + if method == "Console.messageAdded" and tab_id in self._quiet_runtime_tabs: + if tab_id in self._native_runtime_tabs: + return + message = params.get("message") or {} + if message.get("source") == "console-api": + context_id = self._console_context_id(tab_id) + if context_id is None: + # Keep native text from early page scripts while a new + # default realm is being discovered. No object preview + # subscription or page-side console hook is involved. + self._pending_console_text.setdefault(tab_id, deque(maxlen=1000)).append(message) + else: + await self._emit_console_text(tab_id, context_id, message) + return # PR3: keep the live top-frame url fresh and release the fresh-blank # normalization once the page actually navigates, so getTargetInfo # stops reporting ":" after the first real navigation. if method == "Page.frameNavigated": frame = params.get("frame") or {} + if tab_id in self._quiet_runtime_tabs and isinstance(frame.get("id"), str): + if not frame.get("parentId"): + # A replacement root document destroys all child realms, + # even if Chrome omits child frameDetached events during + # a renderer swap and reuses numeric context IDs. + self._forget_tab_discoveries(tab_id) + await self._retire_tab_contexts(tab_id) + self._tab_main_frame[tab_id] = frame["id"] + else: + await self._retire_frame_contexts(tab_id, frame["id"]) + self._invalidate_frame_discovery(tab_id, frame["id"]) + self._schedule_frame_refresh(tab_id, frame["id"]) # Top frame only: no parentId. if isinstance(frame, dict) and not frame.get("parentId"): new_url = frame.get("url") if isinstance(new_url, str) and new_url and new_url != ":": self._tab_url[tab_id] = new_url self._fresh_blank_tabs.discard(tab_id) + elif method == "Page.frameDetached" and tab_id in self._quiet_runtime_tabs: + frame_id = params.get("frameId") + if isinstance(frame_id, str): + key = (tab_id, frame_id) + self._frame_generations.pop(key, None) + self._context_refresh_tasks.pop(key, None) + await self._retire_frame_contexts(tab_id, frame_id) + elif (method == "Page.lifecycleEvent" and tab_id in self._quiet_runtime_tabs + and params.get("name") in ("init", "DOMContentLoaded")): + frame_id = params.get("frameId") + if isinstance(frame_id, str): + self._schedule_frame_refresh(tab_id, frame_id, rediscover=True) + elif method == "DOM.documentUpdated" and tab_id in self._quiet_runtime_tabs: + frame_id = self._tab_main_frame.get(tab_id) + if frame_id is not None: + self._schedule_frame_refresh(tab_id, frame_id, rediscover=True) # PR3: a default-context creation releases the Runtime.enable barrier. elif method == "Runtime.executionContextCreated": ctx = params.get("context") or {} @@ -1081,6 +1401,8 @@ def _rewrite_event_frame_id(self, tab_id: int, method: str, # (executionContextCreated) are the carriers of the top-frame id. if params.get("frameId") == real: params["frameId"] = synthetic + if params.get("parentFrameId") == real: + params["parentFrameId"] = synthetic frame = params.get("frame") if isinstance(frame, dict): if frame.get("id") == real: @@ -1109,6 +1431,12 @@ def _evict_tab(self, tab_id: int) -> None: """Drop all per-tab state for a closed/detached tab and wake any outstanding Runtime.enable barrier so it doesn't hang on a dead tab.""" self._tab_sessions.pop(tab_id, None) + self._quiet_runtime_tabs.discard(tab_id) + self._native_runtime_tabs.discard(tab_id) + self._execution_contexts.pop(tab_id, None) + self._pending_console_text.pop(tab_id, None) + self._world_names.pop(tab_id, None) + self._forget_tab_discoveries(tab_id) for sid in [sid for sid, target in self._aux_sessions.items() if target == tab_id]: self._aux_sessions.pop(sid, None) self._ext.evict_tab_sessions(tab_id) diff --git a/tests/daemon/e2e/_real_browser.py b/tests/daemon/e2e/_real_browser.py index 596a3b1..8d64461 100644 --- a/tests/daemon/e2e/_real_browser.py +++ b/tests/daemon/e2e/_real_browser.py @@ -97,11 +97,14 @@ def launch_cft_with_extension( ext_dir: Path, *, cdp_port: int = 0, + remote_debugging: bool = True, + headless: bool = False, ) -> ChromeHandle: - """Launch Chrome for Testing with --load-extension, wait for CDP ready. + """Launch Chrome for Testing with --load-extension. - Detects the CDP websocket URL from either the DevToolsActivePort file - (port=0) or stderr (Chrome 148+ with explicit port skips the file). + By default, wait for the CDP websocket URL from DevToolsActivePort or + stderr. With remote_debugging=False there is no browser CDP socket; callers + wait for the extension relay instead. """ profile_dir = Path(tempfile.mkdtemp(prefix="bd-e2e-chrome-")) stderr_path = profile_dir / "_cft_stderr.log" @@ -109,10 +112,8 @@ def launch_cft_with_extension( args = [ str(cft_binary), f"--user-data-dir={profile_dir}", - f"--remote-debugging-port={cdp_port}", "--no-first-run", "--no-default-browser-check", - "--remote-allow-origins=*", "--no-proxy-server", "--enable-features=UserScriptUserExtensionToggle", # See the same pair in `launch_chrome.py` / e2e conftest: without them @@ -123,6 +124,11 @@ def launch_cft_with_extension( f"--load-extension={ext_dir}", "about:blank", ] + if remote_debugging: + args[2:2] = [f"--remote-debugging-port={cdp_port}", + "--remote-allow-origins=*"] + if headless: + args.insert(2, "--headless=new") proc = subprocess.Popen( args, stdout=subprocess.DEVNULL, @@ -130,6 +136,10 @@ def launch_cft_with_extension( start_new_session=True, ) try: + if not remote_debugging: + stderr_fh.close() + return ChromeHandle(ws_url="", profile_path=profile_dir, + pid=proc.pid, port=0) active_file = profile_dir / "DevToolsActivePort" deadline = time.monotonic() + 15.0 while time.monotonic() < deadline: diff --git a/tests/daemon/e2e/conftest.py b/tests/daemon/e2e/conftest.py index 5a2bac0..bd959ea 100644 --- a/tests/daemon/e2e/conftest.py +++ b/tests/daemon/e2e/conftest.py @@ -608,6 +608,25 @@ def e2e_chrome(cft_binary, patched_ext_dir): shutil.rmtree(handle.profile_path, ignore_errors=True) +@pytest.fixture +def extension_only_chrome(cft_binary, patched_ext_dir, request): + """Chrome with extension automation and no remote debugging port. + + Headful by default; input telemetry can request headless isolation from + unrelated desktop cursor movement through indirect parametrization. + """ + from ._real_browser import kill_chrome, launch_cft_with_extension + + handle = launch_cft_with_extension(cft_binary, patched_ext_dir, + remote_debugging=False, + headless=getattr(request, "param", None) == "headless") + try: + yield handle + finally: + kill_chrome(handle.pid) + shutil.rmtree(handle.profile_path, ignore_errors=True) + + # --------------------------------------------------------------------------- # Artifact dump on failure # --------------------------------------------------------------------------- diff --git a/tests/daemon/e2e/test_debugger_pause_extension.py b/tests/daemon/e2e/test_debugger_pause_extension.py new file mode 100644 index 0000000..72a8c9f --- /dev/null +++ b/tests/daemon/e2e/test_debugger_pause_extension.py @@ -0,0 +1,220 @@ +"""Extension-only Chrome remains usable on pages containing debugger statements. + +Repeat with ``uv run pytest tests/daemon/e2e/test_debugger_pause_extension.py -v``. +Chrome has no remote debugging port; the agent talks only to chrome.debugger +through the extension. ``_artifacts/debugger-pause-extension.json`` records the +navigation, evaluation, real form input, default console text and executor rebind. +""" +from __future__ import annotations + +import json +from http.server import BaseHTTPRequestHandler, ThreadingHTTPServer +import subprocess +import threading +import uuid + +import pytest + +from .conftest import published_endpoint +from .helpers import bs_home, drop_ledger_session, run_skill, seed_ledger_session + + +_HTML = """<!doctype html><meta charset="utf-8"><title>Debugger fixture + +

Ready

""" + + +@pytest.fixture +def debugger_site(): + class Handler(BaseHTTPRequestHandler): + def do_GET(self): + body = _HTML.encode() + self.send_response(200) + self.send_header("Content-Type", "text/html; charset=utf-8") + self.send_header("Content-Length", str(len(body))) + self.end_headers() + self.wfile.write(body) + + def log_message(self, *args): + pass + + server = ThreadingHTTPServer(("127.0.0.1", 0), Handler) + thread = threading.Thread(target=server.serve_forever, daemon=True) + thread.start() + try: + yield f"http://127.0.0.1:{server.server_port}" + finally: + server.shutdown() + server.server_close() + thread.join(timeout=2) + + +@pytest.fixture +def e2e_chrome(extension_only_chrome): + """Override the usual harness: no Chrome CDP socket or DevTools connection.""" + return extension_only_chrome + + +def test_debugger_statements_do_not_pause_agent_calls_or_loaded_page_rebind( + e2e_daemon, e2e_chrome, ext_ready, e2e_artifacts_dir, debugger_site, +): + pytest.importorskip("playwright.sync_api") + assert not (e2e_chrome.profile_path / "DevToolsActivePort").exists() + sid = f"e2e-debugger-{uuid.uuid4().hex}" + seed_ledger_session(bs_home("extension"), sid, backend="extension", name="debugger") + artifact = e2e_artifacts_dir / "debugger-pause-extension.json" + progress = artifact.with_suffix(".progress.json") + progress.unlink(missing_ok=True) + events = [] + observer_ready = threading.Event() + observer_stop = threading.Event() + + def observe(): + # Subscribe to the same session's synthesized target announcements. + # No page domain or debugger command is sent by this observer. + from websockets.sync.client import connect + endpoint = published_endpoint(e2e_daemon.runtime_dir) + with connect(endpoint.replace("http://", "ws://") + f"/cdp?session={sid}") as ws: + ws.send(json.dumps({"id": 1, "method": "Target.setAutoAttach", "params": { + "autoAttach": True, "waitForDebuggerOnStart": False, "flatten": True}})) + while not observer_stop.is_set(): + try: + event = json.loads(ws.recv(timeout=0.2)) + except TimeoutError: + continue + if event.get("id") == 1: + observer_ready.set() + if event.get("method") in {"Debugger.paused", "Debugger.resumed", "Page.frameNavigated"}: + events.append(event) + + observer = threading.Thread(target=observe, daemon=True) + observer.start() + assert observer_ready.wait(5), "passive extension event observer did not connect" + evidence = {"remote_debugging_port": False, "stages": [], "native_events": events} + script = f"site = {debugger_site!r}\nprogress_path = {str(progress)!r}\n" + ''' +import json +from pathlib import Path +progress = [] +def note(stage, **observations): + progress.append({'stage': stage, **observations}) + Path(progress_path).write_text(json.dumps(progress, indent=2) + '\\n') +note('bound') +messages = [] +page.on('console', lambda message: messages.append({'level': message.type, 'text': message.text})) +context_events = [] +context_observer = context.new_cdp_session(page) +context_observer.on('Runtime.executionContextCreated', + lambda event: context_events.append({'type':'created', **event})) +context_observer.on('Runtime.executionContextDestroyed', + lambda event: context_events.append({'type':'destroyed', **event})) +page.set_default_timeout(8000) +page.goto(site, timeout=8000) +note('navigated') +print('NAVIGATED', flush=True) +assert page.evaluate('() => { debugger; return 42; }') == 42 +print('EVALUATED', flush=True) +note('evaluated') +benchmark = """() => { + const iterations = 100000; + let start = performance.now(); + for (let i=0; i= 3, (expected, messages) +note('startup-console-across-navigation', messages=messages) +child = page.frames[1] +note('scriptless-child-bound', url=child.url, contexts=list(context_events)) +child.wait_for_load_state() +note('scriptless-child-loaded') +assert child.evaluate('document.body.innerText') == 'Child context' +note('scriptless-child-evaluated') +assert child.locator('body').inner_text() == 'Child context' +note('scriptless-child-context') +assert 'Name' in snapshot() +page.get_by_role('textbox', name='Name').fill('Ada Lovelace') +page.get_by_role('button', name='Submit').click() +page.get_by_text('Submitted: Ada Lovelace', exact=True).wait_for() +assert 'Submitted: Ada Lovelace' in read_markdown() +page.evaluate("console.log('plain text', 42)") +page.evaluate("console.warn('warning text', 1); console.error('error text', 2)") +page.wait_for_timeout(100) +assert {'level':'log', 'text':'plain text 42'} in messages, messages +assert {'level':'warning', 'text':'warning text 1'} in messages, messages +assert {'level':'error', 'text':'error text 2'} in messages, messages +before = page.evaluate('fixtureTicks') +page.wait_for_timeout(80) +assert page.evaluate('fixtureTicks') > before +print('OBSERVATIONS ' + json.dumps({'messages':messages, 'ticks':page.evaluate('fixtureTicks')}), flush=True) +note('input-console-heartbeat', messages=messages, ticks=page.evaluate('fixtureTicks')) +context_observer.detach() +''' + + def record(script, label): + try: + result = run_skill(script, backend="extension", runtime_dir=e2e_daemon.runtime_dir, + extra_env={"BD_SESSION": sid}, timeout=45) + stage = {"label": label, "returncode": result.returncode, + "stdout": result.stdout, "stderr": result.stderr} + except subprocess.TimeoutExpired as exc: + def as_text(value): + return value.decode(errors="replace") if isinstance(value, bytes) else value + stage = {"label": label, "timeout": exc.timeout, + "stdout": as_text(exc.stdout), "stderr": as_text(exc.stderr)} + evidence["stages"].append(stage) + if progress.exists(): + evidence["progress"] = json.loads(progress.read_text()) + artifact.write_text(json.dumps(evidence, indent=2) + "\n") + assert stage.get("returncode") == 0, evidence + + try: + record(script, "navigation-evaluation-input-console") + record("reset()", "reset-loaded-page") + record("messages = []\npage.on('console', lambda message: messages.append({'level':message.type,'text':message.text}))\n" + "assert page.evaluate('() => { debugger; return 42; }') == 42\n" + "assert page.get_by_text('Submitted: Ada Lovelace', exact=True).count() == 1\n" + "before = page.evaluate('fixtureTicks')\npage.wait_for_timeout(80)\n" + "assert page.evaluate('fixtureTicks') > before\npage.reload()\n" + "page.wait_for_timeout(100)\n" + "assert {'level':'log','text':'fixture startup 42'} in messages, messages\n" + "assert {'level':'warning','text':'fixture startup warning 1'} in messages, messages\n" + "assert {'level':'error','text':'fixture startup error 2'} in messages, messages\n" + "print('REBOUND_OK', messages)", + "rebind-loaded-page") + observed_urls = {event.get("params", {}).get("frame", {}).get("url") + for event in events if event["method"] == "Page.frameNavigated"} + assert debugger_site + "/" in observed_urls, evidence + assert debugger_site.replace("127.0.0.1", "localhost") + "/" in observed_urls, evidence + assert not [event for event in events if event["method"] == "Debugger.paused"], evidence + finally: + observer_stop.set() + observer.join(timeout=2) + artifact.write_text(json.dumps(evidence, indent=2) + "\n") + drop_ledger_session(bs_home("extension"), sid) diff --git a/tests/daemon/e2e/test_runtime_console_cost.py b/tests/daemon/e2e/test_runtime_console_cost.py new file mode 100644 index 0000000..1b2f378 --- /dev/null +++ b/tests/daemon/e2e/test_runtime_console_cost.py @@ -0,0 +1,123 @@ +"""Real-Chrome console overhead and execution-context lifecycle evidence. + +Run ``uv run pytest tests/daemon/e2e/test_runtime_console_cost.py -v``. +The JSON artifact compares table serialization with the Runtime subscription +off/on, and records main-world evaluation after navigation and executor reset. +""" +from __future__ import annotations + +import json +from http.server import BaseHTTPRequestHandler, ThreadingHTTPServer +import threading +import uuid + +import pytest + +from .helpers import bs_home, drop_ledger_session, run_skill, seed_ledger_session + + +@pytest.fixture +def e2e_chrome(extension_only_chrome): + return extension_only_chrome + + +@pytest.fixture +def context_site(): + class Handler(BaseHTTPRequestHandler): + def do_GET(self): + title = "Context fixture" if self.path == "/" else "Next context" + child = '' if self.path == "/frame" else "" + body = f"{title}

Changed

{child}".encode() + self.send_response(200) + self.send_header("Content-Type", "text/html") + self.send_header("Content-Length", str(len(body))) + self.end_headers() + self.wfile.write(body) + + def log_message(self, *args): + pass + + server = ThreadingHTTPServer(("127.0.0.1", 0), Handler) + thread = threading.Thread(target=server.serve_forever, daemon=True) + thread.start() + try: + yield f"http://127.0.0.1:{server.server_port}" + finally: + server.shutdown() + server.server_close() + thread.join(timeout=2) + + +def test_console_subscription_is_opt_in_and_contexts_survive_navigation( + e2e_daemon, ext_ready, e2e_artifacts_dir, context_site, +): + pytest.importorskip("playwright.sync_api") + sid = f"e2e-console-{uuid.uuid4().hex}" + seed_ledger_session(bs_home("extension"), sid, backend="extension", name="console") + artifact = e2e_artifacts_dir / "runtime-console-cost.json" + artifact.unlink(missing_ok=True) + script = f"artifact_path = {str(artifact)!r}\nsite = {context_site!r}\n" + ''' +import json +from pathlib import Path +page.goto(site) +page.evaluate('window.fixtureValue = 41') +assert page.evaluate('fixtureValue + 1') == 42 +measure = """() => { + const rows = Array.from({length: 30000}, (_, i) => ({a:i, b:i+1, c:i+2, d:i+3})); + const samples = []; + for (let i=0; i<3; i++) { + const start = performance.now(); + console.table(rows); + samples.push(performance.now() - start); + } + return samples.sort((a,b) => a-b)[1]; +}""" +result = {'default_ms': page.evaluate(measure)} +cdp = context.new_cdp_session(page) +cdp.send('Runtime.disable') +result['disabled_ms'] = page.evaluate(measure) +cdp.send('Runtime.enable') +result['enabled_ms'] = page.evaluate(measure) +cdp.send('Runtime.disable') +cdp.detach() +Path(artifact_path).write_text(json.dumps(result, indent=2) + '\\n') +page.goto(site + '/next') +result['after_navigation'] = page.evaluate('({title:document.title, previous:typeof fixtureValue})') +page.set_content('

setcontent works

') +result['set_content'] = page.locator('body').inner_text() +assert result['set_content'] == 'setcontent works', result +page.goto(site + '/frame') +frame = page.frames[1] +frame.wait_for_load_state() +result['child_context'] = frame.evaluate('document.body.innerText') +assert result['child_context'] == 'Child context' +assert page.get_by_text('Changed', exact=True).count() == 1 +Path(artifact_path).write_text(json.dumps(result, indent=2) + '\\n') +print('CONSOLE_CONTEXTS_OK') +''' + try: + execution = run_skill(script, backend="extension", runtime_dir=e2e_daemon.runtime_dir, + extra_env={"BD_SESSION": sid}, timeout=90) + evidence = {"returncode": execution.returncode, "stdout": execution.stdout, + "stderr": execution.stderr} + if artifact.exists(): + evidence["observations"] = json.loads(artifact.read_text()) + artifact.write_text(json.dumps(evidence, indent=2) + "\n") + assert execution.returncode == 0, evidence + result = evidence["observations"] + assert result["after_navigation"] == {"title": "Next context", "previous": "undefined"} + assert result["default_ms"] <= result["disabled_ms"] * 3 + 12, result + assert result["enabled_ms"] > result["disabled_ms"] + 5, result + reset_result = run_skill("reset()", backend="extension", runtime_dir=e2e_daemon.runtime_dir, + extra_env={"BD_SESSION": sid}, timeout=30) + assert reset_result.returncode == 0, reset_result.stderr + rebound = run_skill("assert page.frames[1].evaluate('document.body.innerText') == 'Child context'\n" + "assert page.evaluate('typeof fixtureValue') == 'undefined'\nprint('REBOUND_OK')", + backend="extension", runtime_dir=e2e_daemon.runtime_dir, + extra_env={"BD_SESSION": sid}, timeout=45) + evidence["rebind"] = {"returncode": rebound.returncode, "stdout": rebound.stdout, + "stderr": rebound.stderr} + artifact.write_text(json.dumps(evidence, indent=2) + "\n") + assert rebound.returncode == 0, evidence + finally: + drop_ledger_session(bs_home("extension"), sid) diff --git a/tests/daemon/test_facade_extension_unit.py b/tests/daemon/test_facade_extension_unit.py index 6892fdd..31dea66 100644 --- a/tests/daemon/test_facade_extension_unit.py +++ b/tests/daemon/test_facade_extension_unit.py @@ -671,7 +671,8 @@ async def _noop_close(_reason): await relay.stop() -async def test_runtime_enable_forwarded_and_acked(): +async def test_runtime_enable_forwarded_and_acked(monkeypatch): + monkeypatch.setenv("BW_CAPTURE_CONSOLE", "1") async with _wired() as (relay, ext, client, bridge): # First attach a tab so we have a session. await ext.announce_attached(tab_id=8, group_title=bridge.auto_title) @@ -837,9 +838,10 @@ async def test_child_target_lifecycle_events_are_not_forwarded_to_playwright(): ), new_frames -async def test_runtime_enable_does_disable_enable_dance_and_gates_on_event(): +async def test_runtime_enable_does_disable_enable_dance_and_gates_on_event(monkeypatch): """PR3 fix #2: Runtime.enable issues Runtime.disable→enable to force a re-emit, and gates its response on the default executionContextCreated.""" + monkeypatch.setenv("BW_CAPTURE_CONSOLE", "1") async with _wired() as (relay, ext, client, bridge): sid = await _attach_one(ext, client, tab_id=8, group_title=bridge.auto_title) From d8c928049b252a3e4636359a468ed09fa7fd2b56 Mon Sep 17 00:00:00 2001 From: Randy Date: Wed, 7 Oct 2026 21:09:37 +0800 Subject: [PATCH 3/4] feat(executor): screenshots keep the page caret by default Playwright's screenshot() defaults to caret="hide", which writes inline caret-color styles on every editable element (shadow roots and child frames included) and restores them afterwards; page code can observe those shared-DOM mutations. The executor's live surface now installs repl/screenshot_defaults.py, which makes Page/Locator/ElementHandle screenshot() default to caret="initial" for executor contexts. All other options keep native behavior, and an explicit caret (including caret="hide") is passed through unchanged. Co-Authored-By: Claude --- src/browserwright/_executor/process.py | 3 + src/browserwright/repl/screenshot_defaults.py | 63 +++++++++ tests/daemon/e2e/test_screenshot_caret.py | 126 ++++++++++++++++++ 3 files changed, 192 insertions(+) create mode 100644 src/browserwright/repl/screenshot_defaults.py create mode 100644 tests/daemon/e2e/test_screenshot_caret.py diff --git a/src/browserwright/_executor/process.py b/src/browserwright/_executor/process.py index b34f311..49eaa1b 100644 --- a/src/browserwright/_executor/process.py +++ b/src/browserwright/_executor/process.py @@ -1187,6 +1187,9 @@ def _bind_live_surface(self, g: dict[str, Any]) -> None: helper modules are seeded from the live surface too. """ from ..repl.markdown import make_read_markdown + from ..repl.screenshot_defaults import install_screenshot_defaults + + install_screenshot_defaults(self._context) g.pop("__bw_playwright_handle__", None) g["page"] = self._page diff --git a/src/browserwright/repl/screenshot_defaults.py b/src/browserwright/repl/screenshot_defaults.py new file mode 100644 index 0000000..3ad921c --- /dev/null +++ b/src/browserwright/repl/screenshot_defaults.py @@ -0,0 +1,63 @@ +"""Screenshot defaults for the executor's live Playwright surface. + +Playwright's ``screenshot()`` defaults to ``caret="hide"``, which writes inline +``caret-color: transparent`` on every editable element (shadow roots and child +frames included) and restores it afterwards. Those style writes land in the +shared DOM, so page code can observe them (MutationObserver, style reads) even +though the capture itself is a protocol command. + +For contexts registered here, ``Page``/``Locator``/``ElementHandle`` +``screenshot()`` default to ``caret="initial"`` instead. Every other option — +path, clip, type, quality, scale, mask — keeps native behavior, and a caller +that passes ``caret`` explicitly (including ``caret="hide"``) gets exactly that. +""" +from __future__ import annotations + +import functools +import weakref +from typing import Any + +_CONTEXTS: weakref.WeakSet[Any] = weakref.WeakSet() +_INSTALLED = False + + +def _page_of(obj: Any, cls_name: str) -> Any | None: + if cls_name == "Page": + return obj + if cls_name == "Locator": + return obj.page + # ElementHandle has no public ``page``; it keeps its owning frame without a + # browser round-trip, including handles returned by ``evaluate_handle``. + try: + from playwright._impl._sync_base import mapping + return mapping.from_impl(obj._impl_obj._frame.page) + except Exception: # noqa: BLE001 - unknown internals: keep native defaults. + return None + + +def install_screenshot_defaults(context: Any) -> None: + """Register ``context`` and patch the sync screenshot methods once.""" + global _INSTALLED + from playwright.sync_api import BrowserContext, ElementHandle, Locator, Page + + if not isinstance(context, BrowserContext): + return + _CONTEXTS.add(context) + if _INSTALLED: + return + _INSTALLED = True + + for cls in (Page, Locator, ElementHandle): + native = cls.screenshot + + def wrap(native: Any, cls_name: str) -> Any: + @functools.wraps(native) + def screenshot(self: Any, *args: Any, **kwargs: Any) -> Any: + if kwargs.get("caret") is None: + page = _page_of(self, cls_name) + if page is not None and page.context in _CONTEXTS: + kwargs["caret"] = "initial" + return native(self, *args, **kwargs) + return screenshot + + setattr(cls, "screenshot", wrap(native, cls.__name__)) diff --git a/tests/daemon/e2e/test_screenshot_caret.py b/tests/daemon/e2e/test_screenshot_caret.py new file mode 100644 index 0000000..66bd3d2 --- /dev/null +++ b/tests/daemon/e2e/test_screenshot_caret.py @@ -0,0 +1,126 @@ +"""Screenshot defaults preserve page caret styling, observed by page code. + +Repeat with ``uv run pytest tests/daemon/e2e/test_screenshot_caret.py -v``. +The JSON artifact records shared-DOM mutations and the PNG/JPEG files verify +the native screenshot return value, path handling, and clipping behavior. +""" +from __future__ import annotations + +import json +import textwrap +import uuid + +import pytest + +from .helpers import bs_home, drop_ledger_session, run_skill, seed_ledger_session + + +@pytest.fixture +def e2e_chrome(extension_only_chrome): + return extension_only_chrome + + +@pytest.mark.parametrize("extension_only_chrome", ["headless"], indirect=True) +def test_screenshot_defaults_preserve_caret_and_explicit_options( + e2e_daemon, e2e_chrome, ext_ready, e2e_artifacts_dir, +): + pytest.importorskip("playwright.sync_api") + assert not (e2e_chrome.profile_path / "DevToolsActivePort").exists() + sid = f"e2e-caret-{uuid.uuid4().hex}" + seed_ledger_session(bs_home("extension"), sid, backend="extension", name="caret") + artifact = e2e_artifacts_dir / "screenshot-caret.json" + artifact.unlink(missing_ok=True) + script = f"artifact_dir = {str(e2e_artifacts_dir)!r}\n" + textwrap.dedent(''' + import json + import struct + from pathlib import Path + page.set_content(""" + + + +
Editable
+
+ + """) + initial = page.evaluate('readCaretStyles()') + result = {'initial': initial, 'cases': {}} + def capture(name, take): + page.evaluate('styleMutations.splice(0)') + data = take() + page.wait_for_timeout(50) + result['cases'][name] = { + 'styles': page.evaluate('readCaretStyles()'), + 'mutations': page.evaluate('styleMutations.splice(0)'), + 'bytes': len(data), 'png': data.startswith(b'\\x89PNG\\r\\n\\x1a\\n'), + } + return data + directory = Path(artifact_dir) + page_path = directory / 'caret-page.png' + data = capture('page_default', lambda: page.screenshot(path=page_path)) + result['page_file_equals_return'] = page_path.read_bytes() == data + capture('page_none', lambda: page.screenshot(caret=None)) + capture('locator_default', lambda: page.locator('#field').screenshot()) + handle = page.locator('#field').element_handle() + capture('handle_default', lambda: handle.screenshot()) + handle.dispose() + clip = capture('page_clip_initial', lambda: page.screenshot( + caret='initial', clip={'x':0, 'y':0, 'width':160, 'height':100}, scale='css')) + result['clip_dimensions'] = list(struct.unpack('>II', clip[16:24])) + jpeg_path = directory / 'caret-page.jpg' + jpeg = capture('page_jpeg_initial', lambda: page.screenshot( + path=jpeg_path, caret='initial', quality=65)) + result['jpeg_magic'] = jpeg.startswith(b'\\xff\\xd8\\xff') + result['jpeg_file_equals_return'] = jpeg_path.read_bytes() == jpeg + capture('page_explicit_hide', lambda: page.screenshot(caret='hide')) + capture('locator_explicit_hide', lambda: page.locator('#field').screenshot(caret='hide')) + (directory / 'screenshot-caret.json').write_text(json.dumps(result, indent=2) + '\\n') + print('SCREENSHOT_CARET_OK') + ''') + try: + execution = run_skill(script, backend="extension", runtime_dir=e2e_daemon.runtime_dir, + extra_env={"BD_SESSION": sid}, timeout=90) + evidence = {"returncode": execution.returncode, "stdout": execution.stdout, + "stderr": execution.stderr} + if artifact.exists(): + evidence["observations"] = json.loads(artifact.read_text()) + artifact.write_text(json.dumps(evidence, indent=2) + "\n") + assert execution.returncode == 0, evidence + result = evidence["observations"] + for name in ("page_default", "page_none", "locator_default", "handle_default", + "page_clip_initial", "page_jpeg_initial"): + case = result["cases"][name] + assert case["mutations"] == [], (name, case) + assert case["styles"] == result["initial"], (name, case) + assert case["bytes"] > 100, (name, case) + if name != "page_jpeg_initial": + assert case["png"] is True, (name, case) + for name in ("page_explicit_hide", "locator_explicit_hide"): + case = result["cases"][name] + assert any("transparent" in (mutation["before"] or "") + for mutation in case["mutations"]), (name, case) + # Native CSSOM restoration normalizes inline declaration spacing. + # Its explicit hiding must restore the original computed colors. + assert [style['computed'] for style in case['styles']] == [ + style['computed'] for style in result['initial']], (name, case) + assert result["page_file_equals_return"] is True, result + assert result["jpeg_file_equals_return"] is True, result + assert result["jpeg_magic"] is True, result + assert result["clip_dimensions"] == [160, 100], result + finally: + drop_ledger_session(bs_home("extension"), sid) From 9fb15975b53444a1f2ea6fce2886d80eec464c5e Mon Sep 17 00:00:00 2001 From: Randy Date: Wed, 7 Oct 2026 21:31:31 +0800 Subject: [PATCH 4/4] fix(bind): require a live primary session before accepting a Page match Identity matching reads the top-frame id over a fresh auxiliary CDP session, which answers even when the Page's own primary session has gone stale. After an extension service-worker replacement the tab is re-attached without Playwright's domain subscriptions, so the executor's issue #86 in-place rebind accepted the stale Page and the next page.goto hung at commit (test_extension_sw_loss_orphans_resident_executor). The former main-world marker probe rejected such Pages implicitly. Now the single identity match must also answer a constant evaluation on its own session; otherwise there is no usable Page and the existing recycle-and-recover path runs, as on main. The probe reads and writes no page state. Co-Authored-By: Claude --- src/browserwright/repl/playwright_handle.py | 24 ++++++++++++++++++++- 1 file changed, 23 insertions(+), 1 deletion(-) diff --git a/src/browserwright/repl/playwright_handle.py b/src/browserwright/repl/playwright_handle.py index f7fc947..50cc1cb 100644 --- a/src/browserwright/repl/playwright_handle.py +++ b/src/browserwright/repl/playwright_handle.py @@ -546,6 +546,13 @@ def page_for_target(context: Any, sess: Any, target_id: str, attachments are independent of Playwright's primary session and never place state in the document. Lightweight test doubles without an agent CDP surface retain the strict unique-URL fallback. + + Identity alone does not prove the Page is usable: the auxiliary probe is a + fresh attachment, so it answers even when the Page's PRIMARY session went + stale (e.g. the extension service worker was replaced and the tab was + re-attached without Playwright's domain subscriptions). The single + identity match must therefore also answer a round-trip on its own primary + session (``_page_responds``) before it is accepted. """ pages = list(context.pages) if not pages: @@ -560,7 +567,9 @@ def page_for_target(context: Any, sess: Any, target_id: str, else: matches = [page for page in pages if _target_id_for_page(page) == target_id] - return matches[0] if len(matches) == 1 else None + if len(matches) == 1 and _page_responds(matches[0]): + return matches[0] + return None # Test-double compatibility only: without an agent CDP path, require a # unique URL match. Never use the old singleton/last-match heuristics. @@ -575,6 +584,19 @@ def page_for_target(context: Any, sess: Any, target_id: str, return matches[0] if len(matches) == 1 else None +def _page_responds(page: Any) -> bool: + """Does the Page's own (primary) session still serve evaluation? + + A constant expression: it reads and writes no page state. A stale primary + session has no live execution context and fails fast with a + TargetClosed-family error, which the caller treats as "no usable Page". + """ + try: + return page.evaluate("() => true") is True + except Exception: + return False + + def _target_id_for_page(page: Any) -> str | None: """Read exact CDP target identity without document state or URL guesses.""" from .isolated_world import target_id_for_page