[Feature] Add sandbox pools #7

Yunnglin · 2025-12-11T09:14:41Z

No description provided.

gemini-code-assist · 2025-12-11T09:15:15Z

Summary of Changes

Hello @Yunnglin, I'm Gemini Code Assist¹! 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 introduces a significant new feature: sandbox pooling. The primary goal is to enhance the efficiency and scalability of sandbox environments by allowing sandboxes to be pre-initialized and reused for multiple tasks. This reduces the overhead of creating and destroying sandboxes for each execution, making the system more responsive for rapid, repeated tool invocations. The changes involve a refactored manager architecture, new configuration options, and expanded API capabilities to support this pooling mechanism.

Highlights

Sandbox Pooling Implementation: Introduced core logic for managing a pool of reusable sandboxes, including abstract methods in SandboxManager and concrete implementations in LocalSandboxManager and HttpSandboxManager. This allows for pre-provisioning and reusing sandboxes to reduce overhead.
Sandbox Manager Factory: A new SandboxManagerFactory and a @register_manager decorator were added to simplify the creation and registration of different sandbox manager types (local and HTTP), enhancing flexibility and extensibility.
New API Endpoints for Pooling: The sandbox server now exposes /pool/initialize and /pool/execute API endpoints, enabling clients to remotely initialize and utilize sandbox pools for tool execution.
Enhanced Sandbox Statuses and Configuration: New IDLE and BUSY statuses were added to the SandboxStatus enum, alongside a dedicated SandboxManagerConfig model, to support the detailed state management and configuration of sandbox pools.
Comprehensive Testing: A new test file (tests/test_manage_pool.py) was added with extensive unit tests covering the new sandbox pooling functionality, including concurrent execution, timeouts, and error handling for both local and HTTP managers.

Using Gemini Code Assist

The 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 /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands on the current page.

Feature	Command	Description
Code Review	`/gemini review`	Performs a code review for the current pull request in its current state.
Pull Request Summary	`/gemini summary`	Provides a summary of the current pull request in its current state.
Comment	@gemini-code-assist	Responds in comments when explicitly tagged, both in pull request comments and review comments.
Help	`/gemini help`	Displays a list of available commands.

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 .gemini/ folder in the base of the repository. Detailed instructions can be found here.

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.

Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩

gemini-code-assist

Code Review

This pull request introduces a significant new feature: sandbox pooling. This allows for reusing sandboxes to improve performance, which is a great addition. The implementation includes both a local manager with queuing and an HTTP manager that delegates to a server. I appreciate the introduction of a SandboxManagerFactory and the use of a SandboxManagerConfig object, which makes the system more extensible and configurable. The addition of comprehensive tests for the new pooling feature is also excellent.

My feedback focuses on a critical issue in the local pool implementation that could lead to deadlocks, a performance improvement for the HTTP pool initialization, and a couple of improvements to the tests to make them more accurate and robust.

ms_enclave/sandbox/manager/local_manager.py

gemini-code-assist · 2025-12-11T09:17:11Z

tests/test_manage_pool.py

+        # Pool still has IDs but sandboxes are gone
+        self.assertEqual(len(self.manager._sandbox_pool), 2)


This test asserts that after cleanup_all_sandboxes is called, the _sandbox_pool still contains sandbox IDs. This is validating incorrect behavior. When all sandboxes are cleaned up, the pool should also be cleared to prevent the manager from being in an inconsistent state. The HttpSandboxManager correctly clears its pool tracking on cleanup. The LocalSandboxManager should do the same, and this test should be updated to assert that the pool is empty.

Suggested change

# Pool still has IDs but sandboxes are gone

self.assertEqual(len(self.manager._sandbox_pool), 2)

# Pool should also be cleared, and the initialized flag reset.

self.assertEqual(len(self.manager._sandbox_pool), 0)

self.assertFalse(self.manager._pool_initialized)

ms_enclave/sandbox/manager/http_manager.py

gemini-code-assist · 2025-12-11T09:17:11Z

tests/test_manager.py

        sandbox_id = await self.manager.create_sandbox(SandboxType.DOCKER, config)
        sandbox = await self.manager.get_sandbox(sandbox_id)
