fix(automation): purge expired local-mode run workspaces

[trungminhdo4-glitch]

HUMAN: I reviewed the cleanup changes after the maintainer feedback. I checked that candidate discovery is now filesystem first that the DB is only used to classify discovered runs, that orphaned workspaces can still be cleaned up, and that failed deletions no longer block newer cleanup candidates. I also checked that active PENDING/RUNNING workspaces stay protected and that the existing safe delete path was not unnecessarily changed.
regards

Summary

• add configurable retention for terminal local-mode automation workspaces
• purge eligible workspaces during startup and periodically in bounded batches
• protect active runs and reject unsafe paths, links, and junction escapes
• keep filesystem retention independent from database-row retention

Why

Persistent local agent-server runs leave their workspaces under
${AUTOMATION_WORKSPACE_BASE}/automation-runs/<run_id> indefinitely.
This eventually consumes significant local disk space.

Safety properties

• pending and running workspaces are never removed
• candidate paths must remain under the configured workspace root
• symlink and junction escapes are rejected
• cleanup runs in bounded batches
• missing directories and individual deletion failures are tolerated
• filesystem removal is moved off the event loop
• the periodic cleaner is owned and awaited by the application lifecycle

Tests

• 96 targeted tests passed
• 2 Windows symlink tests skipped because directory symlink privileges were unavailable
• Ruff check and format check passed
• Python compilation check passed
• retested after synchronizing with the current main branch

A full Docker/PostgreSQL suite was not run locally.
GitHub Actions for this fork PR are awaiting maintainer approval.

Fixes #271

Disclosure: This contribution was prepared with AI assistance and independently
reviewed and tested locally before publication.

[VascoSch92]

hey @trungminhdo4-glitch

Thanks for the PR, and for being upfront about the AI assistance.

First off, the deletion path itself is solid. Building the path from the UUID rather than anything user-supplied, refusing symlinks and junctions on both the root and the entry, the containment re-check after resolve, rmtree in a thread. That's the part I'd have worried about and it's fine.

My problem is with what we iterate over. Right now we walk DB rows and turn them into paths. I think it needs to be the other way around: walk automation-runs/, and use the DB only to decide whether each directory is safe to delete. That one change fixes three separate things.

Orphans. A directory whose row is gone is invisible to us, permanently. That's not theoretical: #251 is about deleting exactly the rows we key on here, so once it lands it will strand every workspace it cleans up. Same thing happens locally any time someone resets automations.db or repoints AUTOMATION_DB_URL. The issue specifically asks for the filesystem policy to be independent of the row policy, and right now it's downstream of it.

The scan never shrinks. Missing dirs don't consume batch budget, which is the right call on its own, but it means once we've caught up we re-walk every terminal row ever recorded, every hour, with a to_thread hop each.

Errors starve everything behind them. errors and refused do count against the batch and we always start from the oldest, so a handful of undeletable dirs blocks everything newer indefinitely. With batch_size=1 and one dir whose rmtree always raises PermissionError, I ran ten cycles and the newer workspace never got touched. You already solved this exact problem for missing.

Roughly what I have in mind:

candidates = await asyncio.to_thread(_scan_candidates, runs_root, batch_size)  # {run_id: mtime}
async with session_factory() as session:
    rows = (await session.execute(
        select(AutomationRun.id, AutomationRun.status, AutomationRun.completed_at)
        .where(AutomationRun.id.in_(candidates))
    )).all()

for run_id, mtime in candidates.items():
    row = by_id.get(run_id)
    if row is None:                       # orphan
        if mtime < cutoff: doomed.append(run_id)
    elif row.status not in TERMINAL_STATES:
        continue                          # pending/running, never touched
    elif (row.completed_at or mtime) < cutoff:
        doomed.append(run_id)

_scan_candidates is just an os.scandir that skips non-directories, links and anything that doesn't parse as a UUID. _delete_workspace stays exactly as you wrote it. Side benefit: the session is closed before any rmtree runs, whereas today we hold one, and on SQLite a read transaction, across the whole batch.

Can you tag me once you idd the changes?
Thanks

[trungminhdo4-glitch]

@VascoSch92 Thanks for the detailed review und danke für die Zeit und konstruktive kritik. I’ve pushed the requested changes. Cleanup now discovers candidates from automation-runs/ first and uses the DB only for batched classification. Orphan workspaces are handled by mtime, active runs remain protected, the DB session is closed before deletion, and failed/refused deletions no longer consume the successful deletion budget.

I also synchronized the branch with current main. The focused cleanup tests and adjacent lifecycle/config tests pass, along with Ruff checks. PTAL when you have a chance.

[all-hands-bot]

👋 This PR needs a couple of things fixed before OpenHands can review it:

• the PR description's HUMAN: section needs at least 20 characters describing what you tested, not just the template placeholder

Push an update once this is addressed and this check re-runs automatically.

This is an automated check - no AI was used to generate this comment.