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
- Updated CHANGELOG.md with bug fix entry
- Updated CONTRIBUTORS.md with PR #8257

ISSUES CLOSED: #7527

CHANGELOG.md

@@ -6,6 +6,12 @@ The format follows [Keep a Changelog](https://keepachangelog.com/en/1.1.0/).
 ## [Unreleased]
 
 ### Fixed
+- **CleanupService sandbox cache stale after purge** (#7527): `_purge_sandboxes()`
+  now invalidates `_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. Added BDD regression tests
+  (`features/cleanup_service_cache_invalidation.feature`) to verify correct cache
+  lifecycle across scan→purge→scan workflows.
 
 - **Cross-actor subgraph cycle detection reads actor_ref field** (#1431): Fixed
   `_detect_subgraph_cycles()`, `_map_node()`, and the `compile_actor()` main loop

CONTRIBUTORS.md

@@ -8,6 +8,8 @@
 * Luis Mendes <luis.p.mendes@gmail.com>
 * Rui Hu <rui.hu@cleverthis.com>
 
+* HAL 9000 has contributed automated bug fixes, cache invalidation improvements, and BDD test coverage for the CleanupService sandbox lifecycle (PR #8257, fixes #7527).
+
 # Details
 
 Below are some of the specific details of various contributions.

features/cleanup_service_cache_invalidation.feature

@@ -0,0 +1,31 @@
+@unit @regression
+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
+
+  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,174 @@
+"""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.
+"""
+
+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
+
+
+def _make_settings(**overrides: object) -> Settings:
+    s = Settings()
+    for key, value in overrides.items():
+        object.__setattr__(s, key, value)
+    return s
+
+
+_STALE_MAX_AGE_HOURS = 1
+_STALE_MTIME_OFFSET = 999_999
+
+
+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:
+    tmp = Path(tempfile.gettempdir())
+    sandbox = tmp / 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:
+    tmp = Path(tempfile.gettempdir())
+    sandbox = tmp / name
+    sandbox.mkdir(exist_ok=True)
+    return sandbox
+
+
+def _register_cleanup(context: Context, path: Path) -> None:
+    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("cache invalidation has a CleanupService with a pre-populated sandbox cache")
+def step_cache_inv_prepopulated(context: Context) -> None:
+    context.cache_inv_settings = _make_settings(cleanup_sandbox_max_age_hours=_STALE_MAX_AGE_HOURS)
+    context.cache_inv_service = CleanupService(context.cache_inv_settings)
+    na = _unique_sandbox_name("ca-sandbox-plan-aaa")
+    nb = _unique_sandbox_name("ca-sandbox-plan-bbb")
+    da = _make_real_stale_sandbox(na)
+    db = _make_real_stale_sandbox(nb)
+    _register_cleanup(context, da)
+    _register_cleanup(context, db)
+    context.cache_inv_service._sandbox_dirs_cache = [da, db]
+    context.cache_inv_pre_cached_paths = [da, db]
+
+
+@given("cache invalidation has a CleanupService with stale sandbox directories on disk")
+def step_cache_inv_real_stale(context: Context) -> None:
+    na = _unique_sandbox_name("ca-sandbox-planA")
+    nb = _unique_sandbox_name("ca-sandbox-planB")
+    da = _make_real_stale_sandbox(na)
+    db = _make_real_stale_sandbox(nb)
+    _register_cleanup(context, da)
+    _register_cleanup(context, db)
+    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_real_dirs = [da, db]
+
+
+@given("cache invalidation has a CleanupService with a fresh non-stale sandbox cache")
+def step_cache_inv_fresh(context: Context) -> None:
+    context.cache_inv_settings = _make_settings()
+    context.cache_inv_service = CleanupService(context.cache_inv_settings)
+    nm = _unique_sandbox_name("ca-sandbox-plan-fresh")
+    fd = _make_real_fresh_sandbox(nm)
+    _register_cleanup(context, fd)
+    context.cache_inv_service._sandbox_dirs_cache = [fd]
+    context.cache_inv_fresh_dir = fd
+
+
+@when("cache invalidation calls _purge_sandboxes")
+def step_cache_inv_purge(context: Context) -> None:
+    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:
+    svc = context.cache_inv_service
+    svc._sandbox_dirs_cache = list(context.cache_inv_real_dirs)
+    context.cache_inv_first_scan = svc.scan()
+    context.cache_inv_purge_report = svc.purge()
+    context.cache_inv_second_scan = svc.scan()
+
+
+@when("cache invalidation calls purge then scan")
+def step_cache_inv_purge_scan(context: Context) -> None:
+    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_get_dirs(context: Context) -> None:
+    svc = context.cache_inv_service
+    report = CleanupReport(dry_run=False)
+    svc._purge_sandboxes(report)
+    nm = _unique_sandbox_name("ca-sandbox-plan-new")
+    nd = _make_real_stale_sandbox(nm)
+    _register_cleanup(context, nd)
+    context.cache_inv_new_dir = nd
+    context.cache_inv_repopulated = svc._get_sandbox_dirs()
+
+
+@then("cache invalidation sandbox dirs cache should be None")
+def step_cache_inv_none(context: Context) -> None:
+    assert context.cache_inv_service._sandbox_dirs_cache is None, (
+        f"Expected _sandbox_dirs_cache to be None after purge, 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(context: Context) -> None:
+    ss = context.cache_inv_second_scan
+    cp = {str(d) for d in context.cache_inv_real_dirs}
+    sp = {i.path for i in ss.stale_items if i.resource_type == "sandbox"}
+    overlap = cp & sp
+    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_no_precached(context: Context) -> None:
+    sr = context.cache_inv_scan_after_purge
+    cp = {str(p) for p in context.cache_inv_pre_cached_paths}
+    sp = {i.path for i in sr.stale_items}
+    overlap = cp & sp
+    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_repopulated(context: Context) -> None:
+    svc = context.cache_inv_service
+    nd = context.cache_inv_new_dir
+    rep = context.cache_inv_repopulated
+    assert nd in rep, f"Expected repopulated to contain {nd}, got {rep!r}"
+    assert svc._sandbox_dirs_cache is not None, "Expected cache to be set after _get_sandbox_dirs"
+    assert nd in svc._sandbox_dirs_cache, f"Expected cache to contain {nd}, 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 ────────────────────────────────────────