feature (sandbox): add configurable Nginx timeouts and improve startup reliability#411
Conversation
There was a problem hiding this comment.
Pull request overview
This PR enhances sandbox reliability and configurability by adding support for configurable Nginx proxy timeouts and implementing robust startup procedures for GUI-based sandboxes.
Changes:
- Added
NGINX_TIMEOUTenvironment variable support (default: 60 seconds) for configurable Nginx proxy timeouts across all sandbox types - Implemented robust startup procedures in GUI and custom sandboxes by cleaning stale pid/lock files and waiting for X server/VNC readiness before starting dependent services
- Updated all Dockerfiles to export and substitute both
SECRET_TOKENandNGINX_TIMEOUTenvironment variables with fallback defaults
Reviewed changes
Copilot reviewed 14 out of 14 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
| src/agentscope_runtime/sandbox/manager/sandbox_manager.py | Injects NGINX_TIMEOUT environment variable when creating sandbox containers |
| src/agentscope_runtime/sandbox/box/mobile/box/config/nginx.conf | Adds proxy timeout directives using ${NGINX_TIMEOUT} placeholder (has critical bug - no envsubst) |
| src/agentscope_runtime/sandbox/box/gui/box/config/supervisord.conf | Improves service startup with stale file cleanup and X server readiness checks |
| src/agentscope_runtime/sandbox/box/gui/box/config/nginx.conf.template | Adds proxy timeout directives at http level for global coverage |
| src/agentscope_runtime/sandbox/box/gui/Dockerfile | Exports and substitutes environment variables for runtime configuration |
| src/agentscope_runtime/sandbox/box/filesystem/Dockerfile | Exports and substitutes environment variables for runtime configuration |
| src/agentscope_runtime/sandbox/box/browser/Dockerfile | Exports and substitutes environment variables for runtime configuration |
| src/agentscope_runtime/sandbox/box/base/box/config/nginx.conf.template | Adds proxy timeout directives at location level (inconsistent with other sandboxes) |
| src/agentscope_runtime/sandbox/box/base/Dockerfile | Exports and substitutes environment variables for runtime configuration |
| examples/sandbox/custom_sandbox/box/config/supervisord.conf | Improves service startup with stale file cleanup and X server readiness checks |
| examples/sandbox/custom_sandbox/box/config/nginx.conf.template | Adds proxy timeout directives at http level for global coverage |
| examples/sandbox/custom_sandbox/Dockerfile | Exports and substitutes environment variables for runtime configuration |
| cookbook/zh/sandbox/advanced.md | Updates documentation example to reflect new CMD pattern |
| cookbook/en/sandbox/advanced.md | Updates documentation example to reflect new CMD pattern |
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request introduces configurable Nginx timeouts and significantly improves the startup reliability of sandbox services. The changes are well-structured, particularly the enhancements to supervisord.conf which replace fixed sleep calls with more robust readiness checks. I've provided a few suggestions to further improve maintainability and correctness. One suggestion is to move long CMD commands in Dockerfiles to separate startup scripts for better readability. Another is to remove an unnecessary flag in a bash command. Finally, I've pointed out a missing default value for an environment variable in the mobile sandbox's startup script, which could lead to configuration errors. Overall, this is a great set of improvements.
Description
This PR improves sandbox image startup and configurability.
Changes
envsubst:SECRET_TOKEN(default:secret_token_123)NGINX_TIMEOUT(default:60)${NGINX_TIMEOUT}in Nginx config/templates.supervisordservices more robust by cleaning stale pid/lock files and waiting for X/VNC readiness; passSECRET_TOKENvia supervisord env.NGINX_TIMEOUTwhen creating sandboxes insandbox_manager.py.Result
More reliable sandbox startup and configurable Nginx timeouts.
Type of Change
Component(s) Affected
Checklist