Fix Docker sandbox timeout handling#14
Conversation
There was a problem hiding this comment.
Code Review
This pull request introduces a mechanism to handle command timeouts inside the Docker sandbox using GNU timeout, along with robust process-tree termination when a timeout occurs. It also updates the Python, Shell, and Multi-Code executors to correctly propagate and report timeout statuses, and adds corresponding integration tests. The review feedback identifies two critical issues: first, the recursive kill_tree shell function fails to pass the signal argument down to recursive calls, preventing descendant processes from being terminated; second, if the timeout is zero or negative, _outer_timeout returns a non-positive value, which will cause asyncio.wait_for to immediately raise a TimeoutError and block execution.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
b89c2cd to
dea033e
Compare
dea033e to
abc0b49
Compare
What changed
Why
Docker exec timeouts could leave child processes running after the caller reported a timeout. This made timeout handling incomplete and could leave work continuing inside the sandbox.
Validation
git diff --checkDOCKER_HOST=unix:///Users/yunlin/.colima/default/docker.sock conda run -n eval pytest tests/test_tool.py -k 'timeout' -qDOCKER_HOST=unix:///Users/yunlin/.colima/default/docker.sock conda run -n eval pytest tests/test_tool.py -q