feat(audit): user-initiated cleanup for old audit data (15/30/90 days) with orphan sweep and typed-confirmation gate#1852
Closed
Dani Akash (DaniAkash) wants to merge 5 commits into
Closed
feat(audit): user-initiated cleanup for old audit data (15/30/90 days) with orphan sweep and typed-confirmation gate#1852Dani Akash (DaniAkash) wants to merge 5 commits into
Dani Akash (DaniAkash) wants to merge 5 commits into
Conversation
Adds a user-initiated cleanup path for old BrowserClaw audit data.
Three fixed thresholds (15/30/90 days) live in a new shared constant
so the server route validation and client dialog options stay in
lockstep.
Service (services/audit-cleanup.ts):
- listCandidates() returns per-threshold session/dispatch/bytes counts;
cheap enough for the client to poll on a 30s interval.
- cleanupOlderThan(days) deletes every session whose latest dispatch
is older than the cutoff. 'Old' is last-activity based so a
long-lived session with a fresh straggler dispatch survives.
- Transaction spans all three audit tables. File unlinks run AFTER
commit and are best-effort; the worst-case outcome is orphaned
files (harmless), not visible-but-broken rows.
Route (routes/audit/cleanup.ts):
- GET /audit/cleanup/candidates
- POST /audit/cleanup with olderThanDays in {15, 30, 90}
- Schema derives its literal union from the shared constant.
Also exposes ReplayStorage.pathFor(sessionId) so the cleanup service
can size-check files without reimplementing the sanitise+join logic
that lives inside replay-storage.
Tests: 8 service tests (age-based semantics, last-activity guard,
file unlink, idempotency, empty DB, candidates-match-cleanup, all
three tables deleted together) + 4 route tests (candidates shape,
happy path, threshold rejection, missing body). Full claw-server
suite: 412 pass, 0 fail.
Every cleanupOlderThan() now finishes with an orphan sweep that unlinks replay + screenshot files whose id no longer exists in the audit DB. Also runs once on server startup (deferred 30s so it never delays the socket going live). Catches partial-failure drift, crashed writers, and prior manual DB edits. Safety guards: - In-flight guard: skips files younger than 5 minutes. Protects fresh writes whose DB row has not flushed yet. - Strict filename filter: only *.ndjson (replays) or integer-basename *.jpg (screenshots). Foreign files in either directory are left alone, not deleted. - DB-first snapshot: known ids read before scanning disk so rows that appear mid-scan (with newborn files) don't get nuked. - MAX_SWEEP_ENTRIES cap of 100k per directory. Pathological cases log a warning and truncate instead of hanging the request. - Best-effort throughout: readdir/stat/unlink failures logged, never throw. Response shape gains an 'orphans' block so age-based and drift-based counts stay distinguishable in the log and the UI totals. Also exports REPLAY_DIR_NAME / REPLAY_FILE_EXTENSION and SCREENSHOTS_DIR_NAME / SCREENSHOT_FILE_EXTENSION so the sweep and the writers share one source of truth for directory + extension. Tests: 10 orphan tests covering all guards plus integration with the bundled cleanup call. Full claw-server suite: 422 pass, 0 fail.
Documents the accepted concurrency behaviour: a dispatch that lands after cleanupOlderThan has computed its target set but before the transaction commits is serialised behind the cleanup by bun:sqlite's single-writer WAL, ends up in tool_dispatches AFTER the deletes, and resurrects the doomed session as a zombie with only the fresh row and no start/end. Kept as its own commit so the semantic (deliberate) can be reverted without touching the age-based service tests.
Adds a Storage button to the /audit header that opens a two-stage shadcn AlertDialog for the destructive /audit/cleanup endpoint. Stage 1 lets the user pick a threshold (15/30/90 days). Only ranges with at least one session appear as options, per Dani's spec; if none of the three has any candidates, the whole button is hidden. Stage 2 is a typed-confirmation gate: the user has to type 'delete N sessions older than D days' verbatim before the destructive Delete button unlocks. Case-sensitive exact match. The phrase mirrors the read-back line above the input and rebuilds when the range changes so muscle memory from a prior cleanup cannot unlock a new one. Same UX as GitHub repo delete / Stripe account delete. On success: the mutation invalidates useTasks + useAuditCleanupCandidates so the audit list, empty state, and Storage button all refetch on the same tick. On failure: dialog stays on Stage 2 with the typed input cleared so hitting Enter cannot silently retry. Client hooks (useAuditCleanupCandidates + useAuditCleanup) live in modules/api/audit.hooks.ts alongside the existing useTasks factories, per the react-query-kit rules in CLAUDE.md. Tests: 15 helper tests (phrase builder + case-sensitive matcher + byte formatter) + 3 button-visibility tests. The interactive two-stage flow is exercised by the pure-logic helpers plus the server-side cleanup tests.
Contributor
✅ Tests passed — 1352/1356
|
Contributor
Two Greptile catches from PR review: 1. CleanupDialog reset effect had [open, ranges] deps, so the 30s candidates poll (fresh array reference on every refetch) would collapse Stage 2 back to Stage 1 and clear the typed phrase on a user who took longer than 30 seconds to read + type the confirmation. Guard the reset on the false->true transition of 'open' via a useRef so only the actual open event triggers it. 2. audit-cleanup-startup exported wasStartupSweepInvokedForTesting + resetStartupSweepForTesting labelled 'test-only', but the actual test uses runStartupSweep + a logger spy directly. Remove the dead probe helpers and the scheduledForTesting flag they backed.
Contributor
Author
This was referenced Jul 17, 2026
Contributor
Author
|
Closing in favor of #1993. The TS claw-server is being retired, so this feature was reimplemented in the Rust claw-server. It also evolved in the process: instead of a one-shot delete with fixed 15/30/90-day thresholds behind a typed-confirmation gate, #1993 ships a user-controlled retention policy ("automatically delete audit log older than 7 days / 30 days / custom / never") enforced by a background sweep, unified across audit metadata, screenshots, and recordings, with automatic disk reclamation via SQLite incremental vacuum, and a Manage audit files dialog showing current storage usage. No functionality is lost — see #1993 for the replacement. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What's new
An in-cockpit way to reclaim disk and prune the BrowserClaw audit log. Deletes sessions, replay files, and screenshots older than one of three fixed thresholds (15, 30, 90 days). A dedicated Storage button appears in the /audit header when there is data old enough to matter, and hides itself completely otherwise. Behind the button, a two-stage AlertDialog with a typed-confirmation gate protects the destructive call.
Design
Definition of "old"
A session is old when
max(dispatch.created_at) < now - N days. Last-activity based, not session-start based: a long-lived session with a fresh straggler dispatch survives even if it started 100 days ago. Prevents the classic "old start ⇒ delete" bug.Two endpoints
GET /audit/cleanup/candidatesreturns per-threshold session/dispatch/bytes counts. Cheap enough for the client to poll on a 30s interval so options appear/disappear naturally as data ages past a threshold.POST /audit/cleanupaccepts{ olderThanDays: 15 | 30 | 90 }. Zod union derived from the shared constant array so adding a new threshold is one-line.Deletion algorithm
sessionIdsanddispatchIdsBEFORE mutating anything.db.transaction()deletes fromtool_dispatches,agent_session_starts, andagent_session_endsatomically.Orphan sweep (separate commit)
Every cleanup call finishes with, and server startup runs (deferred 30s), a sweep that unlinks replay + screenshot files whose id has no row in the DB. Catches partial-failure drift and crashed writers.
Safety guards:
*.ndjson(replays) or integer-basename*.jpg(screenshots). Foreign files in the directories are left alone.readdir/stat/unlinkfailures logged, never throw.Typed-confirmation gate (Stage 2)
The Delete button will not unlock until the user types `delete N sessions older than D days` verbatim. Case-sensitive exact match, whitespace trimmed. The phrase mirrors the read-back line above the input and rebuilds when the range changes so muscle memory from a prior cleanup cannot unlock a new one. Same UX pattern as GitHub repo delete / Stripe account delete / Vercel project delete.
Failure clears the typed input so hitting Enter cannot silently retry.
Commits (four)
feat(claw-server): age-based audit cleanup service + route + tests(`125650f90`) — the base feature: shared constant, service, route, 8 age-based tests + 4 route tests.feat(claw-server): orphan file sweep bundled into cleanup + startup pass(`b746cd324`) — the sweep, its startup wiring, and 10 orphan-sweep tests. Purely additive on top of commit 1: revertable in isolation.test(claw-server): concurrent-write survives cleanup transaction(`96966e878`) — documents the accepted concurrency behaviour of a dispatch arriving during cleanup (bun:sqlite WAL serialises it, session resurrects as zombie with only the fresh row).feat(claw-app): audit cleanup dialog with typed-confirmation gate(`8583ba1e4`) — client hooks, two-stage dialog, phrase-gate helpers, Storage button in /audit header, 15 helper tests + 3 visibility tests.Test plan
bun run typecheck(claw-server + claw-app): cleanbunx biome checkon all touched files: cleanbun test(claw-server): 423 pass, 0 fail (was 399 before; +24 new tests across 3 files)bun test(claw-app): each file individually passes (18/18 across new files); full suite hits the bun parse-race flake per ci(tests): retry each bun test suite once on failure #1851 which passes on rerun