-        self.assertIn(sandbox.status, [SandboxStatus.STOPPED, SandboxStatus.STOPPING])
+        self.assertIn(sandbox.status, [SandboxStatus.RUNNING])


While this assertion is now correct for a newly created sandbox, the test name test_stop_sandbox is misleading because the test doesn't actually call stop_sandbox. The test should either be renamed to reflect that it's testing creation status, or it should be expanded to test the stop functionality.

For example, you could add this after the creation and initial assertion:

# Now test stopping await self.manager.stop_sandbox(sandbox_id) sandbox = await self.manager.get_sandbox(sandbox_id) self.assertEqual(sandbox.status, SandboxStatus.STOPPED)

Copilot

Pull request overview

This PR adds sandbox pooling functionality to the ms-enclave sandbox system, enabling pre-initialized sandbox reuse for improved performance and resource management.

Key Changes:

Introduces sandbox pooling with FIFO queue management for concurrent execution
Adds SandboxManagerFactory with decorator-based registration pattern for manager types
Implements pool operations (initialize_pool, execute_tool_in_pool) for both local and HTTP managers

Reviewed changes

Copilot reviewed 13 out of 13 changed files in this pull request and generated 14 comments.

Show a summary per file

File	Description
`ms_enclave/sandbox/model/config.py`	Adds `SandboxManagerConfig` class with pool configuration options
`ms_enclave/sandbox/model/base.py`	Adds `IDLE` and `BUSY` status enums, `SandboxManagerType` enum for manager types
`ms_enclave/sandbox/model/responses.py`	Adds `success` property to `ToolResult` for convenient status checking
`ms_enclave/sandbox/model/__init__.py`	Exports new model types for external use
`ms_enclave/sandbox/manager/base.py`	Implements `SandboxManagerFactory` and pool-related abstract methods
`ms_enclave/sandbox/manager/local_manager.py`	Implements local pool initialization and FIFO execution with condition variables
`ms_enclave/sandbox/manager/http_manager.py`	Implements HTTP-based pool operations delegating to server endpoints
`ms_enclave/sandbox/manager/__init__.py`	Exports `SandboxManagerFactory` for public API
`ms_enclave/sandbox/server/server.py`	Adds `/pool/initialize` and `/pool/execute` endpoints, updates constructor to use config
`tests/test_sandbox.py`	Updates Docker image references and removes ToolFactory tests
`tests/test_manager.py`	Adds factory tests, simplifies manager tests by removing redundant test classes
`tests/test_manage_pool.py`	Comprehensive new test suite for pool functionality covering initialization, execution, concurrency, FIFO, errors, and HTTP integration
`examples/sandbox_usage_examples.py`	Updates Docker image reference to standard Python image

ms_enclave/sandbox/model/config.py

ms_enclave/sandbox/manager/http_manager.py

ms_enclave/sandbox/manager/local_manager.py

Copilot · 2025-12-11T09:21:40Z

ms_enclave/sandbox/server/server.py

+        @self.app.post('/pool/initialize')
+        async def initialize_pool(
+            pool_size: Optional[int] = None, sandbox_type: Optional[SandboxType] = None, config: Optional[Dict] = None
+        ):


The endpoint defines 'config' as Optional[Dict] but it should be a request body parameter using FastAPI's Body or a Pydantic model. Currently, FastAPI will try to parse this from query parameters, which won't work for complex configuration objects. Consider using a Pydantic request model or explicitly marking config with Body(None) to accept it as a JSON request body.

Copilot · 2025-12-11T09:21:40Z

ms_enclave/sandbox/server/server.py

+                raise HTTPException(status_code=500, detail=str(e))
+
+        @self.app.post('/pool/execute', response_model=ToolResult)
+        async def execute_tool_in_pool(tool_name: str, parameters: Dict[str, Any], timeout: Optional[float] = None):


The 'parameters' parameter will be parsed as a query parameter by FastAPI, but tool parameters are typically complex objects that should be passed in the request body. Consider using a Pydantic request model or explicitly marking parameters with Body(...) to accept it as JSON in the request body.

ms_enclave/sandbox/model/config.py

Copilot · 2025-12-11T09:21:41Z

ms_enclave/sandbox/model/config.py

