feature(sandbox): Fix heartbeat timeout recovery and session-bound sandbox lifecycle#413
Conversation
Summary of ChangesHello @rayrayraykk, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request significantly enhances the robustness and reliability of sandbox lifecycle management by introducing a more detailed container state model and refining the heartbeat mechanism. The changes enable better tracking of session-bound sandboxes, automatic recovery from inactivity, and consistent handling of environment variables, ultimately leading to a more stable and resilient sandbox environment. Highlights
Ignored Files
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
There was a problem hiding this comment.
Code Review
This pull request significantly enhances the sandbox's heartbeat handling and recovery mechanisms. Key improvements include integrating session tracking directly into the ContainerModel, normalizing environment variables for robustness, and introducing a comprehensive restore_session logic. The changes are well-structured, reflecting a clear intent to improve the lifecycle management of sandbox containers. The accompanying test updates, including a new integration test, provide good coverage for the new functionality.
There was a problem hiding this comment.
Pull request overview
This PR enhances sandbox heartbeat handling and recovery from inactivity timeouts by introducing session-bound container lifecycle management.
Changes:
- Introduces
ContainerStateenum and extendsContainerModelwith lifecycle fields (state, last_active_at, recycled_at, etc.) for tracking container status - Refactors heartbeat and recycle tracking from separate mappings to container-level fields within
ContainerModel - Implements
restore_session()method to recreate recycled containers when users return after timeout - Adds session_ctx_id binding to sandbox creation in sync/async contexts
- Normalizes environment variables in boxlite_client to string key/value pairs
- Adds integration test verifying the full heartbeat timeout → reap → restore → usage cycle
Reviewed changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 18 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/sandbox/test_heartbeat_timeout_restore.py | New integration test verifying heartbeat timeout recovery flow |
| tests/sandbox/test_heartbeat.py | Updated tests using stub clients for heartbeat functionality |
| src/agentscope_runtime/sandbox/model/container.py | Extended ContainerModel with state tracking and lifecycle timestamps |
| src/agentscope_runtime/sandbox/model/init.py | Exported ContainerState enum |
| src/agentscope_runtime/sandbox/manager/sandbox_manager.py | Implemented restore_session, updated cleanup/reap logic, modified create_from_pool binding |
| src/agentscope_runtime/sandbox/manager/heartbeat_mixin.py | Refactored heartbeat/recycle tracking to use container-level fields |
| src/agentscope_runtime/sandbox/box/sandbox.py | Added session_ctx_id generation and binding in sandbox creation |
| src/agentscope_runtime/common/container_clients/boxlite_client.py | Normalized environment variables to string pairs |
| .github/workflows/unit_test_sandbox.yml | Added new test to CI workflow |
Comments suppressed due to low confidence (1)
src/agentscope_runtime/sandbox/manager/sandbox_manager.py:1482
- The docstring for scan_heartbeat_once does not follow the required template. According to custom guideline 1000000, all methods must have complete docstrings with Args and Returns sections in the specified format.
def scan_heartbeat_once(self) -> dict:
"""
Scan all session_ctx_id in session_mapping and reap those idle
beyond timeout. Uses redis distributed lock to avoid multi-instance
double reap.
|
Link to #289 |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 11 out of 11 changed files in this pull request and generated 6 comments.
Comments suppressed due to low confidence (1)
src/agentscope_runtime/sandbox/manager/sandbox_manager.py:1499
- Docstring does not follow the required format. According to the coding guidelines [MUST], all methods must have complete docstrings with Args and Returns sections following the specified template. The method should document:
Args:
(No arguments)
Returns:
dict:
Dictionary containing scan metrics with keys: "scanned_sessions", "reaped_sessions", "skipped_no_heartbeat", "skipped_no_running_containers", "skipped_lock_busy", "skipped_not_idle_after_double_check", "errors"
def scan_heartbeat_once(self) -> dict:
"""
Scan all session_ctx_id in session_mapping and reap those idle
beyond timeout. Uses redis distributed lock to avoid multi-instance
double reap.
"""
Description
[Describe what this PR does and why]
This PR improves sandbox heartbeat handling and recovery after inactivity.
session_ctx_idto sandbox creation (sync/async) for better session tracking.ContainerModelwith lifecycle fields (state,last_active_at,recycled_at, etc.) and keep backward compatibility withmeta["session_ctx_id"].HeartbeatMixinto read/write heartbeat and recycle status on container models, and only callrestore_sessionwhen implemented.run_shell_commandworks again.Type of Change
Component(s) Affected
Checklist