From 5e028b459c2c7f17ccf47e571d9b64e4f95143f5 Mon Sep 17 00:00:00 2001 From: ScriptedAlchemy Date: Fri, 10 Jul 2026 16:34:22 +0000 Subject: [PATCH] fix(db): keep FTS repair out of search reads --- src/db/connection.rs | 5 +- src/db/search.rs | 97 +++++++++++++++--- tests/storage_suite/corruption_test.rs | 131 ++++++++++++++++++++++--- 3 files changed, 205 insertions(+), 28 deletions(-) diff --git a/src/db/connection.rs b/src/db/connection.rs index b9db320e..2ffac9ac 100644 --- a/src/db/connection.rs +++ b/src/db/connection.rs @@ -324,10 +324,11 @@ impl Database { } } - /// Rebuilds the FTS5 index from the content table. + /// Maintenance-only: rebuilds the FTS5 index from the content table. /// /// This fixes FTS-only corruption (e.g. from an interrupted bulk load) - /// without requiring a full re-index of the codebase. + /// without requiring a full re-index of the codebase. Callers must hold + /// exclusive maintenance ownership; read paths must never invoke this. pub async fn rebuild_fts(&self) -> Result<()> { self.conn .execute("INSERT INTO nodes_fts(nodes_fts) VALUES('rebuild')", ()) diff --git a/src/db/search.rs b/src/db/search.rs index 21c46ffb..8d3cbf6b 100644 --- a/src/db/search.rs +++ b/src/db/search.rs @@ -1,5 +1,6 @@ // Rust guideline compliant 2025-10-17 use libsql::params; +use std::sync::atomic::{AtomicBool, Ordering}; use super::connection::Database; use super::rows::row_to_node; @@ -7,6 +8,8 @@ use super::sql::{build_qmark_placeholders, collect_rows}; use crate::errors::{Result, TraceDecayError}; use crate::types::*; +static FTS_REPAIR_WARNING_EMITTED: AtomicBool = AtomicBool::new(false); + #[derive(Debug, Clone, PartialEq, Eq)] pub struct DependencyImportUse { pub module: String, @@ -18,9 +21,9 @@ pub struct DependencyImportUse { impl Database { /// Searches nodes by name, qualified name, docstring, or signature. /// - /// Attempts an FTS5 prefix match first. If the FTS index is corrupted, - /// it is automatically rebuilt and the query retried. If FTS returns no - /// results, falls back to a `LIKE` query. + /// Attempts an FTS5 prefix match first. If only the FTS index is corrupted, + /// falls back to a read-only `LIKE` query. Whole-database corruption is + /// returned to the caller for recovery. pub async fn search_nodes(&self, query: &str, limit: usize) -> Result> { debug_assert!(!query.is_empty(), "search_nodes called with empty query"); debug_assert!(limit > 0, "search_nodes limit must be positive"); @@ -40,22 +43,21 @@ impl Database { return Ok(Vec::new()); } - // Try FTS search, with one self-healing retry on corruption. + // Try FTS search. Search is a read path: repairs must run offline. let fts_result = self.search_nodes_fts(&fts_query, limit).await; match fts_result { Ok(ref results) if !results.is_empty() => return fts_result, Ok(_) => {} // empty — fall through to LIKE - Err(ref e) if Self::is_corruption_error(e) => { - eprintln!("[tracedecay] FTS index corruption detected — rebuilding…"); - if self.rebuild_fts().await.is_ok() { - match self.search_nodes_fts(&fts_query, limit).await { - Ok(results) if !results.is_empty() => return Ok(results), - Ok(_) => {} // fall through to LIKE - Err(e) => return Err(e), + Err(e) if Self::is_corruption_error(&e) => match self.non_fts_schema_intact().await { + Ok(true) => { + if !FTS_REPAIR_WARNING_EMITTED.swap(true, Ordering::Relaxed) { + eprintln!( + "[tracedecay] FTS index corruption detected; using LIKE fallback. Offline repair required." + ); } } - // rebuild_fts failed — fall through to LIKE as last resort - } + Ok(false) | Err(_) => return Err(e), + }, Err(e) => return Err(e), } @@ -92,6 +94,71 @@ impl Database { Ok(results) } + /// Validates every non-FTS table and its indexes without asking `SQLite` + /// to inspect the known-corrupt FTS virtual table or shadow tables. + async fn non_fts_schema_intact(&self) -> Result { + let mut rows = self + .conn() + .query( + "SELECT name FROM sqlite_schema + WHERE type = 'table' + AND name NOT IN ( + 'nodes_fts', 'nodes_fts_data', 'nodes_fts_idx', + 'nodes_fts_content', 'nodes_fts_docsize', 'nodes_fts_config' + ) + ORDER BY name", + (), + ) + .await + .map_err(|e| TraceDecayError::Database { + message: format!("failed to enumerate non-FTS tables: {e}"), + operation: "search_nodes".to_string(), + })?; + let mut tables = Vec::new(); + while let Some(row) = rows.next().await.map_err(|e| TraceDecayError::Database { + message: format!("failed to read non-FTS table name: {e}"), + operation: "search_nodes".to_string(), + })? { + tables.push( + row.get::(0) + .map_err(|e| TraceDecayError::Database { + message: format!("invalid non-FTS table name: {e}"), + operation: "search_nodes".to_string(), + })?, + ); + } + drop(rows); + if !tables.iter().any(|table| table == "nodes") { + return Ok(false); + } + + for table in tables { + let sql = format!("PRAGMA quick_check({})", quote_sqlite_identifier(&table)); + let mut rows = + self.conn() + .query(&sql, ()) + .await + .map_err(|e| TraceDecayError::Database { + message: format!("failed to check non-FTS table '{table}': {e}"), + operation: "search_nodes".to_string(), + })?; + let mut saw_result = false; + while let Some(row) = rows.next().await.map_err(|e| TraceDecayError::Database { + message: format!("failed to read non-FTS table check for '{table}': {e}"), + operation: "search_nodes".to_string(), + })? { + saw_result = true; + if row.get::(0).unwrap_or_default() != "ok" { + return Ok(false); + } + } + if !saw_result { + return Ok(false); + } + } + Ok(true) + } + /// Executes the FTS5 query and returns ranked results. async fn search_nodes_fts(&self, fts_query: &str, limit: usize) -> Result> { let mut rows = self @@ -315,3 +382,7 @@ impl Database { } } } + +fn quote_sqlite_identifier(identifier: &str) -> String { + format!("\"{}\"", identifier.replace('"', "\"\"")) +} diff --git a/tests/storage_suite/corruption_test.rs b/tests/storage_suite/corruption_test.rs index c78fface..146da8f2 100644 --- a/tests/storage_suite/corruption_test.rs +++ b/tests/storage_suite/corruption_test.rs @@ -2,7 +2,7 @@ //! //! These tests verify: //! - `quick_check` detects real page-level corruption -//! - FTS self-healing in `search_nodes` recovers from a corrupt FTS index +//! - `search_nodes` falls back without mutating a corrupt FTS index //! - `rebuild_fts` restores query capability after FTS damage //! - `begin_bulk_load` no longer disables fsync (`synchronous = OFF`) //! - The dirty sentinel lifecycle works correctly @@ -458,8 +458,8 @@ async fn corrupt_db_detected_and_repaired_on_reopen() { } #[tokio::test] -async fn fts_corruption_healed_by_search_nodes() { - let (db, _dir, _path) = setup_db().await; +async fn fts_corruption_falls_back_without_rebuild_or_write() { + let (db, _dir, db_path) = setup_db().await; // Insert data so FTS has content let nodes = vec![ @@ -472,22 +472,127 @@ async fn fts_corruption_healed_by_search_nodes() { let results = db.search_nodes("important_handler", 10).await.unwrap(); assert_eq!(results[0].node.id, "e1"); - // Drop and re-insert one row of FTS with mismatched data to create - // inconsistency (simulate partial crash during trigger execution) - db.conn() - .execute_batch( - "INSERT INTO nodes_fts(nodes_fts, rowid, name, qualified_name, docstring, signature) - VALUES('delete', 1, 'important_handler', 'crate::important_handler', 'Documentation for important_handler', 'fn important_handler()');", + // Capture an FTS segment, then corrupt only its payload on disk. The nodes + // table and primary database B-trees remain healthy. + let mut rows = db + .conn() + .query( + "SELECT block FROM nodes_fts_data WHERE id > 10 ORDER BY id DESC LIMIT 1", + (), ) .await .unwrap(); + let segment = rows + .next() + .await + .unwrap() + .unwrap() + .get::>(0) + .unwrap(); + drop(rows); + db.checkpoint().await.unwrap(); + db.close(); + + // Corrupt both FTS and an unrelated table. Checking only `nodes` would + // incorrectly permit the LIKE fallback because its B-tree is still sound. + let mut bytes = std::fs::read(&db_path).unwrap(); + let offset = bytes + .windows(segment.len()) + .position(|candidate| candidate == segment) + .expect("FTS segment must be present in the checkpointed database"); + bytes[offset..offset + 8].fill(0xff); + std::fs::write(&db_path, bytes).unwrap(); + + let (db, _) = Database::open(&db_path).await.unwrap(); + assert!( + !db.quick_check().await.unwrap(), + "fixture must trigger SQLite's FTS integrity failure" + ); + let changes_before = db.conn().total_changes(); - // The FTS index is now inconsistent — missing a row that exists in content. - // search_nodes should still find it via LIKE fallback even if FTS misses it. let results = db.search_nodes("important_handler", 10).await.unwrap(); + assert_eq!(results[0].node.id, "e1", "LIKE fallback must still match"); + assert_eq!( + db.conn().total_changes(), + changes_before, + "search must not rebuild or otherwise write" + ); + + let mut rows = db + .conn() + .query( + "SELECT rowid FROM nodes_fts WHERE nodes_fts MATCH '\"important_handler\"*'", + (), + ) + .await + .unwrap(); assert!( - !results.is_empty(), - "search should recover via self-healing or LIKE fallback" + rows.next().await.is_err(), + "the corrupt FTS index must remain untouched for offline repair" ); + drop(rows); close_db(db).await; } + +#[tokio::test] +async fn whole_database_corruption_propagates_without_write() { + let (db, _dir, db_path) = setup_db().await; + db.insert_nodes(&[sample_node("whole-db", "whole_db_probe")]) + .await + .unwrap(); + + let mut rows = db + .conn() + .query( + "SELECT block FROM nodes_fts_data WHERE id > 10 ORDER BY id DESC LIMIT 1", + (), + ) + .await + .unwrap(); + let segment = rows + .next() + .await + .unwrap() + .unwrap() + .get::>(0) + .unwrap(); + drop(rows); + let mut rows = db + .conn() + .query( + "SELECT rootpage FROM sqlite_schema WHERE name = 'edges'", + (), + ) + .await + .unwrap(); + let root_page = rows.next().await.unwrap().unwrap().get::(0).unwrap() as u64; + drop(rows); + let mut rows = db.conn().query("PRAGMA page_size", ()).await.unwrap(); + let page_size = rows.next().await.unwrap().unwrap().get::(0).unwrap() as u64; + drop(rows); + db.checkpoint().await.unwrap(); + db.close(); + + let mut bytes = std::fs::read(&db_path).unwrap(); + let fts_offset = bytes + .windows(segment.len()) + .position(|candidate| candidate == segment) + .expect("FTS segment must be present in the checkpointed database"); + bytes[fts_offset..fts_offset + 8].fill(0xff); + bytes[((root_page - 1) * page_size) as usize] = 0xff; + std::fs::write(&db_path, bytes).unwrap(); + + let (db, _) = Database::open(&db_path).await.unwrap(); + let changes_before = db.conn().total_changes(); + let error = db.search_nodes("whole_db_probe", 10).await.unwrap_err(); + assert!( + Database::is_corruption_error(&error), + "unexpected error: {error}" + ); + assert_eq!( + db.conn().total_changes(), + changes_before, + "search must not write while reporting whole-database corruption" + ); + db.close(); +}