add stateful jupyter notebook option and make browser more reliable #199
Conversation
|
@codex review |
![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 👍.
| self._jupyter_session: LocalJupyterSession | None = None | ||
| self._execution_lock: asyncio.Lock | None = None | ||
|
|
||
| if self._execution_backend == "dangerously_use_local_jupyter": | ||
| self._execution_lock = asyncio.Lock() | ||
| self._jupyter_session = LocalJupyterSession( | ||
| connection_file=self._local_jupyter_connection_file, |
There was a problem hiding this comment.
When the Python tool is configured for the new dangerously_use_local_jupyter backend, the constructor immediately executes self._execution_lock = asyncio.Lock(). asyncio.Lock calls get_running_loop, so constructing PythonTool() in synchronous code (e.g. in gpt_oss/chat.py or gpt-oss-mcp-server/python_server.py) now raises RuntimeError: no running event loop whenever PYTHON_EXECUTION_BACKEND is set to this backend. That means the CLI and MCP server crash before they even start when users enable the stateful Jupyter option. The lock should be created lazily inside an async context or via a loop-safe factory so the tool can still be instantiated outside a running loop.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
@codex can you fix it?
There was a problem hiding this comment.
Note
To use Codex here, create an environment for this repo.
There was a problem hiding this comment.
@codex please try again
There was a problem hiding this comment.
Note
To use Codex here, create an environment for this repo.
|
[deleted comment that was just "Yes 🚀" that might mess with the codex flow] |
functions.browser.searchas opposed tobrowser.searchas long as there is no conflicting function defined.