fix(approval): close prompt_toolkit deadlock vectors (#15216)#16574
Merged
Conversation
…reads When tools execute concurrently via ThreadPoolExecutor, worker threads could not see the thread-local approval/sudo callbacks registered by the CLI. This caused dangerous-command prompts to fall back to plain input(), which deadlocks against prompt_toolkit's raw terminal mode. Capture parent-thread callbacks before launching workers, register them locally in each _run_tool thread, and clear them on exit. Mirrors the existing fix pattern from cli.py run_agent() for the main agent worker thread (GHSA-qg5c-hvr5-hjgr / #13617).
PR #13734 fixed the concurrent-tool-executor vector (ThreadPoolExecutor workers didn't inherit the CLI's TLS approval callback). Two vectors remained that could still land in the deadlocking input() fallback: 1. _spawn_background_review spawns a raw threading.Thread with no approval callback installed, so any dangerous-command guard the review agent trips falls back to input() -> deadlock against the parent's prompt_toolkit TUI (same class as delegate_task subagents, fixed in 023b1bf / #15491). Install a _bg_review_auto_deny callback at thread start, clear on finally. 2. prompt_dangerous_approval's fallback unconditionally spawned a daemon thread calling input() when approval_callback was None. That fallback can never succeed under prompt_toolkit because the user's Enter goes to pt's raw-mode stdin capture. Detect an active pt Application via get_app_or_none() and fail closed (deny + log) instead, so future threads that forget to install a callback degrade gracefully instead of hanging 60s invisibly. Regression guards: - tests/run_agent/test_background_review.py verifies the review worker thread sees a callable auto-deny callback mid-run and that the slot is cleared in the finally block. - tests/tools/test_approval.py TestFailClosedUnderPromptToolkit verifies prompt_dangerous_approval returns 'deny' fast under a mocked pt Application, and that a real callback still wins over the guard.
Release-notes contributor attribution for the salvaged PR #13734 fix.
This was referenced Apr 27, 2026
4 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Approval prompts no longer deadlock against prompt_toolkit. The fallback in
tools/approval.pyspawned a daemon thread callinginput()whenever a thread reached it without an approval callback; under prompt_toolkit's raw-mode stdin capture that thread never sees Enter, producing an invisible 60s freeze (#15216).Salvages @andrewhosf's #13734 (concurrent-tool-executor vector) and closes the two remaining vectors.
Changes
_execute_tool_calls_concurrent(from fix(agent): propagate approval callbacks to concurrent tool worker threads #13734, @andrewhosf): snapshot the parent thread's approval + sudo TLS callbacks and register them on eachThreadPoolExecutorworker; clear on exit._spawn_background_review: install a_bg_review_auto_denycallback on thebg-reviewthread (mirrors_subagent_auto_denyfrom 023b1bf / fix(delegate): resolve subagent approval prompts without deadlocking parent TUI #15491); clear infinally.prompt_dangerous_approvalfallback: whenprompt_toolkit.application.current.get_app_or_none()is not None AND no callback is registered, deny fast with a logged warning instead of spawning the deadlockinginput()thread. Fail-closed so future threads that forget to install a callback degrade gracefully.tests/run_agent/test_background_review.py(bg-review installs + clears an auto-deny callback) andtests/tools/test_approval.py::TestFailClosedUnderPromptToolkit(denies fast under pt + callback still wins over guard).Validation
E2E verification (real imports, no mocks):
get_app_or_none()patched to return an app and callback=None,prompt_dangerous_approvalreturns"deny"within 5s instead of blocking oninput().Closes #15216. Closes #15194 (duplicate, also closed by reporter).
Credits @andrewhosf (concurrent-tool vector, #13734) and @stillmuchtolearn (background-review vector, diagnosis in #15216 comments).