diff --git a/backend/packages/harness/deerflow/sandbox/local/local_sandbox_provider.py b/backend/packages/harness/deerflow/sandbox/local/local_sandbox_provider.py index 651db11ec..0510a2473 100644 --- a/backend/packages/harness/deerflow/sandbox/local/local_sandbox_provider.py +++ b/backend/packages/harness/deerflow/sandbox/local/local_sandbox_provider.py @@ -119,3 +119,13 @@ class LocalSandboxProvider(SandboxProvider): # For Docker-based providers (e.g., AioSandboxProvider), cleanup # happens at application shutdown via the shutdown() method. pass + + def reset(self) -> None: + # reset_sandbox_provider() must also clear the module singleton. + global _singleton + _singleton = None + + def shutdown(self) -> None: + # LocalSandboxProvider has no extra resources beyond the shared + # singleton, so shutdown uses the same cleanup path as reset. + self.reset() diff --git a/backend/packages/harness/deerflow/sandbox/sandbox_provider.py b/backend/packages/harness/deerflow/sandbox/sandbox_provider.py index ecb1f7a67..0aa4d619a 100644 --- a/backend/packages/harness/deerflow/sandbox/sandbox_provider.py +++ b/backend/packages/harness/deerflow/sandbox/sandbox_provider.py @@ -37,6 +37,10 @@ class SandboxProvider(ABC): """ pass + def reset(self) -> None: + """Clear cached state that survives provider instance replacement.""" + pass + _default_sandbox_provider: SandboxProvider | None = None @@ -65,11 +69,18 @@ def reset_sandbox_provider() -> None: The next call to `get_sandbox_provider()` will create a new instance. Useful for testing or when switching configurations. + Providers can override `reset()` to clear any module-level state they keep + alive across instances (for example, `LocalSandboxProvider`'s cached + `LocalSandbox` singleton). Without it, config/mount changes would not take + effect on the next acquire(). + Note: If the provider has active sandboxes, they will be orphaned. Use `shutdown_sandbox_provider()` for proper cleanup. """ global _default_sandbox_provider - _default_sandbox_provider = None + if _default_sandbox_provider is not None: + _default_sandbox_provider.reset() + _default_sandbox_provider = None def shutdown_sandbox_provider() -> None: diff --git a/backend/tests/test_local_sandbox_provider_mounts.py b/backend/tests/test_local_sandbox_provider_mounts.py index 5c50a1aa0..5e7a06b6d 100644 --- a/backend/tests/test_local_sandbox_provider_mounts.py +++ b/backend/tests/test_local_sandbox_provider_mounts.py @@ -639,3 +639,148 @@ class TestLocalSandboxProviderMounts: provider = LocalSandboxProvider() assert [m.container_path for m in provider._path_mappings] == ["/mnt/skills", "/mnt/data"] + + +class TestLocalSandboxProviderResetClearsSingleton: + """Regression coverage for issue #2815. + + The module-level LocalSandbox singleton must be cleared whenever the + provider is reset or shut down — otherwise stale path mappings and + mount policy survive config reloads and test teardown. + """ + + def _build_config(self, skills_dir, mounts): + from deerflow.config.sandbox_config import SandboxConfig + + sandbox_config = SandboxConfig( + use="deerflow.sandbox.local:LocalSandboxProvider", + mounts=mounts, + ) + return SimpleNamespace( + skills=SimpleNamespace( + container_path="/mnt/skills", + get_skills_path=lambda: skills_dir, + use="deerflow.skills.storage.local_skill_storage:LocalSkillStorage", + ), + sandbox=sandbox_config, + ) + + def test_reset_sandbox_provider_clears_local_singleton(self, tmp_path): + from deerflow.config.sandbox_config import VolumeMountConfig + from deerflow.sandbox import local as local_module + from deerflow.sandbox.local import local_sandbox_provider as lsp_module + from deerflow.sandbox.sandbox_provider import ( + get_sandbox_provider, + reset_sandbox_provider, + ) + + skills_dir = tmp_path / "skills" + skills_dir.mkdir() + first_dir = tmp_path / "first" + first_dir.mkdir() + second_dir = tmp_path / "second" + second_dir.mkdir() + + first_cfg = self._build_config( + skills_dir, + [VolumeMountConfig(host_path=str(first_dir), container_path="/mnt/first", read_only=False)], + ) + second_cfg = self._build_config( + skills_dir, + [VolumeMountConfig(host_path=str(second_dir), container_path="/mnt/second", read_only=False)], + ) + + # Make sure no leftover singleton from a prior test interferes. + lsp_module._singleton = None + reset_sandbox_provider() + + try: + with patch("deerflow.sandbox.sandbox_provider.get_app_config", return_value=first_cfg), patch("deerflow.config.get_app_config", return_value=first_cfg): + provider = get_sandbox_provider() + provider.acquire() + + assert lsp_module._singleton is not None + first_container_paths = {m.container_path for m in lsp_module._singleton.path_mappings} + assert "/mnt/first" in first_container_paths + + reset_sandbox_provider() + + # The whole point of the regression: reset must drop the cached LocalSandbox. + assert lsp_module._singleton is None + + with patch("deerflow.sandbox.sandbox_provider.get_app_config", return_value=second_cfg), patch("deerflow.config.get_app_config", return_value=second_cfg): + provider2 = get_sandbox_provider() + provider2.acquire() + + assert provider2 is not provider + second_container_paths = {m.container_path for m in lsp_module._singleton.path_mappings} + assert "/mnt/second" in second_container_paths + assert "/mnt/first" not in second_container_paths + finally: + lsp_module._singleton = None + reset_sandbox_provider() + + # Sanity: the local sandbox module still exposes the singleton symbol + # at the same module path (guards against accidental rename). + assert hasattr(local_module.local_sandbox_provider, "_singleton") + + def test_shutdown_sandbox_provider_clears_local_singleton(self, tmp_path): + from deerflow.config.sandbox_config import VolumeMountConfig + from deerflow.sandbox.local import local_sandbox_provider as lsp_module + from deerflow.sandbox.sandbox_provider import ( + get_sandbox_provider, + reset_sandbox_provider, + shutdown_sandbox_provider, + ) + + skills_dir = tmp_path / "skills" + skills_dir.mkdir() + mount_dir = tmp_path / "mount" + mount_dir.mkdir() + + cfg = self._build_config( + skills_dir, + [VolumeMountConfig(host_path=str(mount_dir), container_path="/mnt/data", read_only=False)], + ) + + lsp_module._singleton = None + reset_sandbox_provider() + + try: + with patch("deerflow.sandbox.sandbox_provider.get_app_config", return_value=cfg), patch("deerflow.config.get_app_config", return_value=cfg): + provider = get_sandbox_provider() + provider.acquire() + + assert lsp_module._singleton is not None + + shutdown_sandbox_provider() + + assert lsp_module._singleton is None + finally: + lsp_module._singleton = None + reset_sandbox_provider() + + def test_provider_reset_method_is_idempotent(self, tmp_path): + from deerflow.sandbox.local import local_sandbox_provider as lsp_module + from deerflow.sandbox.local.local_sandbox_provider import LocalSandboxProvider + + skills_dir = tmp_path / "skills" + skills_dir.mkdir() + cfg = self._build_config(skills_dir, []) + + lsp_module._singleton = None + + try: + with patch("deerflow.config.get_app_config", return_value=cfg): + provider = LocalSandboxProvider() + provider.acquire() + assert lsp_module._singleton is not None + + provider.reset() + assert lsp_module._singleton is None + + # Calling reset again on an already-cleared singleton is safe. + provider.reset() + assert lsp_module._singleton is None + finally: + lsp_module._singleton = None