feat(store): sweep to reclaim orphaned legacy shared blobs - #353
Conversation
Blob directories are per-database since #351, and deletes deliberately never touch the legacy shared <root>/blobs -- reads still fall back to it, and only a check against every database in the directory can prove a file there is dead. So nothing reclaims it in the normal course of things. sweep_legacy_blobs collects the referenced hashes from every .db in the directory, then removes the legacy files none of them names. It aborts rather than deleting if a database cannot be read, and refuses outright when there is no database to consult -- an incomplete reference set makes live content look like garbage. A .db without a store_blobs table is skipped as unrelated, checked via sqlite_master so a genuinely broken database still surfaces. Exposed as 'agentflare docs sweep-legacy-blobs [--dry-run]'. On this install the dry run reports 107 of 372 legacy files unreferenced.
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
📝 WalkthroughWalkthroughAdds database-aware sweeping of legacy shared blobs, dry-run reporting, reclamation tests, IO error conversion, and a ChangesLegacy blob sweep
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant DocsCLI
participant sweep_legacy_blobs
participant StoreDatabases
participant LegacyBlobDirectory
DocsCLI->>sweep_legacy_blobs: invoke with root and dry_run
sweep_legacy_blobs->>StoreDatabases: collect referenced blob hashes
sweep_legacy_blobs->>LegacyBlobDirectory: scan shard files
sweep_legacy_blobs->>LegacyBlobDirectory: delete unreferenced files when not dry-run
sweep_legacy_blobs-->>DocsCLI: return JSON LegacySweepReport
Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@crates/agentflare-store/src/maintenance.rs`:
- Around line 324-338: Preserve the sweep safety boundary across all affected
sites: in crates/agentflare-store/src/maintenance.rs:324-338, propagate
individual read_dir entry errors and require at least one successfully consulted
actual store_blobs database rather than any .db file; in src/cli/docs.rs:96-102,
dispatch SweepLegacyBlobs before opening DocsStore and lazily open the store
only for commands that require it; in
crates/agentflare-store/src/maintenance.rs:491-513, add coverage confirming a
legacy blob directory containing only unrelated databases is refused without
deleting blobs.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 0be924de-5adf-433e-8066-65efd8b80a35
📒 Files selected for processing (3)
crates/agentflare-store/src/lib.rscrates/agentflare-store/src/maintenance.rssrc/cli/docs.rs
…hable The guard counted any .db file and the CLI opened DocsStore before dispatching, so a home with legacy blobs but no store left gained a fresh empty database and swept every blob. Consult first, refuse on an empty store set, propagate read_dir entry errors, and dispatch the sweep before the store is opened.
|
Review pass — CodeRabbit's critical finding was valid on all three sites. Fixed in 677fea8 before merge. The Consult first, then refuse — and count stores, not
|
Follow-up to #351 (item #385).
Why
#351 gave each database its own blob directory and made deletes never touch the legacy shared
~/.agentflare/blobs— reads still fall back to it, and only a check against every database in the directory can prove a file there is dead. That was the right trade (leaking a file is recoverable; deleting one another store needs is not), but it means nothing reclaims that directory in the normal course of things.On this install the dry run finds 107 of 372 legacy files unreferenced (538 KB).
What
agentflare_store::maintenance::sweep_legacy_blobs(root, dry_run)collects the referenced hashes from every.dbinroot, then removes the files inroot/blobs/that none of them names.Safety properties, each with a test:
.dbwithout astore_blobstable is skipped as unrelated, checked viasqlite_masterrather than by swallowing a failed query — so a database that is broken rather than unrelated still surfaces instead of silently contributing nothing.SQLITE_OPEN_READ_ONLYand without migrations: the sweep must not alter what it only consults.CLI
Lives under
docsbecause that is where the cache commands are and the docs database is what wrote most of these files, but the sweep consultsstore.dbtoo, so it is safe regardless of which store owns a given blob. Output is the JSON report.Real run against this install:
{ "scanned": 372, "reclaimed": 107, "bytes_reclaimed": 551024, "databases": ["flare-docs.db", "store.db"], "dry_run": true }Verification
cargo fmt --all --check, the CI clippy invocation (--locked --workspace --all-targets --all-features -- -D warnings -A unsafe_code -A clippy::pedantic), andcargo test --workspaceare all clean. Four new tests inmaintenance.rs; the CLI path was exercised end to end with--dry-runagainst the real~/.agentflare.One small addition outside the sweep:
agentflare_store::Errorgained anIo(#[from] std::io::Error)variant, which the directory walk needs.Summary by CodeRabbit
New Features
--dry-runoption to preview which files would be reclaimed without deleting them.Bug Fixes