From 7ad7d1f20d01e12a43d2982cb27f545de4b65446 Mon Sep 17 00:00:00 2001 From: autonomic-bot Date: Thu, 11 Jun 2026 06:24:01 +0000 Subject: [PATCH] =?UTF-8?q?fix(shot):=20A1=20=E2=80=94=20blank-retry=20kee?= =?UTF-8?q?ps=20the=20LARGER=20frame=20(retry=20snapped=20to=20temp=20path?= =?UTF-8?q?,=20os.replace=20only=20if=20>=3D=20first;=20worse=20late=20fra?= =?UTF-8?q?me=20discarded=20+=20temp=20cleaned);=20regression=20test=20[99?= =?UTF-8?q?99,4801]->9999;=20207=20unit=20tests=20pass,=20lint=20PASS?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- runner/harness/screenshot.py | 20 ++++++++++++++++---- tests/unit/test_screenshot.py | 13 +++++++++++++ 2 files changed, 29 insertions(+), 4 deletions(-) diff --git a/runner/harness/screenshot.py b/runner/harness/screenshot.py index f7936ef..15c1867 100644 --- a/runner/harness/screenshot.py +++ b/runner/harness/screenshot.py @@ -66,7 +66,9 @@ def settle(page, idle_timeout_ms: int = SETTLE_TIMEOUT_MS) -> None: def _snap_with_blank_retry(page, out_path: str) -> None: """Screenshot the page; if the PNG is blank/spinner-sized, retry ONCE after a longer settle. - The retry overwrites the tiny frame with a strictly-later one (same page, more paint time).""" + The retry is snapped to a temp path and kept only if it is >= the first frame's size — later + is usually more painted, but a page can also regress (redirect, error overlay) and a worse + frame must never overwrite a better one (adversary finding A1).""" page.screenshot(path=out_path, full_page=False) try: first = os.path.getsize(out_path) @@ -80,9 +82,19 @@ def _snap_with_blank_retry(page, out_path: str) -> None: flush=True, ) _settle(page, BLANK_RETRY_SETTLE_MS) - page.screenshot(path=out_path, full_page=False) - with contextlib.suppress(OSError): - print(f" screenshot: retry frame {os.path.getsize(out_path)} B", flush=True) + retry_path = out_path + ".retry" + try: + page.screenshot(path=retry_path, full_page=False) + retry = os.path.getsize(retry_path) + if retry >= first: + os.replace(retry_path, out_path) + print(f" screenshot: retry frame kept ({retry} B >= {first} B)", flush=True) + else: + os.remove(retry_path) + print(f" screenshot: retry frame discarded ({retry} B < {first} B)", flush=True) + finally: + with contextlib.suppress(OSError): + os.remove(retry_path) def screenshot_path(run_artifact_dir: str) -> str: diff --git a/tests/unit/test_screenshot.py b/tests/unit/test_screenshot.py index a25ca5a..cb5b238 100644 --- a/tests/unit/test_screenshot.py +++ b/tests/unit/test_screenshot.py @@ -94,6 +94,19 @@ def test_snap_retry_keeps_late_frame_even_if_still_blank(tmp_path): S._snap_with_blank_retry(page, out) assert page.shots == 2 assert os.path.getsize(out) == 4801 + assert not os.path.exists(out + ".retry"), "temp retry frame must be cleaned up" + + +def test_snap_retry_never_regresses_to_smaller_frame(tmp_path): + """Adversary finding A1: a partial-but-real first frame (just under the threshold) must + survive a retry that comes back WORSE (page regressed to blank during the extra settle) — + the larger frame wins.""" + out = str(tmp_path / "shot.png") + page = _FakePage([9999, 4801]) + S._snap_with_blank_retry(page, out) + assert page.shots == 2 + assert os.path.getsize(out) == 9999, "retry must never overwrite a larger frame (A1)" + assert not os.path.exists(out + ".retry"), "temp retry frame must be cleaned up" def test_blank_threshold_brackets_observed_sizes():