diff --git a/CHANGELOG.md b/CHANGELOG.md index e6b2b4039..61b8208f3 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,7 @@ All notable changes to OriginWeave are documented in this file. The format follo ### Added +- Kept the inherited protocol-failure cleanup checks executable after shared-deadline integration by observing the correct cleanup path for ordinary and forced-close trials; all failure, cleanup and diagnostic-redaction assertions remain intact. - Controlled browser trials reject malformed pre-shutdown exit counts, retain validated driver and cleanup outcomes after ordinary task failure, and report mid-request protocol failures without remote diagnostic text; failed trials remain failures, startup retries keep their existing narrow scope, and obsolete HTTP-body close signals are no longer accepted. - Failed controlled Agent Task runs now report whether their original browser process ended after shutdown, alongside temporary-profile cleanup; a failed task never becomes a pass merely because cleanup succeeded. If process observation itself fails, termination remains unproven. This covers the original browser process only, not all descendants or arbitrary browser recovery. - Controlled pinned-Chromium Agent Task success now binds the ChromeDriver browser root to its exact Linux `/proc//stat` start-time identity, binds every still-live PID from the already sampled bounded Chromium root-plus-descendant set before shutdown, explicitly records descendants that already exited between the `/proc` lineage snapshot and identity capture, and fails closed unless every retained exact identity terminates after session/driver shutdown; root disappearance or identity change remains an error, PID reuse counts only as termination of the original identity, and this does not attest cgroup/task ownership, processes appearing only after the sample, or OS-wide orphan absence. diff --git a/docs/doctoring.md b/docs/doctoring.md index 986d6060a..0ffbe0317 100644 --- a/docs/doctoring.md +++ b/docs/doctoring.md @@ -118,6 +118,12 @@ The repair restores the existing non-boolean integer range check, forwards only The regressions execute the production success predicate, actual ordinary/forced-close browser-pass and outer-trial paths, and aggregate evidence generation with controlled faults. They check invalid counts, observed root survival, driver cleanup, early failure without invented identity evidence, private-message exclusion, and final rejection with the original trial denominator intact. Unknown fields are not forwarded and malformed known cleanup values are rejected. A sixth regression covers all three final aggregate exception boundaries and server teardown. Controlled tests do not establish live Linux/Chromium termination, cgroup ownership, OS-wide orphan absence or release acceptance; exact-head hosted compatibility remains mandatory. +### Shared-deadline inherited-test reconciliation + +At #147 `af1b98ba377c73b88baa9633e2232e7f76e76f36`, all six parent review-evidence methods were restored, but the forced-close `BadStatusLine` and `IncompleteRead` subcases still injected the old individual root waiter. Native discovery executed all 228 tests and failed those two subcases; a fresh focused run reproduced both failures in 10.417 seconds because the real combined observer consumed its deadline. This was test injection drift after parent integration, not evidence of a production teardown regression. + +The existing test now specifies each lane's actual observer, return value and exact arguments. Ordinary trials keep the individual `False` result. Forced-close trials inject `(False, False)` from the shared observer and require `(321, 654, ((321, 654),))`, preserving root-only capture rather than inventing a complete descendant set. Every existing failed-trial, profile-cleanup, live-root, driver-termination, error-type and redaction assertion remains. No production code, deadline, retry, failed denominator or coverage gate changes. This repair does not address the separate ordinary-pass two-deadline finding or prove real Linux/pinned-Chromium acceptance. + ## References Amazon Web Services. (n.d.). *Set up the Amazon EKS Pod Identity Agent*. Retrieved August 6, 2026, from https://docs.aws.amazon.com/eks/latest/userguide/pod-id-agent-setup.html diff --git a/scripts/ci/run_mv3_compatibility.py b/scripts/ci/run_mv3_compatibility.py index e28696458..b202c8997 100644 --- a/scripts/ci/run_mv3_compatibility.py +++ b/scripts/ci/run_mv3_compatibility.py @@ -621,6 +621,76 @@ def _wait_for_linux_process_identity_set_exit( time.sleep(min(0.05, remaining_seconds)) +def _wait_for_linux_process_teardown( + root_process_id: int, + root_start_time_ticks: int, + process_identities: tuple[tuple[int, int], ...], + *, + timeout_seconds: float = PROCESS_EXIT_TIMEOUT_SECONDS, +) -> tuple[bool, bool]: + """Observe root and sampled-set termination under one bounded shared deadline.""" + + if ( + isinstance(root_process_id, bool) + or not isinstance(root_process_id, int) + or root_process_id <= 0 + ): + raise ValueError("invalid Linux root process identifier") + if ( + isinstance(root_start_time_ticks, bool) + or not isinstance(root_start_time_ticks, int) + or root_start_time_ticks <= 0 + ): + raise ValueError("invalid Linux root process start time") + if not process_identities or len(process_identities) > MAX_BROWSER_PROCESS_TREE_SIZE: + raise ValueError("invalid Linux process identity-set size") + + expected: dict[int, tuple[int, int]] = {} + for identity in process_identities: + if not isinstance(identity, tuple) or len(identity) != 2: + raise ValueError("invalid Linux process identity") + process_id, start_time_ticks = identity + if isinstance(process_id, bool) or not isinstance(process_id, int) or process_id <= 0: + raise ValueError("invalid Linux process identifier") + if ( + isinstance(start_time_ticks, bool) + or not isinstance(start_time_ticks, int) + or start_time_ticks <= 0 + ): + raise ValueError("invalid Linux process start time") + if process_id in expected: + raise ValueError("Linux process identity-set PIDs must be unique") + expected[process_id] = identity + + root_identity = (root_process_id, root_start_time_ticks) + if expected.get(root_process_id) != root_identity: + raise ValueError("Linux root process identity must belong to the sampled process set") + if ( + isinstance(timeout_seconds, bool) + or not isinstance(timeout_seconds, (int, float)) + or timeout_seconds < 0 + or not math.isfinite(timeout_seconds) + ): + raise ValueError("invalid Linux process teardown timeout") + + deadline = time.monotonic() + float(timeout_seconds) + while True: + live_process_ids: set[int] = set() + for process_id, expected_identity in expected.items(): + current_identity = _read_linux_proc_stat_process_identity(process_id) + if current_identity == expected_identity: + live_process_ids.add(process_id) + + root_terminated = root_process_id not in live_process_ids + process_set_terminated = not live_process_ids + if process_set_terminated: + return root_terminated, True + remaining_seconds = deadline - time.monotonic() + if remaining_seconds <= 0: + return root_terminated, False + time.sleep(min(0.05, remaining_seconds)) + + def _sample_linux_process_rss_bytes(process_id: int) -> int: """Read one attributed Linux process RSS through a bounded ``/proc`` status file.""" @@ -1386,15 +1456,22 @@ def _run_agent_task_browser_pass( if browser_process_id is None or browser_process_start_time_ticks is None: raise RuntimeError("Agent Task browser process identity was not captured") - browser_process_terminated = _wait_for_linux_process_identity_exit( - browser_process_id, - browser_process_start_time_ticks, + full_process_set_captured = chromium_process_identities is not None + teardown_identities = ( + chromium_process_identities + if chromium_process_identities is not None + else ((browser_process_id, browser_process_start_time_ticks),) ) - chromium_process_set_terminated: bool | None = None - if chromium_process_identities is not None: - chromium_process_set_terminated = _wait_for_linux_process_identity_set_exit( - chromium_process_identities + browser_process_terminated, observed_process_set_terminated = ( + _wait_for_linux_process_teardown( + browser_process_id, + browser_process_start_time_ticks, + teardown_identities, ) + ) + chromium_process_set_terminated = ( + observed_process_set_terminated if full_process_set_captured else None + ) if ( browser_failure_type is not None or session_cleanup_failure_type is not None @@ -1779,15 +1856,22 @@ def _run_agent_task_forced_close_browser_pass( if browser_process_id is None or browser_process_start_time_ticks is None: raise RuntimeError("Agent Task forced-close browser process identity was not captured") - browser_process_terminated = _wait_for_linux_process_identity_exit( - browser_process_id, - browser_process_start_time_ticks, + full_process_set_captured = chromium_process_identities is not None + teardown_identities = ( + chromium_process_identities + if chromium_process_identities is not None + else ((browser_process_id, browser_process_start_time_ticks),) ) - chromium_process_set_terminated: bool | None = None - if chromium_process_identities is not None: - chromium_process_set_terminated = _wait_for_linux_process_identity_set_exit( - chromium_process_identities + browser_process_terminated, observed_process_set_terminated = ( + _wait_for_linux_process_teardown( + browser_process_id, + browser_process_start_time_ticks, + teardown_identities, ) + ) + chromium_process_set_terminated = ( + observed_process_set_terminated if full_process_set_captured else None + ) if ( browser_failure_type is not None or session_cleanup_failure_type is not None diff --git a/tests/test_agent_task_failure_process_set_termination_contract.py b/tests/test_agent_task_failure_process_set_termination_contract.py index 6fe48ea9d..e59819c50 100644 --- a/tests/test_agent_task_failure_process_set_termination_contract.py +++ b/tests/test_agent_task_failure_process_set_termination_contract.py @@ -24,8 +24,10 @@ def test_browser_pass_retains_sampled_process_set_teardown_after_failure(self) - end = runner.index("\ndef _run_agent_task_trial(", start) browser_pass = runner[start:end] for expected in ( - "if chromium_process_identities is not None:", - "chromium_process_set_terminated = _wait_for_linux_process_identity_set_exit(", + "full_process_set_captured = chromium_process_identities is not None", + "teardown_identities = (", + "_wait_for_linux_process_teardown(", + "observed_process_set_terminated if full_process_set_captured else None", 'failure_evidence["chromium_process_set_terminated"]', ): with self.subTest(expected=expected): diff --git a/tests/test_agent_task_failure_process_termination_contract.py b/tests/test_agent_task_failure_process_termination_contract.py index 910bf0248..3fd4eab45 100644 --- a/tests/test_agent_task_failure_process_termination_contract.py +++ b/tests/test_agent_task_failure_process_termination_contract.py @@ -36,10 +36,14 @@ def test_failure_cleanup_release_record_preserves_evidence_limits(self) -> None: ) def test_real_failure_path_distinguishes_observed_exit_from_observation_error(self) -> None: - """Exercise both owning helpers without launching a browser or trusting fake success.""" + """Exercise the shared teardown observer without launching a browser or trusting fake success.""" - for exit_observation in (True, False, PermissionError("controlled read failure")): - with self.subTest(exit_observation=type(exit_observation).__name__): + for teardown_observation in ( + (True, True), + (False, False), + PermissionError("controlled read failure"), + ): + with self.subTest(teardown_observation=repr(teardown_observation)): namespace = self._namespace("agent_task_failure_observation_boundary") browser_pass = namespace["_run_agent_task_browser_pass"] run_trial = namespace["_run_agent_task_trial"] @@ -60,15 +64,15 @@ def request(_port, method, target, *_args): raise RuntimeError("private controlled browser failure") return {} - exit_wait = mock.Mock(return_value=exit_observation) - if isinstance(exit_observation, Exception): - exit_wait.side_effect = exit_observation + teardown_wait = mock.Mock(return_value=teardown_observation) + if isinstance(teardown_observation, Exception): + teardown_wait.side_effect = teardown_observation replacements = { "_free_loopback_port": lambda: 12345, "_wait_for_driver": lambda _port: None, "_json_request": request, "_read_linux_proc_stat_process_identity": lambda _pid: (321, 654), - "_wait_for_linux_process_identity_exit": exit_wait, + "_wait_for_linux_process_teardown": teardown_wait, } with mock.patch.dict(browser_pass.__globals__, replacements), mock.patch.object( namespace["subprocess"], "Popen", return_value=driver @@ -82,16 +86,19 @@ def request(_port, method, target, *_args): driver.terminate.assert_called_once_with() driver.wait.assert_called_once_with(timeout=5) - exit_wait.assert_called_once_with(321, 654) + teardown_wait.assert_called_once_with(321, 654, ((321, 654),)) self.assertIs(result["passed"], False) self.assertIs(result["profile_cleaned"], True) self.assertNotIn("private controlled browser failure", repr(result)) - if isinstance(exit_observation, Exception): + if isinstance(teardown_observation, Exception): self.assertEqual(result["failure_type"], "PermissionError") self.assertNotIn("browser_process_terminated", result) else: self.assertEqual(result["failure_type"], "RuntimeError") - self.assertIs(result["browser_process_terminated"], exit_observation) + self.assertIs( + result["browser_process_terminated"], teardown_observation[0] + ) + self.assertNotIn("chromium_process_set_terminated", result) def test_browser_pass_retains_failure_process_termination_evidence(self) -> None: """A browser-pass failure after identity capture must survive teardown as evidence.""" diff --git a/tests/test_agent_task_forced_close_process_termination_contract.py b/tests/test_agent_task_forced_close_process_termination_contract.py index 0ad1b9fa4..cea7d58a4 100644 --- a/tests/test_agent_task_forced_close_process_termination_contract.py +++ b/tests/test_agent_task_forced_close_process_termination_contract.py @@ -27,8 +27,7 @@ def test_forced_close_browser_pass_binds_and_waits_for_process_identities(self) "_snapshot_linux_process_evidence", "_read_linux_process_identity_set", "_terminate_owned_process_bounded", - "_wait_for_linux_process_identity_exit", - "_wait_for_linux_process_identity_set_exit", + "_wait_for_linux_process_teardown", '"driver_process_terminated"', '"driver_kill_fallback_used"', '"browser_process_terminated"', @@ -97,12 +96,10 @@ def test_forced_close_browser_failure_is_returned_after_teardown_waits(self) -> self.assertIn(expected, browser_pass) shutdown = browser_pass.index("_terminate_owned_process_bounded(driver)") - root_wait = browser_pass.index("_wait_for_linux_process_identity_exit(") - set_wait = browser_pass.index("_wait_for_linux_process_identity_set_exit(") - failure_return = browser_pass.index("browser_failure_type is not None", set_wait) - self.assertLess(shutdown, root_wait) - self.assertLess(root_wait, failure_return) - self.assertLess(set_wait, failure_return) + teardown_wait = browser_pass.index("_wait_for_linux_process_teardown(") + failure_return = browser_pass.index("browser_failure_type is not None", teardown_wait) + self.assertLess(shutdown, teardown_wait) + self.assertLess(teardown_wait, failure_return) def test_forced_close_driver_shutdown_timeout_is_bounded_and_typed(self) -> None: """A wedged ChromeDriver after SIGKILL must become failure evidence, not escape.""" diff --git a/tests/test_agent_task_forced_close_shared_teardown_deadline_contract.py b/tests/test_agent_task_forced_close_shared_teardown_deadline_contract.py new file mode 100644 index 000000000..329194b3e --- /dev/null +++ b/tests/test_agent_task_forced_close_shared_teardown_deadline_contract.py @@ -0,0 +1,90 @@ +"""Contract for one total post-shutdown teardown deadline in the forced-close lane.""" + +from __future__ import annotations + +import pathlib +import runpy +import unittest + +ROOT = pathlib.Path(__file__).resolve().parents[1] +RUNNER = ROOT / "scripts" / "ci" / "run_mv3_compatibility.py" + + +class AgentTaskForcedCloseSharedTeardownDeadlineContractTests(unittest.TestCase): + """Prevent root and process-set teardown polling from multiplying the budget.""" + + def test_runner_exposes_one_combined_teardown_waiter(self) -> None: + """Root and sampled-set evidence must be observed under one timeout authority.""" + + namespace = runpy.run_path( + str(RUNNER), run_name="forced_close_shared_teardown_deadline" + ) + self.assertIn("_wait_for_linux_process_teardown", namespace) + + def test_combined_waiter_preserves_partial_evidence_at_one_deadline(self) -> None: + """A root may exit while a descendant remains live when the one deadline expires.""" + + namespace = runpy.run_path( + str(RUNNER), run_name="forced_close_shared_teardown_behavior" + ) + waiter = namespace["_wait_for_linux_process_teardown"] + + class FakeTime: + def __init__(self) -> None: + self.now = 0.0 + + def monotonic(self) -> float: + return self.now + + def sleep(self, seconds: float) -> None: + self.now += seconds + + fake_time = FakeTime() + root_identity = (101, 1_001) + child_identity = (202, 2_002) + + def fake_read(process_id: int) -> tuple[int, int] | None: + if process_id == root_identity[0]: + return None if fake_time.now >= 0.05 else root_identity + if process_id == child_identity[0]: + return child_identity + raise AssertionError(f"unexpected process id: {process_id}") + + waiter.__globals__["time"] = fake_time + waiter.__globals__["_read_linux_proc_stat_process_identity"] = fake_read + + root_terminated, process_set_terminated = waiter( + root_identity[0], + root_identity[1], + (root_identity, child_identity), + timeout_seconds=0.10, + ) + self.assertIs(root_terminated, True) + self.assertIs(process_set_terminated, False) + self.assertLessEqual(fake_time.now, 0.1000001) + + def test_combined_waiter_requires_root_identity_in_the_sampled_set(self) -> None: + """A separate root identity may not be paired with an unrelated process set.""" + + namespace = runpy.run_path( + str(RUNNER), run_name="forced_close_shared_teardown_identity" + ) + waiter = namespace["_wait_for_linux_process_teardown"] + with self.assertRaises(ValueError): + waiter(101, 1_001, ((202, 2_002),), timeout_seconds=0) + + def test_forced_close_browser_pass_uses_only_the_combined_waiter(self) -> None: + """The forced-close pass must not run independent root and set timeout windows.""" + + runner = RUNNER.read_text(encoding="utf-8") + start = runner.index("def _run_agent_task_forced_close_browser_pass(") + end = runner.index("\ndef _run_agent_task_forced_close_trial(", start) + browser_pass = runner[start:end] + + self.assertIn("_wait_for_linux_process_teardown(", browser_pass) + self.assertNotIn("_wait_for_linux_process_identity_exit(", browser_pass) + self.assertNotIn("_wait_for_linux_process_identity_set_exit(", browser_pass) + + +if __name__ == "__main__": + unittest.main() diff --git a/tests/test_agent_task_review_evidence_contract.py b/tests/test_agent_task_review_evidence_contract.py index c627a3526..3544b3931 100644 --- a/tests/test_agent_task_review_evidence_contract.py +++ b/tests/test_agent_task_review_evidence_contract.py @@ -80,7 +80,7 @@ def test_protocol_faults_keep_real_browser_cleanup_evidence(self) -> None: namespace = runpy.run_path(str(RUNNER)) trial = namespace[f"_run_{lane}_trial"] driver = mock.Mock() - exit_wait = mock.Mock(return_value=False) + teardown_wait = mock.Mock(return_value=(False, False)) def request(_port, method, target, *_args): if target == "/session": @@ -95,7 +95,7 @@ def request(_port, method, target, *_args): "_free_loopback_port": lambda: 12345, "_wait_for_driver": lambda *_: None, "_json_request": request, "_read_linux_proc_stat_process_identity": lambda *_: (321, 654), - "_wait_for_linux_process_identity_exit": exit_wait, + "_wait_for_linux_process_teardown": teardown_wait, } with mock.patch.dict(trial.__globals__, replacements), mock.patch.object(namespace["subprocess"], "Popen", return_value=driver): result = trial(pathlib.Path("unused-chrome"), pathlib.Path("unused-driver"), "http://127.0.0.1/fixture", 3) @@ -105,7 +105,7 @@ def request(_port, method, target, *_args): self.assertIs(result["driver_process_terminated"], True) self.assertEqual(result["failure_type"], type(error).__name__) self.assertNotIn("private marker", repr(result)) - exit_wait.assert_called_once_with(321, 654) + teardown_wait.assert_called_once_with(321, 654, ((321, 654),)) driver.terminate.assert_called_once_with() def test_all_trial_boundaries_redact_protocol_failures_before_identity_capture(self) -> None: diff --git a/tests/test_agent_task_shared_teardown_deadline.py b/tests/test_agent_task_shared_teardown_deadline.py new file mode 100644 index 000000000..2a2930231 --- /dev/null +++ b/tests/test_agent_task_shared_teardown_deadline.py @@ -0,0 +1,37 @@ +"""Contract for one total post-shutdown teardown deadline in the ordinary Agent Task lane.""" + +from __future__ import annotations + +import pathlib +import unittest + +ROOT = pathlib.Path(__file__).resolve().parents[1] +RUNNER = ROOT / "scripts" / "ci" / "run_mv3_compatibility.py" + + +class AgentTaskSharedTeardownDeadlineContractTests(unittest.TestCase): + """Prevent ordinary Agent Task teardown polling from multiplying the budget.""" + + def test_browser_pass_uses_only_the_combined_teardown_waiter(self) -> None: + """Root and sampled-set evidence must share one timeout authority after shutdown.""" + + runner = RUNNER.read_text(encoding="utf-8") + start = runner.index("def _run_agent_task_browser_pass(") + end = runner.index("\ndef _run_agent_task_trial(", start) + browser_pass = runner[start:end] + + self.assertIn("_wait_for_linux_process_teardown(", browser_pass) + self.assertNotIn("_wait_for_linux_process_identity_exit(", browser_pass) + self.assertNotIn("_wait_for_linux_process_identity_set_exit(", browser_pass) + + shutdown = browser_pass.index("_terminate_owned_process_bounded(") + teardown_wait = browser_pass.index("_wait_for_linux_process_teardown(") + failure_return = browser_pass.index( + "or driver_cleanup_failure_type is not None" + ) + self.assertLess(shutdown, teardown_wait) + self.assertLess(teardown_wait, failure_return) + + +if __name__ == "__main__": + unittest.main()