fix: wake E2B PTY collection on process exit#3629
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.
Reviewed commit: 0ebfe8ca91
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| except Exception: | ||
| pass | ||
| finally: | ||
| entry.done = True | ||
| entry.output_notify.set() |
There was a problem hiding this comment.
When handle.wait() raises for something other than command completion (for example, an E2B stream/RPC error while a long-running command is still alive), this finally still marks the entry done. _finalize_pty_update() then treats _entry_done() as terminal, removes the session, and calls _terminate_pty_entry(), so a transient waiter failure can return process_id=None/no exit code and kill an otherwise live PTY or background command. Only mark the entry done after observing a real terminal result/exit code, or explicitly classify terminal command-exit exceptions separately.
Useful? React with 👍 / 👎.
he-yufeng
force-pushed
the
fix/e2b-exit-wakeup
branch
from
0ebfe8c to
62b561c
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 62b561cc9d
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| on_data=_append_output, | ||
| ) | ||
| entry.handle = handle | ||
| asyncio.create_task(self._run_pty_waiter(entry)) |
There was a problem hiding this comment.
For tty=True sessions, this waiter is attached to the handle returned by E2B pty.create, which starts a login shell (/bin/bash -i -l) rather than the command sent on the next line (E2B SDK). When a child command exits from inside that PTY, such as python3 exiting after stdin, the shell remains alive so wait() never completes and pty_write_stdin(..., yield_time_s=...) still waits for the full yield window unless the shell itself exits; the new test fakes the PTY handle finishing on child exit, but real E2B does not do that.
Useful? React with 👍 / 👎.
2 participants
Summary
Fixes #3609
To verify
PYTHONPATH=src python -m pytest tests/extensions/sandbox/test_e2b.py -qPYTHONPATH=src python -m ruff check src/agents/extensions/sandbox/e2b/sandbox.py tests/extensions/sandbox/test_e2b.pyPYTHONPATH=src python -m py_compile src/agents/extensions/sandbox/e2b/sandbox.py tests/extensions/sandbox/test_e2b.pygit diff --check upstream/main...HEAD