diff --git a/features/steps/cleanup_service_cache_invalidation_steps.py b/features/steps/cleanup_service_cache_invalidation_steps.py index 8ae99dbc8..61d904298 100644 --- a/features/steps/cleanup_service_cache_invalidation_steps.py +++ b/features/steps/cleanup_service_cache_invalidation_steps.py @@ -5,6 +5,14 @@ so that a subsequent ``scan()`` call re-reads the filesystem instead of returning already-deleted paths. All tests use real filesystem operations — no mocks, no patches. + +Test isolation strategy +----------------------- +Each scenario creates a private temporary directory (via ``tempfile.mkdtemp()``) +and uses an ``_IsolatedCleanupService`` subclass that overrides +``_get_sandbox_dirs()`` to scan only that private directory. This prevents +the test from interfering with other concurrently-running scenarios that also +create ``ca-sandbox-*`` directories in the shared system temp directory. """ from __future__ import annotations @@ -36,6 +44,45 @@ _STALE_MTIME_OFFSET = 999_999 _CLEANUP_MAX_AGE_ENV = "CLEVERAGENTS_CLEANUP_SANDBOX_MAX_AGE_HOURS" +class _IsolatedCleanupService(CleanupService): + """CleanupService subclass that scans a private temp directory. + + Overrides ``_get_sandbox_dirs()`` to look only in ``_private_tmp`` + instead of the system-wide ``tempfile.gettempdir()``. This prevents + the test from reading or deleting sandbox directories created by other + concurrently-running scenarios. + """ + + def __init__(self, settings: Settings, private_tmp: Path) -> None: + super().__init__(settings) + self._private_tmp = private_tmp + + def _get_sandbox_dirs(self) -> list[Path]: + """Return sandbox dirs from the private temp directory only.""" + if self._sandbox_dirs_cache is not None: + return self._sandbox_dirs_cache + if not self._private_tmp.exists(): + self._sandbox_dirs_cache = [] + return self._sandbox_dirs_cache + dirs: list[Path] = [] + try: + entries = list(self._private_tmp.iterdir()) + except OSError: + self._sandbox_dirs_cache = [] + return self._sandbox_dirs_cache + for p in entries: + try: + is_dir = p.is_dir() + except OSError: + continue + if is_dir and any( + p.name.startswith(pfx) for pfx in ("ca-sandbox-", "ca-cow-sandbox-") + ): + dirs.append(p) + self._sandbox_dirs_cache = dirs + return dirs + + def _make_settings(**overrides: object) -> Settings: """Create a real Settings instance with optional field overrides. @@ -66,28 +113,40 @@ def _unique_sandbox_name(prefix: str = "ca-sandbox-test") -> str: return f"{prefix}-{uuid.uuid4().hex[:12]}" -def _make_real_stale_sandbox(name: str) -> Path: - """Create a real stale sandbox directory in the system temp dir. +def _make_private_tmp(context: Context) -> Path: + """Create a private temp directory for this scenario and register cleanup.""" + private_tmp = Path(tempfile.mkdtemp(prefix="ca-test-isolation-")) + + def _remove() -> None: + if private_tmp.exists(): + shutil.rmtree(str(private_tmp), ignore_errors=True) + + if not hasattr(context, "_cleanup_handlers"): + context._cleanup_handlers = [] + context._cleanup_handlers.append(_remove) + return private_tmp + + +def _make_real_stale_sandbox(parent: Path, name: str) -> Path: + """Create a real stale sandbox directory inside *parent*. Sets the mtime far in the past so ``_is_sandbox_stale`` returns True when ``cleanup_sandbox_max_age_hours=1``. """ - tmp = Path(tempfile.gettempdir()) - sandbox = tmp / name + sandbox = parent / name sandbox.mkdir(exist_ok=True) old_time = time.time() - _STALE_MTIME_OFFSET os.utime(str(sandbox), (old_time, old_time)) return sandbox -def _make_real_fresh_sandbox(name: str) -> Path: - """Create a real fresh (non-stale) sandbox directory in the system temp dir. +def _make_real_fresh_sandbox(parent: Path, name: str) -> Path: + """Create a real fresh (non-stale) sandbox directory inside *parent*. Uses the current mtime so the directory is not considered stale under the default ``cleanup_sandbox_max_age_hours=48`` setting. """ - tmp = Path(tempfile.gettempdir()) - sandbox = tmp / name + sandbox = parent / name sandbox.mkdir(exist_ok=True) return sandbox @@ -109,21 +168,20 @@ def _register_cleanup(context: Context, path: Path) -> None: @given("cache invalidation has a CleanupService with a pre-populated sandbox cache") def step_cache_inv_service_prepopulated(context: Context) -> None: - """Create a CleanupService whose cache holds real stale sandbox paths.""" + """Create an isolated CleanupService whose cache holds real stale sandbox paths.""" + private_tmp = _make_private_tmp(context) context.cache_inv_settings = _make_settings( cleanup_sandbox_max_age_hours=_STALE_MAX_AGE_HOURS ) - context.cache_inv_service = CleanupService(context.cache_inv_settings) + context.cache_inv_service = _IsolatedCleanupService( + context.cache_inv_settings, private_tmp + ) - # Create two real stale sandbox directories in the system temp dir + # Create two real stale sandbox directories in the private temp dir name_a = _unique_sandbox_name("ca-sandbox-plan-aaa") name_b = _unique_sandbox_name("ca-sandbox-plan-bbb") - dir_a = _make_real_stale_sandbox(name_a) - dir_b = _make_real_stale_sandbox(name_b) - - # Register cleanup in case purge does not delete them - _register_cleanup(context, dir_a) - _register_cleanup(context, dir_b) + dir_a = _make_real_stale_sandbox(private_tmp, name_a) + dir_b = _make_real_stale_sandbox(private_tmp, name_b) # Pre-populate the cache with the real Path objects context.cache_inv_service._sandbox_dirs_cache = [dir_a, dir_b] @@ -132,36 +190,37 @@ def step_cache_inv_service_prepopulated(context: Context) -> None: @given("cache invalidation has a CleanupService with stale sandbox directories on disk") def step_cache_inv_service_real_stale_dirs(context: Context) -> None: - """Create real stale sandbox directories in the system temp dir.""" + """Create real stale sandbox directories in a private temp dir.""" + private_tmp = _make_private_tmp(context) name_a = _unique_sandbox_name("ca-sandbox-planA") name_b = _unique_sandbox_name("ca-sandbox-planB") - dir_a = _make_real_stale_sandbox(name_a) - dir_b = _make_real_stale_sandbox(name_b) - - # Register cleanup in case purge does not delete them - _register_cleanup(context, dir_a) - _register_cleanup(context, dir_b) + dir_a = _make_real_stale_sandbox(private_tmp, name_a) + dir_b = _make_real_stale_sandbox(private_tmp, name_b) # Use minimum allowed max_age_hours=1; dirs are ~11.5 days old so they # are immediately stale. context.cache_inv_settings = _make_settings( cleanup_sandbox_max_age_hours=_STALE_MAX_AGE_HOURS ) - context.cache_inv_service = CleanupService(context.cache_inv_settings) + context.cache_inv_service = _IsolatedCleanupService( + context.cache_inv_settings, private_tmp + ) context.cache_inv_real_dirs = [dir_a, dir_b] @given("cache invalidation has a CleanupService with a fresh non-stale sandbox cache") def step_cache_inv_service_nonstale_cache(context: Context) -> None: - """Create a CleanupService whose cache holds a real non-stale sandbox dir.""" + """Create an isolated CleanupService whose cache holds a real non-stale sandbox dir.""" + private_tmp = _make_private_tmp(context) # Use default max_age_hours=48 so the fresh dir (current mtime) is NOT stale context.cache_inv_settings = _make_settings() - context.cache_inv_service = CleanupService(context.cache_inv_settings) + context.cache_inv_service = _IsolatedCleanupService( + context.cache_inv_settings, private_tmp + ) - # Create a real fresh (non-stale) sandbox directory + # Create a real fresh (non-stale) sandbox directory in the private temp dir name = _unique_sandbox_name("ca-sandbox-plan-fresh") - fresh_dir = _make_real_fresh_sandbox(name) - _register_cleanup(context, fresh_dir) + fresh_dir = _make_real_fresh_sandbox(private_tmp, name) # Pre-populate the cache with the real Path object context.cache_inv_service._sandbox_dirs_cache = [fresh_dir] @@ -189,11 +248,13 @@ def step_cache_inv_scan_purge_scan(context: Context) -> None: svc._sandbox_dirs_cache = list(real_dirs) context.cache_inv_first_scan = svc.scan() - # Purge: actually deletes the directories and invalidates the cache + # Purge: actually deletes the directories and invalidates the cache. + # Because svc is an _IsolatedCleanupService, _get_sandbox_dirs() will + # re-read only the private temp dir — not the system-wide /tmp. context.cache_inv_purge_report = svc.purge() # Second scan: cache was invalidated, so _get_sandbox_dirs re-reads - # the filesystem. The real dirs are now gone from disk. + # the private temp dir. The real dirs are now gone from disk. context.cache_inv_second_scan = svc.scan() @@ -214,13 +275,15 @@ def step_cache_inv_purge_then_get_dirs(context: Context) -> None: report = CleanupReport(dry_run=False) svc._purge_sandboxes(report) - # Create a new real sandbox dir so _get_sandbox_dirs has something to find + # Create a new real sandbox dir in the private temp dir so + # _get_sandbox_dirs has something to find after cache invalidation. + private_tmp = svc._private_tmp new_name = _unique_sandbox_name("ca-sandbox-plan-new") - new_dir = _make_real_stale_sandbox(new_name) + new_dir = _make_real_stale_sandbox(private_tmp, new_name) _register_cleanup(context, new_dir) context.cache_inv_new_dir = new_dir - # Call _get_sandbox_dirs to repopulate the cache from the real filesystem + # Call _get_sandbox_dirs to repopulate the cache from the private temp dir context.cache_inv_repopulated = svc._get_sandbox_dirs()