fix(cli): schedule prompt_toolkit terminal awaitables#22987
fix(cli): schedule prompt_toolkit terminal awaitables#22987pablomoralesm wants to merge 1 commit into
Conversation
fa180fe to
3bc8df8
Compare
|
Thanks — agreed this overlaps with #22989 on #22958. The distinction I was trying to cover here is the prompt_toolkit 3.0.52+ behavior where From a quick comparison, #22989 changes the prompt implementation itself ( Happy to rebase this onto #22989, fold the awaitable scheduling/test into that approach, or close this if maintainers prefer the other patch as the landing vehicle. |
|
Closing this as superseded by changes that have since landed on The original slash-confirmation/input failure is now covered by:
Those changes address the user-facing The remaining Thanks for the pointers on the related fixes. |
Summary
prompt_toolkit.application.run_in_terminal(...)on the active prompt_toolkit app loop before calling it from background slash-command threads./new,/reset,/clear, and/undowhen prompt_toolkit returns an awaitable._cprint, and Ctrl+Z suspend._prompt_text_inputfrom a process-loop-like background thread whilerun_in_terminalexecutes on the app-loop thread.Related issues / PRs
run_in_terminal(...)awaitable root cause of fix(cli): RuntimeWarning in /new, /clear, /undo, /reload-mcp — coroutine was never awaited #22970 and duplicate report bug: /reload-mcp triggers RuntimeWarning from unawaited run_in_terminal coroutine #23009:RuntimeWarning: coroutine 'run_in_terminal.<locals>.run' was never awaited.input()-> prompt_toolkit prompt). This PR focuses on safely scheduling/awaitingrun_in_terminal(...)from background-thread call sites and includes a regression test for that path.Why
/newand other destructive slash commands run their confirmation flow from the CLIprocess_loopbackground thread while prompt_toolkit owns the terminal on its application event loop. In prompt_toolkit 3.x,run_in_terminal(...)returns/schedules an awaitable and must be called from the app loop. Calling it directly from the background thread can drop the prompt and emit:How to test
Reproduction before this fix:
/new.After this fix:
/new.3to cancel./new cancelledwithout the coroutine warning.Platforms tested
Test Plan
Verified in a fresh clone following the CONTRIBUTING setup instructions on commit
3bc8df818.Passed:
venv/bin/python -m py_compile cli.py tests/cli/test_cprint_bg_thread.pyvenv/bin/python scripts/check-windows-footguns.py cli.py tests/cli/test_cprint_bg_thread.pyscripts/run_tests.sh tests/cli/test_cprint_bg_thread.py tests/hermes_cli/test_destructive_slash_confirm_gate.py17 passedManual verification:
/new, selected3to cancel, and verified the confirmation prompt worked without the unawaited coroutine warning.Full-suite note:
scripts/run_tests.sh testsin a fresh clone. It did not complete green (16333 passed, 75 skipped, 33 failed, 5057 errors), with broad unrelated-looking failures across plugin/skill/TUI/website tests. A representative reported failing test passed when rerun in isolation, so I did not treat this as related to this CLI fix.scripts/run_tests.shwith no args currently fails locally withARGS[@]: unbound variable;scripts/run_tests.sh testswas used for the full-suite attempt.