From 26cf4565984d34bbbe58a32f7ec8784144efcd41 Mon Sep 17 00:00:00 2001 From: Teknium <127238744+teknium1@users.noreply.github.com> Date: Sun, 23 Aug 2026 16:01:08 -0700 Subject: [PATCH] =?UTF-8?q?refactor:=20reconcile=20with=20#93269=20?= =?UTF-8?q?=E2=80=94=20one=20shared=20shutdown=20predicate,=20keep=20both?= =?UTF-8?q?=20guard=20sites?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit PR #93269 (kshitijk4poor) landed the outer-handler break for the same symptom while this branch was in flight. Keep his guard (it covers shutdown errors from local post-processing and does the resume-hint + best-effort persist) and keep this branch's inner-retry-handler return (it fires BEFORE the ⚠️ retry trace, credential rotation, and fallback attempts that the outer handler never sees). Point his _is_interpreter_shutdown_error at tools/interpreter_shutdown.py so the class has exactly one text-matching site, preserving his RuntimeError type gate and all 7 of his tests. --- agent/conversation_loop.py | 45 +++++++++----------------------------- 1 file changed, 10 insertions(+), 35 deletions(-) diff --git a/agent/conversation_loop.py b/agent/conversation_loop.py index cb2e067b1a..93a41ebf90 100644 --- a/agent/conversation_loop.py +++ b/agent/conversation_loop.py @@ -279,15 +279,18 @@ def _is_interpreter_shutdown_error(exc: Exception) -> bool: During teardown, ``concurrent.futures`` refuses new work with ``RuntimeError: cannot schedule new futures after interpreter shutdown`` (or the shorter ``... after shutdown`` variant from a plain - ThreadPoolExecutor). Both are documented in #58720. The common - prefix catches both; the module-global ``is_finalizing`` flag can - lag the error by a hair, so matching the error text is the safe - fallback for that race. + ThreadPoolExecutor). Both are documented in #58720. + + Delegates to the shared predicate in ``tools.interpreter_shutdown`` + (same home as cron delivery and concurrent tool submission) so the + shutdown-race bug class has one text-matching site. Keeps the + RuntimeError type gate from the original (#93269): unlike the raw + predicate, a ValueError carrying similar text must not match here. """ if isinstance(exc, RuntimeError): - msg = str(exc).lower() - if "cannot schedule new futures" in msg: - return True + from tools.interpreter_shutdown import interpreter_shutting_down + + return interpreter_shutting_down(exc) return False @@ -8410,34 +8413,6 @@ def run_conversation( _is_local_processing_error = _hit_local and not _hit_api - # Interpreter finalization: the process is exiting (TUI quit, - # SIGTERM, one-shot CLI done) while a background turn — most - # commonly the post-turn review fork's daemon thread — is still - # mid-request. Every further API attempt raises "cannot schedule - # new futures after interpreter shutdown", so retrying is futile - # and the un-gated ❌ prints spam the user's shell AFTER the TUI - # already exited (call #4, #5, #6...). Same shutdown-race class - # as cron delivery (#55924/#58720) and concurrent tool submission; - # shared predicate in tools/interpreter_shutdown.py. Log one - # warning, no traceback, no synthetic history append (nothing can - # persist it anymore), and leave the loop immediately. - from tools.interpreter_shutdown import interpreter_shutting_down - - if interpreter_shutting_down(e): - logger.warning( - "Interpreter is shutting down — abandoning turn after " - "API call #%d (%s)", - api_call_count, - e, - ) - _turn_exit_reason = "interpreter_shutdown" - failed = True - final_response = ( - "Turn abandoned: the process was shutting down before " - "the model call could complete." - ) - break - if _is_local_processing_error: error_msg = ( f"Error during local message processing after "