diff --git a/.github/workflows/docs-pages.yml b/.github/workflows/docs-pages.yml index 2a86bcc4..4a3d5dca 100644 --- a/.github/workflows/docs-pages.yml +++ b/.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: diff --git a/autograder/autograder.py b/autograder/autograder.py index 73ff5417..f6d6d1ea 100644 --- a/autograder/autograder.py +++ b/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", ) diff --git a/docs/core/pipeline-architecture.md b/docs/core/pipeline-architecture.md index dadb64ca..b89f3b47 100644 --- a/docs/core/pipeline-architecture.md +++ b/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 diff --git a/docs/pipeline/README.md b/docs/pipeline/README.md index d37e51d1..27952a30 100644 --- a/docs/pipeline/README.md +++ b/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. diff --git a/tests/unit/pipeline/test_sandbox_cleanup.py b/tests/unit/pipeline/test_sandbox_cleanup.py index ddf3fc0a..df74fbf6 100644 --- a/tests/unit/pipeline/test_sandbox_cleanup.py +++ b/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,11 +55,11 @@ 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 @@ -67,4 +67,4 @@ def test_cleanup_no_sandbox_no_call(mock_get_manager, pipeline_exec): pipeline = AutograderPipeline() pipeline._cleanup_sandbox(pipeline_exec) - mock_manager.release_sandbox.assert_not_called() + mock_manager.destroy_sandbox.assert_not_called()