fix(cli): install async exception filter on active loop - #69018
Open
steezkelly wants to merge 1 commit into
Open
fix(cli): install async exception filter on active loop#69018steezkelly wants to merge 1 commit into
steezkelly wants to merge 1 commit into
Conversation
Collaborator
teknium1
reviewed
Jul 30, 2026
teknium1
left a comment
Contributor
There was a problem hiding this comment.
Thanks for tracing the installation-order failure. The premise is confirmed on current main: cli.py:17095 calls get_running_loop() before app.run() at cli.py:17106, then suppresses the resulting RuntimeError at cli.py:17097.
Problems
tests/cli/test_cli_asyncio_exception_handler.py:40usesinspect.getsource(HermesCLI.run)and asserts an implementation string.AGENTS.mdprohibits source-reading tests, and this assertion cannot establish thatApplication.run()receives and executes the callback.
Suggested changes
- Replace that assertion with a behavioral wiring test that captures the
pre_runpassed toApplication.run, invokes it from a live asyncio loop, and verifies the loop filter is installed.
The production approach is otherwise narrowly scoped to the affected CLI path. This is an automated hermes-sweeper review.
|
|
||
| def test_cli_installs_exception_filter_after_prompt_toolkit_loop_starts(): | ||
| source = inspect.getsource(HermesCLI.run) | ||
|
|
Contributor
There was a problem hiding this comment.
Please replace this source-inspection assertion with a behavioral test that captures Application.run(..., pre_run=...), invokes the callback inside a running loop, and verifies the handler. AGENTS.md prohibits tests that read production source text, and this does not prove the callback wiring executes.
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
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
pre_runcallback, when its event loop is actually runningApplication.run()wiringRoot cause
HermesCLI.run()calledasyncio.get_running_loop()synchronously beforeApplication.run()started prompt_toolkit's loop. The resultingRuntimeErrorwas swallowed, so the filter was never installed and benign late httpx/httpcore cleanup surfaced as an interactive "Event loop is closed" exception screen.Verification
pytest tests/cli/test_cli_asyncio_exception_handler.py -q— 2 passedpytest tests/cli tests/hermes_cli/test_suppress_eio_on_interrupt.py tests/test_model_tools_async_bridge.py -q -k "not test_session_not_found_goes_to_stdout_in_full_mode"— 1129 passed, 1 deselectedtest_session_not_found_goes_to_stdout_in_full_mode; it reproduces unchanged on the base checkout and passes in isolationruff check cli.py tests/cli/test_cli_asyncio_exception_handler.py— passedgit diff --checkandpy_compile— passed