From 2c37b1af25f936d48fb00edeebd661a7d0b0e775 Mon Sep 17 00:00:00 2001 From: jethac Date: Wed, 29 Jul 2026 17:29:42 -0700 Subject: [PATCH] test(approval): event-based waits for blocking-approval E2E polls (PR #63522 port) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Manual port of @jethac's #63522 onto the pruned tree: 3 of the original 6 fixed-budget poll sites survive (3-concurrent-agents wait, session-A/B routing wait, two-session queue wait). Replaced each 2.5-5s hard-ceiling poll loop with the PR's _wait_until(predicate) helper — generous 30s ceiling reached only on genuine failure, instant return on the green path, assert with message instead of silent fall-through. Dropped: the 3 hunks targeting pruned code, and the unrelated scripts/release.py mailmap hunk (AUTHOR_MAP is frozen; contributor mapping handled via contributors/emails/). --- contributors/emails/jethachan@gmail.com | 1 + tests/gateway/test_approve_deny_commands.py | 40 ++++++++++++++------- 2 files changed, 28 insertions(+), 13 deletions(-) create mode 100644 contributors/emails/jethachan@gmail.com 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", [])