Uh oh!
There was an error while loading. Please reload this page.
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.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
b89c2cd to
dea033eComparedea033e to
abc0b49CompareUh oh!
There was an error while loading. Please reload this page.
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