From a1aa9833247ef1c4aba48d0e69d91d2fd0117ad6 Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Mon, 10 Aug 2026 20:37:25 +0900 Subject: [PATCH 01/22] test(mv3): require ephemeral profile isolation evidence --- tests/test_mv3_ephemeral_profile_contract.py | 79 ++++++++++++++++++++ 1 file changed, 79 insertions(+) create mode 100644 tests/test_mv3_ephemeral_profile_contract.py diff --git a/tests/test_mv3_ephemeral_profile_contract.py b/tests/test_mv3_ephemeral_profile_contract.py new file mode 100644 index 000000000..8ea105321 --- /dev/null +++ b/tests/test_mv3_ephemeral_profile_contract.py @@ -0,0 +1,79 @@ +"""Fail-first contract for bounded ephemeral Chromium profile evidence.""" + +from __future__ import annotations + +import importlib.util +import pathlib +import unittest +from unittest import mock + +ROOT = pathlib.Path(__file__).resolve().parents[1] +RUNNER_PATH = ROOT / "scripts" / "ci" / "run_mv3_compatibility.py" + + +def _load_runner(): + """Load the compatibility runner without executing its command-line entry point.""" + + spec = importlib.util.spec_from_file_location("originweave_mv3_runner", RUNNER_PATH) + if spec is None or spec.loader is None: + raise RuntimeError("unable to load MV3 compatibility runner") + module = importlib.util.module_from_spec(spec) + spec.loader.exec_module(module) + return module + + +class ManifestV3EphemeralProfileContractTests(unittest.TestCase): + """Require each Chromium trial to prove isolated profile creation and cleanup.""" + + def test_restart_trial_reports_isolated_ephemeral_profile_cleanup(self) -> None: + """Trial evidence must prove an empty new profile and deletion after browser use.""" + + runner = _load_runner() + observed_profiles: list[pathlib.Path] = [] + call_count = 0 + + def fake_browser_pass( + _chrome_bin: pathlib.Path, + _chromedriver_bin: pathlib.Path, + _fixture_url: str, + profile_dir: str, + expected_storage_persistence: str, + ) -> dict[str, object]: + nonlocal call_count + profile_path = pathlib.Path(profile_dir) + if call_count == 0: + self.assertTrue(profile_path.is_dir()) + self.assertEqual(list(profile_path.iterdir()), []) + profile_path.joinpath("profile-created-by-browser").write_text( + "fixture", encoding="utf-8" + ) + else: + self.assertEqual(profile_path, observed_profiles[0]) + self.assertTrue(profile_path.joinpath("profile-created-by-browser").is_file()) + observed_profiles.append(profile_path) + call_count += 1 + return { + "browser_version": runner.PINNED_CHROME_VERSION, + "worker_start_count": call_count, + "storage_persistence": expected_storage_persistence, + "surfaces": {"fixture": True}, + } + + with mock.patch.object(runner, "_run_browser_pass", side_effect=fake_browser_pass): + result = runner._run_restart_trial( + pathlib.Path("/unused/chrome"), + pathlib.Path("/unused/chromedriver"), + "http://127.0.0.1/fixture", + 1, + ) + + self.assertEqual(len(observed_profiles), 2) + self.assertFalse(observed_profiles[0].exists()) + surfaces = result["surfaces"] + self.assertIsInstance(surfaces, dict) + self.assertIs(surfaces.get("ephemeral-profile-isolation"), True) + self.assertIs(surfaces.get("ephemeral-profile-cleanup"), True) + + +if __name__ == "__main__": + unittest.main() From 96a4e949d96b5794ef473ccf813987b8e69ea566 Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Mon, 10 Aug 2026 20:41:58 +0900 Subject: [PATCH 02/22] test(mv3): prove ephemeral profile lifecycle directly --- tests/test_mv3_ephemeral_profile_contract.py | 15 +++++++-------- 1 file changed, 7 insertions(+), 8 deletions(-) diff --git a/tests/test_mv3_ephemeral_profile_contract.py b/tests/test_mv3_ephemeral_profile_contract.py index 8ea105321..d50954aa7 100644 --- a/tests/test_mv3_ephemeral_profile_contract.py +++ b/tests/test_mv3_ephemeral_profile_contract.py @@ -1,4 +1,4 @@ -"""Fail-first contract for bounded ephemeral Chromium profile evidence.""" +"""Regression contract for bounded ephemeral Chromium profile lifecycle.""" from __future__ import annotations @@ -23,10 +23,10 @@ def _load_runner(): class ManifestV3EphemeralProfileContractTests(unittest.TestCase): - """Require each Chromium trial to prove isolated profile creation and cleanup.""" + """Prove each Chromium trial creates, reuses, and deletes one isolated profile.""" - def test_restart_trial_reports_isolated_ephemeral_profile_cleanup(self) -> None: - """Trial evidence must prove an empty new profile and deletion after browser use.""" + def test_restart_trial_uses_empty_profile_then_deletes_it(self) -> None: + """A trial must start empty, reuse only its own profile, then remove it.""" runner = _load_runner() observed_profiles: list[pathlib.Path] = [] @@ -68,11 +68,10 @@ def fake_browser_pass( ) self.assertEqual(len(observed_profiles), 2) + self.assertEqual(observed_profiles[0], observed_profiles[1]) self.assertFalse(observed_profiles[0].exists()) - surfaces = result["surfaces"] - self.assertIsInstance(surfaces, dict) - self.assertIs(surfaces.get("ephemeral-profile-isolation"), True) - self.assertIs(surfaces.get("ephemeral-profile-cleanup"), True) + self.assertNotIn(str(observed_profiles[0]), repr(result)) + self.assertIs(result.get("passed"), True) if __name__ == "__main__": From f0875d1b07be44376e01025f34904e9cf2ba2ce1 Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Tue, 11 Aug 2026 08:24:45 +0900 Subject: [PATCH 03/22] test(mv3): require browser process-group cleanup --- tests/test_mv3_ephemeral_profile_contract.py | 36 ++++++++++++++++++++ 1 file changed, 36 insertions(+) diff --git a/tests/test_mv3_ephemeral_profile_contract.py b/tests/test_mv3_ephemeral_profile_contract.py index d50954aa7..9502f2e88 100644 --- a/tests/test_mv3_ephemeral_profile_contract.py +++ b/tests/test_mv3_ephemeral_profile_contract.py @@ -4,6 +4,8 @@ import importlib.util import pathlib +import signal +import tempfile import unittest from unittest import mock @@ -73,6 +75,40 @@ def fake_browser_pass( self.assertNotIn(str(observed_profiles[0]), repr(result)) self.assertIs(result.get("passed"), True) + def test_browser_pass_owns_and_terminates_the_chromedriver_process_group(self) -> None: + """Failure cleanup must signal the isolated driver group, not only its leader.""" + + runner = _load_runner() + driver = mock.Mock() + driver.pid = 4242 + driver.wait.return_value = 0 + + with tempfile.TemporaryDirectory(prefix="originweave-mv3-cleanup-") as profile_dir: + with ( + mock.patch.object(runner.subprocess, "Popen", return_value=driver) as popen, + mock.patch.object( + runner, + "_wait_for_driver", + side_effect=RuntimeError("controlled startup failure"), + ), + mock.patch.object(runner.os, "killpg") as kill_process_group, + ): + with self.assertRaisesRegex(RuntimeError, "controlled startup failure"): + runner._run_browser_pass( + pathlib.Path("/unused/chrome"), + pathlib.Path("/unused/chromedriver"), + "http://127.0.0.1/fixture", + profile_dir, + "initialized", + ) + + _, popen_kwargs = popen.call_args + self.assertIs(popen_kwargs.get("start_new_session"), True) + kill_process_group.assert_called_once_with(driver.pid, signal.SIGTERM) + driver.wait.assert_called_once_with(timeout=5) + driver.terminate.assert_not_called() + driver.kill.assert_not_called() + if __name__ == "__main__": unittest.main() From 832e4961cc28c5bc226f489177d524193b5809f2 Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Tue, 11 Aug 2026 08:27:05 +0900 Subject: [PATCH 04/22] fix(mv3): terminate isolated browser process group --- scripts/ci/run_mv3_compatibility.py | 24 +++++++++++++++++------- 1 file changed, 17 insertions(+), 7 deletions(-) diff --git a/scripts/ci/run_mv3_compatibility.py b/scripts/ci/run_mv3_compatibility.py index 4cb3c732a..44eb95abf 100755 --- a/scripts/ci/run_mv3_compatibility.py +++ b/scripts/ci/run_mv3_compatibility.py @@ -17,6 +17,7 @@ import json import os import pathlib +import signal import socket import string import subprocess @@ -332,6 +333,19 @@ def _exercise_real_click(driver_port: int, session_id: str) -> str: return str(text) +def _terminate_process_group(driver: subprocess.Popen[str]) -> None: + """Terminate the isolated ChromeDriver process group and all inherited children.""" + + with contextlib.suppress(ProcessLookupError): + os.killpg(driver.pid, signal.SIGTERM) + try: + driver.wait(timeout=5) + except subprocess.TimeoutExpired: + with contextlib.suppress(ProcessLookupError): + os.killpg(driver.pid, signal.SIGKILL) + driver.wait(timeout=5) + + def _run_browser_pass( chrome_bin: pathlib.Path, chromedriver_bin: pathlib.Path, @@ -350,6 +364,7 @@ def _run_browser_pass( stdout=subprocess.DEVNULL, stderr=subprocess.STDOUT, text=True, + start_new_session=True, ) try: _wait_for_driver(driver_port) @@ -444,12 +459,7 @@ def _run_browser_pass( _webdriver_path(session_id, ""), {}, ) - driver.terminate() - try: - driver.wait(timeout=5) - except subprocess.TimeoutExpired: - driver.kill() - driver.wait(timeout=5) + _terminate_process_group(driver) def _run_restart_trial( @@ -610,4 +620,4 @@ def main() -> int: if __name__ == "__main__": - raise SystemExit(main()) + raise SystemExit(main()) \ No newline at end of file From bd79a30636a2c225267a1e7728d708035f30852b Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Sun, 16 Aug 2026 08:09:19 +0900 Subject: [PATCH 05/22] merge: align MV3 download restart fix from prerequisite --- tests/fixtures/mv3_basic/service_worker.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/fixtures/mv3_basic/service_worker.js b/tests/fixtures/mv3_basic/service_worker.js index 37e8129d5..33ac59efd 100644 --- a/tests/fixtures/mv3_basic/service_worker.js +++ b/tests/fixtures/mv3_basic/service_worker.js @@ -87,7 +87,7 @@ async function exerciseDownload(sender) { downloadId = await chrome.downloads.download({ url, filename: "originweave-mv3/download.txt", - conflictAction: "overwrite", + conflictAction: "uniquify", saveAs: false, }); } catch (_error) { From 41b184d73c945a01a2c6eb5c2c43307a9062b86e Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Sun, 16 Aug 2026 08:09:53 +0900 Subject: [PATCH 06/22] merge: preserve prerequisite MV3 restart regression --- tests/test_mv3_downloads_contract.py | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/tests/test_mv3_downloads_contract.py b/tests/test_mv3_downloads_contract.py index 94696376c..53218b965 100644 --- a/tests/test_mv3_downloads_contract.py +++ b/tests/test_mv3_downloads_contract.py @@ -50,6 +50,13 @@ def test_service_worker_executes_and_verifies_a_real_loopback_download(self) -> self.assertIn(expected, worker) self.assertNotIn('chrome.runtime.getURL("download.txt")', worker) + def test_restart_pair_never_overwrites_the_previous_controlled_download(self) -> None: + """Restart evidence must not race Chrome while replacing the first pass's file.""" + + worker = (FIXTURE / "service_worker.js").read_text(encoding="utf-8") + self.assertIn('conflictAction: "uniquify"', worker) + self.assertNotIn('conflictAction: "overwrite"', worker) + def test_download_failures_emit_only_bounded_stage_diagnostics(self) -> None: """Fixture diagnostics must name a reviewed stage without retaining raw browser errors.""" From 90d9ed7f79858bed42963443c2cb584de9dc0f8a Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Sun, 16 Aug 2026 22:12:52 +0900 Subject: [PATCH 07/22] test(mv3): inherit cleanup import normalization --- tests/test_mv3_session_cleanup_exception_contract.py | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/tests/test_mv3_session_cleanup_exception_contract.py b/tests/test_mv3_session_cleanup_exception_contract.py index 57fc943a3..3c30557bc 100644 --- a/tests/test_mv3_session_cleanup_exception_contract.py +++ b/tests/test_mv3_session_cleanup_exception_contract.py @@ -7,7 +7,7 @@ import signal import tempfile import unittest -from unittest import mock +import unittest.mock ROOT = pathlib.Path(__file__).resolve().parents[1] RUNNER = ROOT / "scripts" / "ci" / "run_mv3_compatibility.py" @@ -89,9 +89,13 @@ def fake_json_request( with tempfile.TemporaryDirectory(prefix="originweave-cleanup-contract-") as profile_dir: with ( - mock.patch.object(globals_["subprocess"], "Popen", return_value=fake_driver) as popen, - mock.patch.object(globals_["os"], "killpg") as kill_process_group, - mock.patch.dict( + unittest.mock.patch.object( + globals_["subprocess"], "Popen", return_value=fake_driver + ) as popen, + unittest.mock.patch.object( + globals_["os"], "killpg" + ) as kill_process_group, + unittest.mock.patch.dict( globals_, { "_free_loopback_port": lambda: 43123, From 3dc0cfd1fc6636ab3b258e00e01715f96c7ca535 Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Mon, 17 Aug 2026 05:50:53 +0900 Subject: [PATCH 08/22] test(mv3): preserve cleanup cause across group teardown failure --- ..._mv3_session_cleanup_exception_contract.py | 88 +++++++++++++------ 1 file changed, 63 insertions(+), 25 deletions(-) diff --git a/tests/test_mv3_session_cleanup_exception_contract.py b/tests/test_mv3_session_cleanup_exception_contract.py index 3c30557bc..9b43f30a9 100644 --- a/tests/test_mv3_session_cleanup_exception_contract.py +++ b/tests/test_mv3_session_cleanup_exception_contract.py @@ -36,13 +36,40 @@ def wait(self, timeout: float) -> int: class ManifestV3SessionCleanupExceptionTests(unittest.TestCase): """Unexpected cleanup failures must remain visible after process-group teardown.""" - def test_unreviewed_session_cleanup_exception_is_not_silently_suppressed(self) -> None: - """A new exception class must propagate after bounded process-group cleanup.""" + @staticmethod + def _surfaces() -> dict[str, str]: + """Return one fully passing controlled compatibility surface set.""" + + return { + "workerStartCount": "1", + "storagePersistence": "initialized", + "workerReply": "pong", + "content": "ready", + "storage": "ready", + "dnr": "blocked", + "tabs": "ready", + "windows": "ready", + "scripting": "ready", + "scriptingExecuted": "ready", + "commands": "ready", + "sidePanel": "ready", + "bookmarks": "ready", + "history": "ready", + "downloads": "ready", + } + + def _run_with_cleanup_failure( + self, + cleanup_failure: Exception, + fake_driver: _FakeDriver, + *, + killpg_side_effect: Exception | None = None, + ) -> tuple[object, object, object, object]: + """Run the production browser-pass boundary with controlled cleanup failures.""" namespace = runpy.run_path(str(RUNNER), run_name="mv3_cleanup_contract") run_browser_pass = namespace["_run_browser_pass"] globals_ = run_browser_pass.__globals__ - fake_driver = _FakeDriver() def fake_json_request( _driver_port: int, @@ -66,34 +93,16 @@ def fake_json_request( if method == "POST" and path.endswith("/url"): return {"value": None} if method == "DELETE" and path.endswith("/session/session-1"): - raise _UnexpectedCleanupFailure("must not be normalized") + raise cleanup_failure raise AssertionError(f"unexpected WebDriver request: {method} {path}") - surfaces = { - "workerStartCount": "1", - "storagePersistence": "initialized", - "workerReply": "pong", - "content": "ready", - "storage": "ready", - "dnr": "blocked", - "tabs": "ready", - "windows": "ready", - "scripting": "ready", - "scriptingExecuted": "ready", - "commands": "ready", - "sidePanel": "ready", - "bookmarks": "ready", - "history": "ready", - "downloads": "ready", - } - with tempfile.TemporaryDirectory(prefix="originweave-cleanup-contract-") as profile_dir: with ( unittest.mock.patch.object( globals_["subprocess"], "Popen", return_value=fake_driver ) as popen, unittest.mock.patch.object( - globals_["os"], "killpg" + globals_["os"], "killpg", side_effect=killpg_side_effect ) as kill_process_group, unittest.mock.patch.dict( globals_, @@ -102,13 +111,13 @@ def fake_json_request( "_wait_for_driver": lambda _port: None, "_json_request": fake_json_request, "_wait_for_extension_evidence": ( - lambda _port, _session, _expected: surfaces + lambda _port, _session, _expected: self._surfaces() ), "_exercise_real_click": lambda _port, _session: "clicked", }, ), ): - with self.assertRaises(_UnexpectedCleanupFailure): + try: run_browser_pass( pathlib.Path("/controlled/chrome"), pathlib.Path("/controlled/chromedriver"), @@ -116,12 +125,41 @@ def fake_json_request( profile_dir, "initialized", ) + except Exception as error: # noqa: BLE001 - return exact boundary error. + return namespace, error, popen, kill_process_group + raise AssertionError("cleanup failure unexpectedly became success") + + def test_unreviewed_session_cleanup_exception_is_not_silently_suppressed(self) -> None: + """A new exception class must propagate after bounded process-group cleanup.""" + fake_driver = _FakeDriver() + expected = _UnexpectedCleanupFailure("must not be normalized") + _namespace, error, popen, kill_process_group = self._run_with_cleanup_failure( + expected, + fake_driver, + ) + + self.assertIs(error, expected) _, popen_kwargs = popen.call_args self.assertIs(popen_kwargs.get("start_new_session"), True) kill_process_group.assert_called_once_with(fake_driver.pid, signal.SIGTERM) self.assertEqual(fake_driver.wait_timeouts, [5]) + def test_reviewed_cleanup_error_survives_process_group_signal_failure(self) -> None: + """A later process-group signal error must not replace a reviewed session failure.""" + + fake_driver = _FakeDriver() + session_error = RuntimeError("session delete failed") + namespace, error, _popen, kill_process_group = self._run_with_cleanup_failure( + session_error, + fake_driver, + killpg_side_effect=PermissionError("process-group signal denied"), + ) + + self.assertIsInstance(error, namespace["WebDriverSessionCleanupError"]) + self.assertIs(error.__cause__, session_error) + kill_process_group.assert_called_once_with(fake_driver.pid, signal.SIGTERM) + if __name__ == "__main__": unittest.main() From f1169ddf440f81e3e5e0adcb3fc6893fd99ab342 Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Wed, 19 Aug 2026 01:10:10 -0700 Subject: [PATCH 09/22] test(mv3): preserve ChromeDriver status authority on stack realignment --- ...st_mv3_driver_status_authority_contract.py | 87 +++++++++++++++++++ 1 file changed, 87 insertions(+) create mode 100644 tests/test_mv3_driver_status_authority_contract.py diff --git a/tests/test_mv3_driver_status_authority_contract.py b/tests/test_mv3_driver_status_authority_contract.py new file mode 100644 index 000000000..f57509d0a --- /dev/null +++ b/tests/test_mv3_driver_status_authority_contract.py @@ -0,0 +1,87 @@ +"""Regression contracts for bounded ChromeDriver startup/status authority.""" + +from __future__ import annotations + +import pathlib +import runpy +import unittest +import unittest.mock + +ROOT = pathlib.Path(__file__).resolve().parents[1] +RUNNER = ROOT / "scripts" / "ci" / "run_mv3_compatibility.py" + + +class ManifestV3DriverStatusAuthorityTests(unittest.TestCase): + """Startup evidence must come from the pinned ChromeDriver status shape.""" + + def test_ready_status_rejects_a_different_chromedriver_build(self) -> None: + """A ready loopback endpoint cannot impersonate the pinned driver build.""" + + namespace = runpy.run_path(str(RUNNER), run_name="mv3_driver_status_authority") + wait_for_driver = namespace["_wait_for_driver"] + globals_ = wait_for_driver.__globals__ + + def mismatched_status( + _driver_port: int, + method: str, + path: str, + _payload=None, + *, + timeout: float = 5.0, + ) -> dict[str, object]: + self.assertEqual(method, "GET") + self.assertEqual(path, "/status") + self.assertGreater(timeout, 0) + return { + "value": { + "ready": True, + "build": {"version": "149.0.0.0 (controlled-mismatch)"}, + } + } + + with unittest.mock.patch.dict(globals_, {"_json_request": mismatched_status}): + with self.assertRaisesRegex(RuntimeError, "ChromeDriver status identity mismatch"): + wait_for_driver(43123) + + def test_malformed_status_value_is_bounded_and_retried(self) -> None: + """Malformed external status JSON must not escape as AttributeError.""" + + namespace = runpy.run_path(str(RUNNER), run_name="mv3_driver_status_protocol") + wait_for_driver = namespace["_wait_for_driver"] + globals_ = wait_for_driver.__globals__ + monotonic_values = iter((0.0, 0.0, 21.0)) + status_calls = 0 + + def malformed_status( + _driver_port: int, + method: str, + path: str, + _payload=None, + *, + timeout: float = 5.0, + ) -> dict[str, object]: + nonlocal status_calls + status_calls += 1 + self.assertEqual(method, "GET") + self.assertEqual(path, "/status") + self.assertGreater(timeout, 0) + return {"value": []} + + with ( + unittest.mock.patch.dict(globals_, {"_json_request": malformed_status}), + unittest.mock.patch.object( + globals_["time"], "monotonic", side_effect=lambda: next(monotonic_values) + ), + unittest.mock.patch.object(globals_["time"], "sleep", return_value=None), + ): + with self.assertRaisesRegex( + RuntimeError, + r"ChromeDriver did not become ready \(status_protocol_error\)", + ): + wait_for_driver(43123) + + self.assertEqual(status_calls, 1) + + +if __name__ == "__main__": + unittest.main() From 2fef789c42de292f63ef4113949237f02b5b20bd Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Wed, 19 Aug 2026 01:15:26 -0700 Subject: [PATCH 10/22] fix(mv3): preserve status authority across profile stack --- scripts/ci/run_mv3_compatibility.py | 28 ++++++++++++++++++++++++---- 1 file changed, 24 insertions(+), 4 deletions(-) diff --git a/scripts/ci/run_mv3_compatibility.py b/scripts/ci/run_mv3_compatibility.py index 79e3b449c..676c1aed6 100644 --- a/scripts/ci/run_mv3_compatibility.py +++ b/scripts/ci/run_mv3_compatibility.py @@ -213,17 +213,37 @@ def _json_request( def _wait_for_driver(driver_port: int) -> None: - """Wait for local ChromeDriver readiness while retaining only a safe failure class.""" + """Wait for the exact pinned local ChromeDriver and reject foreign ready endpoints.""" deadline = time.monotonic() + STARTUP_TIMEOUT_SECONDS last_failure_kind = "not_observed" while time.monotonic() < deadline: try: status = _json_request(driver_port, "GET", "/status", timeout=1.0) - if status.get("value", {}).get("ready") is True: - return except (OSError, ValueError, RuntimeError, json.JSONDecodeError) as exc: last_failure_kind = str(_failure_evidence(exc)["failure_kind"]) + time.sleep(0.1) + continue + + status_value = status.get("value") + if not isinstance(status_value, dict): + last_failure_kind = "status_protocol_error" + time.sleep(0.1) + continue + + ready = status_value.get("ready") + if ready is True: + build = status_value.get("build") + build_version = build.get("version") if isinstance(build, dict) else None + expected_prefix = f"{PINNED_CHROME_VERSION} (" + if not isinstance(build_version, str) or not ( + build_version == PINNED_CHROME_VERSION + or build_version.startswith(expected_prefix) + ): + raise RuntimeError("ChromeDriver status identity mismatch") + return + if ready is not False: + last_failure_kind = "status_protocol_error" time.sleep(0.1) raise RuntimeError( f"ChromeDriver did not become ready ({last_failure_kind})" @@ -793,4 +813,4 @@ def main() -> int: if __name__ == "__main__": - raise SystemExit(main()) + raise SystemExit(main()) \ No newline at end of file From 336a0ad9899f1dc2348201af7f8f9686c355f72e Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Wed, 19 Aug 2026 03:09:21 -0700 Subject: [PATCH 11/22] test(mv3): restore atomic ChromeDriver port authority regression --- tests/test_mv3_driver_status_authority_contract.py | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/tests/test_mv3_driver_status_authority_contract.py b/tests/test_mv3_driver_status_authority_contract.py index f57509d0a..f7bc97bf4 100644 --- a/tests/test_mv3_driver_status_authority_contract.py +++ b/tests/test_mv3_driver_status_authority_contract.py @@ -14,6 +14,14 @@ class ManifestV3DriverStatusAuthorityTests(unittest.TestCase): """Startup evidence must come from the pinned ChromeDriver status shape.""" + def test_driver_owns_ephemeral_port_selection_without_a_release_bind_race(self) -> None: + """ChromeDriver itself must bind port zero instead of racing on a released probe port.""" + + source = RUNNER.read_text(encoding="utf-8") + self.assertNotIn("def _free_loopback_port", source) + self.assertNotIn("driver_port = _free_loopback_port()", source) + self.assertIn('"--port=0"', source) + def test_ready_status_rejects_a_different_chromedriver_build(self) -> None: """A ready loopback endpoint cannot impersonate the pinned driver build.""" From 06f7a90c314d5d13b39cad3a8e31cfcdc78f422a Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Wed, 19 Aug 2026 03:18:27 -0700 Subject: [PATCH 12/22] fix(mv3): preserve atomic driver port with process-group isolation --- scripts/ci/run_mv3_compatibility.py | 108 +++++++++++++++--- ...mv3_browser_version_diagnostic_contract.py | 5 +- tests/test_mv3_ephemeral_profile_contract.py | 10 +- ...st_mv3_primary_failure_cleanup_contract.py | 19 ++- ...test_mv3_process_group_cleanup_contract.py | 5 +- ..._mv3_session_cleanup_exception_contract.py | 27 ++--- 6 files changed, 114 insertions(+), 60 deletions(-) diff --git a/scripts/ci/run_mv3_compatibility.py b/scripts/ci/run_mv3_compatibility.py index 676c1aed6..f0e9267fd 100644 --- a/scripts/ci/run_mv3_compatibility.py +++ b/scripts/ci/run_mv3_compatibility.py @@ -16,8 +16,8 @@ import json import os import pathlib +import queue import signal -import socket import string import subprocess import tempfile @@ -40,6 +40,8 @@ STARTUP_TIMEOUT_SECONDS = 20.0 FIXTURE_TIMEOUT_SECONDS = 20.0 MAX_WEBDRIVER_RESPONSE_BYTES = 1_048_576 +MAX_CHROMEDRIVER_STARTUP_LINE_BYTES = 512 +CHROMEDRIVER_BOUND_PORT_PREFIX = "ChromeDriver was started successfully on port " W3C_ELEMENT_KEY = "element-6066-11e4-a52e-4f735466cecf" PATH_TOKEN_CHARACTERS = frozenset(string.ascii_letters + string.digits + "-_.") SURFACE_EVIDENCE_KEYS = ( @@ -127,14 +129,6 @@ def _failure_evidence(error: BaseException) -> dict[str, Any]: return {"failure_kind": "runtime_error"} -def _free_loopback_port() -> int: - """Reserve and release one loopback TCP port for a short-lived local service.""" - - with socket.socket(socket.AF_INET, socket.SOCK_STREAM) as sock: - sock.bind(("127.0.0.1", 0)) - return int(sock.getsockname()[1]) - - def _path_token(value: str, label: str) -> str: """Validate one ChromeDriver-issued identifier before interpolating a path.""" @@ -457,6 +451,91 @@ def _teardown_driver_process(driver: subprocess.Popen[str]) -> Exception | None: return None +def _start_chromedriver( + chromedriver_bin: pathlib.Path, +) -> tuple[subprocess.Popen[str], int]: + """Let ChromeDriver atomically bind an ephemeral port and report the bound authority. + + The process owns port allocation by binding port zero itself. Its combined output is + continuously drained so the pipe cannot become a back-pressure failure, but only the + reviewed startup-port record is retained. Raw ChromeDriver output never enters evidence. + """ + + driver = subprocess.Popen( + [str(chromedriver_bin), "--port=0", "--allowed-ips=127.0.0.1"], + stdout=subprocess.PIPE, + stderr=subprocess.STDOUT, + text=True, + encoding="utf-8", + errors="replace", + bufsize=1, + start_new_session=True, + ) + if driver.stdout is None: + teardown_error = _teardown_driver_process(driver) + startup_error = RuntimeError("ChromeDriver startup output pipe was unavailable") + if teardown_error is not None: + startup_error.add_note( + "ChromeDriver process teardown also failed: " + f"{type(teardown_error).__name__}" + ) + raise startup_error + + startup_events: queue.Queue[tuple[str, int | None]] = queue.Queue(maxsize=1) + + def publish(event: tuple[str, int | None]) -> None: + try: + startup_events.put_nowait(event) + except queue.Full: + return + + def drain_output() -> None: + for raw_line in driver.stdout: + if not raw_line.startswith(CHROMEDRIVER_BOUND_PORT_PREFIX): + continue + if len(raw_line.encode("utf-8")) > MAX_CHROMEDRIVER_STARTUP_LINE_BYTES: + publish(("invalid", None)) + continue + + line = raw_line.rstrip("\r\n") + if not line.endswith("."): + publish(("invalid", None)) + continue + port_text = line[len(CHROMEDRIVER_BOUND_PORT_PREFIX) : -1] + if not port_text.isdecimal(): + publish(("invalid", None)) + continue + port = int(port_text) + if not 1 <= port <= 65_535: + publish(("invalid", None)) + continue + publish(("ready", port)) + publish(("eof", None)) + + threading.Thread( + target=drain_output, + name="originweave-chromedriver-output-drain", + daemon=True, + ).start() + + try: + event_kind, bound_port = startup_events.get(timeout=STARTUP_TIMEOUT_SECONDS) + except queue.Empty: + event_kind, bound_port = "timeout", None + + if event_kind == "ready" and bound_port is not None: + return driver, bound_port + + teardown_error = _teardown_driver_process(driver) + startup_error = RuntimeError("ChromeDriver did not publish a valid bound port") + if teardown_error is not None: + startup_error.add_note( + "ChromeDriver process teardown also failed: " + f"{type(teardown_error).__name__}" + ) + raise startup_error + + def _run_browser_pass( chrome_bin: pathlib.Path, chromedriver_bin: pathlib.Path, @@ -466,17 +545,10 @@ def _run_browser_pass( ) -> dict[str, Any]: """Run one fresh browser process against a shared bounded compatibility profile.""" - driver_port = _free_loopback_port() session_id: str | None = None download_dir = pathlib.Path(profile_dir) / "downloads" download_dir.mkdir(mode=0o700, parents=True, exist_ok=True) - driver = subprocess.Popen( - [str(chromedriver_bin), f"--port={driver_port}", "--allowed-ips=127.0.0.1"], - stdout=subprocess.DEVNULL, - stderr=subprocess.STDOUT, - text=True, - start_new_session=True, - ) + driver, driver_port = _start_chromedriver(chromedriver_bin) primary_error: BaseException | None = None try: _wait_for_driver(driver_port) @@ -813,4 +885,4 @@ def main() -> int: if __name__ == "__main__": - raise SystemExit(main()) \ No newline at end of file + raise SystemExit(main()) diff --git a/tests/test_mv3_browser_version_diagnostic_contract.py b/tests/test_mv3_browser_version_diagnostic_contract.py index c79da5ac8..c2f78060b 100644 --- a/tests/test_mv3_browser_version_diagnostic_contract.py +++ b/tests/test_mv3_browser_version_diagnostic_contract.py @@ -73,13 +73,10 @@ def fake_json_request( prefix="originweave-browser-version-contract-" ) as profile_dir: with ( - unittest.mock.patch.object( - globals_["subprocess"], "Popen", return_value=fake_driver - ), unittest.mock.patch.dict( globals_, { - "_free_loopback_port": lambda: 43123, + "_start_chromedriver": lambda _binary: (fake_driver, 43123), "_wait_for_driver": lambda _port: None, "_json_request": fake_json_request, }, diff --git a/tests/test_mv3_ephemeral_profile_contract.py b/tests/test_mv3_ephemeral_profile_contract.py index 7e7a09abd..5fc07593c 100644 --- a/tests/test_mv3_ephemeral_profile_contract.py +++ b/tests/test_mv3_ephemeral_profile_contract.py @@ -73,6 +73,9 @@ def fake_browser_pass( def test_browser_pass_owns_and_terminates_the_chromedriver_process_group(self) -> None: """Failure cleanup must signal the isolated driver group, not only its leader.""" + source = RUNNER.read_text(encoding="utf-8") + self.assertIn("start_new_session=True", source) + namespace = runpy.run_path(str(RUNNER), run_name="mv3_process_group_contract") run_browser_pass = namespace["_run_browser_pass"] globals_ = run_browser_pass.__globals__ @@ -82,13 +85,10 @@ def test_browser_pass_owns_and_terminates_the_chromedriver_process_group(self) - with tempfile.TemporaryDirectory(prefix="originweave-mv3-cleanup-") as profile_dir: with ( - unittest.mock.patch.object( - globals_["subprocess"], "Popen", return_value=driver - ) as popen, unittest.mock.patch.dict( globals_, { - "_free_loopback_port": lambda: 43123, + "_start_chromedriver": lambda _binary: (driver, 43123), "_wait_for_driver": unittest.mock.Mock( side_effect=RuntimeError("controlled startup failure") ), @@ -105,8 +105,6 @@ def test_browser_pass_owns_and_terminates_the_chromedriver_process_group(self) - "initialized", ) - _, popen_kwargs = popen.call_args - self.assertIs(popen_kwargs.get("start_new_session"), True) kill_process_group.assert_called_once_with(driver.pid, signal.SIGTERM) driver.wait.assert_called_once_with(timeout=5) driver.terminate.assert_not_called() diff --git a/tests/test_mv3_primary_failure_cleanup_contract.py b/tests/test_mv3_primary_failure_cleanup_contract.py index 133e7159f..8b1644fa3 100644 --- a/tests/test_mv3_primary_failure_cleanup_contract.py +++ b/tests/test_mv3_primary_failure_cleanup_contract.py @@ -77,18 +77,13 @@ def fake_json_request( raise AssertionError(f"unexpected WebDriver request: {method} {path}") with tempfile.TemporaryDirectory(prefix="originweave-primary-cleanup-") as profile_dir: - with ( - unittest.mock.patch.object( - globals_["subprocess"], "Popen", return_value=fake_driver - ), - unittest.mock.patch.dict( - globals_, - { - "_free_loopback_port": lambda: 43123, - "_wait_for_driver": lambda _port: None, - "_json_request": fake_json_request, - }, - ), + with unittest.mock.patch.dict( + globals_, + { + "_start_chromedriver": lambda _binary: (fake_driver, 43123), + "_wait_for_driver": lambda _port: None, + "_json_request": fake_json_request, + }, ): with self.assertRaises(RuntimeError) as raised: run_browser_pass( diff --git a/tests/test_mv3_process_group_cleanup_contract.py b/tests/test_mv3_process_group_cleanup_contract.py index 48188369e..d6ad9cdb9 100644 --- a/tests/test_mv3_process_group_cleanup_contract.py +++ b/tests/test_mv3_process_group_cleanup_contract.py @@ -75,9 +75,6 @@ def fake_json_request( with tempfile.TemporaryDirectory(prefix="originweave-group-cleanup-") as profile_dir: with ( - unittest.mock.patch.object( - globals_["subprocess"], "Popen", return_value=driver - ), unittest.mock.patch.object( globals_["os"], "killpg", @@ -86,7 +83,7 @@ def fake_json_request( unittest.mock.patch.dict( globals_, { - "_free_loopback_port": lambda: 43123, + "_start_chromedriver": lambda _binary: (driver, 43123), "_wait_for_driver": lambda _port: None, "_json_request": fake_json_request, "_wait_for_extension_evidence": ( diff --git a/tests/test_mv3_session_cleanup_exception_contract.py b/tests/test_mv3_session_cleanup_exception_contract.py index 0106438d9..09767d452 100644 --- a/tests/test_mv3_session_cleanup_exception_contract.py +++ b/tests/test_mv3_session_cleanup_exception_contract.py @@ -121,22 +121,17 @@ def fake_json_request( raise AssertionError(f"unexpected WebDriver request: {method} {path}") with tempfile.TemporaryDirectory(prefix="originweave-cleanup-contract-") as profile_dir: - with ( - unittest.mock.patch.object( - globals_["subprocess"], "Popen", return_value=fake_driver - ), - unittest.mock.patch.dict( - globals_, - { - "_free_loopback_port": lambda: 43123, - "_wait_for_driver": lambda _port: None, - "_json_request": fake_json_request, - "_wait_for_extension_evidence": ( - lambda _port, _session, _expected: self._surfaces() - ), - "_exercise_real_click": lambda _port, _session: "clicked", - }, - ), + with unittest.mock.patch.dict( + globals_, + { + "_start_chromedriver": lambda _binary: (fake_driver, 43123), + "_wait_for_driver": lambda _port: None, + "_json_request": fake_json_request, + "_wait_for_extension_evidence": ( + lambda _port, _session, _expected: self._surfaces() + ), + "_exercise_real_click": lambda _port, _session: "clicked", + }, ): try: run_browser_pass( From d1fc75122becf99b6ab55198a6703caaa8ea8a3e Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Wed, 19 Aug 2026 05:35:44 -0700 Subject: [PATCH 13/22] test(mv3): preserve portable ChromeDriver decoding --- ...t_mv3_subprocess_compatibility_contract.py | 44 +++++++++++++++++++ 1 file changed, 44 insertions(+) create mode 100644 tests/test_mv3_subprocess_compatibility_contract.py diff --git a/tests/test_mv3_subprocess_compatibility_contract.py b/tests/test_mv3_subprocess_compatibility_contract.py new file mode 100644 index 000000000..f7eed458e --- /dev/null +++ b/tests/test_mv3_subprocess_compatibility_contract.py @@ -0,0 +1,44 @@ +"""Regression contract for portable, explicit ChromeDriver output decoding.""" + +from __future__ import annotations + +import ast +import pathlib +import unittest + +ROOT = pathlib.Path(__file__).resolve().parents[1] +RUNNER = ROOT / "scripts" / "ci" / "run_mv3_compatibility.py" + + +class ChromeDriverSubprocessCompatibilityContractTests(unittest.TestCase): + """Keep subprocess decoding explicit instead of version-gated Popen kwargs.""" + + def test_chromedriver_popen_uses_binary_pipe_with_explicit_utf8_decode(self) -> None: + """Popen must avoid text-decoding kwargs and decode bounded output explicitly.""" + + source = RUNNER.read_text(encoding="utf-8") + tree = ast.parse(source) + start = next( + node + for node in tree.body + if isinstance(node, ast.FunctionDef) and node.name == "_start_chromedriver" + ) + popen_calls = [ + node + for node in ast.walk(start) + if isinstance(node, ast.Call) + and isinstance(node.func, ast.Attribute) + and isinstance(node.func.value, ast.Name) + and node.func.value.id == "subprocess" + and node.func.attr == "Popen" + ] + self.assertEqual(len(popen_calls), 1) + keyword_names = {keyword.arg for keyword in popen_calls[0].keywords} + self.assertTrue({"text", "encoding", "errors"}.isdisjoint(keyword_names)) + + self.assertIn('raw_line_bytes.decode("utf-8", errors="replace")', source) + self.assertIn("len(raw_line_bytes) > MAX_CHROMEDRIVER_STARTUP_LINE_BYTES", source) + + +if __name__ == "__main__": + unittest.main() \ No newline at end of file From 8514aecce2e35712028ba613468d38d62af40203 Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Wed, 19 Aug 2026 05:38:45 -0700 Subject: [PATCH 14/22] fix(mv3): preserve portable decoding in profile isolation --- scripts/ci/run_mv3_compatibility.py | 18 ++++++++---------- 1 file changed, 8 insertions(+), 10 deletions(-) diff --git a/scripts/ci/run_mv3_compatibility.py b/scripts/ci/run_mv3_compatibility.py index f0e9267fd..6d3817708 100644 --- a/scripts/ci/run_mv3_compatibility.py +++ b/scripts/ci/run_mv3_compatibility.py @@ -366,7 +366,7 @@ def _exercise_real_click(driver_port: int, session_id: str) -> str: return str(text) -def _teardown_driver_process(driver: subprocess.Popen[str]) -> Exception | None: +def _teardown_driver_process(driver: subprocess.Popen[bytes]) -> Exception | None: """Reap ChromeDriver and, for real Popen instances, its isolated process group. Production ChromeDriver launches expose a positive `pid` and run in a fresh @@ -453,7 +453,7 @@ def _teardown_driver_process(driver: subprocess.Popen[str]) -> Exception | None: def _start_chromedriver( chromedriver_bin: pathlib.Path, -) -> tuple[subprocess.Popen[str], int]: +) -> tuple[subprocess.Popen[bytes], int]: """Let ChromeDriver atomically bind an ephemeral port and report the bound authority. The process owns port allocation by binding port zero itself. Its combined output is @@ -465,10 +465,6 @@ def _start_chromedriver( [str(chromedriver_bin), "--port=0", "--allowed-ips=127.0.0.1"], stdout=subprocess.PIPE, stderr=subprocess.STDOUT, - text=True, - encoding="utf-8", - errors="replace", - bufsize=1, start_new_session=True, ) if driver.stdout is None: @@ -482,6 +478,7 @@ def _start_chromedriver( raise startup_error startup_events: queue.Queue[tuple[str, int | None]] = queue.Queue(maxsize=1) + port_prefix_bytes = CHROMEDRIVER_BOUND_PORT_PREFIX.encode("ascii") def publish(event: tuple[str, int | None]) -> None: try: @@ -490,13 +487,14 @@ def publish(event: tuple[str, int | None]) -> None: return def drain_output() -> None: - for raw_line in driver.stdout: - if not raw_line.startswith(CHROMEDRIVER_BOUND_PORT_PREFIX): + for raw_line_bytes in driver.stdout: + if not raw_line_bytes.startswith(port_prefix_bytes): continue - if len(raw_line.encode("utf-8")) > MAX_CHROMEDRIVER_STARTUP_LINE_BYTES: + if len(raw_line_bytes) > MAX_CHROMEDRIVER_STARTUP_LINE_BYTES: publish(("invalid", None)) continue + raw_line = raw_line_bytes.decode("utf-8", errors="replace") line = raw_line.rstrip("\r\n") if not line.endswith("."): publish(("invalid", None)) @@ -885,4 +883,4 @@ def main() -> int: if __name__ == "__main__": - raise SystemExit(main()) + raise SystemExit(main()) \ No newline at end of file From 4f7aaf080a29afaee197f54c791d0c7d9ccce1a4 Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Wed, 19 Aug 2026 05:39:48 -0700 Subject: [PATCH 15/22] chore(mv3): match prerequisite regression exactly --- tests/test_mv3_subprocess_compatibility_contract.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/test_mv3_subprocess_compatibility_contract.py b/tests/test_mv3_subprocess_compatibility_contract.py index f7eed458e..166c471c1 100644 --- a/tests/test_mv3_subprocess_compatibility_contract.py +++ b/tests/test_mv3_subprocess_compatibility_contract.py @@ -41,4 +41,4 @@ def test_chromedriver_popen_uses_binary_pipe_with_explicit_utf8_decode(self) -> if __name__ == "__main__": - unittest.main() \ No newline at end of file + unittest.main() From 8a6e0fdfa7e9588bf7fae1e29ab0f2e397a291ab Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Wed, 19 Aug 2026 23:39:38 -0700 Subject: [PATCH 16/22] test(mv3): restore bounded startup-line regression from prerequisite --- ...t_mv3_subprocess_compatibility_contract.py | 32 +++++++++++++++++-- 1 file changed, 30 insertions(+), 2 deletions(-) diff --git a/tests/test_mv3_subprocess_compatibility_contract.py b/tests/test_mv3_subprocess_compatibility_contract.py index 166c471c1..020c5fdb4 100644 --- a/tests/test_mv3_subprocess_compatibility_contract.py +++ b/tests/test_mv3_subprocess_compatibility_contract.py @@ -1,9 +1,11 @@ -"""Regression contract for portable, explicit ChromeDriver output decoding.""" +"""Regression contract for portable, bounded ChromeDriver output decoding.""" from __future__ import annotations import ast +import io import pathlib +import runpy import unittest ROOT = pathlib.Path(__file__).resolve().parents[1] @@ -11,7 +13,7 @@ class ChromeDriverSubprocessCompatibilityContractTests(unittest.TestCase): - """Keep subprocess decoding explicit instead of version-gated Popen kwargs.""" + """Keep subprocess decoding explicit and bound retained startup-line memory.""" def test_chromedriver_popen_uses_binary_pipe_with_explicit_utf8_decode(self) -> None: """Popen must avoid text-decoding kwargs and decode bounded output explicitly.""" @@ -39,6 +41,32 @@ def test_chromedriver_popen_uses_binary_pipe_with_explicit_utf8_decode(self) -> self.assertIn('raw_line_bytes.decode("utf-8", errors="replace")', source) self.assertIn("len(raw_line_bytes) > MAX_CHROMEDRIVER_STARTUP_LINE_BYTES", source) + def test_startup_output_reader_never_requests_an_unbounded_line(self) -> None: + """A newline-free subprocess record must be drained in bounded reads.""" + + namespace = runpy.run_path(str(RUNNER), run_name="mv3_startup_output_bound") + read_line = namespace["_read_chromedriver_startup_line"] + maximum = namespace["MAX_CHROMEDRIVER_STARTUP_LINE_BYTES"] + + class RecordingStream(io.BytesIO): + def __init__(self, initial_bytes: bytes) -> None: + super().__init__(initial_bytes) + self.requested_sizes: list[int] = [] + + def readline(self, size: int = -1) -> bytes: + self.requested_sizes.append(size) + return super().readline(size) + + stream = RecordingStream(b"x" * (maximum * 4) + b"\nnext\n") + raw_line, oversized = read_line(stream) + + self.assertTrue(oversized) + self.assertLessEqual(len(raw_line), maximum + 1) + self.assertTrue(stream.requested_sizes) + self.assertNotIn(-1, stream.requested_sizes) + self.assertLessEqual(max(stream.requested_sizes), maximum + 1) + self.assertEqual(read_line(stream), (b"next\n", False)) + if __name__ == "__main__": unittest.main() From 4ced8d1260fc82893438185e7477c6cb417ae935 Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Wed, 19 Aug 2026 23:41:47 -0700 Subject: [PATCH 17/22] fix(mv3): preserve bounded startup reads in profile-isolation child --- scripts/ci/run_mv3_compatibility.py | 23 +++++++++++++++++++++-- 1 file changed, 21 insertions(+), 2 deletions(-) diff --git a/scripts/ci/run_mv3_compatibility.py b/scripts/ci/run_mv3_compatibility.py index 6d3817708..0d119d4d6 100644 --- a/scripts/ci/run_mv3_compatibility.py +++ b/scripts/ci/run_mv3_compatibility.py @@ -451,6 +451,22 @@ def _teardown_driver_process(driver: subprocess.Popen[bytes]) -> Exception | Non return None +def _read_chromedriver_startup_line(stream: Any) -> tuple[bytes, bool]: + """Read and drain one ChromeDriver startup record using only bounded reads.""" + + raw_line_bytes = stream.readline(MAX_CHROMEDRIVER_STARTUP_LINE_BYTES + 1) + if not raw_line_bytes: + return b"", False + + oversized = len(raw_line_bytes) > MAX_CHROMEDRIVER_STARTUP_LINE_BYTES + if oversized and not raw_line_bytes.endswith(b"\n"): + while True: + remainder = stream.readline(MAX_CHROMEDRIVER_STARTUP_LINE_BYTES + 1) + if not remainder or remainder.endswith(b"\n"): + break + return raw_line_bytes, oversized + + def _start_chromedriver( chromedriver_bin: pathlib.Path, ) -> tuple[subprocess.Popen[bytes], int]: @@ -487,10 +503,13 @@ def publish(event: tuple[str, int | None]) -> None: return def drain_output() -> None: - for raw_line_bytes in driver.stdout: + while True: + raw_line_bytes, oversized = _read_chromedriver_startup_line(driver.stdout) + if not raw_line_bytes: + break if not raw_line_bytes.startswith(port_prefix_bytes): continue - if len(raw_line_bytes) > MAX_CHROMEDRIVER_STARTUP_LINE_BYTES: + if oversized: publish(("invalid", None)) continue From 57403cc6b1cd6848fd71e6bdd4f9b00438acedb7 Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Thu, 20 Aug 2026 22:42:44 -0700 Subject: [PATCH 18/22] chore(mv3): remap profile isolation onto live parent --- scripts/ci/run_mv3_compatibility.py | 52 ++++++++++++++++++++++++----- 1 file changed, 44 insertions(+), 8 deletions(-) diff --git a/scripts/ci/run_mv3_compatibility.py b/scripts/ci/run_mv3_compatibility.py index 7680a3649..c307e7356 100644 --- a/scripts/ci/run_mv3_compatibility.py +++ b/scripts/ci/run_mv3_compatibility.py @@ -80,6 +80,16 @@ "download-not-evaluated", } ) +WEBDRIVER_ERROR_CODES = frozenset( + { + "invalid argument", + "no such element", + "session not created", + "stale element reference", + "timeout", + "unknown error", + } +) class CompatibilitySurfaceError(RuntimeError): @@ -94,6 +104,14 @@ def __init__(self, observed: dict[str, str]) -> None: super().__init__("Manifest V3 fixture surfaces did not converge") +class WebDriverProtocolError(RuntimeError): + """Report one allow-listed WebDriver error code without browser-controlled text.""" + + def __init__(self, code: object, _message: object) -> None: + self.code = code if isinstance(code, str) and code in WEBDRIVER_ERROR_CODES else "unknown" + super().__init__(f"WebDriver protocol error: {self.code}") + + class WebDriverSessionCleanupError(RuntimeError): """Report a reviewed WebDriver session-delete failure after process teardown.""" @@ -120,6 +138,8 @@ def _failure_evidence(error: BaseException) -> dict[str, Any]: if isinstance(error, CompatibilitySurfaceError): return {"failure_kind": "surface_mismatch", "observed": error.observed} + if isinstance(error, WebDriverProtocolError): + return {"failure_kind": "webdriver_protocol_error", "error_code": error.code} if isinstance(error, json.JSONDecodeError): return {"failure_kind": "json_decode_error"} if isinstance(error, OSError): @@ -178,6 +198,7 @@ def _json_request( body = None if payload is None else json.dumps(payload).encode("utf-8") connection = http.client.HTTPConnection("127.0.0.1", driver_port, timeout=timeout) + transport_protocol_failed = False try: try: connection.request( @@ -189,20 +210,30 @@ def _json_request( response = connection.getresponse() raw = response.read(MAX_WEBDRIVER_RESPONSE_BYTES + 1) except http.client.HTTPException: - raise RuntimeError("WebDriver transport protocol failure") from None + transport_protocol_failed = True + if transport_protocol_failed: + raise RuntimeError("WebDriver transport protocol failure") if len(raw) > MAX_WEBDRIVER_RESPONSE_BYTES: raise RuntimeError("WebDriver response exceeded the bounded JSON limit") - if response.status >= 400: - raise RuntimeError(f"WebDriver HTTP {response.status} error") finally: connection.close() - decoded = json.loads(raw.decode("utf-8")) + try: + decoded = json.loads(raw.decode("utf-8")) + except json.JSONDecodeError: + if response.status >= 400: + raise RuntimeError(f"WebDriver HTTP {response.status} error") from None + raise if not isinstance(decoded, dict): raise RuntimeError("WebDriver returned a non-object JSON payload") + if response.status >= 400: + value = decoded.get("value") + if isinstance(value, dict) and value.get("error"): + raise WebDriverProtocolError(value.get("error"), value.get("message")) + raise RuntimeError(f"WebDriver HTTP {response.status} error") value = decoded.get("value") if isinstance(value, dict) and value.get("error"): - raise RuntimeError("WebDriver returned a protocol error") + raise WebDriverProtocolError(value.get("error"), value.get("message")) return decoded @@ -588,7 +619,6 @@ def _run_browser_pass( "--disable-component-update", "--disable-sync", "--disable-dev-shm-usage", - "--no-sandbox", f"--user-data-dir={profile_dir}", f"--disable-extensions-except={FIXTURE}", f"--load-extension={FIXTURE}", @@ -845,7 +875,13 @@ def main() -> int: trial_number, ) ) - except (OSError, ValueError, RuntimeError, json.JSONDecodeError) as exc: + except ( + OSError, + ValueError, + RuntimeError, + json.JSONDecodeError, + subprocess.TimeoutExpired, + ) as exc: failed_trial: dict[str, Any] = { "trial_number": trial_number, "passed": False, @@ -904,4 +940,4 @@ def main() -> int: if __name__ == "__main__": - raise SystemExit(main()) \ No newline at end of file + raise SystemExit(main()) From e2ef39648dd3e7727eacffcdce760ef9e4cab365 Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Fri, 21 Aug 2026 02:07:07 -0700 Subject: [PATCH 19/22] test(mv3): reproduce descendant leak after leader exit --- ...test_mv3_process_group_cleanup_contract.py | 64 ++++++++++++++++++- 1 file changed, 62 insertions(+), 2 deletions(-) diff --git a/tests/test_mv3_process_group_cleanup_contract.py b/tests/test_mv3_process_group_cleanup_contract.py index d6ad9cdb9..c33aa8cab 100644 --- a/tests/test_mv3_process_group_cleanup_contract.py +++ b/tests/test_mv3_process_group_cleanup_contract.py @@ -1,11 +1,15 @@ -"""Regression contract for session-cleanup causality during process-group teardown.""" +"""Regression contracts for bounded process-group cleanup.""" from __future__ import annotations +import os import pathlib import runpy import signal +import subprocess +import sys import tempfile +import time import unittest import unittest.mock @@ -14,7 +18,7 @@ class ManifestV3ProcessGroupCleanupContractTests(unittest.TestCase): - """Preserve the reviewed session failure when process-group signaling also fails.""" + """Prove cleanup causality and descendant process-group termination.""" @staticmethod def _surfaces() -> dict[str, str]: @@ -115,6 +119,62 @@ def fake_json_request( getattr(raised.exception, "__notes__", []), ) + @unittest.skipUnless(os.name == "posix" and hasattr(os, "killpg"), "requires POSIX process groups") + def test_teardown_reaps_descendant_that_ignores_sigterm_after_leader_exits(self) -> None: + """A fast-exiting leader cannot make a SIGTERM-resistant descendant look reaped.""" + + namespace = runpy.run_path(str(RUNNER), run_name="mv3_real_group_cleanup_contract") + teardown_driver_process = namespace["_teardown_driver_process"] + child_program = ( + "import signal,time\n" + "signal.signal(signal.SIGTERM, signal.SIG_IGN)\n" + "print('ready', flush=True)\n" + "time.sleep(60)\n" + ) + leader_program = ( + "import subprocess,sys,time\n" + f"child = subprocess.Popen([sys.executable, '-c', {child_program!r}], " + "stdout=subprocess.PIPE, text=True)\n" + "assert child.stdout is not None\n" + "assert child.stdout.readline().strip() == 'ready'\n" + "print(child.pid, flush=True)\n" + "time.sleep(60)\n" + ) + driver = subprocess.Popen( + [sys.executable, "-c", leader_program], + stdout=subprocess.PIPE, + stderr=subprocess.DEVNULL, + text=True, + start_new_session=True, + ) + self.assertIsNotNone(driver.stdout) + assert driver.stdout is not None + child_pid = int(driver.stdout.readline().strip()) + self.assertGreater(child_pid, 0) + process_group_id = driver.pid + + try: + self.assertIsNone(teardown_driver_process(driver)) + deadline = time.monotonic() + 1.0 + while time.monotonic() < deadline: + try: + os.killpg(process_group_id, 0) + except ProcessLookupError: + break + time.sleep(0.05) + else: + self.fail("process-group teardown left a SIGTERM-resistant descendant alive") + finally: + try: + os.killpg(process_group_id, signal.SIGKILL) + except ProcessLookupError: + pass + try: + driver.wait(timeout=5) + except subprocess.TimeoutExpired: + driver.kill() + driver.wait(timeout=5) + if __name__ == "__main__": unittest.main() From 54e76d9f356b91a0b743635d7151b0d72c937f5f Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Fri, 21 Aug 2026 02:14:37 -0700 Subject: [PATCH 20/22] fix(mv3): reap surviving browser process groups --- scripts/ci/run_mv3_compatibility.py | 83 +++++++++++++++++++---------- 1 file changed, 54 insertions(+), 29 deletions(-) diff --git a/scripts/ci/run_mv3_compatibility.py b/scripts/ci/run_mv3_compatibility.py index c307e7356..59dab6366 100644 --- a/scripts/ci/run_mv3_compatibility.py +++ b/scripts/ci/run_mv3_compatibility.py @@ -39,6 +39,8 @@ REQUEST_TIMEOUT_SECONDS = 5.0 STARTUP_TIMEOUT_SECONDS = 20.0 FIXTURE_TIMEOUT_SECONDS = 20.0 +PROCESS_GROUP_EXIT_TIMEOUT_SECONDS = 5.0 +PROCESS_GROUP_EXIT_POLL_SECONDS = 0.05 MAX_WEBDRIVER_RESPONSE_BYTES = 1_048_576 MAX_CHROMEDRIVER_STARTUP_LINE_BYTES = 512 CHROMEDRIVER_BOUND_PORT_PREFIX = "ChromeDriver was started successfully on port " @@ -397,14 +399,51 @@ def _exercise_real_click(driver_port: int, session_id: str) -> str: return str(text) +def _wait_for_process_group_exit(process_group_id: int) -> Exception | None: + """Wait a bounded interval until one isolated process group no longer exists.""" + + deadline = time.monotonic() + PROCESS_GROUP_EXIT_TIMEOUT_SECONDS + while True: + try: + os.killpg(process_group_id, 0) + except ProcessLookupError: + return None + except OSError as error: + return error + if time.monotonic() >= deadline: + return RuntimeError( + "ChromeDriver process group remained alive after bounded teardown" + ) + time.sleep(PROCESS_GROUP_EXIT_POLL_SECONDS) + + +def _kill_and_reap_process_group( + driver: subprocess.Popen[bytes], process_group_id: int +) -> Exception | None: + """Force one surviving isolated process group down and verify bounded disappearance.""" + + try: + os.killpg(process_group_id, signal.SIGKILL) + except ProcessLookupError: + pass + except OSError as kill_error: + return kill_error + try: + driver.wait(timeout=5) + except (OSError, subprocess.TimeoutExpired) as wait_error: + return wait_error + return _wait_for_process_group_exit(process_group_id) + + def _teardown_driver_process(driver: subprocess.Popen[bytes]) -> Exception | None: """Reap ChromeDriver and, for real Popen instances, its isolated process group. Production ChromeDriver launches expose a positive `pid` and run in a fresh - process session, so teardown first signals the entire process group with - bounded SIGTERM→SIGKILL recovery. A pid-less test double retains the older - bounded leader-only path so cleanup-failure contracts can isolate session - semantics without sending operating-system signals. + process session. Teardown signals the group with SIGTERM, reaps the leader, + verifies whether descendants still occupy the group, and applies bounded + SIGKILL recovery before reporting success. A pid-less test double retains the + older bounded leader-only path so cleanup-failure contracts can isolate + session semantics without sending operating-system signals. """ driver_pid = getattr(driver, "pid", None) @@ -418,19 +457,8 @@ def _teardown_driver_process(driver: subprocess.Popen[bytes]) -> Exception | Non return wait_error return None except OSError as terminate_error: - try: - os.killpg(driver_pid, signal.SIGKILL) - driver.wait(timeout=5) - except ProcessLookupError: - try: - driver.wait(timeout=5) - except (OSError, subprocess.TimeoutExpired) as wait_error: - terminate_error.add_note( - "bounded ChromeDriver process-group fallback reap failed: " - f"{type(wait_error).__name__}" - ) - return terminate_error - except (OSError, subprocess.TimeoutExpired) as fallback_error: + fallback_error = _kill_and_reap_process_group(driver, driver_pid) + if fallback_error is not None: terminate_error.add_note( "bounded ChromeDriver process-group kill fallback also failed: " f"{type(fallback_error).__name__}" @@ -440,22 +468,19 @@ def _teardown_driver_process(driver: subprocess.Popen[bytes]) -> Exception | Non try: driver.wait(timeout=5) - return None except subprocess.TimeoutExpired: - try: - os.killpg(driver_pid, signal.SIGKILL) - except ProcessLookupError: - pass - except OSError as kill_error: - return kill_error - try: - driver.wait(timeout=5) - except (OSError, subprocess.TimeoutExpired) as wait_error: - return wait_error - return None + return _kill_and_reap_process_group(driver, driver_pid) except OSError as wait_error: return wait_error + try: + os.killpg(driver_pid, 0) + except ProcessLookupError: + return None + except OSError as probe_error: + return probe_error + return _kill_and_reap_process_group(driver, driver_pid) + try: driver.terminate() except OSError as terminate_error: From ff6590e9680c34a413111ebfe4fd5211da8afe19 Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Fri, 21 Aug 2026 02:15:11 -0700 Subject: [PATCH 21/22] test(mv3): close descendant fixture pipe --- tests/test_mv3_process_group_cleanup_contract.py | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/test_mv3_process_group_cleanup_contract.py b/tests/test_mv3_process_group_cleanup_contract.py index c33aa8cab..11060329f 100644 --- a/tests/test_mv3_process_group_cleanup_contract.py +++ b/tests/test_mv3_process_group_cleanup_contract.py @@ -150,6 +150,7 @@ def test_teardown_reaps_descendant_that_ignores_sigterm_after_leader_exits(self) self.assertIsNotNone(driver.stdout) assert driver.stdout is not None child_pid = int(driver.stdout.readline().strip()) + driver.stdout.close() self.assertGreater(child_pid, 0) process_group_id = driver.pid From 90849dfb6db169069b7b8055cc0a739b7884a4c3 Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Fri, 21 Aug 2026 02:21:26 -0700 Subject: [PATCH 22/22] test(mv3): model process-group exit after SIGTERM --- tests/test_mv3_ephemeral_profile_contract.py | 20 ++++++++++++++++++-- 1 file changed, 18 insertions(+), 2 deletions(-) diff --git a/tests/test_mv3_ephemeral_profile_contract.py b/tests/test_mv3_ephemeral_profile_contract.py index 5fc07593c..846013759 100644 --- a/tests/test_mv3_ephemeral_profile_contract.py +++ b/tests/test_mv3_ephemeral_profile_contract.py @@ -83,6 +83,14 @@ def test_browser_pass_owns_and_terminates_the_chromedriver_process_group(self) - driver.pid = 4242 driver.wait.return_value = 0 + def fake_kill_process_group(process_group_id: int, process_signal: int) -> None: + self.assertEqual(process_group_id, driver.pid) + if process_signal == signal.SIGTERM: + return + if process_signal == 0: + raise ProcessLookupError + raise AssertionError(f"unexpected process-group signal: {process_signal}") + with tempfile.TemporaryDirectory(prefix="originweave-mv3-cleanup-") as profile_dir: with ( unittest.mock.patch.dict( @@ -94,7 +102,9 @@ def test_browser_pass_owns_and_terminates_the_chromedriver_process_group(self) - ), }, ), - unittest.mock.patch.object(globals_["os"], "killpg") as kill_process_group, + unittest.mock.patch.object( + globals_["os"], "killpg", side_effect=fake_kill_process_group + ) as kill_process_group, ): with self.assertRaisesRegex(RuntimeError, "controlled startup failure"): run_browser_pass( @@ -105,7 +115,13 @@ def test_browser_pass_owns_and_terminates_the_chromedriver_process_group(self) - "initialized", ) - kill_process_group.assert_called_once_with(driver.pid, signal.SIGTERM) + self.assertEqual( + kill_process_group.call_args_list, + [ + unittest.mock.call(driver.pid, signal.SIGTERM), + unittest.mock.call(driver.pid, 0), + ], + ) driver.wait.assert_called_once_with(timeout=5) driver.terminate.assert_not_called() driver.kill.assert_not_called()