diff --git a/contributors/emails/jethachan@gmail.com b/contributors/emails/jethachan@gmail.com new file mode 100644 index 00000000000..0a23ebe1cf5 --- /dev/null +++ b/contributors/emails/jethachan@gmail.com @@ -0,0 +1 @@ +jethac diff --git a/tests/gateway/test_approve_deny_commands.py b/tests/gateway/test_approve_deny_commands.py index bf3a44515e9..ed668b526dc 100644 --- a/tests/gateway/test_approve_deny_commands.py +++ b/tests/gateway/test_approve_deny_commands.py @@ -76,6 +76,26 @@ def _clear_approval_state(): mod._pending.clear() +def _wait_until(predicate, timeout=30.0, interval=0.02): + """Wait until *predicate()* is truthy or *timeout* elapses; return its value. + + Replaces the fixed-iteration ``for _ in range(N): sleep(0.05)`` polls these + E2E tests used to wait for a background agent thread to reach the gateway + approval notify. Those budgets (2.5-5s) race the scheduler: under CI worker + contention the thread can be starved past the deadline, so the notify fires + after the poll gives up and the assertion sees an empty list. The generous + ceiling here is only reached on genuine failure; the common path returns the + instant the condition holds, adding no latency to green runs. + (Ported from PR #63522 by @jethac.) + """ + deadline = time.monotonic() + timeout + result = predicate() + while not result and time.monotonic() < deadline: + time.sleep(interval) + result = predicate() + return result + + # ------------------------------------------------------------------ # Blocking gateway approval infrastructure (tools/approval.py) # ------------------------------------------------------------------ @@ -390,10 +410,8 @@ class TestBlockingApprovalE2E: t.start() # Wait for all 3 to block - for _ in range(100): - if len(notified) >= 3: - break - time.sleep(0.05) + assert _wait_until(lambda: len(notified) >= 3), \ + "not all 3 agents reached the gateway approval notify" assert len(notified) == 3 assert len(_gateway_queues.get(session_key, [])) == 3 @@ -571,10 +589,7 @@ class TestCrossSessionApprovalIsolation: t = threading.Thread(target=worker_a) t.start() try: - for _ in range(50): - if notified_a or notified_b: - break - time.sleep(0.05) + _wait_until(lambda: notified_a or notified_b) # The prompt must land in session A (the originator), never B. assert len(notified_a) == 1, "approval prompt did not route to session A" @@ -636,11 +651,10 @@ class TestCrossSessionApprovalIsolation: tb.start() try: # Wait until both sessions have a pending approval in their queue. - for _ in range(100): - if (len(_gateway_queues.get("sess-A", [])) >= 1 - and len(_gateway_queues.get("sess-B", [])) >= 1): - break - time.sleep(0.05) + assert _wait_until( + lambda: len(_gateway_queues.get("sess-A", [])) >= 1 + and len(_gateway_queues.get("sess-B", [])) >= 1 + ), "both sessions never reached a pending approval" # Each command must be parked in its OWN session queue. qa = _gateway_queues.get("sess-A", [])