fix: improve run_sync's python 3.14 compatibility #2006
Conversation
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
src/agents/run.py
Outdated
| # We intentionally leave the default loop open even if we had to create one above. Session | ||
| # instances and other helpers stash loop-bound primitives between calls and expect to find | ||
| # the same default loop every time run_sync is invoked on this thread. | ||
| return default_loop.run_until_complete( |
There was a problem hiding this comment.
The new implementation leaves the coroutine returned by self.run() attached to the default loop and simply calls default_loop.run_until_complete(...) (lines 751‑754). When the loop is interrupted before completion—e.g. a user presses Ctrl+C and triggers KeyboardInterrupt—run_until_complete unwinds immediately and does not cancel the task it created. Because we now keep the loop open for reuse, that unfinished task remains pending on the loop and resumes the next time run_sync is invoked, so an aborted agent run keeps running concurrently with the next run. This regression is easy to reproduce: start run_sync, hit Ctrl+C to abort it, then call run_sync again and observe the first run continue. The previous asyncio.run call avoided this by creating a fresh loop per invocation and tearing it down on interruption. Please wrap the scheduled coroutine in a task and cancel/await it when run_until_complete exits with an exception (especially KeyboardInterrupt) so that aborted runs do not linger on the shared loop.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
@codex i've resolved this issue. can you review the changes again?
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| ) | ||
| ) | ||
|
|
||
| try: | ||
| return default_loop.run_until_complete(task) | ||
| except BaseException: |
There was a problem hiding this comment.
The new run_sync implementation stops using asyncio.run/Runner, but it never performs the cleanup that Runner.close() used to do. After the default_loop.run_until_complete(task) call the code simply returns, so _asyncgens is never drained with loop.shutdown_asyncgens(). Any async generators created inside AgentRunner.run (or user hooks/tools) that aren’t fully consumed now keep their finally blocks from running unless another loop run happens later. You can reproduce the regression by invoking run_sync on a coroutine that does g = agen(); await g.asend(None) where agen has a finally: cleanup. Before this change cleanup would run as soon as run_sync returned; now it never runs because the loop is left open and the registered async generators are never closed. This leaks resources and makes async generators accumulate across runs. Please call default_loop.run_until_complete(default_loop.shutdown_asyncgens()) (and then return the result) so the loop still stays open but async generators are finalized after every synchronous run.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
@codex improved this; can you review the changes again?
There was a problem hiding this comment.
Codex Review: Didn't find any major issues. What shall we delve into next?
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
This pull request improves run_sync method internals in response to the codex suggestion at #1961 (comment)