From 8ba669cf57c9c08c1dfbc71bbf02932d9dbf3b46 Mon Sep 17 00:00:00 2001 From: Patrick Buckley Date: Tue, 16 Jun 2026 16:49:21 -0700 Subject: [PATCH] fix(deadline): prefer a ready result over a same-window deadline/cancel MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit run_with_deadline checked the deadline/cancel before reading the result queue, so a call that completed in the same scheduling window could be reported as a spurious timeout. Drain the queue first. Also from review: - test_validate_regex_pattern stubs run_with_deadline, so the probe regex never runs — use a benign pattern instead of a real backtracking literal (the literal tripped a ReDoS scanner). - output_guard_judge docstring: reference IntentJudge._parse_verdict instead of brittle judge.py line numbers. The _runner BaseException catch is intentional and kept: it relays (not swallows) whatever fn() raises to the caller via the queue; narrowing to Exception would let a BaseException escape the worker so the caller never gets a value, degrading the no-hang guarantee. --- tests/test_validate_regex_pattern.py | 4 +++- turnstone/core/deadline.py | 16 ++++++++++++++-- turnstone/core/output_guard_judge.py | 2 +- 3 files changed, 18 insertions(+), 4 deletions(-) diff --git a/tests/test_validate_regex_pattern.py b/tests/test_validate_regex_pattern.py index 3ba0a36f..514d3094 100644 --- a/tests/test_validate_regex_pattern.py +++ b/tests/test_validate_regex_pattern.py @@ -27,7 +27,9 @@ def test_catastrophic_backtracking_returns_message(monkeypatch) -> None: raise DeadlineExceededError monkeypatch.setattr("turnstone.console.server.run_with_deadline", _deadline) - assert _validate_regex_pattern(r"(a+)+$") == "Regex appears to have catastrophic backtracking" + # The pattern is arbitrary — run_with_deadline is stubbed to raise, so the + # probe never runs; a real backtracking literal here would only trip CodeQL. + assert _validate_regex_pattern(r"\w+") == "Regex appears to have catastrophic backtracking" def test_probe_error_returns_generic_message(monkeypatch) -> None: diff --git a/turnstone/core/deadline.py b/turnstone/core/deadline.py index 5afd9aef..d0dbfd5e 100644 --- a/turnstone/core/deadline.py +++ b/turnstone/core/deadline.py @@ -68,6 +68,18 @@ def run_with_deadline( deadline = time.monotonic() + timeout while True: + # Prefer a result that has already arrived over a deadline or cancel + # firing in the same scheduling window — otherwise a completed call + # could be reported as a spurious timeout/cancel under jitter. + try: + ok, payload = box.get_nowait() + except queue.Empty: + pass + else: + if ok: + return payload # type: ignore[return-value] # ok=True ⇒ payload is _T + raise payload # type: ignore[misc] # ok=False ⇒ payload is the raised exc + if cancel_event is not None and cancel_event.is_set(): raise DeadlineCancelledError remaining = deadline - time.monotonic() @@ -78,5 +90,5 @@ def run_with_deadline( except queue.Empty: continue if ok: - return payload # type: ignore[return-value] # ok=True ⇒ payload is _T - raise payload # type: ignore[misc] # ok=False ⇒ payload is the raised exc + return payload # type: ignore[return-value] + raise payload # type: ignore[misc] diff --git a/turnstone/core/output_guard_judge.py b/turnstone/core/output_guard_judge.py index 96c35507..e3de8a30 100644 --- a/turnstone/core/output_guard_judge.py +++ b/turnstone/core/output_guard_judge.py @@ -12,7 +12,7 @@ Design: a static tool result doesn't benefit from multi-turn — the text is already in hand. - JSON-in-content verdict. 4-strategy parser inlined from - :class:`IntentJudge` (``judge.py:1603-1659``). + :meth:`IntentJudge._parse_verdict`. - Wall-clock deadline via :func:`turnstone.core.deadline.run_with_deadline`, which runs the call on a *daemon* worker and polls the cancel event each second. A timeout or cancel abandons the call rather than waiting it out,