From 624f8d64741aa2360571370f1dc87d69c3efec9a Mon Sep 17 00:00:00 2001
From: Desi Villanueva <217994822+dezgit2025@users.noreply.github.com>
Date: Thu, 1 Oct 2026 11:07:52 -0400
Subject: [PATCH] test: make process-group timeout test deterministic

The test raced a child that wrote a sentinel after 1 s against a 1 s
timeout. Measured margin on an idle Mac was 33-51 ms, so scheduling
jitter on CI runners flipped the result and the required macOS check
failed randomly.

The child now sleeps 30 s and the parent records its PID. After the
timeout the test polls until that PID is gone (5 s deadline) and still
asserts no sentinel was written. A broken group kill (killpg failing,
only the parent killed) fails the test with 'child survived the group
kill'.
---
 CHANGELOG.md                                  |  3 ++
 .../codex_fix/tests/test_v5_runner_review.py  | 29 +++++++++++++++----
 2 files changed, 27 insertions(+), 5 deletions(-)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 269033a..73f427f 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -22,6 +22,9 @@ Format: [Keep a Changelog](https://keepachangelog.com/). Versioning: [SemVer](ht
   no persisted checkout credentials, and the release tag passed via env instead
   of template expansion. Adds a `gitleaks` secret-scan job (checksum-verified
   binary, redacted output) and documents an optional pre-commit hook.
+- Makes the subprocess timeout test deterministic: it raced a 1-second child
+  against a 1-second timeout (~40 ms margin) and failed randomly on macOS. It
+  now checks that the child process is gone instead of racing the clock.
 
 ## [0.6.0] — 2026-09-20
 
diff --git a/src/session_recall/codex_fix/tests/test_v5_runner_review.py b/src/session_recall/codex_fix/tests/test_v5_runner_review.py
index dce674e..29410c2 100644
--- a/src/session_recall/codex_fix/tests/test_v5_runner_review.py
+++ b/src/session_recall/codex_fix/tests/test_v5_runner_review.py
@@ -3,6 +3,7 @@
 from __future__ import annotations
 
 import json
+import os
 import subprocess
 import sys
 import time
@@ -132,16 +133,34 @@ def test_real_subprocess_output_flood_is_killed_and_bounded(tmp_path):
         )
 
 
+def _process_gone(pid: int, deadline: float) -> bool:
+    end = time.monotonic() + deadline
+    while time.monotonic() < end:
+        try:
+            os.kill(pid, 0)
+        except ProcessLookupError:
+            return True
+        except PermissionError:
+            return True  # PID reused by another user's process: ours is gone
+        time.sleep(0.05)
+    return False
+
+
 def test_real_subprocess_timeout_kills_child_process_group(tmp_path):
+    # The child outlives the timeout by a wide margin, so the outcome depends
+    # only on whether the process group is killed, not on scheduling luck.
     sentinel = tmp_path / "child-survived"
-    child = "import time; from pathlib import Path; time.sleep(1); Path(%r).write_text('bad')" % str(sentinel)
+    pid_file = tmp_path / "child.pid"
+    child = "import time; from pathlib import Path; time.sleep(30); Path(%r).write_text('bad')" % str(sentinel)
     parent = (
-        "import subprocess,sys,time; "
-        f"subprocess.Popen([sys.executable,'-c',{child!r}]); time.sleep(30)"
+        "import subprocess,sys,time; from pathlib import Path; "
+        f"p = subprocess.Popen([sys.executable,'-c',{child!r}]); "
+        f"Path({str(pid_file)!r}).write_text(str(p.pid)); time.sleep(30)"
     )
     with pytest.raises(subprocess.TimeoutExpired):
-        _actual_run([sys.executable, "-c", parent], timeout=1, cwd=tmp_path)
-    time.sleep(1.2)
+        _actual_run([sys.executable, "-c", parent], timeout=2, cwd=tmp_path)
+    child_pid = int(pid_file.read_text())
+    assert _process_gone(child_pid, deadline=5.0), "child survived the group kill"
     assert not sentinel.exists()