Repository navigation
Conversation
…reclaim The docs cache had no ceiling of any kind. Soft-deleted rows (#334/#337) were never physically removed, no policy capped total size, and nothing ever handed freed pages back to the filesystem -- a long-lived install refreshing a rotating set of packages grows without bound (#339). agentflare-store gains a maintenance module: Store::gc purges tombstones past a 7-day retention, evicts the least recently updated live documents while the project is over a 256 MB budget, then reclaims. Blob accounting is the part worth reading twice -- a tombstone gave up its own blob reference when it was soft-deleted, so purging must not release it again, while an evicted live document still holds one. History snapshots hold references of their own and are released as their rows go, the same leak the soft-delete path closed earlier in this issue. Size is measured as live content plus referenced blobs, not the .db file: blobs dominate a docs cache (5.7 MB against a 1.8 MB database when measured), and the file does not shrink at the moment rows are deleted, which would make it useless as the loop variable for eviction. db-kit now asks for auto_vacuum=INCREMENTAL and synchronous=NORMAL, and sets foreign_keys=ON explicitly instead of inheriting it from the linked SQLite build. The auto_vacuum pragma goes ahead of journal_mode and that ordering is load-bearing: switching to WAL writes the header, after which the setting is silently ignored -- and it reads back as INCREMENTAL until the first table lands, so a test that checks too early passes either way. Databases created before this stay in NONE mode, so gc runs one full VACUUM to convert them, rebuilding the external-content FTS index behind it, and uses incremental_vacuum from then on. Collection runs on the flare-docs fetch path rather than a schedule -- every route that grows the cache funnels through it, so nothing has to own a timer -- and non-empty runs are journaled to ~/.agentflare/audit/gc.jsonl beside the git shim's log.
|
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:
📝 WalkthroughWalkthroughAdds SQLite pragma and foreign-key enforcement, implements store cache garbage collection with blob cleanup and vacuuming, exposes it through ChangesDocs cache maintenance
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant DocsMCP
participant DocsStore
participant Store
participant SQLite
participant AuditLog
DocsMCP->>DocsStore: fetch and store documentation
DocsMCP->>DocsStore: run cache garbage collection
DocsStore->>Store: gc for fixed project
Store->>SQLite: purge, evict, and reclaim pages
Store-->>DocsStore: return GcReport
DocsStore-->>DocsMCP: return fetch outcome
DocsMCP->>AuditLog: append gc report when changes occurred
Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (3)
src/mcp_server/flare_docs.rs (1)
182-199: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueConsider a
tracingline for the swallowed GC error.Not failing the fetch is right, but the current shape leaves a permanently failing GC completely invisible — the audit journal only records successful, non-empty runs. A
debug!/warn!on the error arm would make an unbounded cache diagnosable.🤖 Prompt for 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. In `@src/mcp_server/flare_docs.rs` around lines 182 - 199, Add a tracing debug or warning entry around the ignored result from flare_git_core::audit::log_event in the GC reporting flow, recording failures while preserving the non-failing fetch behavior. Keep successful audit logging and the report.purged + report.evicted == 0 early return unchanged.crates/agentflare-store/src/maintenance.rs (2)
124-127: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winOne
BEGIN IMMEDIATEper evicted document.The purge phase batches all expired ids into a single transaction; eviction opens a separate immediate transaction (plus a separate
blob_unreftransaction set) per victim. Under a real over-budget run that is potentially thousands of exclusive-lock commits.release_own_blobistruefor every victim, so the slice can go through in one call — you lose per-victimevictedaccounting, but the count is just the slice length.♻️ Batch the eviction phase
- for id in self.eviction_candidates(project_id, opts.max_bytes)? { - report.blobs_released += self.hard_delete_docs(std::slice::from_ref(&id), true)?; - report.evicted += 1; - } + let victims = self.eviction_candidates(project_id, opts.max_bytes)?; + report.evicted = victims.len(); + report.blobs_released += self.hard_delete_docs(&victims, true)?;🤖 Prompt for 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. In `@crates/agentflare-store/src/maintenance.rs` around lines 124 - 127, Batch all eviction candidates into a single collection, then call hard_delete_docs once with the full slice and release_own_blob enabled instead of deleting each document inside the loop. Update report.evicted from the number of candidates while preserving report.blobs_released from the batched deletion result.
205-212: 🚀 Performance & Scalability | 🔵 Trivial | 💤 Low valueHoist the history-hash statement out of the id loop.
tx.prepareruns once per id;prepare_cached(or preparing before the loop, aswrite_batch_itemsdoes inflare-docs) avoids re-parsing on every document — relevant once purge handles a week's worth of tombstones.🤖 Prompt for 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. In `@crates/agentflare-store/src/maintenance.rs` around lines 205 - 212, Move preparation of the history-hash query out of the per-id loop in the surrounding maintenance operation, reusing a single prepared statement for each id lookup. Prefer the transaction’s cached preparation mechanism or prepare it before the loop, while preserving the existing query parameters, row collection, and statement lifetime behavior.
🤖 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-db-kit/src/open.rs`:
- Around line 34-40: Update the journal-mode configuration in the connection
setup to use conn.pragma_update_and_check for "journal_mode" instead of
conn.pragma_update, while preserving the existing auto_vacuum ordering and WAL
value.
In `@crates/agentflare-store/src/maintenance.rs`:
- Around line 79-93: Update MaintenanceStore::cache_bytes so the blob total is
scoped to the requested project by summing only blobs referenced by that
project’s live documents, using the existing document-to-blob relationship. Keep
the content calculation project-scoped and ensure the returned total reflects
only bytes attributable to project_id, preventing eviction from targeting more
than that project owns.
In `@src/mcp_server/flare_docs.rs`:
- Line 163: Update the fetch flow around gc_docs_cache so garbage collection
runs via spawn_blocking rather than inline on the async runtime. Await the
blocking task if completion is required, or detach it consistently with the
existing ignored-failure behavior, and apply the same change to the additional
GC call near the later cache path.
---
Nitpick comments:
In `@crates/agentflare-store/src/maintenance.rs`:
- Around line 124-127: Batch all eviction candidates into a single collection,
then call hard_delete_docs once with the full slice and release_own_blob enabled
instead of deleting each document inside the loop. Update report.evicted from
the number of candidates while preserving report.blobs_released from the batched
deletion result.
- Around line 205-212: Move preparation of the history-hash query out of the
per-id loop in the surrounding maintenance operation, reusing a single prepared
statement for each id lookup. Prefer the transaction’s cached preparation
mechanism or prepare it before the loop, while preserving the existing query
parameters, row collection, and statement lifetime behavior.
In `@src/mcp_server/flare_docs.rs`:
- Around line 182-199: Add a tracing debug or warning entry around the ignored
result from flare_git_core::audit::log_event in the GC reporting flow, recording
failures while preserving the non-failing fetch behavior. Keep successful audit
logging and the report.purged + report.evicted == 0 early return unchanged.
🪄 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: e20ad123-dcf3-4425-b995-b4dda555e818
📒 Files selected for processing (7)
crates/agentflare-db-kit/CHANGELOG.mdcrates/agentflare-db-kit/src/open.rscrates/agentflare-store/src/lib.rscrates/agentflare-store/src/maintenance.rscrates/flare-docs/src/lib.rscrates/flare-docs/src/store.rssrc/mcp_server/flare_docs.rs
| conn.pragma_update(None, "auto_vacuum", "INCREMENTAL")?; | ||
| conn.pragma_update(None, "journal_mode", "WAL")?; | ||
| // The durability level WAL is designed around: a crash can lose the last | ||
| // few transactions, a power loss still cannot corrupt the file. FULL | ||
| // fsyncs every commit, which for local agent stores buys nothing over | ||
| // WAL's own guarantees. | ||
| conn.pragma_update(None, "synchronous", "NORMAL")?; |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Any existing precedent in-repo for setting journal_mode?
rg -nP --type=rust 'journal_mode' -C2
# rusqlite version actually resolved
fd -H '^Cargo.lock$' --max-depth 2 --exec rg -n -A2 '^name = "rusqlite"$' {}Repository: getappz/agentflare
Length of output: 156
🌐 Web query:
rusqlite pragma_update journal_mode WAL ExecuteReturnedResults
💡 Result:
In rusqlite, the ExecuteReturnedResults error occurs when a SQL statement returns rows (e.g., via a SELECT or a PRAGMA that returns a result set) while being executed by a method that expects no output, such as Connection::execute or execute_batch [1][2][3]. The PRAGMA journal_mode=WAL statement is a common source of this error because, in SQLite, this command returns the resulting journal mode [4]. If your rusqlite configuration includes the extra_check feature (which is often enabled or implied in certain environments or older versions), rusqlite validates that statements do not return unexpected rows and will panic or return the ExecuteReturnedResults error if they do [5][6]. To resolve this when setting the journal mode, you should use the pragma_update_and_check method instead of pragma_update [7][8]. This method is specifically designed to handle PRAGMAs that return data by providing a closure to process the result row [9][10]: conn.pragma_update_and_check(None, "journal_mode", &"WAL", |_| Ok()).unwrap; Alternatively, if you are not using extra_check and simply want to ensure the command executes without processing output, verify your use of Connection::execute_batch, though pragma_update_and_check remains the recommended, robust approach for handling pragmas that return values [7][11][10].
Citations:
- 1: https://docs.rs/rusqlite/latest/rusqlite/enum.Error.html
- 2: https://docs.rs/rusqlite/latest/src/rusqlite/error.rs.html
- 3: https://github.com/rusqlite/rusqlite/blob/master/src/error.rs
- 4: https://sqlite.org/pragma.html
- 5: cljoly/rusqlite_migration@e4d489c
- 6: https://docs.rs/rusqlite/latest/src/rusqlite/statement.rs.html
- 7: Fix broken docs and examples when some rusqlite features are used cljoly/rusqlite_migration#139
- 8: https://docs.rs/rusqlite_migration/latest/rusqlite_migration/
- 9: https://docs.rs/rusqlite/0.25.4/rusqlite/struct.Connection.html
- 10: https://github.com/rusqlite/rusqlite/blob/master/src/pragma.rs
- 11: Impossible to execute a pragma in 0.14.0 rusqlite/rusqlite#400
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n== open.rs ==\n'
sed -n '1,120p' crates/agentflare-db-kit/src/open.rs
printf '\n== rusqlite pin in Cargo.lock ==\n'
rg -n -A4 -B2 '^name = "rusqlite"$|^version = ' Cargo.lock Cargo.toml crates/**/Cargo.lock
printf '\n== any local uses of pragma_update / pragma_update_and_check ==\n'
rg -n 'pragma_update(_and_check)?\(|journal_mode|synchronous|auto_vacuum' crates/ -g '*.rs'Repository: getappz/agentflare
Length of output: 50375
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '== rusqlite version from Cargo.lock ==\n'
rg -n -A3 -B2 '^name = "rusqlite"$' Cargo.lock
printf '\n== rusqlite version from manifests ==\n'
rg -n 'rusqlite' Cargo.toml crates/*/Cargo.toml
printf '\n== local open.rs call sites ==\n'
rg -n 'pragma_update\(|pragma_update_and_check\(' crates/agentflare-db-kit/src/open.rs crates -g '*.rs' | head -n 40Repository: getappz/agentflare
Length of output: 3096
🌐 Web query:
rusqlite 0.40.1 pragma_update journal_mode ExecuteReturnedResults pragma_update_and_check
💡 Result:
In rusqlite 0.40.1, the methods pragma_update and pragma_update_and_check are provided on the Connection and Transaction types to manage SQLite pragmas [1][2][3]. pragma_update The pragma_update method is used to set a new value for a specific pragma [3]. It executes the pragma command using execute_batch [3]. Because it is designed for simple updates, it does not retrieve results, and it will return an error if the underlying SQLite operation unexpectedly returns rows [3]. pragma_update_and_check The pragma_update_and_check method is intended for pragmas that return a value upon being updated (e.g., verifying that a journal_mode change was successful) [3]. It executes the pragma using query_row, allowing you to provide a closure (f) to process the returned row and verify the result [3]. ExecuteReturnedResults ExecuteReturnedResults is a variant of the rusqlite::Error enum [4]. It is returned when an operation called via execute (which expects no output) unexpectedly returns rows from the database [5][6]. This commonly occurs if you use execute on a SQL statement that produces a result set, or if a pragma operation that you expected to be silent actually returns data [5]. If you encounter this error, it serves as a suggestion to use a query-based method instead [5][6]. Example usage: // Using pragma_update for simple settings db.pragma_update(None, "user_version", 1)?; // Using pragma_update_and_check to verify a change db.pragma_update_and_check(None, "journal_mode", "OFF", |row| { let mode: String = row.get(0)?; if mode == "off" { Ok() } else { Err(rusqlite::Error::ExecuteReturnedResults) // Or a custom error } })?;
Citations:
- 1: https://docs.rs/rusqlite/latest/rusqlite/struct.Connection.html
- 2: https://docs.rs/rusqlite/latest/rusqlite/struct.Transaction.html
- 3: https://github.com/rusqlite/rusqlite/blob/master/src/pragma.rs
- 4: https://docs.rs/rusqlite/latest/rusqlite/enum.Error.html
- 5: https://docs.rs/rusqlite/latest/src/rusqlite/error.rs.html
- 6: https://github.com/rusqlite/rusqlite/blob/master/src/error.rs
Use pragma_update_and_check for journal_mode. PRAGMA journal_mode returns a row, so pragma_update can raise ExecuteReturnedResults here. Switch to pragma_update_and_check (or query_row) and keep the auto_vacuum ordering as-is.
🤖 Prompt for 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.
In `@crates/agentflare-db-kit/src/open.rs` around lines 34 - 40, Update the
journal-mode configuration in the connection setup to use
conn.pragma_update_and_check for "journal_mode" instead of conn.pragma_update,
while preserving the existing auto_vacuum ordering and WAL value.
…ictions Two findings from review, both about eviction removing more than it should. cache_bytes summed all of store_blobs while the content term was project-scoped, so in a multi-project database another project's bytes decided when this one evicted -- and since evicting this project's documents never brought that total down, it emptied the project and still finished over budget. Only blobs referenced by this project's live documents count now. Eviction also estimated each document's contribution up front and deleted a whole batch against it. A blob two documents share is only reclaimed when the second one goes, so the estimate under-counts what the batch frees, and an estimate that can never reach its target walks the candidate list to the end. Replaced with a loop that re-measures after each eviction and stops the moment the project fits, bounded by the candidate query returning None. Cache maintenance also ran inline on the single-threaded MCP runtime, which is what that function's own doc comment gives as the reason the fetch uses spawn_blocking. A purge can remove thousands of rows, and the first run against a pre-auto_vacuum database rewrites the whole file and rebuilds the search index. It now goes to the blocking pool, detached -- the caller's answer does not depend on it. Not changed: the journal_mode pragma flagged as needing pragma_update_and_check. rusqlite 0.40.1's pragma_update runs the statement through execute_batch, which discards returned rows, so ExecuteReturnedResults cannot arise -- and the line predates this branch.
|
Both correctness findings fixed in 4f5593c; the pragma one is a false positive. 1. Global blob total in a project-scoped budget — fixed. let blobs: i64 = conn.query_row(
- "SELECT COALESCE(SUM(size), 0) FROM store_blobs",
- [],
+ "SELECT COALESCE(SUM(b.size), 0) FROM store_blobs b
+ WHERE EXISTS (SELECT 1 FROM store_documents d
+ WHERE d.blob_hash = b.hash
+ AND d.project_id = ?1 AND d.deleted_at IS NULL)",
+ params![project_id],
|row| row.get(0),
)?;Scoping alone was not enough, though. Two documents in the same project can share a blob, which is only reclaimed when the second one goes — so the up-front per-document estimate ( - for id in self.eviction_candidates(project_id, opts.max_bytes)? {
+ while self.cache_bytes(project_id)? > opts.max_bytes {
+ let Some(id) = self.next_eviction_candidate(project_id)? else {
+ break;
+ };
report.blobs_released += self.hard_delete_docs(std::slice::from_ref(&id), true)?;
report.evicted += 1;
}Test: 2. GC inline on the async runtime — fixed. Moved to the blocking pool and detached, for the reason that function's own doc comment already gives. fn gc_docs_cache(&self) {
- let Ok(Ok(report)) = self.with_flare_docs_store(|store| store.gc(GcOpts::default())) else {
- return;
- };
- if report.purged + report.evicted == 0 {
+ if self.ensure_flare_docs_store().is_err() {
return;
}
- if let Some(path) = flare_git_core::audit::default_path("gc.jsonl") {
+ let store = std::sync::Arc::clone(&self.flare_docs_store);
+ tokio::task::spawn_blocking(move || {
+ let Ok(guard) = store.lock() else { return };
+ let Some(store) = guard.as_ref() else { return };
+ let Ok(report) = store.gc(GcOpts::default()) else { return };
+ if report.purged + report.evicted == 0 {
+ return;
+ }
+ // ... journal to gc.jsonl
+ });
}3. Full gate green after the fixes: |
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 `@src/mcp_server.rs`:
- Around line 106-108: Update flare_docs_store and its access paths, especially
gc_docs_cache and with_flare_docs_store, so MCP handlers do not synchronously
block on the store mutex while garbage collection runs. Use an async-awaitable
lock or separate GC ownership from request-path access, ensuring handlers yield
while waiting and the existing store operations remain correct.
🪄 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: 286e146b-da0d-4444-a56b-85eadd4594ca
📒 Files selected for processing (3)
crates/agentflare-store/src/maintenance.rssrc/mcp_server.rssrc/mcp_server/flare_docs.rs
🚧 Files skipped from review as they are similar to previous changes (1)
- src/mcp_server/flare_docs.rs
Detaching GC onto the blocking pool moved the work off the runtime but not the contention: it holds the same std::sync::Mutex every docs handler takes, so a request arriving mid-maintenance blocked the MCP runtime's only thread for as long as a purge or a full VACUUM ran -- stalling every other tool call, not just the docs ones. The store now sits behind a tokio mutex. Handlers await it; the GC task, already off the runtime, takes it with blocking_lock.
|
Fixed in 2da7cde — you're right, detaching moved the work off the runtime but not the contention. - flare_docs_store: std::sync::Arc<std::sync::Mutex<Option<flare_docs::DocsStore>>>,
+ flare_docs_store: std::sync::Arc<tokio::sync::Mutex<Option<flare_docs::DocsStore>>>,
- fn with_flare_docs_store<T>(
+ async fn with_flare_docs_store<T>(
&self,
f: impl FnOnce(&flare_docs::DocsStore) -> T,
) -> Result<T, ErrorData> {
- self.ensure_flare_docs_store()?;
- let guard = self
- .flare_docs_store
- .lock()
- .map_err(|e| ErrorData::internal_error(e.to_string(), None))?;
+ self.ensure_flare_docs_store().await?;
+ let guard = self.flare_docs_store.lock().await;
Ok(f(guard.as_ref().expect("ensure_flare_docs_store just initialized it")))
}The GC task, already off the runtime inside tokio::task::spawn_blocking(move || {
- let Ok(guard) = store.lock() else {
- return;
- };
+ // `blocking_lock`, not `lock().await`: this closure is already
+ // off the runtime, and a request waiting on the same mutex
+ // awaits rather than blocking behind it.
+ let guard = store.blocking_lock();All 11 call sites now Gate green: |
…our's content (#351) store.db and flare-docs.db share ~/.agentflare/ and so derived the same blobs/ directory while keeping separate store_blobs ref-count tables. Identical bytes in both stores collide by content address, and neither side can see the other's references, so a reclaim in one silently stranded the other's content. PR #348 made this routine by running GC on every docs fetch. Writes now go to <root>/blobs-<db stem>/, reads fall back to the legacy shared <root>/blobs/, and deletes only ever unlink from the store's own directory. Worst case leaks a legacy file nothing references; it can never remove content another store still needs. No migration, so blobs already on disk keep reading back.
…(pr open wrapper, post-hoc stamp) and explicit takeover (#895) * feat(pr-identity): agentflare pr open/owner/adopt, post-hoc stamp, doctor line (#348 phase 3) Agentflare-Agent: claude-code Agentflare-Branch: task/348-cross-instance-prs-phase-3-stamp-prs-age Agentflare-Item: 348 Agentflare-Session: 62141f4c-1715-4319-89f6-57750848e4d0 * fix(doctor): list only PR numbers for other-instance PRs (CodeQL cleartext-logging alert) Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com> Agentflare-Agent: claude-code Agentflare-Branch: task/348-cross-instance-prs-phase-3-stamp-prs-age Agentflare-Item: 348 Agentflare-Session: 04c55c4c-961f-4a8b-be6b-d5a951c8d91f * fix(doctor): report only a count of other-instance PRs (CodeQL cleartext-logging) Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com> Agentflare-Agent: claude-code Agentflare-Branch: task/348-cross-instance-prs-phase-3-stamp-prs-age Agentflare-Item: 348 Agentflare-Session: 04c55c4c-961f-4a8b-be6b-d5a951c8d91f * fix(doctor): return a count from pr_owner so no client-derived text is printed (CodeQL) Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com> Agentflare-Agent: claude-code Agentflare-Branch: task/348-cross-instance-prs-phase-3-stamp-prs-age Agentflare-Item: 348 Agentflare-Session: 04c55c4c-961f-4a8b-be6b-d5a951c8d91f * fix(pr): pr adopt re-stamps the origin so ownership gates follow a takeover resolve_owner honours the takeover comment but origin_allows/marks_item/find_by_item_marker read only the body stamp, so after adopt the previous owner still passed the gates and both instances could work the PR. adopt now rewrites the stamp to the adopter. Agentflare-Agent: claude-code Agentflare-Branch: task/348-cross-instance-prs-phase-3-stamp-prs-age Agentflare-Item: 348 Agentflare-Session: 2e7fb763-e49d-4d67-b764-96b4a24c61c7
Closes #339.
The docs cache had no ceiling of any kind: soft-deleted rows (#334/#337) were never physically removed, no policy capped total size, and nothing ever handed freed pages back to the filesystem. Step 1 of this issue (blob release on delete, #343) stopped the leak on the write path; this bounds the cache itself.
What lands
agentflare-store::maintenance—Store::gc(project_id, GcOpts) -> GcReport, three phases:agentflare-db-kit—auto_vacuum=INCREMENTAL,synchronous=NORMAL, and an explicitforeign_keys=ON.flare-docs/ MCP handler —DocsStore::gc, called on the fetch path, journaled to~/.agentflare/audit/gc.jsonl.Three things worth reading twice
Blob accounting differs by phase. A tombstone released its own blob reference when it was soft-deleted (
doc_delete,upsert_batch_reconciled), so purging it must not release again — that would take a reference the row no longer holds and empty a blob a live document still points at. An evicted live document does still hold one. History snapshots hold references of their own either way, and are released as their rows go — the same leak class the soft-delete path closed earlier in this issue.Size is live content plus referenced blobs, not the
.dbfile. Blobs dominate a docs cache (measured 5.7 MB against a 1.8 MB database), so a cap watching only the file would never fire. The file also doesn't shrink at the moment rows are deleted, which makes it useless as the loop variable for eviction — pages come back at the reclaim step, after the decisions are already made.The pragma ordering is load-bearing.
auto_vacuumgoes ahead ofjournal_mode: switching to WAL writes the header, after which the pragma is silently ignored. Worse, it reads back as INCREMENTAL on an empty file and only reverts once the first table lands — so the obvious test passes no matter which order you use. The test here opens, drops, and reopens for that reason. Databases created before this stay in NONE mode, sogcruns one fullVACUUMto convert them (rebuilding the external-content FTS index behind it, per the noteEXTERNAL_CONTENT_FTS_MIGRATIONleft for this work) and usesincremental_vacuumfrom then on.Deliberate calls
updated_atorder, not true LRU. A real last-accessed column costs a write on every read; for a cache that is refreshed rather than mutated the two orders barely differ.foreign_keys=ONchanges nothing today — the bundled SQLite already defaults it on. It is set so the guarantee comes from the code rather than alibsqlite3-sysbuild flag, and pinned by a test asserting enforcement is live during migrations too (which is why a migration that removes parent rows has to clear their children first — the dedup migration already does).Tests
Six new tests in
maintenance, plus three indb-kit. Each maintenance test was verified to fail under a targeted mutation:expired_tombstones_are_purged_and_fresh_ones_kept,purging_a_document_releases_the_blobs_its_history_still_heldpurging_a_document_releases_the_blobs_its_history_still_heldeviction_drops_the_least_recently_updated_document_first,eviction_releases_the_evicted_documents_blobeviction_releases_the_evicted_documents_blobOne honest gap, documented in the test itself: removing the post-
VACUUMFTS rebuild kills nothing. The SQLite in use preserves rowids acrossVACUUM, sodocuments_stay_searchable_after_a_vacuuming_gcpins the outcome (index and base table still agree by rowid, search still answers) rather than the mechanism. The rebuild stays because SQLite documents renumbering as permitted for tables without an explicit INTEGER PRIMARY KEY — whichstore_documents, keyed on a TEXT id, is — and the failure mode if a future version exercises that permission is a search index that returns nothing, with no error anywhere.cargo fmt --check,cargo clippy --locked --workspace --all-targets --all-features -- -D warnings -A unsafe_code -A clippy::pedantic, andcargo test --workspaceall pass locally.Known hazard left in place
flare-docs.dbandstore.dbshare~/.agentflare/blobs/but keep separatestore_blobsref-count tables, so one store's unref could in principle delete a file the other still references. Near-zero probability, one-line fix (namespace the blobs directory by db stem) — out of scope here, worth its own issue.Summary by CodeRabbit
New Features
Bug Fixes