From 3dbb897df732e2f8f1c0e7ab114b02a5078ca023 Mon Sep 17 00:00:00 2001 From: Richard Abrich Date: Thu, 3 Sep 2026 19:47:10 -0400 Subject: [PATCH 1/2] fix: wait for a quiet frame tree before keeping a secret-masked screenshot Three immediate retries lost to SPA login pages that attach and detach iframes after Sign in. Retry until the tree is still, up to eight seconds. Discard every PNG taken while frames moved. Still refuse if it never settles. Opened by an agent session, not the founder. --- openadapt_flow/backends/playwright_backend.py | 19 +++++++++- tests/test_browser_attach.py | 38 ++++++++++++++++++- 2 files changed, 54 insertions(+), 3 deletions(-) diff --git a/openadapt_flow/backends/playwright_backend.py b/openadapt_flow/backends/playwright_backend.py index 478cc1fd..cc1fc0cc 100644 --- a/openadapt_flow/backends/playwright_backend.py +++ b/openadapt_flow/backends/playwright_backend.py @@ -12,6 +12,7 @@ import hmac import math import re +import time import uuid from dataclasses import dataclass, field from datetime import datetime, timezone @@ -33,7 +34,11 @@ ) VIEWPORT: tuple[int, int] = (1280, 800) -_MASKED_SCREENSHOT_ATTEMPTS = 3 +# Secret-masked screenshots must not keep bytes from a changing frame tree. +# Login pages often attach and detach iframes for a few seconds after a click. +# Retry until the tree is quiet. Still refuse if it never settles. +_MASKED_SCREENSHOT_TIMEOUT_S = 8.0 +_MASKED_SCREENSHOT_RETRY_SLEEP_S = 0.05 _MODIFIER_ALIASES = { "meta": "Meta", @@ -2300,10 +2305,14 @@ def screenshot(self) -> bytes: if not self._screenshot_mask_selectors: return self.page.screenshot(type="png", full_page=False, **base_options) - for _attempt in range(_MASKED_SCREENSHOT_ATTEMPTS): + deadline = time.monotonic() + _MASKED_SCREENSHOT_TIMEOUT_S + while True: generation = self._screenshot_frame_generation frames = tuple(self.page.frames) if generation != self._screenshot_frame_generation: + if time.monotonic() >= deadline: + break + time.sleep(_MASKED_SCREENSHOT_RETRY_SLEEP_S) continue options = dict(base_options) options["mask"] = [ @@ -2319,6 +2328,9 @@ def screenshot(self) -> bytes: self.page.evaluate("() => null") except Exception: if generation != self._screenshot_frame_generation: + if time.monotonic() >= deadline: + break + time.sleep(_MASKED_SCREENSHOT_RETRY_SLEEP_S) continue raise current_frames = tuple(self.page.frames) @@ -2328,6 +2340,9 @@ def screenshot(self) -> bytes: return png # ``png`` is intentionally discarded here. It never reaches the # recorder, disk, or a compiled bundle. + if time.monotonic() >= deadline: + break + time.sleep(_MASKED_SCREENSHOT_RETRY_SLEEP_S) raise ScreenshotMaskStabilityError( "the browser frame tree changed during every secret-masked " "screenshot attempt; recording was refused" diff --git a/tests/test_browser_attach.py b/tests/test_browser_attach.py index d23199b5..6d418d6e 100644 --- a/tests/test_browser_attach.py +++ b/tests/test_browser_attach.py @@ -2284,9 +2284,45 @@ def attach_and_detach_during_every_capture(**kwargs): "screenshot", attach_and_detach_during_every_capture, ) + patch_context.setattr( + "openadapt_flow.backends.playwright_backend._MASKED_SCREENSHOT_TIMEOUT_S", + 0.0, + ) + patch_context.setattr( + "openadapt_flow.backends.playwright_backend._MASKED_SCREENSHOT_RETRY_SLEEP_S", + 0.0, + ) with pytest.raises(ScreenshotMaskStabilityError, match="frame tree"): race_backend.screenshot() - assert churn_attempts == 3 + assert churn_attempts >= 1 + + settle_attempts = 0 + + def churn_twice_then_keep(**kwargs): + nonlocal settle_attempts + settle_attempts += 1 + if settle_attempts <= 2: + race_page.evaluate( + """attempt => { + const frame = document.createElement('iframe'); + frame.id = `settle-${attempt}`; + frame.srcdoc = ''; + document.body.appendChild(frame); + frame.remove(); + }""", + settle_attempts, + ) + return original_screenshot(**kwargs) + + with monkeypatch.context() as patch_context: + patch_context.setattr( + race_page, + "screenshot", + churn_twice_then_keep, + ) + png = race_backend.screenshot() + assert png + assert settle_attempts >= 3 finally: if frame_race_session.page is not None: frame_race_session.page.evaluate( From 2528c58f6dafba97624ae6de494798177e6e048d Mon Sep 17 00:00:00 2001 From: Richard Abrich Date: Thu, 3 Sep 2026 19:49:37 -0400 Subject: [PATCH 2/2] test: reproduce secret-mask refuse after three iframe storms A fake login page attaches and detaches iframes for the old three-try budget, then goes quiet. That is the ServiceNow Sign-in failure. The record must keep a masked frame. A tree that never settles still refuses. Opened by an agent session, not the founder. --- tests/test_browser_attach.py | 116 +++++++++++++++++++++++++++++++++++ 1 file changed, 116 insertions(+) diff --git a/tests/test_browser_attach.py b/tests/test_browser_attach.py index 6d418d6e..1c31a551 100644 --- a/tests/test_browser_attach.py +++ b/tests/test_browser_attach.py @@ -268,6 +268,122 @@ def screenshot(self, **kwargs): assert not any(page.listeners.values()) +def test_masked_screenshot_survives_three_iframe_storms_then_settles( + monkeypatch: pytest.MonkeyPatch, +) -> None: + """Reproduce the ServiceNow Sign-in refuse with a fake page. + + After Sign in, a login SPA attaches and detaches iframes. The old loop + tried three captures with no wait and then raised + ScreenshotMaskStabilityError. This page storms for those three captures + and then goes quiet. The record must keep a masked frame, not stop. + """ + + class Frame: + def __init__(self, name: str) -> None: + self.name = name + + def locator(self, selector): + return f"locator:{self.name}:{selector}" + + class Page: + def __init__(self) -> None: + self.frames = [Frame("main")] + self.listeners: dict[str, list] = {} + self.capture_count = 0 + + def on(self, event, listener): + self.listeners.setdefault(event, []).append(listener) + + def remove_listener(self, event, listener): + self.listeners[event].remove(listener) + + def evaluate(self, _script): + return None + + def screenshot(self, **kwargs): + self.capture_count += 1 + if self.capture_count <= 3: + frame = Frame(f"storm-{self.capture_count}") + self.frames.append(frame) + for listener in self.listeners.get("frameattached", []): + listener(frame) + self.frames.pop() + for listener in self.listeners.get("framedetached", []): + listener(frame) + return b"png" + + monkeypatch.setattr( + "openadapt_flow.backends.playwright_backend._MASKED_SCREENSHOT_RETRY_SLEEP_S", + 0.0, + ) + page = Page() + backend = PlaywrightBackend( # type: ignore[arg-type] + page, + screenshot_mask_selectors=("input[type='password']",), + ) + assert backend.screenshot() == b"png" + assert page.capture_count >= 4 + backend.stop_screenshot_mask_tracking() + + +def test_masked_screenshot_still_refuses_a_frame_tree_that_never_settles( + monkeypatch: pytest.MonkeyPatch, +) -> None: + """A tree that never goes quiet must still refuse. Do not keep those bytes.""" + + class Frame: + def __init__(self, name: str) -> None: + self.name = name + + def locator(self, selector): + return f"locator:{self.name}:{selector}" + + class Page: + def __init__(self) -> None: + self.frames = [Frame("main")] + self.listeners: dict[str, list] = {} + self.capture_count = 0 + + def on(self, event, listener): + self.listeners.setdefault(event, []).append(listener) + + def remove_listener(self, event, listener): + self.listeners[event].remove(listener) + + def evaluate(self, _script): + return None + + def screenshot(self, **kwargs): + self.capture_count += 1 + frame = Frame(f"churn-{self.capture_count}") + self.frames.append(frame) + for listener in self.listeners.get("frameattached", []): + listener(frame) + self.frames.pop() + for listener in self.listeners.get("framedetached", []): + listener(frame) + return b"png" + + monkeypatch.setattr( + "openadapt_flow.backends.playwright_backend._MASKED_SCREENSHOT_TIMEOUT_S", + 0.0, + ) + monkeypatch.setattr( + "openadapt_flow.backends.playwright_backend._MASKED_SCREENSHOT_RETRY_SLEEP_S", + 0.0, + ) + page = Page() + backend = PlaywrightBackend( # type: ignore[arg-type] + page, + screenshot_mask_selectors=("input[type='password']",), + ) + with pytest.raises(ScreenshotMaskStabilityError, match="frame tree"): + backend.screenshot() + assert page.capture_count >= 1 + backend.stop_screenshot_mask_tracking() + + def test_declared_secret_selectors_use_css_string_escaping() -> None: selectors = _secret_screenshot_selectors({"päss", 'quote"\\line\nend'})