#7375·hive

[Bug]: Janitor's worker deep-clean crash-resume check only verifies conversations/ is gone, leaving data/ and stray files permanently unswept

Author: Prabal864Created Aug 16, 2026Updated Sep 9, 2026

Description

deep_clean_worker() in core/framework/maintenance/retention.py is tier-2 of the janitor: it deletes a finished colony worker's conversations/, data/, and stray session-root files, after first writing a result.json tombstone. Its docstring makes a crash-safety promise:

Order is crash-safe: the tombstone is written before any deletion so (a) resume_worker refuses the worker from that point on and (b) a rerun after a crash simply finishes the deletions.

That promise only half-holds. The crash-resume check only verifies that conversations/ is gone — not data/, not the stray files the same function is responsible for removing:

python
# core/framework/maintenance/retention.py
tombstone_exists = result_path.exists()
already_cleaned = tombstone_exists and not (wdir / "conversations").exists()
if already_cleaned:
    return report

The actual deletion loop below processes conversations, then data, then any stray files in the worker dir, one target at a time (each wrapped in its own try/except). If the janitor process is killed — OOM, host restart, disk full, container preemption, SIGKILL — after conversations/ has been removed but before data/ and the stray files are reached, the tombstone is already on disk (it's written first). On the next janitor run, already_cleaned evaluates True immediately and the function returns without ever touching data/ or the leftover files — permanently. There's no further trigger that re-examines this worker; tier 2 treats it as fully cleaned from then on.

Location

python
# core/framework/maintenance/retention.py, deep_clean_worker()
tombstone_exists = result_path.exists()
already_cleaned = tombstone_exists and not (wdir / "conversations").exists()
if already_cleaned:
    return report

Called from core/framework/maintenance/janitor.py:311 as part of the normal tier-2 run_once(tiers={2}) sweep — no unusual configuration needed to reach it.

Steps to Reproduce

Verified directly against the real function (unmodified core/framework/maintenance/retention.py), simulating the exact post-crash state (tombstone written, conversations/ already removed, data/ and a stray file still present — i.e. the crash landed between the two for sub in ("conversations", "data") iterations):

python
from framework import config
config.HIVE_HOME = hive_home          # temp dir
config.COLONIES_DIR = colonies_dir

from framework.maintenance.retention import deep_clean_worker, DeleteDisposer, Manifest

wdir = colonies_dir / "c1" / "workers" / wid
(wdir / "data").mkdir(parents=True)
(wdir / "data" / "web_scrape_1.txt").write_text("x" * 500)
(wdir / "meta.json").write_text('{"worker_id": "..."}')
(wdir / "tasks.json").write_text('{"tasks": []}')
(wdir / "reminder_state.json").write_text('{"turns_total": 3}')
# conversations/ is intentionally NOT created — it's already gone, simulating
# a crash that finished that half of the deletion loop.
(wdir / "result.json").write_text('{"status": "completed", ...}')  # tombstone already written

report = deep_clean_worker("c1", wid, wdir, disposer=DeleteDisposer(), manifest=Manifest())

Actual output:

report.files: 0   report.bytes_freed: 0
data/ exists: True
data/web_scrape_1.txt exists: True
reminder_state.json exists: True

deep_clean_worker() returns immediately having done nothing, and will continue to do nothing on every subsequent janitor run — data/ and the stray file are permanently retained.

Test Coverage Gap

core/tests/test_janitor_worker_deepclean.py::test_crash_resume_converges does cover a crash-resume scenario, but only the case where the crash happens before any deletion starts (both conversations/ and data/ still present). It never exercises the partial state where conversations/ alone is already gone — which is exactly the gap above, and exactly the more likely crash point in practice since conversations/ (transcript files) is processed first and is typically larger/slower to delete than data/.

Impact

Silent, permanent leak of worker artifact data — data/ can hold tool output, scraped files, or other generated content, and stray session-root files are left behind too. This is retention policy's own job to purge, and it fails without any error, log line, or manifest entry: the tombstone alone makes the worker look "fully cleaned" from then on. Worse than an ordinary leak because re-running the janitor — retention's own recovery mechanism — cannot fix it; the leaked files are permanently invisible to tier 2 once conversations/ is gone.

Expected Behavior

The crash-resume check should confirm every target the loop is responsible for is actually gone (conversations/, data/, and no leftover non-keep-file stray files) before treating the worker as fully cleaned — not just conversations/. Concretely, already_cleaned should also check not (wdir / "data").exists() and that no stray files remain, or (more simply) the tombstone-exists fast path should be dropped in favor of just re-running the idempotent deletion loop, which already no-ops correctly (per test_deep_clean_is_idempotent) when nothing is left to delete.

Environment

  • OS: Windows 11 (repro is platform-independent — pure logic bug, not OS-specific)
  • Python: 3.14.4

Additional Context

Happy to open a PR fixing the crash-resume check plus a regression test covering this exact partial-crash state, if this is a direction the maintainers want.