Skip to content

fix(cli): schedule prompt_toolkit terminal awaitables#22987

Closed
pablomoralesm wants to merge 1 commit into
NousResearch:mainfrom
pablomoralesm:fix/prompt-toolkit-run-in-terminal-awaitable
Closed

fix(cli): schedule prompt_toolkit terminal awaitables#22987
pablomoralesm wants to merge 1 commit into
NousResearch:mainfrom
pablomoralesm:fix/prompt-toolkit-run-in-terminal-awaitable

Conversation

@pablomoralesm

@pablomoralesm pablomoralesm commented May 10, 2026

Copy link
Copy Markdown

Summary

  • Schedule prompt_toolkit.application.run_in_terminal(...) on the active prompt_toolkit app loop before calling it from background slash-command threads.
  • Fix destructive slash command confirmation prompts such as /new, /reset, /clear, and /undo when prompt_toolkit returns an awaitable.
  • Apply the same awaitable handling to related terminal-bridged call sites for curses picker, background _cprint, and Ctrl+Z suspend.
  • Add a regression test covering _prompt_text_input from a process-loop-like background thread while run_in_terminal executes on the app-loop thread.

Related issues / PRs

Why

/new and other destructive slash commands run their confirmation flow from the CLI process_loop background 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:

RuntimeWarning: coroutine 'run_in_terminal.<locals>.run' was never awaited

How to test

Reproduction before this fix:

  1. Start interactive Hermes CLI.
  2. Enter /new.
  3. Observe the destructive-command confirmation fail with the unawaited coroutine warning.

After this fix:

  1. Start interactive Hermes CLI.
  2. Enter /new.
  3. The confirmation prompt appears.
  4. Enter 3 to cancel.
  5. Hermes reports /new cancelled without the coroutine warning.

Platforms tested

  • macOS
  • Python 3.11
  • prompt_toolkit 3.0.52

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.py
  • venv/bin/python scripts/check-windows-footguns.py cli.py tests/cli/test_cprint_bg_thread.py
  • scripts/run_tests.sh tests/cli/test_cprint_bg_thread.py tests/hermes_cli/test_destructive_slash_confirm_gate.py
    • 17 passed

Manual verification:

  • Started Hermes with an isolated temporary home directory, ran /new, selected 3 to cancel, and verified the confirmation prompt worked without the unawaited coroutine warning.

Full-suite note:

  • Attempted scripts/run_tests.sh tests in 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.sh with no args currently fails locally with ARGS[@]: unbound variable; scripts/run_tests.sh tests was used for the full-suite attempt.

@pablomoralesm pablomoralesm force-pushed the fix/prompt-toolkit-run-in-terminal-awaitable branch from fa180fe to 3bc8df8 Compare May 10, 2026 05:36
@alt-glitch alt-glitch added type/bug Something isn't working comp/cli CLI entry point, hermes_cli/, setup wizard P2 Medium — degraded but workaround exists labels May 10, 2026
@alt-glitch

Copy link
Copy Markdown
Collaborator

Competing fix for #22958 alongside #22989. Also addresses root cause of #22970 (RuntimeWarning from unawaited coroutine).

@pablomoralesm

Copy link
Copy Markdown
Author

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 run_in_terminal(...) returns an awaitable/Task. In the background-thread slash-command path, calling it directly can drop the terminal operation and produce the unawaited coroutine warning seen in #22970.

From a quick comparison, #22989 changes the prompt implementation itself (input() -> prompt_toolkit.shortcuts.prompt()), while this PR adds the missing bridge that schedules/awaits run_in_terminal(...) on the active Application loop and adds a regression test that calls _prompt_text_input from a separate thread to match process_loop.

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.

@pablomoralesm

Copy link
Copy Markdown
Author

Closing this as superseded by changes that have since landed on main.

The original slash-confirmation/input failure is now covered by:

Those changes address the user-facing /new, /clear, /undo, and /reload-mcp confirmation path this PR was opened for, and current main now conflicts with this branch.

The remaining run_in_terminal(...) awaitable/output-loss angle appears to be better tracked separately in #23297 / #23302, so keeping this PR open would mostly duplicate and conflict with the current implementation.

Thanks for the pointers on the related fixes.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

comp/cli CLI entry point, hermes_cli/, setup wizard P2 Medium — degraded but workaround exists type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants