Merge pull request #279 from webtech-network/fix/destroy-sandbox-after-grading

Destroy grading sandboxes at pipeline cleanup

Author: ArthurCRodrigues

.github/workflows/docs-pages.yml

@@ -8,12 +8,6 @@ on:
       - 'mkdocs.yml'
       - 'requirements-docs.txt'
       - '.github/workflows/docs-pages.yml'
-  pull_request:
-    paths:
-      - 'docs/**'
-      - 'mkdocs.yml'
-      - 'requirements-docs.txt'
-      - '.github/workflows/docs-pages.yml'
   workflow_dispatch:
 
 permissions:
@@ -44,17 +38,14 @@ jobs:
         run: mkdocs build --strict
 
       - name: Setup Pages
-        if: github.event_name != 'pull_request'
         uses: actions/configure-pages@v5
 
       - name: Upload Pages artifact
-        if: github.event_name != 'pull_request'
         uses: actions/upload-pages-artifact@v3
         with:
           path: ./site
 
   deploy:
-    if: github.event_name != 'pull_request'
     needs: build
     runs-on: ubuntu-latest
     environment:

autograder/autograder.py

@@ -86,7 +86,7 @@ def run(self, submission: Submission):
 
         pipeline_execution.finish_execution() # Generates GradingResult object in pipeline execution
 
-        # Cleanup: Release sandbox back to pool if it was created
+        # Cleanup: Destroy sandbox if it was created
         self._cleanup_sandbox(pipeline_execution)
 
         logger.info(
@@ -98,16 +98,17 @@ def run(self, submission: Submission):
         return pipeline_execution
 
     def _cleanup_sandbox(self, pipeline_execution: PipelineExecution) -> None:
-        """Release sandbox back to pool after pipeline execution."""
+        """Destroy sandbox after pipeline execution to avoid cross-submission reuse."""
         try:
             sandbox = pipeline_execution.sandbox
             if sandbox:
                 from sandbox_manager.manager import get_sandbox_manager
                 manager = get_sandbox_manager()
                 language = pipeline_execution.submission.language
-                manager.release_sandbox(language, sandbox)
+                manager.destroy_sandbox(language, sandbox)
+                pipeline_execution.sandbox = None
                 logger.info(
-                    "Sandbox released: external_user_id=%s, language=%s",
+                    "Sandbox destroyed: external_user_id=%s, language=%s",
                     pipeline_execution.submission.user_id,
                     language.value if language else "none",
                 )

docs/core/pipeline-architecture.md

@@ -24,7 +24,7 @@ LOAD_TEMPLATE -> BUILD_TREE -> SANDBOX -> PRE_FLIGHT -> AI_BATCH -> GRADE -> FOC
 2. Each step receives the same `PipelineExecution` and appends one `StepResult`.
 3. If a step fails, execution stops early.
 4. `finish_execution()` assembles `GradingResult` from grade/focus/feedback artifacts.
-5. Sandbox cleanup runs at the end.
+5. Sandbox cleanup runs at the end and destroys any sandbox used by the submission.
 
 ## Core data contract
 

docs/pipeline/README.md

@@ -39,7 +39,7 @@ Key behaviors:
 - After each step, the pipeline checks the latest `StepResult`. If it failed, the pipeline sets its status to `FAILED` and breaks.
 - Unhandled exceptions set the status to `INTERRUPTED`.
 - After all steps (or early exit), `finish_execution()` is called to assemble the final `GradingResult`.
-- Sandbox cleanup always runs at the end, regardless of success or failure.
+- Sandbox cleanup always runs at the end, regardless of success or failure, and destroys the sandbox used by that submission.
 
 ### PipelineExecution
 
@@ -191,7 +191,7 @@ When a step fails:
 2. The pipeline detects the failure via `get_previous_step()` and calls `set_failure()`.
 3. No further steps are executed.
 4. `finish_execution()` is called — since the status is `FAILED`, `result` remains `None`.
-5. Sandbox cleanup always runs: if a Sandbox step created a sandbox, it is released back to the pool regardless of pipeline outcome.
+5. Sandbox cleanup always runs: if a Sandbox step created a sandbox, it is destroyed regardless of pipeline outcome, and the pool replenishes with a fresh container.
 
 For unhandled exceptions, the status is set to `INTERRUPTED` and the same cleanup logic applies.
 

tests/unit/pipeline/test_sandbox_cleanup.py

@@ -24,7 +24,7 @@ def pipeline_exec(submission):
 
 @patch("sandbox_manager.manager.get_sandbox_manager")
 def test_cleanup_sandbox_called_after_pipeline_run(mock_get_manager, pipeline_exec):
-    """Verifies that _cleanup_sandbox is called and releases the sandbox correctly."""
+    """Verifies that _cleanup_sandbox is called and destroys the sandbox correctly."""
     mock_manager = mock_get_manager.return_value
     mock_sandbox = MagicMock(spec=SandboxContainer)
 
@@ -39,11 +39,11 @@ def test_cleanup_sandbox_called_after_pipeline_run(mock_get_manager, pipeline_ex
     # Let's test _cleanup_sandbox directly to verify it uses the property
     pipeline._cleanup_sandbox(pipeline_exec)
 
-    mock_manager.release_sandbox.assert_called_once_with(Language.PYTHON, mock_sandbox)
+    mock_manager.destroy_sandbox.assert_called_once_with(Language.PYTHON, mock_sandbox)
 
 @patch("sandbox_manager.manager.get_sandbox_manager")
 def test_cleanup_sandbox_even_on_step_failure(mock_get_manager, pipeline_exec):
-    """Ensures that sandbox cleanup occurs even if the pipeline status is FAILED."""
+    """Ensures sandbox destruction occurs even if the pipeline status is FAILED."""
     mock_manager = mock_get_manager.return_value
     mock_sandbox = MagicMock(spec=SandboxContainer)
 
@@ -55,16 +55,16 @@ def test_cleanup_sandbox_even_on_step_failure(mock_get_manager, pipeline_exec):
     pipeline._cleanup_sandbox(pipeline_exec)
 
     # Cleanup should still happen
-    mock_manager.release_sandbox.assert_called_once_with(Language.PYTHON, mock_sandbox)
+    mock_manager.destroy_sandbox.assert_called_once_with(Language.PYTHON, mock_sandbox)
 
 @patch("sandbox_manager.manager.get_sandbox_manager")
 def test_cleanup_no_sandbox_no_call(mock_get_manager, pipeline_exec):
-    """Verifies that no sandbox release call is made if no sandbox is attached."""
+    """Verifies that no sandbox destroy call is made if no sandbox is attached."""
     mock_manager = mock_get_manager.return_value
 
     pipeline_exec.sandbox = None
 
     pipeline = AutograderPipeline()
     pipeline._cleanup_sandbox(pipeline_exec)
 
-    mock_manager.release_sandbox.assert_not_called()
+    mock_manager.destroy_sandbox.assert_not_called()