+    def validate_pool_size(cls, v):
+        """Validate pool size."""
+        if v < 0:
+            raise ValueError('Pool size must be non-negative')
+        return v
+
+    @field_validator('cleanup_interval')
+    def validate_cleanup_interval(cls, v):


Normal methods should have 'self', rather than 'cls', as their first parameter.

Suggested change

def validate_pool_size(cls, v):

"""Validate pool size."""

if v < 0:

raise ValueError('Pool size must be non-negative')

return v

@field_validator('cleanup_interval')

def validate_cleanup_interval(cls, v):

def validate_pool_size(self, v):

"""Validate pool size."""

if v < 0:

raise ValueError('Pool size must be non-negative')

return v

@field_validator('cleanup_interval')

def validate_cleanup_interval(self, v):

Copilot · 2025-12-11T09:21:42Z

tests/test_manage_pool.py

+from unittest.mock import AsyncMock, MagicMock, patch
+


Import of 'AsyncMock' is not used.
Import of 'MagicMock' is not used.
Import of 'patch' is not used.

Suggested change

from unittest.mock import AsyncMock, MagicMock, patch

Copilot · 2025-12-11T09:21:42Z

tests/test_manage_pool.py

+    SandboxInfo,
+    SandboxManagerConfig,
+    SandboxStatus,
+    SandboxType,
+    ToolResult,


Import of 'SandboxInfo' is not used.
Import of 'ToolResult' is not used.

Suggested change

SandboxInfo,

SandboxManagerConfig,

SandboxStatus,

SandboxType,

ToolResult,

SandboxManagerConfig,

SandboxStatus,

SandboxType,

Copilot · 2025-12-11T09:21:43Z

tests/test_manage_pool.py

+        task1.cancel()
+        try:
+            await task1
+        except asyncio.CancelledError:


'except' clause does nothing but pass and there is no explanatory comment.

Suggested change

except asyncio.CancelledError:

except asyncio.CancelledError:

# Task was cancelled as part of test cleanup; this is expected.

Co-authored-by: Copilot <[email protected]>

Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>

Yunnglin added 4 commits December 11, 2025 15:49

update pool

f0943e5

update pool remote

3f49f4a

update manager factory

de07b10

update manager factory

6ab4ff0

Copilot AI review requested due to automatic review settings December 11, 2025 09:14

Copilot started reviewing on behalf of Yunnglin December 11, 2025 09:15 View session

gemini-code-assist bot reviewed Dec 11, 2025

View reviewed changes

Copilot AI reviewed Dec 11, 2025

View reviewed changes

Yunnglin and others added 5 commits December 11, 2025 17:43

Update ms_enclave/sandbox/model/config.py

e56f06c

Co-authored-by: Copilot <[email protected]>

Update ms_enclave/sandbox/manager/http_manager.py

f4ec906

Co-authored-by: Copilot <[email protected]>

Update ms_enclave/sandbox/model/config.py

7d9503d

Co-authored-by: Copilot <[email protected]>

Update ms_enclave/sandbox/manager/local_manager.py

f60d9d3

Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>

fix

d823ac6

Yunnglin merged commit 13770a0 into main Dec 11, 2025
1 check passed

		# Pool still has IDs but sandboxes are gone
		self.assertEqual(len(self.manager._sandbox_pool), 2)

-        # Pool still has IDs but sandboxes are gone
-        self.assertEqual(len(self.manager._sandbox_pool), 2)
+        # Pool should also be cleared, and the initialized flag reset.
+        self.assertEqual(len(self.manager._sandbox_pool), 0)
+        self.assertFalse(self.manager._pool_initialized)

	except asyncio.CancelledError:
	except asyncio.CancelledError:
	# Task was cancelled as part of test cleanup; this is expected.

[Feature] Add sandbox pools #7

[Feature] Add sandbox pools #7

Uh oh!

Conversation

Yunnglin commented Dec 11, 2025

Uh oh!

gemini-code-assist bot commented Dec 11, 2025

Summary of Changes

Highlights

Footnotes

Uh oh!

gemini-code-assist bot left a comment

Choose a reason for hiding this comment

Code Review

Uh oh!

Uh oh!

gemini-code-assist bot Dec 11, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

gemini-code-assist bot Dec 11, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI Dec 11, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 11, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Copilot AI Dec 11, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 11, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 11, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 11, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants