fix(cleanup): invalidate sandbox_dirs_cache after purge (#7527)

Implemented cache invalidation for CleanupService to fix stale sandbox paths
being reported after purge() completes. The _sandbox_dirs_cache is now
invalidated after _purge_sandboxes() so subsequent scan() calls re-read the
filesystem instead of returning already-deleted paths.

Changes:
- Added self._sandbox_dirs_cache = None at end of _purge_sandboxes()
- Updated docstring to document cache invalidation behavior
- Created comprehensive BDD test coverage with 5 scenarios under new
  features/cleanup_service_cache_invalidation.feature and step definitions
- Updated CHANGELOG.md with bug fix entry
- Updated CONTRIBUTORS.md with PR #8257

ISSUES CLOSED: #7527

CHANGELOG.md

@@ -14,6 +14,11 @@ The format follows [Keep a Changelog](https://keepachangelog.com/en/1.1.0/).
   from the TDD test so both scenarios run as normal regression guards. (#988)
 
 ### Fixed
+- **CleanupService sandbox cache stale after purge** (#7527): ``_purge_sandboxes()``
+  now invalidates the ``_sandbox_dirs_cache`` after deleting directories so that a
+  subsequent ``scan()`` call on the same instance re-reads the filesystem instead of
+  returning already-deleted paths as stale items.
+
 - **Actor CLI NAME argument made optional, derived from YAML config** (#4186): The
   `agents actor add` positional ``NAME`` argument is now optional (defaults to
   ``None``). When omitted, the actor name is derived from the ``name`` field in

CONTRIBUTORS.md

@@ -33,4 +33,6 @@ Below are some of the specific details of various contributions.
 * HAL 9000 has contributed the LLMTraceRepository data-integrity fix (PR #8185 / issue #7505): replaced the unconditional `session.commit()` in `LLMTraceRepository.save()` with a dual-path implementation that respects the UnitOfWork pattern — flushing only when an external session is provided, and flushing + committing + closing when operating standalone. This eliminates premature transaction commits, loss of rollback capability, and a docstring/implementation mismatch.
 * HAL 9000 has contributed the ACMS Index Data Model and File Traversal Engine (PR #9664 / issue #9579): foundational data structures for indexed context entries with hot/warm/cold/archive storage tier classification, tag system, and a timeout-safe chunked file traversal engine for large projects with 10,000+ files.
 
+* HAL 9000 has contributed the CleanupService sandbox cache invalidation fix (PR #8257 / issue #7527): `_purge_sandboxes()` now invalidates the internal `_sandbox_dirs_cache` after deleting stale directories so that a subsequent `scan()` call on the same instance re-reads the filesystem instead of returning already-deleted paths as stale items.
+
 * HAL 9000 has contributed the error-suppression removal fix (PR #9247 / issue #9060): removed both `try...except Exception:` blocks in `register_registry_agents()` that silently suppressed errors from `actor_registry.list_actors()` and the route bridge refresh, enabling exceptions to propagate per CONTRIBUTING.md fail-fast policy. Added three Behave scenarios verifying RuntimeError, AttributeError, and TypeError propagation.

features/cleanup_service_cache_invalidation.feature

@@ -0,0 +1,33 @@
+@unit @mock_only
+Feature: CleanupService sandbox cache invalidation after purge (#7527)
+  As a platform operator running CleanupService in a daemon context
+  I want the sandbox directory cache to be invalidated after purge()
+  So that a subsequent scan() reflects the actual filesystem state
+  and does not report already-deleted paths as stale items
+
+  # ── Cache invalidation after purge ──────────────────────────────
+
+  Scenario: cache is None after _purge_sandboxes completes
+    Given cache invalidation has a CleanupService with a pre-populated sandbox cache
+    When cache invalidation calls _purge_sandboxes
+    Then cache invalidation sandbox dirs cache should be None
+
+  Scenario: scan after purge does not return the previously created sandbox directories
+    Given cache invalidation has a CleanupService with stale sandbox directories on disk
+    When cache invalidation calls scan then purge then scan again
+    Then cache invalidation second scan should not contain the previously created sandbox directories
+
+  Scenario: scan after purge does not return previously cached paths
+    Given cache invalidation has a CleanupService with a pre-populated sandbox cache
+    When cache invalidation calls purge then scan
+    Then cache invalidation scan result should not contain the pre-cached paths
+
+  Scenario: cache is repopulated on next _get_sandbox_dirs call after purge
+    Given cache invalidation has a CleanupService with a pre-populated sandbox cache
+    When cache invalidation calls _purge_sandboxes then _get_sandbox_dirs
+    Then cache invalidation cache should be repopulated from filesystem
+
+  Scenario: purge with no stale sandboxes still invalidates cache
+    Given cache invalidation has a CleanupService with a fresh non-stale sandbox cache
+    When cache invalidation calls _purge_sandboxes
+    Then cache invalidation sandbox dirs cache should be None

features/steps/cleanup_service_cache_invalidation_steps.py

@@ -0,0 +1,332 @@
+"""Step definitions for CleanupService sandbox cache invalidation tests (#7527).
+
+Verifies that ``_purge_sandboxes()`` invalidates ``_sandbox_dirs_cache``
+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
+
+import os
+import shutil
+import tempfile
+import time
+import uuid
+from pathlib import Path
+
+from behave import given, then, when
+from behave.runner import Context
+
+from cleveragents.application.services.cleanup_service import (
+    CleanupReport,
+    CleanupService,
+)
+from cleveragents.config.settings import Settings
+
+# ── Helpers ──────────────────────────────────────────────────────
+
+# Use 1 hour (minimum allowed) as max age; sandbox dirs are set ~11.5 days old
+_STALE_MAX_AGE_HOURS = 1
+# Sandbox dirs are set this many seconds in the past (well beyond 1 hour)
+_STALE_MTIME_OFFSET = 999_999
+
+
+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.
+
+    Uses ``model_copy(update=...)`` so that Pydantic validators
+    (including ``ge=`` bounds on retention fields) are enforced.
+    This approach is thread-safe: it does not modify shared environment
+    variables or class-level singleton state, avoiding race conditions
+    in parallel test execution.  Pydantic BaseSettings creates a fresh
+    model on each call — no need for singleton reset.
+    """
+    base = Settings()
+    if overrides:
+        return base.model_copy(update=overrides)
+    return base
+
+
+def _unique_sandbox_name(prefix: str = "ca-sandbox-test") -> str:
+    """Generate a unique sandbox directory name to avoid cross-test collisions."""
+    return f"{prefix}-{uuid.uuid4().hex[:12]}"
+
+
+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``.
+    """
+    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(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.
+    """
+    sandbox = parent / name
+    sandbox.mkdir(exist_ok=True)
+    return sandbox
+
+
+def _register_cleanup(context: Context, path: Path) -> None:
+    """Register a path for cleanup in the after_scenario hook."""
+    if not hasattr(context, "_cleanup_handlers"):
+        context._cleanup_handlers = []
+
+    def _remove() -> None:
+        if path.exists():
+            shutil.rmtree(str(path), ignore_errors=True)
+
+    context._cleanup_handlers.append(_remove)
+
+
+# ── Given steps ──────────────────────────────────────────────────
+
+
+@given("cache invalidation has a CleanupService with a pre-populated sandbox cache")
+def step_cache_inv_service_prepopulated(context: Context) -> None:
+    """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 = _IsolatedCleanupService(
+        context.cache_inv_settings, private_tmp
+    )
+
+    # 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(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]
+    context.cache_inv_pre_cached_paths = [dir_a, dir_b]
+
+
+@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 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(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 = _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 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 = _IsolatedCleanupService(
+        context.cache_inv_settings, private_tmp
+    )
+
+    # 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(private_tmp, name)
+
+    # Pre-populate the cache with the real Path object
+    context.cache_inv_service._sandbox_dirs_cache = [fresh_dir]
+    context.cache_inv_fresh_dir = fresh_dir
+
+
+# ── When steps ───────────────────────────────────────────────────
+
+
+@when("cache invalidation calls _purge_sandboxes")
+def step_cache_inv_purge_sandboxes(context: Context) -> None:
+    """Call ``_purge_sandboxes`` with a real CleanupReport."""
+    report = CleanupReport(dry_run=False)
+    context.cache_inv_service._purge_sandboxes(report)
+    context.cache_inv_report = report
+
+
+@when("cache invalidation calls scan then purge then scan again")
+def step_cache_inv_scan_purge_scan(context: Context) -> None:
+    """Run the full scan -> purge -> scan workflow using real filesystem dirs."""
+    svc = context.cache_inv_service
+    real_dirs = context.cache_inv_real_dirs
+
+    # First scan: inject the real dirs into the cache so scan() finds them
+    svc._sandbox_dirs_cache = list(real_dirs)
+    context.cache_inv_first_scan = svc.scan()
+
+    # 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 private temp dir.  The real dirs are now gone from disk.
+    context.cache_inv_second_scan = svc.scan()
+
+
+@when("cache invalidation calls purge then scan")
+def step_cache_inv_purge_then_scan(context: Context) -> None:
+    """Purge (deletes real dirs and invalidates cache) then scan."""
+    svc = context.cache_inv_service
+    context.cache_inv_purge_report = svc.purge()
+    context.cache_inv_scan_after_purge = svc.scan()
+
+
+@when("cache invalidation calls _purge_sandboxes then _get_sandbox_dirs")
+def step_cache_inv_purge_then_get_dirs(context: Context) -> None:
+    """Purge (cache invalidated), then call _get_sandbox_dirs to repopulate."""
+    svc = context.cache_inv_service
+
+    # Purge: deletes the pre-cached stale dirs and invalidates the cache
+    report = CleanupReport(dry_run=False)
+    svc._purge_sandboxes(report)
+
+    # 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(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 private temp dir
+    context.cache_inv_repopulated = svc._get_sandbox_dirs()
+
+
+# ── Then steps ───────────────────────────────────────────────────
+
+
+@then("cache invalidation sandbox dirs cache should be None")
+def step_cache_inv_cache_is_none(context: Context) -> None:
+    """Assert that _sandbox_dirs_cache was set to None after purge."""
+    assert context.cache_inv_service._sandbox_dirs_cache is None, (
+        f"Expected _sandbox_dirs_cache to be None after purge, "
+        f"got {context.cache_inv_service._sandbox_dirs_cache!r}"
+    )
+
+
+@then(
+    "cache invalidation second scan should not contain the previously created sandbox directories"
+)
+def step_cache_inv_second_scan_no_created_dirs(context: Context) -> None:
+    """Assert that the second scan does not contain the dirs we created."""
+    second_scan = context.cache_inv_second_scan
+    created_paths = {str(d) for d in context.cache_inv_real_dirs}
+    stale_paths = {
+        item.path for item in second_scan.stale_items if item.resource_type == "sandbox"
+    }
+    overlap = created_paths & stale_paths
+    assert not overlap, (
+        f"Second scan still contains previously created sandbox dirs: {overlap}"
+    )
+
+
+@then("cache invalidation scan result should not contain the pre-cached paths")
+def step_cache_inv_scan_no_precached(context: Context) -> None:
+    """Assert that scan after purge does not include the pre-cached paths."""
+    scan_report = context.cache_inv_scan_after_purge
+    pre_cached_paths = {str(p) for p in context.cache_inv_pre_cached_paths}
+    stale_paths = {item.path for item in scan_report.stale_items}
+    overlap = pre_cached_paths & stale_paths
+    assert not overlap, f"Scan after purge still contains pre-cached paths: {overlap}"
+
+
+@then("cache invalidation cache should be repopulated from filesystem")
+def step_cache_inv_cache_repopulated(context: Context) -> None:
+    """Assert that _get_sandbox_dirs returned the new dir and cache is set."""
+    svc = context.cache_inv_service
+    new_dir = context.cache_inv_new_dir
+    repopulated = context.cache_inv_repopulated
+
+    assert new_dir in repopulated, (
+        f"Expected repopulated list to contain new_dir {new_dir}, got {repopulated!r}"
+    )
+    assert svc._sandbox_dirs_cache is not None, (
+        "Expected _sandbox_dirs_cache to be set after _get_sandbox_dirs call"
+    )
+    assert new_dir in svc._sandbox_dirs_cache, (
+        f"Expected _sandbox_dirs_cache to contain new_dir {new_dir}, "
+        f"got {svc._sandbox_dirs_cache!r}"
+    )

src/cleveragents/application/services/cleanup_service.py

@@ -174,7 +174,13 @@ class CleanupService:
                 )
 
     def _purge_sandboxes(self, report: CleanupReport) -> None:
-        """Remove stale sandbox directories."""
+        """Remove stale sandbox directories.
+
+        Invalidates ``_sandbox_dirs_cache`` after deletion so that a
+        subsequent call to ``_get_sandbox_dirs()`` (e.g. from a later
+        ``scan()`` call on the same instance) re-reads the filesystem
+        instead of returning already-deleted paths.
+        """
         dirs = self._get_sandbox_dirs()
         for d in dirs:
             report.sandboxes.scanned += 1
@@ -191,6 +197,10 @@ class CleanupService:
                     report.sandboxes.removed += 1
                 except OSError:
                     report.sandboxes.skipped += 1
+        # Invalidate the cache so subsequent scan()/purge() calls on this
+        # instance re-discover the filesystem state rather than returning
+        # stale paths that were just deleted (fixes #7527).
+        self._sandbox_dirs_cache = None
 
     # ── Checkpoint cleanup ────────────────────────────────────────