From 4181248da24301ec2cf1f3cad485dd5e2dea90da Mon Sep 17 00:00:00 2001 From: ualtinok <94532+ualtinok@users.noreply.github.com> Date: Tue, 27 Oct 2026 02:25:52 +0100 Subject: [PATCH 1/6] mason: seal base call-graph donors and clone them only when sealed A sealed donor is a private, fully built derived graph switched to journal_mode=DELETE, checkpointed and synced, with no -wal/-shm/-journal sibling, plus an atomically published -seal record (fingerprint, size, journal mode). Readers open sealed files with immutable=1; IdentityConnection refuses write opens while the seal record exists. clone_sealed verifies the seal and copies into a fresh, never-reused name with clonefile (APFS), FICLONE (Linux) or a logged plain copy; a live (unsealed) database is refused. The generation sweep reclaims seal records with their database. Co-authored-by: alfonso-aft <289616620+alfonso-aft@users.noreply.github.com> (cherry picked from commit 4da050bd3d2a68effe2d0e2aa90ff7bbede9b7d5) --- crates/aft/src/callgraph_store/mod.rs | 22 +- crates/aft/src/db/file_identity.rs | 2 + crates/aft/src/views/donor.rs | 504 ++++++++++++++++++++++++++ crates/aft/src/views/generation.rs | 16 +- crates/aft/src/views/mod.rs | 1 + 5 files changed, 540 insertions(+), 5 deletions(-) create mode 100644 crates/aft/src/views/donor.rs diff --git a/crates/aft/src/callgraph_store/mod.rs b/crates/aft/src/callgraph_store/mod.rs index 82211065c3..8e38567488 100644 --- a/crates/aft/src/callgraph_store/mod.rs +++ b/crates/aft/src/callgraph_store/mod.rs @@ -9125,7 +9125,12 @@ impl ReadonlyCallGraphStore { ) -> Result { let sqlite_path = crate::views::resolve_derived_path(&view_dir, generation) .map_err(|e| CallGraphStoreError::Unavailable(e.to_string()))?; - let conn = open_readonly_connection(&sqlite_path)?; + // A sealed donor never changes; open it without locks or a WAL. + let conn = if crate::views::donor::is_sealed(&sqlite_path) { + open_sealed_connection(&sqlite_path)? + } else { + open_readonly_connection(&sqlite_path)? + }; ensure_database_ready(&conn)?; configure_serving_reader(&conn)?; let mut inner = CallGraphStore::from_connection( @@ -11741,6 +11746,21 @@ fn open_readonly_connection(path: &Path) -> Result { open_readonly_connection_before(path, Instant::now()) } +/// A sealed donor (see `views::donor`) is opened with `immutable=1`: SQLite +/// then takes no locks and never looks for a WAL, which is what lets a +/// worker clone the file with a plain descriptor. +fn open_sealed_connection(path: &Path) -> Result { + crate::private_storage::prepare_sqlite(path, OpenFlags::SQLITE_OPEN_READ_ONLY)?; + let uri = format!("{}&immutable=1", sqlite_readonly_uri(path)); + let conn = TrackedConnection::open_with_flags( + &uri, + OpenFlags::SQLITE_OPEN_READ_ONLY | OpenFlags::SQLITE_OPEN_URI, + SqliteStore::CallgraphGeneration, + )?; + conn.execute_batch("PRAGMA query_only=ON;")?; + Ok(conn) +} + fn open_readonly_connection_before(path: &Path, deadline: Instant) -> Result { crate::private_storage::prepare_sqlite(path, OpenFlags::SQLITE_OPEN_READ_ONLY)?; let uri = sqlite_readonly_uri(path); diff --git a/crates/aft/src/db/file_identity.rs b/crates/aft/src/db/file_identity.rs index 2e2bc939b6..e526a9e034 100644 --- a/crates/aft/src/db/file_identity.rs +++ b/crates/aft/src/db/file_identity.rs @@ -400,6 +400,7 @@ impl IdentityConnection { pub(crate) fn open(path: impl AsRef, seam: &'static str) -> rusqlite::Result { super::lifecycle::production_write_gate(path.as_ref())?; + crate::views::donor::refuse_sealed_write(path.as_ref())?; let _guard = filesystem_guard(); crate::private_storage::prepare_sqlite(path.as_ref(), rusqlite::OpenFlags::default())?; #[cfg(test)] @@ -422,6 +423,7 @@ impl IdentityConnection { ) -> rusqlite::Result { if !flags.contains(rusqlite::OpenFlags::SQLITE_OPEN_READ_ONLY) { super::lifecycle::production_write_gate(path.as_ref())?; + crate::views::donor::refuse_sealed_write(path.as_ref())?; } let _guard = filesystem_guard(); crate::private_storage::prepare_sqlite(path.as_ref(), flags)?; diff --git a/crates/aft/src/views/donor.rs b/crates/aft/src/views/donor.rs new file mode 100644 index 0000000000..bc3b803d2a --- /dev/null +++ b/crates/aft/src/views/donor.rs @@ -0,0 +1,504 @@ +//! Sealed call-graph donors. +//! +//! A base's derived call graph is built privately before the base pointer can +//! name it. Before that pointer moves, the builder seals the file: it switches +//! it to `journal_mode=DELETE`, checkpoints, syncs it, proves that no `-wal`, +//! `-shm` or `-journal` file remains, and atomically publishes a seal record +//! next to it (`-seal`). From then on the file never changes: +//! +//! * every reader opens it with SQLite `immutable=1` (no locks, no WAL); +//! * a write open through `IdentityConnection` is refused while the seal +//! record exists (see `refuse_sealed_write`); +//! * a worker checkout clones it into a fresh, never-reused private name +//! (`clonefile` on APFS, `FICLONE` reflink on Linux, otherwise a plain copy +//! whose reason is logged) and patches the clone with the manifest diff. +//! +//! A live SQLite database is never cloned or copied here: `clone_sealed` +//! refuses a donor whose seal record is missing or no longer matches. + +use std::io::{Read as _, Write as _}; +use std::path::{Path, PathBuf}; +use std::sync::Mutex; + +use serde::{Deserialize, Serialize}; + +/// Appended to a sealed database's path to name its seal record. The +/// generation sweep treats it as one of the database's sibling files, so the +/// record is reclaimed together with its donor. +pub(crate) const SEAL_SUFFIX: &str = "-seal"; +const SEAL_FORMAT: u32 = 1; + +#[derive(Clone, Debug, Deserialize, Eq, PartialEq, Serialize)] +pub(crate) struct SealRecord { + pub format: u32, + /// BLAKE3 of the sealed file's bytes, as hex. + pub fingerprint: String, + pub size: u64, + pub journal_mode: String, +} + +/// How a worker's private copy of a donor was made. +#[derive(Clone, Debug, Eq, PartialEq)] +pub(crate) enum CloneMechanism { + /// APFS `clonefile(2)`: copy-on-write, no data copied. + #[cfg_attr(not(target_os = "macos"), allow(dead_code))] + Clonefile, + /// Linux `FICLONE` reflink: copy-on-write, no data copied. + #[cfg_attr(not(target_os = "linux"), allow(dead_code))] + Reflink, + /// The filesystem cannot share extents; the sealed bytes were copied. + /// Safe because a sealed file never changes. Carries the reason. + Copy(String), +} + +/// Why a worker graph was built cold instead of from a sealed donor. This is +/// the closed list callers report; ordinary fixtures never produce one. +#[derive(Clone, Debug, Eq, PartialEq)] +pub(crate) enum ColdReason { + /// The base has no sealed donor (for example, a base published before + /// sealing existed). + NoSealedDonor, + /// The donor's seal record is missing, unreadable or does not match. + SealInvalid(String), + /// The donor's recorded manifest fingerprint is not the base's. + BaseFingerprintMismatch, +} + +/// One donor decision for a worker graph, recorded per member directory so +/// tests can count reuse without reading global state of other fixtures. +#[derive(Clone, Debug, Eq, PartialEq)] +pub(crate) enum DonorEvent { + Sealed, + Cloned(CloneMechanism), + Cold(ColdReason), +} + +static EVENTS: Mutex> = Mutex::new(Vec::new()); + +pub(crate) fn record(dir: &Path, event: DonorEvent) { + match &event { + DonorEvent::Cloned(CloneMechanism::Copy(reason)) => log::info!( + "views donor clone fell back to a copy dir={} reason={reason}", + dir.display() + ), + DonorEvent::Cold(reason) => log::info!( + "views worker graph built cold dir={} reason={reason:?}", + dir.display() + ), + _ => {} + } + EVENTS + .lock() + .unwrap_or_else(std::sync::PoisonError::into_inner) + .push((dir.to_path_buf(), event)); +} + +/// Donor events recorded for directories under `prefix`. +#[cfg(test)] +pub(crate) fn events_under(prefix: &Path) -> Vec { + EVENTS + .lock() + .unwrap_or_else(std::sync::PoisonError::into_inner) + .iter() + .filter(|(dir, _)| dir.starts_with(prefix)) + .map(|(_, event)| event.clone()) + .collect() +} + +pub(crate) fn seal_path(database: &Path) -> PathBuf { + let mut name = database.as_os_str().to_owned(); + name.push(SEAL_SUFFIX); + PathBuf::from(name) +} + +pub(crate) fn is_sealed(database: &Path) -> bool { + seal_path(database).exists() +} + +/// Write opens of a sealed database are refused: its readers open it with +/// `immutable=1`, so any change would be invisible to them or corrupt them. +pub(crate) fn refuse_sealed_write(database: &Path) -> rusqlite::Result<()> { + if is_sealed(database) { + return Err(rusqlite::Error::SqliteFailure( + rusqlite::ffi::Error::new(rusqlite::ffi::SQLITE_READONLY), + Some(format!( + "sealed donor {} cannot be opened for writing", + database.display() + )), + )); + } + Ok(()) +} + +fn sibling(database: &Path, suffix: &str) -> PathBuf { + let mut name = database.as_os_str().to_owned(); + name.push(suffix); + PathBuf::from(name) +} + +fn error(step: &str, reason: impl std::fmt::Display) -> String { + format!("donor {step}: {reason}") +} + +/// SQLite header bytes 18 and 19 are the write and read format versions: +/// 1 means a rollback journal, 2 means WAL. +fn header_journal_mode(database: &Path) -> Result<&'static str, String> { + let mut header = [0u8; 20]; + std::fs::File::open(database) + .and_then(|mut file| file.read_exact(&mut header)) + .map_err(|e| error("header", e))?; + match (header[18], header[19]) { + (1, 1) => Ok("delete"), + (2, 2) => Ok("wal"), + other => Err(error( + "header", + format!("unknown format versions {other:?}"), + )), + } +} + +fn fingerprint(database: &Path) -> Result<(String, u64), String> { + let mut file = std::fs::File::open(database).map_err(|e| error("fingerprint", e))?; + let mut hasher = blake3::Hasher::new(); + let mut buffer = vec![0u8; 1 << 16]; + let mut size = 0u64; + loop { + let read = file + .read(&mut buffer) + .map_err(|e| error("fingerprint", e))?; + if read == 0 { + break; + } + size += read as u64; + hasher.update(&buffer[..read]); + } + Ok((hasher.finalize().to_hex().to_string(), size)) +} + +fn no_journal_siblings(database: &Path) -> Result<(), String> { + for suffix in ["-wal", "-shm", "-journal"] { + if sibling(database, suffix).exists() { + return Err(error("seal", format!("{suffix} file remains"))); + } + } + Ok(()) +} + +/// Seals a private, fully built database that no reader can reach yet. +/// Must run before any pointer names it: the journal-mode switch needs +/// exclusive access, and the sync below opens a plain descriptor, which is +/// only safe while no SQLite connection in this process has the file open. +pub(crate) fn seal(database: &Path) -> Result { + { + let connection = + crate::db::file_identity::IdentityConnection::open(database, "views::donor::seal") + .map_err(|e| error("open", e))?; + connection + .query_row("PRAGMA wal_checkpoint(TRUNCATE)", [], |_| Ok(())) + .map_err(|e| error("checkpoint", e))?; + let mode: String = connection + .query_row("PRAGMA journal_mode=DELETE", [], |row| row.get(0)) + .map_err(|e| error("journal_mode", e))?; + if !mode.eq_ignore_ascii_case("delete") { + return Err(error("journal_mode", mode)); + } + } + { + let _files = crate::db::file_identity::filesystem_guard(); + if crate::db::file_identity::open_connections(database) != 0 { + return Err(error("seal", "the database is still open")); + } + std::fs::File::open(database) + .and_then(|file| file.sync_all()) + .map_err(|e| error("fsync", e))?; + } + no_journal_siblings(database)?; + let journal_mode = header_journal_mode(database)?; + if journal_mode != "delete" { + return Err(error("seal", format!("header names {journal_mode}"))); + } + let (fingerprint, size) = fingerprint(database)?; + let record = SealRecord { + format: SEAL_FORMAT, + fingerprint, + size, + journal_mode: journal_mode.into(), + }; + let path = seal_path(database); + let temporary = sibling(&path, &format!(".tmp.{}", super::base_refs::fresh_nonce())); + let bytes = serde_json::to_vec(&record).map_err(|e| error("record", e))?; + let written = (|| -> std::io::Result<()> { + let mut file = std::fs::OpenOptions::new() + .write(true) + .create_new(true) + .open(&temporary)?; + file.write_all(&bytes)?; + file.sync_all()?; + std::fs::rename(&temporary, &path) + })(); + if let Err(e) = written { + let _ = std::fs::remove_file(&temporary); + return Err(error("record", e)); + } + if let Some(parent) = path.parent() { + super::sync_directory(parent).map_err(|e| error("record", e))?; + } + Ok(record) +} + +/// Reads the seal record and checks it against the file's size, header and +/// siblings. The full fingerprint is checked by `verify_fingerprint`. +pub(crate) fn verify(database: &Path) -> Result { + let bytes = std::fs::read(seal_path(database)).map_err(|e| error("record", e))?; + let record: SealRecord = serde_json::from_slice(&bytes).map_err(|e| error("record", e))?; + if record.format != SEAL_FORMAT || record.journal_mode != "delete" { + return Err(error("record", "unsupported seal record")); + } + let size = std::fs::metadata(database) + .map_err(|e| error("metadata", e))? + .len(); + if size != record.size { + return Err(error("verify", "size differs from the seal record")); + } + no_journal_siblings(database)?; + if header_journal_mode(database)? != "delete" { + return Err(error("verify", "the header is not in rollback mode")); + } + Ok(record) +} + +/// Checks the file's bytes against the recorded fingerprint. +pub(crate) fn verify_fingerprint(database: &Path, record: &SealRecord) -> Result<(), String> { + let (fingerprint, size) = fingerprint(database)?; + if fingerprint != record.fingerprint || size != record.size { + return Err(error("verify", "bytes differ from the seal record")); + } + Ok(()) +} + +/// Clones a sealed donor into `destination`, which must not exist yet: a +/// clone name is fresh and never reused. On failure nothing is left behind. +pub(crate) fn clone_sealed(donor: &Path, destination: &Path) -> Result { + let record = verify(donor)?; + if destination.exists() { + return Err(error("clone", "the destination name was used before")); + } + let result = clone_file(donor, destination).and_then(|mechanism| { + let size = std::fs::metadata(destination) + .map_err(|e| error("clone", e))? + .len(); + if size != record.size { + return Err(error("clone", "the copy has the wrong size")); + } + if cfg!(test) { + verify_fingerprint(destination, &record)?; + } + std::fs::File::open(destination) + .and_then(|file| file.sync_all()) + .map_err(|e| error("fsync", e))?; + if let Some(parent) = destination.parent() { + super::sync_directory(parent).map_err(|e| error("fsync", e))?; + } + Ok(mechanism) + }); + if result.is_err() { + let _ = std::fs::remove_file(destination); + } + result +} + +#[cfg(test)] +thread_local! { + /// Forces the copy fallback so tests can cover it on clone-capable disks. + static FORCE_COPY: std::cell::Cell = const { std::cell::Cell::new(false) }; +} + +#[cfg(test)] +pub(crate) fn force_copy_for_test(force: bool) { + FORCE_COPY.with(|cell| cell.set(force)); +} + +fn forced_copy() -> bool { + #[cfg(test)] + { + FORCE_COPY.with(std::cell::Cell::get) + } + #[cfg(not(test))] + { + false + } +} + +fn copy(donor: &Path, destination: &Path, reason: String) -> Result { + let mut source = std::fs::File::open(donor).map_err(|e| error("copy", e))?; + let mut target = std::fs::OpenOptions::new() + .write(true) + .create_new(true) + .open(destination) + .map_err(|e| error("copy", e))?; + std::io::copy(&mut source, &mut target).map_err(|e| error("copy", e))?; + Ok(CloneMechanism::Copy(reason)) +} + +#[cfg(target_os = "macos")] +fn clone_file(donor: &Path, destination: &Path) -> Result { + use std::os::unix::ffi::OsStrExt; + if forced_copy() { + return copy(donor, destination, "forced by test".into()); + } + let source = + std::ffi::CString::new(donor.as_os_str().as_bytes()).map_err(|e| error("clone", e))?; + let target = std::ffi::CString::new(destination.as_os_str().as_bytes()) + .map_err(|e| error("clone", e))?; + // `CLONE_NOFOLLOW` from : refuse a symlinked source. + const CLONE_NOFOLLOW: u32 = 0x0001; + // Both C strings outlive the call. + if unsafe { libc::clonefile(source.as_ptr(), target.as_ptr(), CLONE_NOFOLLOW) } == 0 { + return Ok(CloneMechanism::Clonefile); + } + let reason = std::io::Error::last_os_error(); + copy(donor, destination, format!("clonefile: {reason}")) +} + +#[cfg(target_os = "linux")] +fn clone_file(donor: &Path, destination: &Path) -> Result { + use std::os::fd::AsRawFd; + if forced_copy() { + return copy(donor, destination, "forced by test".into()); + } + // A plain descriptor on the donor is safe: every opener of a sealed file + // uses `immutable=1`, so no SQLite connection holds a POSIX lock on it + // that closing this descriptor could release. + let source = std::fs::File::open(donor).map_err(|e| error("clone", e))?; + let target = std::fs::OpenOptions::new() + .write(true) + .create_new(true) + .open(destination) + .map_err(|e| error("clone", e))?; + // SAFETY: both descriptors are open for the duration of the call. + if unsafe { libc::ioctl(target.as_raw_fd(), libc::FICLONE, source.as_raw_fd()) } == 0 { + return Ok(CloneMechanism::Reflink); + } + let reason = std::io::Error::last_os_error(); + drop(target); + let _ = std::fs::remove_file(destination); + copy(donor, destination, format!("FICLONE: {reason}")) +} + +#[cfg(not(any(target_os = "macos", target_os = "linux")))] +fn clone_file(donor: &Path, destination: &Path) -> Result { + let reason = if forced_copy() { + "forced by test" + } else { + "no copy-on-write clone on this platform" + }; + copy(donor, destination, reason.into()) +} + +#[cfg(test)] +mod tests { + use super::*; + + fn database(dir: &Path) -> PathBuf { + let path = dir.join("derived-donor.sqlite"); + let connection = + crate::db::file_identity::IdentityConnection::open(&path, "views::donor::tests") + .unwrap(); + connection + .execute_batch( + "PRAGMA journal_mode=WAL; CREATE TABLE t(v); INSERT INTO t VALUES (1), (2);", + ) + .unwrap(); + drop(connection); + path + } + + #[test] + fn sealing_leaves_a_rollback_file_with_a_record_and_refuses_write_opens() { + let temp = tempfile::tempdir().unwrap(); + let path = database(temp.path()); + assert!(!is_sealed(&path)); + let record = seal(&path).unwrap(); + assert!(is_sealed(&path)); + assert_eq!(record.journal_mode, "delete"); + for suffix in ["-wal", "-shm", "-journal"] { + assert!(!sibling(&path, suffix).exists(), "{suffix} remains"); + } + assert_eq!(header_journal_mode(&path).unwrap(), "delete"); + assert_eq!(verify(&path).unwrap(), record); + verify_fingerprint(&path, &record).unwrap(); + let refused = + crate::db::file_identity::IdentityConnection::open(&path, "views::donor::tests") + .unwrap_err(); + assert!(refused.to_string().contains("sealed donor"), "{refused}"); + let refused = crate::db::file_identity::IdentityConnection::open_with_flags( + &path, + rusqlite::OpenFlags::SQLITE_OPEN_READ_WRITE, + "views::donor::tests", + ) + .unwrap_err(); + assert!(refused.to_string().contains("sealed donor"), "{refused}"); + // Read-only opens stay allowed, and an immutable reader sees the rows. + let uri = format!( + "file:{}?mode=ro&immutable=1", + path.to_string_lossy().replace('\\', "/") + ); + let reader = rusqlite::Connection::open_with_flags( + uri, + rusqlite::OpenFlags::SQLITE_OPEN_READ_ONLY | rusqlite::OpenFlags::SQLITE_OPEN_URI, + ) + .unwrap(); + let rows: i64 = reader + .query_row("SELECT COUNT(*) FROM t", [], |row| row.get(0)) + .unwrap(); + assert_eq!(rows, 2); + drop(reader); + assert!(!sibling(&path, "-wal").exists()); + } + + #[test] + fn clones_use_fresh_names_keep_the_donor_unchanged_and_refuse_tampering() { + let temp = tempfile::tempdir().unwrap(); + let path = database(temp.path()); + let record = seal(&path).unwrap(); + for (name, forced) in [("clone-a.sqlite", false), ("clone-b.sqlite", true)] { + force_copy_for_test(forced); + let destination = temp.path().join(name); + let mechanism = clone_sealed(&path, &destination).unwrap(); + force_copy_for_test(false); + if forced { + assert_eq!(mechanism, CloneMechanism::Copy("forced by test".into())); + } + verify_fingerprint(&destination, &record).unwrap(); + // The clone is an ordinary private database: writable, unsealed. + assert!(!is_sealed(&destination)); + let connection = crate::db::file_identity::IdentityConnection::open( + &destination, + "views::donor::tests", + ) + .unwrap(); + connection.execute("INSERT INTO t VALUES (3)", []).unwrap(); + drop(connection); + // A name is never reused, even after a successful clone. + assert!(clone_sealed(&path, &destination) + .unwrap_err() + .contains("used before")); + } + verify_fingerprint(&path, &record).unwrap(); + // A donor whose bytes no longer match its record is never cloned. + std::fs::OpenOptions::new() + .append(true) + .open(&path) + .unwrap() + .write_all(b"tamper") + .unwrap(); + let destination = temp.path().join("clone-c.sqlite"); + assert!(clone_sealed(&path, &destination).is_err()); + assert!(!destination.exists()); + // An unsealed (live) database is never cloned. + let live = temp.path().join("live"); + std::fs::create_dir_all(&live).unwrap(); + let live = database(&live); + assert!(clone_sealed(&live, &temp.path().join("clone-d.sqlite")).is_err()); + } +} diff --git a/crates/aft/src/views/generation.rs b/crates/aft/src/views/generation.rs index c955735362..f4fd8c6a9e 100644 --- a/crates/aft/src/views/generation.rs +++ b/crates/aft/src/views/generation.rs @@ -845,6 +845,7 @@ impl ViewStore { ".sqlite-wal", ".sqlite-shm", ".sqlite-journal", + ".sqlite-seal", ".ref", ] .iter() @@ -942,9 +943,15 @@ impl ViewStore { let generation = name .strip_prefix("derived-") .and_then(|s| { - [".sqlite", ".sqlite-wal", ".sqlite-shm", ".sqlite-journal"] - .iter() - .find_map(|suffix| s.strip_suffix(suffix)) + [ + ".sqlite", + ".sqlite-wal", + ".sqlite-shm", + ".sqlite-journal", + ".sqlite-seal", + ] + .iter() + .find_map(|suffix| s.strip_suffix(suffix)) }) .or_else(|| { name.strip_prefix("manifest-") @@ -1143,7 +1150,8 @@ impl ViewStore { continue; } let suffixes = if sqlite { - &["", "-wal", "-shm", "-journal"][..] + // A sealed donor's seal record goes with its database. + &["", "-wal", "-shm", "-journal", super::donor::SEAL_SUFFIX][..] } else { &[""][..] }; diff --git a/crates/aft/src/views/mod.rs b/crates/aft/src/views/mod.rs index 67e7433e8e..72825f1057 100644 --- a/crates/aft/src/views/mod.rs +++ b/crates/aft/src/views/mod.rs @@ -14,6 +14,7 @@ pub mod assembly; pub mod base_builder; pub mod base_refs; pub mod base_sweep; +pub(crate) mod donor; mod generation; #[cfg(test)] pub(crate) use generation::{ From ca2d5888fece3f334d7c6dada207465d26e4059e Mon Sep 17 00:00:00 2001 From: ualtinok <94532+ualtinok@users.noreply.github.com> Date: Tue, 27 Oct 2026 02:26:13 +0100 Subject: [PATCH 2/6] mason: bootstrap the call-graph plane through the base builder and clone sealed donors for workers Fresh v3 bootstrap now accepts planes that complete their own base entries (CompositePlane::completes_fresh_base / complete_v3_base). CallgraphBridge completes a fresh base from the bootstrap's strict-reconciliation attachments under the base build claim (taking over a dead builder's claim, refusing a live one), publishes a readiness successor whose derived graph is sealed as the base's donor, and the claim is released after the worker publishes. Semantic producers and other nonlexical planes stay refused. Base entries now carry resolution_input before the base identity is computed (base_builder::base_entry, used by build_base and the bootstrap's expected identity), and with a registered graph producer only call-graph inputs start pending, so a completed base projects. A v3 worker graph is a fresh clone of the base's sealed donor patched with the manifest diff (apply_manifest_diff_from_blob_reader); a cold build happens only for a reason from donor::ColdReason and is recorded. Fix: deleting a file a worker added (absent from the base) produced a tombstone for a non-inherited path, which ComposedGeneration::compose refuses; such paths are now dropped from the delta without a tombstone. Co-authored-by: alfonso-aft <289616620+alfonso-aft@users.noreply.github.com> (cherry picked from commit 27584ea4496e88dbacc4d889b98e8413f468c9b8) --- crates/aft/src/views/base_builder.rs | 44 +++- crates/aft/src/views/callgraph.rs | 14 +- crates/aft/src/views/first_load.rs | 288 +++++++++++++++++++++++- crates/aft/src/views/materialization.rs | 22 ++ 4 files changed, 356 insertions(+), 12 deletions(-) diff --git a/crates/aft/src/views/base_builder.rs b/crates/aft/src/views/base_builder.rs index 55276b2c29..006311675b 100644 --- a/crates/aft/src/views/base_builder.rs +++ b/crates/aft/src/views/base_builder.rs @@ -349,6 +349,12 @@ impl BuildClaim { pub fn scope(&self) -> &str { &self.scope } + pub fn storage(&self) -> &Path { + &self.storage + } + pub fn family(&self) -> &str { + &self.family + } pub fn attempt(&self) -> i64 { self.attempt } @@ -408,6 +414,12 @@ impl BuildClaim { "base derived graph checkpoint incomplete", ))); } + // Seal it as the base's donor before the pointer can name it: no + // reader can have it open yet, and every later opener reads it + // immutably. Worker checkouts clone it instead of rebuilding. + super::donor::seal(&database) + .map_err(|reason| BaseRefError::Io(std::io::Error::other(reason)))?; + super::donor::record(store.view_dir(), super::donor::DonorEvent::Sealed); let reference = store.view_dir().join(format!("derived-{generation}.ref")); let temporary = store .view_dir() @@ -558,6 +570,25 @@ pub fn read_stable(path: &Path) -> std::io::Result> { )) } +/// A base member entry. Whether a path is resolver configuration is part of +/// the membership digest, so it is fixed here, before the base identity is +/// computed: a later readiness successor cannot change it. +pub(crate) fn base_entry( + path: &super::RelPath, + content: crate::blob_store::v2::ContentHash, + size: u64, + planes: super::manifest_v2::EntryPlanes, +) -> super::manifest_v2::EntryV2 { + let mut entry = super::manifest_v2::EntryV2::regular(content, size, planes); + if let super::manifest_v2::EntryV2::Regular { + resolution_input, .. + } = &mut entry + { + *resolution_input = super::assembly::is_resolution_input(path.as_bytes()); + } + entry +} + /// Inputs of one base build: the membership the configured walker produced /// for `root`, and the effective identity inputs. pub struct BaseBuildInput<'a> { @@ -618,7 +649,7 @@ pub fn build_base( producers: super::manifest_v2::Producers { trigram: policy.fingerprint_hex(), semantic: semantic_producer.clone(), - callgraph: callgraph_producer, + callgraph: callgraph_producer.clone(), }, head_tree: None, ignore_fingerprint, @@ -640,17 +671,24 @@ pub fn build_base( std::ffi::OsString::from(String::from_utf8_lossy(path.as_bytes()).into_owned()); let source = read_stable(&root.join(relative))?; source_bytes += source.len() as u64; + // A registered graph producer completes only the members it needs a + // blob for; any other member stays without callgraph state, so a + // completed base projects. An unregistered producer keeps every + // member pending, as lexical-only bases always have. + let graph_registered = callgraph_producer != super::semantic_runtime::UNREGISTERED_PRODUCER; let planes = super::manifest_v2::EntryPlanes { trigram: Some(PlaneState::pending("lexical plane not built")), semantic: semantic_producer .as_ref() .filter(|_| super::semantic::applies_to(path)) .map(|_| PlaneState::pending("awaiting the base owner")), - callgraph: Some(PlaneState::pending("awaiting the base owner")), + callgraph: (!graph_registered || super::callgraph::applies_to(path)) + .then(|| PlaneState::pending("awaiting the base owner")), }; manifest.insert( path.clone(), - super::manifest_v2::EntryV2::regular( + base_entry( + path, crate::blob_store::v2::ContentHash::of(&source), source.len() as u64, planes, diff --git a/crates/aft/src/views/callgraph.rs b/crates/aft/src/views/callgraph.rs index 80d067008d..10b9fd08d2 100644 --- a/crates/aft/src/views/callgraph.rs +++ b/crates/aft/src/views/callgraph.rs @@ -48,6 +48,15 @@ pub(crate) fn source_language(path: &Path) -> Option { .filter(|language| *language != crate::parser::LangId::Markdown) } +/// Whether a member needs a callgraph blob: a source file in a callgraph +/// language, or resolver configuration such as `package.json`. +pub(crate) fn applies_to(path: &RelPath) -> bool { + super::assembly::is_resolution_input(path.as_bytes()) + || std::str::from_utf8(path.as_bytes()) + .ok() + .is_some_and(|p| source_language(Path::new(p)).is_some()) +} + /// Attach evidence computed from exactly the caller's already-read byte buffer. /// No disk access, blob put, publication or membership decision occurs here. pub fn attach( @@ -234,10 +243,7 @@ impl PlaneAdapter for CallgraphPlane { PRODUCER.into() } fn applies_to(&self, path: &RelPath) -> bool { - super::assembly::is_resolution_input(path.as_bytes()) - || std::str::from_utf8(path.as_bytes()) - .ok() - .is_some_and(|p| source_language(Path::new(p)).is_some()) + applies_to(path) } fn open_generation( &self, diff --git a/crates/aft/src/views/first_load.rs b/crates/aft/src/views/first_load.rs index b33b572a4e..fa7e1bf553 100644 --- a/crates/aft/src/views/first_load.rs +++ b/crates/aft/src/views/first_load.rs @@ -1249,6 +1249,25 @@ pub trait CompositePlane: Send + Sync { None } + /// Whether this plane can complete its entries of a freshly built v3 + /// base itself (see [`Self::complete_v3_base`]). Fresh bootstrap refuses a + /// nonlexical plane that cannot, rather than strand its base entries. + fn completes_fresh_base(&self) -> bool { + false + } + + /// Completes this plane's pending entries of the claimed base from the + /// attachments the bootstrap's strict reconciliation computed for the + /// same bytes, and publishes the result as a readiness successor. + fn complete_v3_base( + &self, + claim: &super::base_builder::BuildClaim, + observed: &BTreeMap, + ) -> Result<(), PlaneError> { + let _ = (claim, observed); + Ok(()) + } + /// Derive a payload key without extraction. Planes with deferred extraction /// may leave this absent and keep their own cheap attachment format. fn key( @@ -1710,8 +1729,8 @@ impl CheckoutDriver { if let Some(plane) = self .planes .iter() + .find(|plane| plane.plane() != FamilyPlane::Trigram && !plane.completes_fresh_base()) .map(|plane| plane.plane()) - .find(|plane| *plane != FamilyPlane::Trigram) { return Err(FreshBootstrapRefusal::NonLexicalPlane(plane)); } @@ -1720,7 +1739,13 @@ impl CheckoutDriver { FamilyPlane::Semantic, )); } - if self.producers.callgraph != super::semantic_runtime::UNREGISTERED_PRODUCER { + // A registered call-graph producer needs a registered plane that + // completes the base's graph entries. + if self.producers.callgraph != super::semantic_runtime::UNREGISTERED_PRODUCER + && !self.planes.iter().any(|plane| { + plane.plane() == FamilyPlane::Callgraph && plane.completes_fresh_base() + }) + { return Err(FreshBootstrapRefusal::NonLexicalPlane( FamilyPlane::Callgraph, )); @@ -1860,10 +1885,12 @@ impl CheckoutDriver { return Ok((snapshot, revision)); } let own = self.build_own_generation(access, &snapshot, DERIVED_CALLGRAPH_SEEDING); + // Every plane of a fresh bootstrap has completed its base entries by + // now (lexical in the build, others in `bootstrap_v3_base`). if self .planes .iter() - .all(|plane| plane.plane() == FamilyPlane::Trigram) + .all(|plane| plane.plane() == FamilyPlane::Trigram || plane.completes_fresh_base()) { if let Some(claim) = self .base_claim @@ -1929,8 +1956,8 @@ impl CheckoutDriver { if let super::snapshot::DiskState::Present { content, size } = state { manifest .insert( - path, - super::manifest_v2::EntryV2::regular(content, size, Default::default()), + path.clone(), + super::base_builder::base_entry(&path, content, size, Default::default()), ) .map_err(|e| SiblingLoader::error(e.to_string()))?; } @@ -1991,6 +2018,7 @@ impl CheckoutDriver { } } }; + let resolved = self.complete_fresh_base(&route, resolved)?; let header = resolved.header(); let generation = resolved.into_generation(&route.storage, &route.family); let manifest = super::manifest_v2::ManifestV2::new_v3( @@ -2005,6 +2033,68 @@ impl CheckoutDriver { Ok(Arc::new(composed.with_v3_route(seed_route))) } + /// Completes the nonlexical planes of a fresh base before a worker + /// publishes over it, so the worker's graph composes a ready base. The + /// base's build claim is the completion lock: this driver uses the claim + /// it took building the base, or takes over a dead builder's claim. A live + /// foreign builder still owns the completion, so the bind refuses. + fn complete_fresh_base( + &self, + route: &super::snapshot::V3Route, + base: super::base_refs::AuthenticatedBase, + ) -> Result { + let completing: Vec<_> = self + .planes + .iter() + .filter(|plane| plane.completes_fresh_base()) + .filter(|plane| { + base.manifest().entries().any(|(_, entry)| { + entry + .plane_state(plane.plane()) + .is_some_and(super::readiness::PlaneState::is_pending) + }) + }) + .cloned() + .collect(); + if completing.is_empty() { + return Ok(base); + } + let scope = base.scope().to_owned(); + drop(base); + let mut held = self + .base_claim + .lock() + .unwrap_or_else(std::sync::PoisonError::into_inner); + if held.as_ref().is_none_or(|claim| claim.scope() != scope) { + match super::base_builder::claim_base(&route.storage, &route.family, &scope, None) + .map_err(|e| SiblingLoader::error(e.to_string()))? + { + super::base_builder::ClaimOutcome::Claimed(claim) => { + if let Some(previous) = held.replace(claim) { + let _ = previous.release(); + } + } + outcome => { + return Err(SiblingLoader::error(format!( + "v3 base plane completion is owned elsewhere: {outcome:?}" + ))) + } + } + } + let claim = held.as_ref().expect("claim held above"); + let observed = self + .observed + .lock() + .unwrap_or_else(std::sync::PoisonError::into_inner) + .entries + .clone(); + for plane in completing { + plane.complete_v3_base(claim, &observed)?; + } + super::base_refs::resolve_base(&route.storage, &route.family, &scope) + .map_err(|e| SiblingLoader::error(e.to_string())) + } + #[allow(clippy::too_many_arguments)] pub fn new_v3( storage: &std::path::Path, @@ -3094,6 +3184,18 @@ impl CheckoutDriver { &super::readiness::FillMap::default(), ) .map_err(|error| SiblingLoader::error(error.to_string()))?; + // A deleted path the base never had (a file this worker added and then + // removed) leaves the delta without an entry or a tombstone: a + // tombstone may only name an inherited base entry. + for (path, entry) in snapshot.live_entries() { + if entry.disk == super::snapshot::DiskState::Absent + && generation + .base() + .is_some_and(|base| base.entry(path).is_none()) + { + delta.inherit(path.clone(), None); + } + } delta .set_base_header(base_header.clone()) .map_err(|error| SiblingLoader::error(error.to_string()))?; @@ -4464,9 +4566,146 @@ mod composite_tests { #[derive(Default)] pub struct CallgraphBridge { pub adapter: Arc, + /// Source files this bridge parsed for call-graph evidence. Unchanged + /// files whose blob already exists reuse it and are not counted. + extractions: std::sync::atomic::AtomicUsize, +} + +impl CallgraphBridge { + pub fn extraction_count(&self) -> usize { + self.extractions.load(std::sync::atomic::Ordering::Relaxed) + } +} + +/// The sealed donor of `base`'s derived graph, or why there is none. +fn sealed_donor(base: &ComposedGeneration) -> Result { + use super::donor::ColdReason; + let route = base.v3_route().ok_or(ColdReason::NoSealedDonor)?; + let donor = super::resolve_derived_path(&route.member_dir(), base.name()) + .map_err(|_| ColdReason::NoSealedDonor)?; + if !super::donor::is_sealed(&donor) { + return Err(ColdReason::NoSealedDonor); + } + super::donor::verify(&donor).map_err(ColdReason::SealInvalid)?; + Ok(donor) +} + +fn remove_private_database(database: &std::path::Path) { + for suffix in ["", "-wal", "-shm", "-journal"] { + let mut name = database.as_os_str().to_owned(); + name.push(suffix); + let _ = std::fs::remove_file(std::path::PathBuf::from(name)); + } } impl CompositePlane for CallgraphBridge { + fn completes_fresh_base(&self) -> bool { + true + } + + fn complete_v3_base( + &self, + claim: &super::base_builder::BuildClaim, + observed: &BTreeMap, + ) -> Result<(), PlaneError> { + let error = |reason: String| PlaneError { + plane: Some(FamilyPlane::Callgraph), + reason, + }; + let base = super::base_refs::resolve_base(claim.storage(), claim.family(), claim.scope()) + .map_err(|e| error(e.to_string()))?; + let route = super::base_refs::route(claim.storage(), claim.family(), claim.scope()); + let mut keys = Vec::new(); + let mut payloads = Vec::new(); + for (path, entry) in base.manifest().entries() { + if !entry + .plane_state(FamilyPlane::Callgraph) + .is_some_and(super::readiness::PlaneState::is_pending) + { + continue; + } + // The base was read independently of the reconciliation; only + // evidence for the very bytes the base names may complete it. + let observation = observed.get(path).filter(|live| { + matches!( + (live.disk, entry), + ( + super::snapshot::DiskState::Present { content, size }, + super::manifest_v2::EntryV2::Regular { content: c, size: s, .. }, + ) if content == *c && size == *s + ) + }); + let attachment = observation + .and_then(|live| live.attachments.get(&FamilyPlane::Callgraph)) + .ok_or_else(|| { + error(format!( + "{path:?}: base member changed before its graph was completed" + )) + })?; + if let Some(cached) = attachment.downcast_ref::() { + keys.push((path.clone(), cached.0)); + } else if let Some(attachment) = + attachment.downcast_ref::() + { + keys.push((path.clone(), attachment.key)); + payloads.push(( + attachment.key, + attachment + .blob + .to_bytes() + .map_err(|e| error(e.to_string()))?, + )); + } else { + return Err(error(format!("{path:?}: callgraph attachment missing"))); + } + } + drop(base); + let mut live = + crate::pins::LivePin::create_v3(claim.storage(), claim.family(), claim.scope()) + .map_err(|e| error(e.to_string()))?; + live.protect(&keys.iter().map(|(_, key)| *key).collect::>()) + .map_err(|e| error(e.to_string()))?; + if !payloads.is_empty() { + let request = payloads + .iter() + .map(|(_, bytes)| bytes.len() as u64) + .sum::() + .saturating_mul(2) + .saturating_add(1 << 20); + let schema = FamilyPlane::Callgraph.payload_schema(); + crate::blob_store::v3::publish(claim.storage(), claim.family(), request, None, |p| { + for (key, bytes) in &payloads { + p.put(key, bytes, schema)?; + } + Ok(()) + }) + .map_err(|e| error(e.to_string()))?; + } + for (_, key) in &keys { + if !v3_contains(&route, key) { + return Err(error( + "base callgraph row disappeared before publication".into(), + )); + } + } + super::base_builder::publish_successor( + claim, + |manifest| { + for (path, key) in &keys { + if let Some(super::manifest_v2::EntryV2::Regular { planes, .. }) = + manifest.get_mut(path) + { + planes.callgraph = Some(super::readiness::PlaneState::ready(key)); + } + } + }, + None, + ) + .map_err(|e| error(e.to_string()))?; + drop(live); + Ok(()) + } + fn plane(&self) -> FamilyPlane { FamilyPlane::Callgraph } @@ -4522,6 +4761,8 @@ impl CompositePlane for CallgraphBridge { .to_lowercase() }; let mut entry = LiveEntry::new(super::snapshot::DiskState::of_bytes(bytes), 0); + self.extractions + .fetch_add(1, std::sync::atomic::Ordering::Relaxed); let attachment = super::callgraph::attach(&mut entry, bytes, &language)?; Ok(Arc::new(attachment)) } @@ -4765,6 +5006,43 @@ impl CompositePlane for CallgraphBridge { super::ViewStore::open_dir(dir.clone()).map_err(|e| error(e.to_string()))?; let database = super::resolve_derived_path(&dir, staging).map_err(|e| error(e.to_string()))?; + // Prefer a private clone of the base's sealed donor, patched with the + // manifest diff, over rebuilding the whole graph. `staging` is a fresh + // name, so the clone never reuses another file's name. + match sealed_donor(base) { + Ok(donor) => { + let base_projected = super::callgraph::project_manifest(base)?; + let projected = super::callgraph::project_manifest(&composed)?; + let mechanism = super::donor::clone_sealed(&donor, &database).map_err(error)?; + match super::materialization::apply_manifest_diff_from_blob_reader( + &database, + &base_projected, + &projected, + &reader, + ) { + Ok(_) => { + super::donor::record(&dir, super::donor::DonorEvent::Cloned(mechanism)); + return Ok(()); + } + Err(e) if super::materialization::is_base_fingerprint_mismatch(&e) => { + remove_private_database(&database); + super::donor::record( + &dir, + super::donor::DonorEvent::Cold( + super::donor::ColdReason::BaseFingerprintMismatch, + ), + ); + } + Err(e) => { + remove_private_database(&database); + return Err(error(e.to_string())); + } + } + } + Err(reason) => { + super::donor::record(&dir, super::donor::DonorEvent::Cold(reason)); + } + } super::callgraph::materialize(&database, &composed, &reader) } fn finish_generation_v3( diff --git a/crates/aft/src/views/materialization.rs b/crates/aft/src/views/materialization.rs index 5beb91763d..bc5af83c3d 100644 --- a/crates/aft/src/views/materialization.rs +++ b/crates/aft/src/views/materialization.rs @@ -276,6 +276,28 @@ pub fn materialize_from_blob_reader( materialize_reading(database_path, manifest, None, reader, None).map(|_| ()) } +/// [`apply_manifest_diff`] over an immutable blob reader, for a private clone +/// of a sealed donor built from `base_manifest`. Without the legacy blob +/// table there are no per-fact resolution inputs, so callers that may read a +/// changed fact are re-resolved; rows are still rewritten only where they +/// change. A clone whose recorded fingerprint is not `base_manifest`'s is +/// refused unchanged (see [`is_base_fingerprint_mismatch`]). +pub(crate) fn apply_manifest_diff_from_blob_reader( + database_path: &Path, + base_manifest: &crate::views::Manifest, + new_manifest: &crate::views::Manifest, + reader: &impl join::ManifestBlobReader, +) -> Result { + materialize_reading( + database_path, + new_manifest, + Some(base_manifest), + reader, + None, + ) + .map(|(stats, _)| stats) +} + fn materialize_reading( database_path: &Path, manifest: &crate::views::Manifest, From ca5ed51b11f5b8df1b7fb398e4fb1b6286ff0e46 Mon Sep 17 00:00:00 2001 From: ualtinok <94532+ualtinok@users.noreply.github.com> Date: Tue, 27 Oct 2026 02:46:01 +0100 Subject: [PATCH 3/6] mason: route views-on call-graph reads through the checkout's v3 view With views on (and the call graph enabled, a heavy-work root and an available cache), callgraph_store_for_ops and its no-wait variant acquire a reader from a CallgraphView: a v3-only CheckoutDriver with the lexical and call-graph planes under its own member scope (.callgraph). Each acquisition verified-binds, publishes pending edits as a new worker generation before its single install, refuses uncertain snapshots (pending intent, unhealthy watcher, unpublished changes) and never falls back to the legacy store or a v1 view. The reader is pinned once per pinned request. Status, zoom, inspect's graph-backed categories and the borrowed-graph disclosure follow the route; warming never binds on the transport loop. An explicitly installed CheckoutQueryRuntime keeps precedence. Views off is unchanged. Fix (views-on): view-materialized graphs store the call-graph blob key as a file's content hash, so source-line freshness checks marked every view answer stale (approximate, no call_expression). The adapter now also accepts the current bytes' derived blob key. Parity tests compare callers/impact/call_tree/trace_to output with a cold legacy store over the same files on a plain clone and a linked worktree, across edits, a deletion, an overflow and a restart; a refused acquisition never answers from a resident legacy store. Co-authored-by: alfonso-aft <289616620+alfonso-aft@users.noreply.github.com> (cherry picked from commit 57a9c36ab4d1a796f1146c43c1af6c7c23607ac8) --- crates/aft/src/commands/callgraph_borrowed.rs | 7 +- .../src/commands/callgraph_store_adapter.rs | 22 +- crates/aft/src/commands/configure.rs | 243 +++++++++ crates/aft/src/commands/inspect.rs | 10 +- crates/aft/src/context.rs | 135 +++++ crates/aft/src/feature_status.rs | 8 + crates/aft/src/views/assembly.rs | 2 +- crates/aft/src/views/callgraph.rs | 464 ++++++++++++++++++ 8 files changed, 884 insertions(+), 7 deletions(-) diff --git a/crates/aft/src/commands/callgraph_borrowed.rs b/crates/aft/src/commands/callgraph_borrowed.rs index d105281e97..5304bf594a 100644 --- a/crates/aft/src/commands/callgraph_borrowed.rs +++ b/crates/aft/src/commands/callgraph_borrowed.rs @@ -276,9 +276,12 @@ pub fn disclose_borrowed_answer( /// Describe the borrowed callgraph this context answers from, or `None` when /// callgraph answers come from this checkout's own data: the owner checkout, -/// a per-checkout query runtime, or a pinned per-HEAD view. +/// a per-checkout query runtime, a checkout's v3 view or a pinned per-HEAD view. pub fn borrowed_callgraph(ctx: &AppContext) -> Option { - if !ctx.shared_artifacts_read_only() || ctx.checkout_query_runtime_active() { + if !ctx.shared_artifacts_read_only() + || ctx.checkout_query_runtime_active() + || ctx.callgraph_views_enabled() + { return None; } if ctx.config().views.enabled diff --git a/crates/aft/src/commands/callgraph_store_adapter.rs b/crates/aft/src/commands/callgraph_store_adapter.rs index fc66058aac..1f75882cdf 100644 --- a/crates/aft/src/commands/callgraph_store_adapter.rs +++ b/crates/aft/src/commands/callgraph_store_adapter.rs @@ -3058,10 +3058,30 @@ fn content_matches_store( }; Ok( stored == crate::cache_freshness::zero_hash().to_hex().as_str() - || stored == current.to_hex().as_str(), + || stored == current.to_hex().as_str() + || stored == view_blob_key(file, current).unwrap_or_default(), ) } +/// A view-materialized graph records each file's call-graph blob key rather +/// than its raw content hash. The key is derived from the content hash, the +/// file's language and the extractor version, so the current bytes match +/// exactly when they derive the stored key. +fn view_blob_key(file: &str, current: blake3::Hash) -> Option { + let language = if crate::views::assembly::is_resolution_input(file.as_bytes()) { + "config".to_owned() + } else { + format!("{:?}", crate::parser::detect_language(Path::new(file))?).to_lowercase() + }; + let key = crate::blob_store::CallgraphKey::from_source_digest( + *current.as_bytes(), + language, + crate::views::callgraph::PRODUCER, + ) + .full_key(); + Some(crate::blob_store::v2::FamilyKey::from(&key).to_hex()) +} + fn file_matches_store( store: &impl CallGraphRead, file: &str, diff --git a/crates/aft/src/commands/configure.rs b/crates/aft/src/commands/configure.rs index 232c9bf474..c285ee410d 100644 --- a/crates/aft/src/commands/configure.rs +++ b/crates/aft/src/commands/configure.rs @@ -15716,6 +15716,249 @@ mod tests { lexical_views_assert_public_parity(&ctx, &root); } + fn configure_callgraph_views(root: &Path, storage: &Path, views: bool) -> RawRequest { + configure_request_with_params(json!({ + "project_root": root, "harness": "opencode", "storage_dir": storage, + "config": [user_tier(json!({ + "views": { "enabled": views }, + "indexes": { "trigram": true, "semantic": false, "callgraph": true } + }))] + })) + } + + /// The navigation answers whose output the views route must reproduce. + fn callgraph_views_answers(ctx: &AppContext, root: &Path) -> Vec<(String, Response)> { + let requests = [ + ("callers", "a.ts", "target", 2), + ("impact", "a.ts", "target", 3), + ("call_tree", "c.ts", "top", 3), + ("trace_to", "a.ts", "target", 5), + ("callers", "b.ts", "caller", 1), + ]; + requests + .into_iter() + .map(|(command, file, symbol, depth)| { + let request = RawRequest { + id: format!("callgraph-views-{command}"), + command: command.into(), + lsp_hints: None, + session_id: None, + params: json!({"file": root.join(file), "symbol": symbol, "depth": depth}), + }; + let response = match command { + "callers" => crate::commands::callers::handle_callers(&request, ctx), + "impact" => crate::commands::impact::handle_impact(&request, ctx), + "call_tree" => crate::commands::call_tree::handle_call_tree(&request, ctx), + _ => crate::commands::trace_to::handle_trace_to(&request, ctx), + }; + (format!("{command} {file} {symbol}"), response) + }) + .collect() + } + + /// Compares the views route with the legacy store over the same files, + /// built cold by a views-off context in separate storage. The legacy + /// store serves no linked worktree, so it reads a plain copy of the + /// checkout's files, and both sides render their root as ``. + fn callgraph_views_assert_parity(ctx: &AppContext, root: &Path) { + let storage = tempfile::tempdir().unwrap(); + let copy = tempfile::tempdir().unwrap(); + let copy_root = std::fs::canonicalize(copy.path()).unwrap(); + for entry in std::fs::read_dir(root).unwrap() { + let entry = entry.unwrap(); + if entry.file_type().unwrap().is_file() && entry.file_name() != ".git" { + std::fs::copy(entry.path(), copy_root.join(entry.file_name())).unwrap(); + } + } + let oracle = test_context(); + assert!( + handle_configure_for_test( + &configure_callgraph_views(©_root, storage.path(), false), + &oracle + ) + .success + ); + { + let _wait = crate::context::override_callgraph_build_wait_ms_for_test(&oracle, 60_000); + let legacy = lazy_ready(oracle.callgraph_store_for_ops()); + assert_eq!(legacy.reader_kind(), "legacy"); + } + let normalized = |data: &serde_json::Value, root: &Path| { + data.to_string() + .replace(&root.display().to_string(), "") + }; + let before = ctx.callgraph_view_route_count(); + let expected = callgraph_views_answers(&oracle, ©_root); + let actual = callgraph_views_answers(ctx, root); + for ((label, expected), (_, actual)) in expected.iter().zip(&actual) { + assert!(expected.success, "legacy {label}: {:?}", expected.data); + assert!(actual.success, "view {label}: {:?}", actual.data); + assert_eq!( + normalized(&actual.data, root), + normalized(&expected.data, ©_root), + "{label}" + ); + } + assert!(ctx.callgraph_view_route_count() > before); + assert_eq!(oracle.callgraph_view_route_count(), 0); + assert!(ctx.callgraph_store().read().unwrap().is_none()); + } + + fn callgraph_views_checkout(kind: OlderCheckoutKind) { + let temp = tempfile::tempdir().unwrap(); + let live = temp.path().join("live"); + init_git_fixture(&live); + for (name, text) in [ + ("a.ts", "export function target() { return 1; }\n"), + ( + "b.ts", + "import { target } from './a';\nexport function caller() { return target(); }\n", + ), + ( + "c.ts", + "import { caller } from './b';\nexport function top() { return caller(); }\n", + ), + ] { + std::fs::write(live.join(name), text).unwrap(); + } + git_commit_all(&live, "graph"); + let scratch = temp.path().join("scratch"); + match kind { + OlderCheckoutKind::LinkedWorktree => git_ok( + &live, + &[ + "worktree", + "add", + "--quiet", + "--detach", + scratch.to_str().unwrap(), + "HEAD", + ], + ), + _ => git_ok(temp.path(), &["clone", "--quiet", "live", "scratch"]), + } + let root = std::fs::canonicalize(&scratch).unwrap(); + let storage = temp.path().join("storage"); + let ctx = test_context(); + assert!( + handle_configure_for_test(&configure_callgraph_views(&root, &storage, true), &ctx) + .success + ); + callgraph_views_assert_parity(&ctx, &root); + + // Edits: a new caller, a changed callee file and a deletion. + std::fs::write( + root.join("d.ts"), + "import { target } from './a';\nexport function second() { return target(); }\n", + ) + .unwrap(); + ctx.record_checkout_watcher_change(&root.join("d.ts")); + callgraph_views_assert_parity(&ctx, &root); + std::fs::write( + root.join("b.ts"), + "import { target } from './a';\nexport function caller() { return target() + target(); }\n", + ) + .unwrap(); + ctx.record_checkout_watcher_change(&root.join("b.ts")); + callgraph_views_assert_parity(&ctx, &root); + std::fs::remove_file(root.join("d.ts")).unwrap(); + ctx.record_checkout_watcher_overflow(); + callgraph_views_assert_parity(&ctx, &root); + assert!(!storage.join("blobs/v2").exists()); + + // Restart: a new context reopens the published view. + ctx.mark_subc_unbound(); + ctx.cancel_unbound_artifact_work(); + drop(ctx); + let restarted = test_context(); + assert!( + handle_configure_for_test( + &configure_callgraph_views(&root, &storage, true), + &restarted + ) + .success + ); + callgraph_views_assert_parity(&restarted, &root); + } + + #[test] + fn callgraph_views_public_linked_worktree_parity_across_edits_and_restart() { + callgraph_views_checkout(OlderCheckoutKind::LinkedWorktree); + } + + #[test] + fn callgraph_views_public_plain_clone_parity_across_edits_and_restart() { + callgraph_views_checkout(OlderCheckoutKind::PlainClone); + } + + #[test] + fn callgraph_views_acquisition_failure_never_uses_the_legacy_store() { + struct Refused(std::sync::atomic::AtomicUsize); + impl crate::views::callgraph::CallgraphReaderSource for Refused { + fn acquire( + &self, + ) -> Result, String> { + self.0.fetch_add(1, Ordering::SeqCst); + Err("callgraph source deliberately refused".into()) + } + fn record_change(&self, _: &Path) {} + } + let temp = tempfile::tempdir().unwrap(); + let root = temp.path().join("project"); + init_git_fixture(&root); + std::fs::write( + root.join("a.ts"), + "export function target() { return 1; }\nexport function caller() { return target(); }\n", + ) + .unwrap(); + let root = std::fs::canonicalize(root).unwrap(); + let storage = temp.path().join("storage"); + // A legacy store built for this very root must not answer either. + let legacy = test_context(); + assert!( + handle_configure_for_test(&configure_callgraph_views(&root, &storage, false), &legacy) + .success + ); + let legacy_store = { + let _wait = crate::context::override_callgraph_build_wait_ms_for_test(&legacy, 60_000); + lazy_ready(legacy.callgraph_store_for_ops()) + }; + let ctx = test_context(); + assert!( + handle_configure_for_test(&configure_callgraph_views(&root, &storage, true), &ctx) + .success + ); + *ctx.callgraph_store().write().unwrap() = Some(legacy_store); + let refused = Arc::new(Refused(Default::default())); + ctx.install_callgraph_source_for_test(refused.clone()); + let _pin = ctx.pin_config(); + let answers = callgraph_views_answers(&ctx, &root); + for (label, answer) in &answers { + assert!(!answer.success, "{label}: {:?}", answer.data); + assert!( + answer.data.to_string().contains("deliberately refused"), + "{label}: {:?}", + answer.data + ); + } + // One pinned request acquires once and keeps its refusal. + let inspect_access = ctx.callgraph_store_for_query_without_wait(); + assert!(matches!( + inspect_access, + CallgraphStoreAccess::Error(ref error) if error.to_string().contains("deliberately refused") + )); + assert_eq!(refused.0.load(Ordering::SeqCst), 1); + assert_eq!(ctx.callgraph_view_route_count(), 0); + // Views off keeps the legacy store exactly as before. + assert_eq!(legacy.callgraph_view_route_count(), 0); + let legacy_answers = callgraph_views_answers(&legacy, &root); + assert!( + legacy_answers[0].1.success, + "{:?}", + legacy_answers[0].1.data + ); + } + fn configure_search_with_max_file_size( root: &std::path::Path, storage: &std::path::Path, diff --git a/crates/aft/src/commands/inspect.rs b/crates/aft/src/commands/inspect.rs index e7c17395d5..7faf0b67a9 100644 --- a/crates/aft/src/commands/inspect.rs +++ b/crates/aft/src/commands/inspect.rs @@ -708,12 +708,16 @@ fn handle_inspect_payload( .iter() .any(|category| category.is_tier2() && snapshot.config.inspect.category_enabled(*category)); start_on_demand_callgraph_for_inspect(ctx, &snapshot.config); - let routed_store = if tier2_enabled && ctx.checkout_query_runtime_active() { + // With views on, the graph-backed categories read the checkout's v3 view, + // pinned once for this request; an unavailable view is reported, never + // replaced by the legacy scanner's store. + let v3_routed = tier2_enabled && ctx.callgraph_views_enabled(); + let routed_store = if tier2_enabled && (ctx.checkout_query_runtime_active() || v3_routed) { ctx.callgraph_store_for_ops() } else { crate::context::CallgraphStoreAccess::Unavailable }; - let checkout_routed = ctx.checkout_query_outcome().is_some(); + let checkout_routed = ctx.checkout_query_outcome().is_some() || v3_routed; let checkout_store = if checkout_routed { match routed_store { crate::context::CallgraphStoreAccess::Ready(store) => Some(store), @@ -761,7 +765,7 @@ fn handle_inspect_payload( } else { None }; - let use_checkout_view = needs_ephemeral_view + let use_checkout_view = (needs_ephemeral_view || v3_routed) && snapshot.config.views.enabled && (checkout_routed || checkout_store.is_some()); let checkout_view_unavailability = (needs_ephemeral_view diff --git a/crates/aft/src/context.rs b/crates/aft/src/context.rs index a8c9d47d79..08c0ab4a0e 100644 --- a/crates/aft/src/context.rs +++ b/crates/aft/src/context.rs @@ -296,6 +296,10 @@ thread_local! { const { std::cell::RefCell::new(Vec::new()) }; static LEXICAL_QUERY_SNAPSHOTS: std::cell::RefCell)>> = const { std::cell::RefCell::new(Vec::new()) }; + /// The call-graph reader each pinned request acquired from its checkout + /// view, so every graph read in one request answers from one generation. + static CALLGRAPH_QUERY_READERS: std::cell::RefCell, String>)>> = + const { std::cell::RefCell::new(Vec::new()) }; } #[cfg(test)] @@ -388,6 +392,8 @@ impl Drop for ConfigPinGuard { if let Some(key) = self.key { LEXICAL_QUERY_SNAPSHOTS .with(|snapshots| snapshots.borrow_mut().retain(|(id, _)| *id != key)); + CALLGRAPH_QUERY_READERS + .with(|readers| readers.borrow_mut().retain(|(id, _)| *id != key)); CHECKOUT_QUERY_OUTCOMES.with(|outcomes| { outcomes .borrow_mut() @@ -4555,6 +4561,8 @@ pub struct AppContext { checkout_driver: crate::views::first_load::InstalledDriver, lexical_view: parking_lot::Mutex>>, lexical_routes: [AtomicU64; 3], + callgraph_view: parking_lot::Mutex>>, + callgraph_view_routes: AtomicU64, /// The views-on semantic lane of this root (see `views::semantic_runtime`). checkout_semantic: Arc, /// Wall-clock milliseconds at which the completion drain should next @@ -5406,6 +5414,8 @@ impl AppContext { checkout_driver: crate::views::first_load::InstalledDriver::default(), lexical_view: parking_lot::Mutex::new(None), lexical_routes: Default::default(), + callgraph_view: parking_lot::Mutex::new(None), + callgraph_view_routes: AtomicU64::new(0), checkout_semantic: Arc::default(), view_disk_limits_next_ms, checkout_query_runtime: RwLock::new(None), @@ -8996,6 +9006,7 @@ impl AppContext { self.arm_view_disk_limits(next.views.enabled); if !next.views.enabled || project_root_changed { self.lexical_view.lock().take(); + self.callgraph_view.lock().take(); } if project_root_changed { self.path_restriction_root_memo.lock().take(); @@ -10534,6 +10545,9 @@ impl AppContext { if let Some(view) = self.lexical_view.lock().as_ref() { view.record_change(path); } + if let Some(view) = self.callgraph_view.lock().as_ref() { + view.record_change(path); + } if let Some(driver) = self.checkout_driver.get() { driver.record_absolute_change(path); } @@ -10547,6 +10561,9 @@ impl AppContext { if let Some(view) = self.lexical_view.lock().as_ref() { view.record_overflow(); } + if let Some(view) = self.callgraph_view.lock().as_ref() { + view.record_overflow(); + } if let Some(driver) = self.checkout_driver.get() { driver.record_watcher_overflow(); } @@ -10558,6 +10575,9 @@ impl AppContext { if let Some(view) = self.lexical_view.lock().as_ref() { view.record_cookie(path); } + if let Some(view) = self.callgraph_view.lock().as_ref() { + view.record_cookie(path); + } if let Some(driver) = self.checkout_driver.get() { driver.record_watcher_cookie(path); } @@ -10930,6 +10950,23 @@ impl AppContext { Err(_) => CallgraphStoreAccess::Building, }; } + if self.callgraph_views_enabled() { + // Warming runs on the transport loop and must not bind a view + // there; only a use of the graph acquires it. + if !demand { + return CallgraphStoreAccess::Building; + } + return match self.callgraph_view_reader() { + Some(Ok(store)) => CallgraphStoreAccess::Ready(store), + Some(Err(reason)) if reason.contains(crate::views::read::CALLGRAPH_DISABLED) => { + CallgraphStoreAccess::Error(CallGraphStoreError::Unavailable(reason)) + } + Some(Err(reason)) => CallgraphStoreAccess::Error(CallGraphStoreError::Unavailable( + format!("callgraph view unavailable: {reason}"), + )), + None => CallgraphStoreAccess::Unavailable, + }; + } if self.config().views.enabled && self.config().indexes.callgraph { let on_demand = self.callgraph_build_on_demand(); // An on-demand root that cannot yet compare its files with the @@ -12061,6 +12098,104 @@ impl AppContext { std::array::from_fn(|index| self.lexical_routes[index].load(Ordering::Relaxed)) } + /// With views enabled, call-graph reads (navigation, zoom, inspect) use + /// the checkout's v3 view rather than the legacy store or a v1 view. An + /// explicitly installed checkout query runtime keeps precedence. + pub(crate) fn callgraph_views_enabled(&self) -> bool { + let config = self.config(); + config.views.enabled + && config.indexes.callgraph + && config.project_root.is_some() + && self.heavy_root_work_allowed() + && self.cache_unavailable_reason().is_none() + && !self.checkout_query_runtime_active() + } + + pub(crate) fn callgraph_view_is_ready(&self) -> bool { + self.callgraph_view + .lock() + .as_ref() + .is_some_and(|view| view.is_ready()) + } + + #[cfg(test)] + pub(crate) fn install_callgraph_source_for_test( + &self, + source: Arc, + ) { + let root = self.canonical_cache_root(); + let family = self.memoized_artifact_cache_key(&root); + *self.callgraph_view.lock() = Some(Arc::new( + crate::views::callgraph::CallgraphView::with_source( + root, + self.storage_paths(), + family, + crate::blob_store::v2::TrigramPolicy { + max_file_size: self.config().search_index_max_file_size, + }, + source, + ), + )); + } + + /// The call-graph reader of this checkout's view, acquired once per + /// pinned request. `None` means views do not route call-graph reads; an + /// acquisition error is final and never answered from another store. + pub(crate) fn callgraph_view_reader( + &self, + ) -> Option, String>> { + if !self.callgraph_views_enabled() { + return None; + } + if let Some(reader) = CALLGRAPH_QUERY_READERS.with(|readers| { + readers + .borrow() + .iter() + .find(|(id, _)| *id == self.config_pin_key()) + .map(|(_, reader)| reader.clone()) + }) { + return Some(reader); + } + let config = self.config(); + let root = self.canonical_cache_root(); + let storage = self.storage_paths(); + let policy = crate::blob_store::v2::TrigramPolicy { + max_file_size: config.search_index_max_file_size, + }; + let family = self.memoized_artifact_cache_key(&root); + let view = { + let mut slot = self.callgraph_view.lock(); + if !slot + .as_ref() + .is_some_and(|view| view.matches(&root, &storage, &family, policy)) + { + match crate::views::callgraph::CallgraphView::new(root, storage, family, policy) { + Ok(view) => *slot = Some(Arc::new(view)), + Err(error) => return Some(Err(error)), + } + } + slot.as_ref().unwrap().clone() + }; + self.record_checkout_pending_watcher_gap(); + let result = view.reader(); + if pinned_config_for(self.config_pin_key()).is_some() { + CALLGRAPH_QUERY_READERS.with(|readers| { + readers + .borrow_mut() + .push((self.config_pin_key(), result.clone())) + }); + } + if result.is_ok() { + self.callgraph_view_routes.fetch_add(1, Ordering::Relaxed); + } + Some(result) + } + + /// Executed v3 call-graph acquisitions (one per pinned request). + pub fn callgraph_view_route_count(&self) -> u64 { + self.callgraph_view_routes.load(Ordering::Relaxed) + } + pub(crate) fn search_exact_memo( &self, ) -> Arc { diff --git a/crates/aft/src/feature_status.rs b/crates/aft/src/feature_status.rs index 0a2f255dc0..f40b34dab2 100644 --- a/crates/aft/src/feature_status.rs +++ b/crates/aft/src/feature_status.rs @@ -403,6 +403,13 @@ pub const fn local_semantic_platform_supported() -> bool { } fn observe_callgraph(ctx: &AppContext) -> IndexObservation { + if ctx.callgraph_views_enabled() { + return if ctx.callgraph_view_is_ready() { + IndexObservation::ready() + } else { + IndexObservation::building() + }; + } let resident = match ctx.callgraph_store().try_read() { Ok(guard) => guard.is_some(), Err(std::sync::TryLockError::Poisoned(poisoned)) => poisoned.into_inner().is_some(), @@ -466,6 +473,7 @@ pub fn observe_callgraph_query(ctx: &AppContext) -> IndexObservation { /// transient busy or locked error while opening the published view database. pub fn observe_callgraph_query_with_contention(ctx: &AppContext) -> (IndexObservation, bool) { let served_by_view = ctx.checkout_query_runtime_active() + || ctx.callgraph_views_enabled() || (ctx.config().views.enabled && ctx .pinned_view_runtime() diff --git a/crates/aft/src/views/assembly.rs b/crates/aft/src/views/assembly.rs index a0ac943df6..98dc6e58be 100644 --- a/crates/aft/src/views/assembly.rs +++ b/crates/aft/src/views/assembly.rs @@ -2203,7 +2203,7 @@ fn manifest_entry_callgraph_key(entry: &ManifestEntry) -> Option<&str> { } } -pub(super) fn is_resolution_input(path: &[u8]) -> bool { +pub(crate) fn is_resolution_input(path: &[u8]) -> bool { let name = path.rsplit(|byte| *byte == b'/').next().unwrap_or(path); name == b"package.json" || name == b"Cargo.toml" diff --git a/crates/aft/src/views/callgraph.rs b/crates/aft/src/views/callgraph.rs index 10b9fd08d2..7ac648bd6b 100644 --- a/crates/aft/src/views/callgraph.rs +++ b/crates/aft/src/views/callgraph.rs @@ -343,6 +343,303 @@ impl PlaneAdapter for CallgraphPlane { } } } +/// Supplies a checkout's verified call graph to one request, without +/// exposing its storage layout. Implementations never publish v1/v2 data and +/// never substitute another store when acquisition fails. +pub(crate) trait CallgraphReaderSource: Send + Sync { + fn acquire(&self) -> Result, String>; + fn record_change(&self, path: &Path); + fn record_overflow(&self) {} + fn record_cookie(&self, _: &Path) {} + /// Source files parsed for call-graph evidence so far. + #[cfg(test)] + fn extraction_count(&self) -> usize { + 0 + } +} + +/// The production source: a v3-only checkout driver carrying the lexical +/// plane (bases are lexical-ready first) and the call-graph plane. +struct CheckoutCallgraphSource { + driver: Arc, + access: ViewAccess, + bridge: Arc, + storage: PathBuf, + family: String, + scope: String, + producers: super::manifest_v2::Producers, +} + +/// True when the snapshot's live entries differ from its published +/// generation: a graph opened for that generation would answer for older +/// bytes than the checkout holds. +fn has_unpublished_changes(snapshot: &Snapshot) -> bool { + snapshot + .live_entries() + .any(|(path, entry)| entry.disk != snapshot.generation().disk_state(path)) +} + +impl CallgraphReaderSource for CheckoutCallgraphSource { + fn acquire(&self) -> Result, String> { + use super::first_load::FirstLoadDriver; + let installed = self.driver.installed_snapshot(); + // After a restart, start from this checkout's own published worker + // generation; verification still reconciles it against the disk. + let seed = if installed.generation().name() == "empty" { + match super::base_refs::open_foreign_v3( + &self.storage, + &self.family, + &self.scope, + &self.producers, + ) { + Ok(Some(generation)) => generation, + Ok(None) => installed.generation().clone(), + Err(error) => { + crate::slog_debug!( + "callgraph v3 seed unavailable; reconciling current tree: {}", + error + ); + installed.generation().clone() + } + } + } else { + installed.generation().clone() + }; + let (mut snapshot, revision) = self + .driver + .verified_bind(&self.access, seed) + .map_err(|error| error.to_string())?; + if has_unpublished_changes(&snapshot) { + // Publish the edits as a new worker generation (a donor clone + // patched with the diff) before the single install, as a fresh + // bootstrap does inside `verified_bind`. A second install would + // see the first one marked overflowed on a checkout without a + // steady watcher subscription and refuse its own cut. + let own = self + .driver + .build_own_generation( + &self.access, + &snapshot, + super::first_load::DERIVED_CALLGRAPH_SEEDING, + ) + .map_err(|error| error.to_string())?; + let cut = snapshot + .install_cut() + .cloned() + .ok_or("callgraph view snapshot has no install cut")?; + let mut delta = super::snapshot::LiveDelta::from_snapshot(&snapshot); + let draft = super::snapshot::derive_successor( + &delta.cut(), + own, + &super::snapshot::carry_disk_state_only, + ); + delta + .replay_and_swap(draft, &super::snapshot::carry_disk_state_only) + .map_err(|error| format!("{error:?}"))?; + snapshot = delta + .snapshot() + .with_install_cut(cut.clone()) + .with_reconciled_epoch(snapshot.reconciled_epoch().unwrap_or(cut.epoch)); + } + self.driver + .install(&self.access, &snapshot, revision) + .map_err(|error| error.to_string())?; + // Read the installed cut directly: `installed_snapshot` would mark a + // checkout without a steady watcher subscription overflowed at once. + // That mark only decides how the next bind reconciles; this cut was + // just verified against the disk. + let (snapshot, _) = self.driver.installed_cut(); + if snapshot.pending_intent().next().is_some() + || snapshot.watcher() != super::snapshot::WatcherState::Healthy + || has_unpublished_changes(&snapshot) + { + return Err(format!( + "callgraph view has unverified writes (pending={:?}, watcher={:?}, unpublished={:?})", + snapshot.pending_intent().take(3).collect::>(), + snapshot.watcher(), + snapshot + .live_entries() + .filter(|(path, entry)| entry.disk + != snapshot.generation().disk_state(path)) + .map(|(path, _)| path.clone()) + .take(3) + .collect::>(), + )); + } + let reader = match self.bridge.adapter.reader(&self.access, &snapshot) { + Ok(reader) => reader, + Err(error) if error.reason == super::read::CALLGRAPH_DISABLED => { + return Err(error.reason) + } + Err(_) => { + // A generation installed from the durable pointer (after a + // restart) has no resident reader yet; it is this checkout's + // own v3 member, so open its derived graph here. + self.bridge + .adapter + .open_generation(&self.access, snapshot.generation()) + .map_err(|error| error.to_string())?; + self.bridge + .adapter + .reader(&self.access, &snapshot) + .map_err(|error| error.to_string())? + } + }; + Ok(reader.store.clone()) + } + + fn record_change(&self, path: &Path) { + self.driver.record_absolute_change(path); + } + fn record_overflow(&self) { + self.driver.record_watcher_overflow(); + } + fn record_cookie(&self, path: &Path) { + self.driver.record_watcher_cookie(path); + } + #[cfg(test)] + fn extraction_count(&self) -> usize { + self.bridge.extraction_count() + } +} + +/// Checkout-local call-graph authority used when views are enabled. Its v3 +/// member is separate from the lexical view's, so lexical service never waits +/// for graph extraction; both share the family's content-addressed rows. +pub(crate) struct CallgraphView { + root: PathBuf, + storage: crate::root_cache::StoragePaths, + family: String, + policy: crate::blob_store::v2::TrigramPolicy, + source: Arc, + query: std::sync::Mutex<()>, + ready: std::sync::atomic::AtomicBool, +} + +/// The v3 member scope of a checkout's call-graph view. +pub(crate) fn view_scope(root: &Path) -> String { + format!( + "{}.callgraph", + crate::path_identity::project_scope_key(root) + ) +} + +impl CallgraphView { + pub(crate) fn matches( + &self, + root: &Path, + storage: &crate::root_cache::StoragePaths, + family: &str, + policy: crate::blob_store::v2::TrigramPolicy, + ) -> bool { + self.root == root + && self.storage == *storage + && self.family == family + && self.policy == policy + } + + pub(crate) fn new( + root: PathBuf, + storage: crate::root_cache::StoragePaths, + family: String, + policy: crate::blob_store::v2::TrigramPolicy, + ) -> Result { + let scope = view_scope(&root); + let producers = super::manifest_v2::Producers { + trigram: policy.fingerprint_hex(), + semantic: None, + callgraph: PRODUCER.into(), + }; + let trigram = Arc::new(super::first_load::TrigramBridge::new( + storage.artifacts.clone(), + policy, + )); + let bridge = Arc::new(super::first_load::CallgraphBridge::default()); + let adapter: Arc = bridge.adapter.clone(); + let driver = Arc::new( + super::first_load::CheckoutDriver::new_v3( + &storage.artifacts, + &family, + &scope, + &root, + producers.clone(), + Arc::new(super::first_load::ConfiguredMembershipWalker), + vec![trigram, bridge.clone()], + ) + .map_err(|error| error.to_string())? + .with_adapters(vec![adapter]), + ); + driver.register_write_intent(); + let source = Arc::new(CheckoutCallgraphSource { + access: driver.access(), + driver, + bridge, + storage: storage.artifacts.clone(), + family: family.clone(), + scope, + producers, + }); + Ok(Self::with_source(root, storage, family, policy, source)) + } + + pub(crate) fn with_source( + root: PathBuf, + storage: crate::root_cache::StoragePaths, + family: String, + policy: crate::blob_store::v2::TrigramPolicy, + source: Arc, + ) -> Self { + Self { + root, + storage, + family, + policy, + source, + query: std::sync::Mutex::new(()), + ready: std::sync::atomic::AtomicBool::new(false), + } + } + + pub(crate) fn record_change(&self, path: &Path) { + self.ready + .store(false, std::sync::atomic::Ordering::Release); + self.source.record_change(path); + } + + pub(crate) fn record_overflow(&self) { + self.ready + .store(false, std::sync::atomic::Ordering::Release); + self.source.record_overflow(); + } + + pub(crate) fn record_cookie(&self, path: &Path) { + self.source.record_cookie(path); + } + + pub(crate) fn is_ready(&self) -> bool { + self.ready.load(std::sync::atomic::Ordering::Acquire) + } + + #[cfg(test)] + pub(crate) fn extraction_count(&self) -> usize { + self.source.extraction_count() + } + + /// Acquires a reader pinned to one verified generation. Acquisitions are + /// serialized so an installed membership and its graph come from the + /// same reconciliation. + pub(crate) fn reader(&self) -> Result, String> { + let _query = self + .query + .lock() + .unwrap_or_else(std::sync::PoisonError::into_inner); + let result = self.source.acquire(); + self.ready + .store(result.is_ok(), std::sync::atomic::Ordering::Release); + result + } +} + fn queryable_manifest( manifest: &super::snapshot::ComposedGeneration, ) -> Result { @@ -365,6 +662,173 @@ fn error(reason: impl std::fmt::Display) -> PlaneError { mod tests { use super::*; + const FILES: &[(&str, &str)] = &[ + ("a.ts", "export function target() { return 1; }\n"), + ( + "b.ts", + "import { target } from './a';\nexport function caller() { return target(); }\n", + ), + ("package.json", "{\"name\": \"donor-fixture\"}\n"), + ("notes.md", "# notes\n"), + ]; + + fn checkout(dir: &Path) -> PathBuf { + std::fs::create_dir_all(dir).unwrap(); + for (name, text) in FILES { + std::fs::write(dir.join(name), text).unwrap(); + } + std::fs::canonicalize(dir).unwrap() + } + + fn callers(store: &ReadonlyCallGraphStore, file: &str, symbol: &str) -> Vec { + let mut names: Vec<_> = store + .callers_of(Path::new(file), symbol, 1) + .unwrap() + .callers + .iter() + .map(|caller| format!("{caller:?}")) + .collect(); + names.sort(); + names + } + + fn view(root: &Path, storage: &Path) -> CallgraphView { + CallgraphView::new( + root.to_path_buf(), + crate::root_cache::StoragePaths::same(storage), + "donor-reuse".into(), + crate::blob_store::v2::TrigramPolicy { + max_file_size: 1 << 20, + }, + ) + .unwrap() + } + + fn events(dir: &Path) -> Vec { + super::super::donor::events_under(dir) + } + + #[test] + fn sealed_base_donor_serves_checkouts_without_reextraction_or_cold_builds() { + use super::super::donor::DonorEvent; + let _serial = crate::blob_store::v3::serial_for_test(); + let temp = tempfile::tempdir().unwrap(); + let storage = temp.path().join("storage"); + let first = checkout(&temp.path().join("first")); + let second = checkout(&temp.path().join("second")); + let member = |root: &Path| { + crate::blob_store::v3::view_dir(&storage, "donor-reuse", &view_scope(root)).unwrap() + }; + + // A fresh checkout bootstraps the base through the base builder, + // completes and seals its graph, then clones the donor for itself. + let first_view = view(&first, &storage); + let first_store = first_view.reader().unwrap(); + assert_eq!(callers(&first_store, "a.ts", "target").len(), 1); + assert_eq!(first_view.extraction_count(), 3, "a.ts, b.ts, package.json"); + let all = events(&storage); + assert_eq!( + all.iter().filter(|e| **e == DonorEvent::Sealed).count(), + 1, + "{all:?}" + ); + assert!( + !all.iter().any(|e| matches!(e, DonorEvent::Cold(_))), + "{all:?}" + ); + assert!(matches!( + events(&member(&first)).as_slice(), + [DonorEvent::Cloned(_)] + )); + + // The base fixed configuration metadata before its first publication. + let producers = super::super::manifest_v2::Producers { + trigram: crate::blob_store::v2::TrigramPolicy { + max_file_size: 1 << 20, + } + .fingerprint_hex(), + semantic: None, + callgraph: PRODUCER.into(), + }; + let worker = super::super::base_refs::open_foreign_v3( + &storage, + "donor-reuse", + &view_scope(&first), + &producers, + ) + .unwrap() + .unwrap(); + let base_scope = worker.base_header().unwrap().scope.clone(); + let base = + super::super::base_refs::resolve_base(&storage, "donor-reuse", &base_scope).unwrap(); + let config = RelPath::new(b"package.json".to_vec()).unwrap(); + assert!(matches!( + base.manifest().get(&config), + Some(EntryV2::Regular { + resolution_input: true, + .. + }) + )); + let donor = super::super::resolve_derived_path(base.view_dir(), base.generation()).unwrap(); + let seal = super::super::donor::verify(&donor).unwrap(); + drop((base, worker)); + + // A second checkout of the same bytes reuses the base's blobs and its + // sealed donor: nothing is parsed and no graph is built cold. + let second_view = view(&second, &storage); + let second_store = second_view.reader().unwrap(); + assert_eq!(second_view.extraction_count(), 0); + assert_eq!( + callers(&second_store, "a.ts", "target"), + callers(&first_store, "a.ts", "target") + ); + assert!(matches!( + events(&member(&second)).as_slice(), + [DonorEvent::Cloned(_)] + )); + + // An edit publishes a new worker generation: another fresh clone of + // the donor, patched with the diff; only the edited file is parsed. + std::fs::write( + second.join("c.ts"), + "import { caller } from './b';\nexport function top() { return caller(); }\n", + ) + .unwrap(); + second_view.record_change(&second.join("c.ts")); + let edited = second_view.reader().unwrap(); + assert_eq!(callers(&edited, "b.ts", "caller").len(), 1); + assert!(callers(&first_store, "b.ts", "caller").is_empty()); + assert_eq!(second_view.extraction_count(), 1); + let second_events = events(&member(&second)); + assert_eq!(second_events.len(), 2, "{second_events:?}"); + assert!(second_events + .iter() + .all(|e| matches!(e, DonorEvent::Cloned(_)))); + // Readers of the donor never see a write. + super::super::donor::verify_fingerprint(&donor, &seal).unwrap(); + + // Deleting a file the worker added leaves no tombstone behind: the + // base never had it. + std::fs::remove_file(second.join("c.ts")).unwrap(); + second_view.record_change(&second.join("c.ts")); + let deleted = second_view.reader().unwrap(); + assert!(callers(&deleted, "b.ts", "caller").is_empty()); + assert_eq!( + callers(&deleted, "a.ts", "target"), + callers(&first_store, "a.ts", "target") + ); + + // A restart reopens the published worker generation without parsing + // or rebuilding anything. + drop((first_store, first_view)); + let restarted = view(&first, &storage); + let reopened = restarted.reader().unwrap(); + assert_eq!(callers(&reopened, "a.ts", "target").len(), 1); + assert_eq!(restarted.extraction_count(), 0); + assert_eq!(events(&member(&first)).len(), 1); + assert!(!storage.join("blobs/v2").exists()); + } + #[test] fn markdown_is_not_a_callgraph_input_but_resolution_config_is() { let plane = CallgraphPlane::default(); From 05bdfa6b2436e56e44ac97c7c64b585da931cee4 Mon Sep 17 00:00:00 2001 From: ualtinok <94532+ualtinok@users.noreply.github.com> Date: Tue, 27 Oct 2026 03:53:04 +0100 Subject: [PATCH 4/6] mason: port views-on call-graph tests to the checkout view route With views on, every call-graph read now goes through the checkout's v3 view, so tests that pinned a v1 view and asserted the old route's answers are ported to the same contracts on the new route, or retired when the old route itself was their subject: Ported (contract kept, assertions as strict): - views_keyed_generation_{answers_building,zoom_reports_building, inspect_reports_pending}_while_derived_store_is_busy: a SQLite lock on the view database opening is reported as busy/building/pending, never as an absent graph. The view route now carries a typed busy flag (CallgraphViewError) and inspect observes it from the request's access. - views_pending_head_callers_... -> views_changed_head_callers_answer_the_ current_tree_not_the_stale_legacy_store: never serve stale rows. - views_publication_skips_a_non_utf8_callgraph_source_and_completes: now also passes on the view route because call-graph attachment gives a non-UTF-8 source an empty graph, as checkout assembly does (fix). - publication_build_does_not_delay_same_root_bind_and_read: the read still completes while a publication is held, and answers the current tree. - callgraph_worktree_published_owner_assembles_its_own_changed_source and callgraph_unindexed_path_differs_from_missing_symbol: a worktree answers from its own source at once; a new file is indexed, not 'not_in_generation'. - scoped_inspect_views_worktree_reports_checkout_only_dead_function: an edit after binding is reflected rather than reported unavailable. - views_removed_callgraph_blob_store_and_view_rebuild_on_restart: the v1 checkpoint-contention section is dropped (the view route no longer reads the v1 database); rebuild, parity and edit checks remain. - blob_removal_stop waits (bounded) for v3 session-end/checkpoint threads. Added: views_callgraph_index_off_answers_off_without_binding_a_view. Retired (subject was the v1 route): - views_keyless_generation_answers_disabled_while_derived_store_is_busy and per_checkout_9::callgraph_ops_report_a_keyless_view_generation_as_disabled: a v1 generation published with the call graph off; v3 keyless refusal stays covered by per_checkout_callgraph keyless tests. - generation_sweep_waits_for_the_last_query_handle: v1 QueryPin vs v1 sweep; the v3 equivalent (a held reader keeps its generation) is asserted in views::callgraph::tests. - inspect_view_pending_reply_names_publication_not_changed_sources: wording of the v1 'view pending' reply. - inspect_views_owner_persists_and_reuses_tier2_contributions: the writer-backed reuse pipeline read the v1 published view; v3-routed dead code is projected per request from the pinned view (no contribution reuse). Co-authored-by: alfonso-aft <289616620+alfonso-aft@users.noreply.github.com> (cherry picked from commit 7773c24d17f3c4e26c12a9a038a524a0f9371857) --- crates/aft/src/commands/configure.rs | 123 ++----------- crates/aft/src/commands/inspect.rs | 27 ++- crates/aft/src/context.rs | 169 +++++++++-------- .../src/executor/view_publication_tests.rs | 136 +------------- crates/aft/src/views/callgraph.rs | 66 ++++++- crates/aft/src/views/mod.rs | 5 +- .../integration/callgraph_worktree_test.rs | 67 +------ .../tests/integration/inspect_command_test.rs | 137 ++------------ .../aft/tests/integration/per_checkout_9.rs | 172 ------------------ 9 files changed, 226 insertions(+), 676 deletions(-) diff --git a/crates/aft/src/commands/configure.rs b/crates/aft/src/commands/configure.rs index c285ee410d..ccb7ce36e3 100644 --- a/crates/aft/src/commands/configure.rs +++ b/crates/aft/src/commands/configure.rs @@ -12128,6 +12128,16 @@ mod tests { }; drop(ctx); crate::views::wait_for_derived_checkpoint_for_test(&lazy_view_dir(storage, root)); + // The checkout's v3 views end their registry sessions and settle their + // checkpoints on short-lived detached threads; a real stop ends the + // process. Give those threads a bounded moment to close. + let v3 = storage.join("blobs").join("v3"); + let deadline = Instant::now() + Duration::from_secs(10); + while Instant::now() < deadline + && !crate::db::file_identity::open_connections_under_for_test(&v3).is_empty() + { + std::thread::sleep(Duration::from_millis(10)); + } let held = crate::db::file_identity::open_connections_under_for_test(storage); assert!( held.is_empty(), @@ -12760,111 +12770,11 @@ mod tests { "both files were extracted again" ); - // Rebuilding from scratch flushes its database before bind returns, so - // there is no background checkpoint to pause. Publish an edit below and - // pause that generation's checkpoint with a real SQLite writer. - // An exclusive rollback-journal transaction makes short-lived lock - // contention observable without racing the scheduler. - enum CheckpointObservation { - Waiting, - Answer(std::thread::Result<(String, String)>), - } - let (begin_tx, begin_rx) = std::sync::mpsc::channel(); - let (locked_tx, locked_rx) = std::sync::mpsc::channel(); - let (release_tx, release_rx) = std::sync::mpsc::channel(); - let (observation_tx, observation_rx) = std::sync::mpsc::channel(); - let waiting_tx = observation_tx.clone(); - crate::views::checkpoint_test_hooks() - .lock() - .unwrap() - .insert( - view_dir.clone(), - crate::views::CheckpointTestHooks { - before_settled: Some(Box::new(move |path| { - begin_rx.recv_timeout(Duration::from_secs(60)).unwrap(); - let connection = crate::db::file_identity::IdentityConnection::open( - path, - "checkpoint contention fixture", - ) - .unwrap(); - let journal: String = connection - .query_row("PRAGMA journal_mode=DELETE", [], |row| row.get(0)) - .unwrap(); - assert_eq!(journal, "delete"); - connection.execute_batch("BEGIN EXCLUSIVE;").unwrap(); - locked_tx.send(path.to_path_buf()).unwrap(); - let _ = release_rx.recv_timeout(Duration::from_secs(60)); - connection - .execute_batch("ROLLBACK; PRAGMA journal_mode=WAL;") - .unwrap(); - })), - before_wait: Some(Box::new(move || { - waiting_tx.send(CheckpointObservation::Waiting).unwrap(); - })), - }, - ); - let source = fs::read_to_string(root.join("lib.rs")).unwrap(); - fs::write( - root.join("lib.rs"), - format!("{source}\n// checkpoint synchronization fixture\n"), - ) - .unwrap(); - let checkpoint_publication = ctx - .publish_view_paths(BTreeSet::from([b"lib.rs".to_vec()]), false) - .unwrap(); - assert!( - checkpoint_publication.published, - "{checkpoint_publication:?}" - ); - let generation = checkpoint_publication - .generation - .expect("incremental checkpoint generation"); - begin_tx.send(()).unwrap(); - let locked_path = locked_rx.recv_timeout(Duration::from_secs(60)).unwrap(); + // With views on, answers come from the checkout's own v3 view; the v1 + // publication's checkpoint no longer stands between a query and its + // graph. The rebuilt checkout answers exactly as before. assert_eq!( - locked_path, - crate::views::resolve_derived_path(&view_dir, &generation).unwrap() - ); - let request: RawRequest = serde_json::from_value(serde_json::json!({ - "id": "checkpoint-busy", "command": "callers", "session_id": "after-removal", - "file": "lib.rs", "symbol": "target" - })) - .unwrap(); - let busy = crate::commands::callers::handle_callers(&request, &ctx); - assert!(!busy.success, "{busy:?}"); - assert_eq!(busy.data["code"], "callgraph_building", "{busy:?}"); - let reader_ctx = Arc::clone(&ctx); - let reader = std::thread::spawn(move || { - let answer = std::panic::catch_unwind(std::panic::AssertUnwindSafe(|| { - blob_removal_answers(&reader_ctx) - })); - observation_tx - .send(CheckpointObservation::Answer(answer)) - .unwrap(); - }); - assert!( - matches!( - observation_rx - .recv_timeout(Duration::from_secs(60)) - .unwrap(), - CheckpointObservation::Waiting - ), - "the fixture read before the checkpoint completion event" - ); - release_tx.send(()).unwrap(); - let CheckpointObservation::Answer(answer) = observation_rx - .recv_timeout(Duration::from_secs(60)) - .unwrap() - else { - panic!("checkpoint waiter did not answer"); - }; - reader.join().unwrap(); - crate::views::checkpoint_test_hooks() - .lock() - .unwrap() - .remove(&view_dir); - assert_eq!( - answer.unwrap(), + blob_removal_answers(&ctx), ( fixture.callers_before.clone(), fixture.call_tree_before.clone() @@ -15897,7 +15807,10 @@ mod tests { impl crate::views::callgraph::CallgraphReaderSource for Refused { fn acquire( &self, - ) -> Result, String> { + ) -> Result< + Arc, + crate::views::callgraph::CallgraphViewError, + > { self.0.fetch_add(1, Ordering::SeqCst); Err("callgraph source deliberately refused".into()) } diff --git a/crates/aft/src/commands/inspect.rs b/crates/aft/src/commands/inspect.rs index 7faf0b67a9..bdd25298d9 100644 --- a/crates/aft/src/commands/inspect.rs +++ b/crates/aft/src/commands/inspect.rs @@ -718,6 +718,24 @@ fn handle_inspect_payload( crate::context::CallgraphStoreAccess::Unavailable }; let checkout_routed = ctx.checkout_query_outcome().is_some() || v3_routed; + // Why the v3 view could not serve, observed once from the access this + // request already holds: a SQLite lock is contention (pending, retry), + // anything else is unavailability. + let routed_observation = v3_routed.then(|| match &routed_store { + crate::context::CallgraphStoreAccess::Error(error) + if error.is_transient_lock_contention() => + { + (crate::feature_status::IndexObservation::building(), true) + } + crate::context::CallgraphStoreAccess::Error(error) => ( + crate::feature_status::IndexObservation::unavailable(error.to_string()), + false, + ), + access => ( + crate::feature_status::callgraph_access_observation(ctx, access), + false, + ), + }); let checkout_store = if checkout_routed { match routed_store { crate::context::CallgraphStoreAccess::Ready(store) => Some(store), @@ -762,12 +780,17 @@ fn handle_inspect_payload( && checkout_store.is_none() { Some(crate::feature_status::observe_callgraph_query_with_contention(ctx)) + } else if checkout_store.is_none() { + routed_observation } else { None }; + // A v3-routed request scans only with a reader in hand; without one its + // graph-backed categories report the gap below rather than running the + // legacy reuse scanner. let use_checkout_view = (needs_ephemeral_view || v3_routed) && snapshot.config.views.enabled - && (checkout_routed || checkout_store.is_some()); + && (checkout_store.is_some() || (checkout_routed && !v3_routed)); let checkout_view_unavailability = (needs_ephemeral_view && snapshot.config.views.enabled && !checkout_routed @@ -834,7 +857,7 @@ fn handle_inspect_payload( if outcomes.contains_key(&category) { continue; } - if (scope_was_provided || !ctx.inspect_writer()) && !use_checkout_view { + if (scope_was_provided || !ctx.inspect_writer() || v3_routed) && !use_checkout_view { continue; } if request_deadline.is_some_and(|deadline| !deadline.has_work_budget()) { diff --git a/crates/aft/src/context.rs b/crates/aft/src/context.rs index 08c0ab4a0e..5a469ac653 100644 --- a/crates/aft/src/context.rs +++ b/crates/aft/src/context.rs @@ -298,7 +298,7 @@ thread_local! { const { std::cell::RefCell::new(Vec::new()) }; /// The call-graph reader each pinned request acquired from its checkout /// view, so every graph read in one request answers from one generation. - static CALLGRAPH_QUERY_READERS: std::cell::RefCell, String>)>> = + static CALLGRAPH_QUERY_READERS: std::cell::RefCell, crate::views::callgraph::CallgraphViewError>)>> = const { std::cell::RefCell::new(Vec::new()) }; } @@ -10958,11 +10958,24 @@ impl AppContext { } return match self.callgraph_view_reader() { Some(Ok(store)) => CallgraphStoreAccess::Ready(store), - Some(Err(reason)) if reason.contains(crate::views::read::CALLGRAPH_DISABLED) => { - CallgraphStoreAccess::Error(CallGraphStoreError::Unavailable(reason)) + // A lock held on the view's database says nothing about the + // graph: report SQLite's busy error, which callers present as + // building, retry shortly. + Some(Err(error)) if error.busy => CallgraphStoreAccess::Error( + CallGraphStoreError::Sqlite(rusqlite::Error::SqliteFailure( + rusqlite::ffi::Error::new(rusqlite::ffi::SQLITE_BUSY), + Some(error.reason), + )), + ), + Some(Err(error)) + if error + .reason + .contains(crate::views::read::CALLGRAPH_DISABLED) => + { + CallgraphStoreAccess::Error(CallGraphStoreError::Unavailable(error.reason)) } - Some(Err(reason)) => CallgraphStoreAccess::Error(CallGraphStoreError::Unavailable( - format!("callgraph view unavailable: {reason}"), + Some(Err(error)) => CallgraphStoreAccess::Error(CallGraphStoreError::Unavailable( + format!("callgraph view unavailable: {error}"), )), None => CallgraphStoreAccess::Unavailable, }; @@ -12143,7 +12156,8 @@ impl AppContext { /// acquisition error is final and never answered from another store. pub(crate) fn callgraph_view_reader( &self, - ) -> Option, String>> { + ) -> Option, crate::views::callgraph::CallgraphViewError>> + { if !self.callgraph_views_enabled() { return None; } @@ -12171,7 +12185,7 @@ impl AppContext { { match crate::views::callgraph::CallgraphView::new(root, storage, family, policy) { Ok(view) => *slot = Some(Arc::new(view)), - Err(error) => return Some(Err(error)), + Err(error) => return Some(Err(error.into())), } } slot.as_ref().unwrap().clone() @@ -20427,10 +20441,15 @@ mod callgraph_store_for_ops_tests { assert!(!message.contains("retry shortly"), "{message}"); } + /// With views on, a commit that changes the graph is answered from the + /// checkout as it now is: the stale legacy store, which still holds the + /// removed call, never answers. #[test] - fn views_pending_head_callers_reports_building_instead_of_serving_legacy() { + fn views_changed_head_callers_answer_the_current_tree_not_the_stale_legacy_store() { let fixture = ViewsCallgraphFixture::new(); - fixture.publish(); + let before = fixture.callers(); + assert!(before.success, "{:#}", before.data); + assert_eq!(before.data["total_callers"], 1, "{:#}", before.data); fixture.commit_source( "pub fn caller() {}\npub fn target() {}\n", "remove stale call", @@ -20447,55 +20466,53 @@ mod callgraph_store_for_ops_tests { let response = fixture.callers(); - assert!(!response.success, "pending HEAD must not serve stale rows"); - assert_eq!(response.data["code"], "callgraph_building"); + assert!(response.success, "{:#}", response.data); assert_eq!( - response.data["message"], - "callers: callgraph store is building in the background; retry shortly" + response.data["total_callers"], 0, + "the changed HEAD must not serve stale rows: {:#}", + response.data ); } impl ViewsCallgraphFixture { - /// Publish the current HEAD as a generation without call graph data, - /// as a session with the call graph off does, and pin it as this - /// context's view, as configure does when it loads that generation. - fn publish_without_callgraph(&self) -> String { - let view = self.ctx.view_runtime_snapshot().expect("view runtime"); - let head = crate::alias::head_tree_entries(&self.root).expect("HEAD entries"); - let desired_head = crate::views::assembly::head_tree_fingerprint(&head); - let report = crate::views::assembly::publish_checkout( - &crate::views::assembly::AssemblyRequest { - storage: view.storage.clone(), - project_root: self.root.clone(), - family: view.family.clone(), - scope: view.scope.clone(), - desired_head: desired_head.clone(), - changed_paths: Default::default(), - semantic_keys: Default::default(), - require_semantic: false, - allow_blob_put: true, - callgraph: false, - }, + /// Hold this checkout's v3 call-graph view database busy the way a + /// writer does mid-maintenance: an exclusive lock that a reader cannot + /// get past. Binds the view first (one answered query), then drops + /// this context's view (turning views off and on again) so the next + /// query must open the database, and waits for the deferred checkpoint + /// so only the returned connection holds the file. + fn hold_checkout_view_derived_busy(&self) -> rusqlite::Connection { + let bound = self.callers(); + assert!(bound.success, "{:#}", bound.data); + self.ctx + .update_config(|config| config.views.enabled = false); + self.ctx.update_config(|config| config.views.enabled = true); + let storage = self.ctx.storage_paths().artifacts; + let family = self.ctx.memoized_artifact_cache_key(&self.root); + let member = crate::blob_store::v3::view_dir( + &storage, + &family, + &crate::views::callgraph::view_scope(&self.root), ) - .expect("keyless view publication"); - assert!(report.published, "keyless generation must publish"); - let manifest = report.manifest.expect("published manifest"); - assert!(crate::views::assembly::manifest_lacks_callgraph(&manifest)); - let generation = report.generation.expect("published generation"); - let pin = - crate::pins::QueryPin::acquire(&view.view_dir, &generation).expect("query pin"); - self.ctx.install_view_runtime( - ViewRuntimeSnapshot { - generation: Some(generation.clone()), - manifest: Some(manifest), - head_fingerprint: desired_head, - pending_paths: BTreeSet::new(), - pending_inputs: BTreeMap::new(), - ..view - }, - Some(pin), + .expect("v3 member directory"); + crate::views::wait_for_derived_checkpoint_for_test(&member); + let generation = crate::views::ViewStore::open_dir(member.clone()) + .expect("v3 member view") + .current_generation() + .expect("v3 pointer") + .expect("published v3 generation"); + let path = + crate::views::resolve_derived_path(&member, &generation).expect("derived path"); + assert!( + path.is_file(), + "derived database {} missing", + path.display() ); - generation + let holder = rusqlite::Connection::open(&path).expect("derived writer"); + holder + .execute_batch("PRAGMA locking_mode=EXCLUSIVE; BEGIN EXCLUSIVE;") + .expect("exclusive derived lock"); + holder } /// Hold the derived database of the pinned generation busy the way a @@ -20522,43 +20539,46 @@ mod callgraph_store_for_ops_tests { } } - /// Whether a generation carries a call graph is a property of its - /// manifest alone. A generation published with the call graph off must - /// answer "disabled" even while its derived database is busy; answering - /// "building, retry" would send the agent into a retry loop for a graph - /// that will never be built from this generation. + /// A call graph turned off is never built and never answered: with views + /// on and `indexes.callgraph=false` the call-graph tools report the index + /// off without binding a checkout view. #[test] - fn views_keyless_generation_answers_disabled_while_derived_store_is_busy() { + fn views_callgraph_index_off_answers_off_without_binding_a_view() { let fixture = ViewsCallgraphFixture::new(); - fixture.publish_without_callgraph(); - let _busy = fixture.hold_pinned_derived_busy(); + fixture + .ctx + .update_config(|config| config.indexes.callgraph = false); let response = fixture.callers(); assert!(!response.success, "{:#}", response.data); assert_eq!( - response.data["code"], "callgraph_unavailable", - "{:#}", - response.data - ); - assert!( - response.data["message"] - .as_str() - .is_some_and(|message| message.contains(crate::views::read::CALLGRAPH_DISABLED)), + response.data["code"], "callgraph_off", "{:#}", response.data ); + assert!(matches!( + fixture.ctx.callgraph_store_for_ops(), + CallgraphStoreAccess::Off + )); + assert_eq!(fixture.ctx.callgraph_view_route_count(), 0); + let member = crate::blob_store::v3::view_dir( + &fixture.ctx.storage_paths().artifacts, + &fixture.ctx.memoized_artifact_cache_key(&fixture.root), + &crate::views::callgraph::view_scope(&fixture.root), + ) + .unwrap(); + assert!(!member.exists()); } - /// The counterpart: a generation that does carry a call graph, read while + /// A generation that does carry a call graph, read while /// its derived database is busy, is temporarily unreadable and says so as /// a retryable `callgraph_building`. This also proves the busy hold above /// really blocks a reader that opens the store. #[test] fn views_keyed_generation_answers_building_while_derived_store_is_busy() { let fixture = ViewsCallgraphFixture::new(); - fixture.publish(); - let _busy = fixture.hold_pinned_derived_busy(); + let busy = fixture.hold_checkout_view_derived_busy(); let response = fixture.callers(); @@ -20572,13 +20592,15 @@ mod callgraph_store_for_ops_tests { response.data["message"], "callers: persisted callgraph store is busy; retry shortly" ); + drop(busy); + let response = fixture.callers(); + assert!(response.success, "{:#}", response.data); } #[test] fn views_keyed_generation_inspect_reports_pending_while_derived_store_is_busy() { let fixture = ViewsCallgraphFixture::new(); fixture.ctx.set_harness(crate::harness::Harness::Runner); - fixture.publish(); let inspect = || { crate::commands::inspect::handle_inspect( &RawRequest { @@ -20594,7 +20616,7 @@ mod callgraph_store_for_ops_tests { &fixture.ctx, ) }; - let busy = fixture.hold_pinned_derived_busy(); + let busy = fixture.hold_checkout_view_derived_busy(); let response = inspect(); @@ -20740,7 +20762,6 @@ mod callgraph_store_for_ops_tests { #[test] fn views_keyed_generation_zoom_reports_building_while_derived_store_is_busy() { let fixture = ViewsCallgraphFixture::new(); - fixture.publish(); let zoom = || { crate::commands::zoom::handle_zoom( &RawRequest { @@ -20757,7 +20778,7 @@ mod callgraph_store_for_ops_tests { &fixture.ctx, ) }; - let busy = fixture.hold_pinned_derived_busy(); + let busy = fixture.hold_checkout_view_derived_busy(); let response = zoom(); diff --git a/crates/aft/src/executor/view_publication_tests.rs b/crates/aft/src/executor/view_publication_tests.rs index 5402a580d2..e286b55edf 100644 --- a/crates/aft/src/executor/view_publication_tests.rs +++ b/crates/aft/src/executor/view_publication_tests.rs @@ -1783,63 +1783,6 @@ fn inspect_view_pending_publication_reschedules_dead_code_without_another_inspec ); } -#[test] -fn inspect_view_pending_reply_names_publication_not_changed_sources() { - use crate::inspect::{InspectCategory, InspectSnapshot, JobScope}; - let fixture = Fixture::new(); - fixture.ctx.set_harness(crate::harness::Harness::Opencode); - let root = fixture.ctx.canonical_cache_root(); - let family = crate::search_index::artifact_cache_key(&root); - crate::root_cache::configure_artifact_access(&root, &family, false); - let snapshot = InspectSnapshot { - project_root: root.clone(), - inspect_dir: fixture.ctx.inspect_dir(), - config: fixture.ctx.config(), - symbol_cache: fixture.ctx.symbol_cache(), - inspect_writer: true, - callgraph_writer: true, - }; - let initial = fixture - .ctx - .inspect_manager() - .tier2_run_with_reuse_blocking_fresh( - snapshot, - InspectCategory::DeadCode, - JobScope::for_project(root), - ); - assert_eq!(initial.payload().unwrap()["callgraph_available"], true); - fixture.change("waiting_for_view"); - let response = crate::commands::inspect::handle_inspect_warm_for_test( - &crate::protocol::RawRequest { - id: "pending-view".into(), - command: "inspect".into(), - params: serde_json::json!({"scope": "tracked.rs", "sections": ["dead_code"]}), - session_id: None, - lsp_hints: None, - }, - &fixture.ctx, - ); - assert!(response.success, "{:?}", response.data); - let text = response.data["text"].as_str().unwrap(); - assert!( - text.contains("dead code waits for this checkout's index view to publish"), - "{text}" - ); - assert!( - text.contains("publication schedules a Tier-2 refresh"), - "{text}" - ); - assert!(!text.contains("sources or configuration changed"), "{text}"); - assert!( - !text.contains("cached source contributions, file set, or analysis configuration changed"), - "{text}" - ); - assert_eq!( - response.data["summary"]["dead_code"]["gaps"][0]["kind"], - "view_pending" - ); -} - #[test] fn publication_busy_readiness_uses_backoff_and_recovers_after_unlock() { let fixture = Fixture::new(); @@ -2660,13 +2603,15 @@ fn publication_build_does_not_delay_same_root_bind_and_read() { Lane::PureRead, "view.read".to_owned(), Box::new(|ctx| { + // The checkout view answers from the tree as it now is, without + // waiting for the held publication. let CallgraphStoreAccess::Ready(store) = ctx.callgraph_store_for_ops() else { - panic!("previous view reader unavailable") + panic!("checkout view reader unavailable") }; assert!(crate::callgraph_store::CallGraphRead::node_for( &store, Path::new("tracked.rs"), - "previous" + "next" ) .is_ok()); Response::success("view.read", serde_json::json!({})) @@ -2684,7 +2629,7 @@ fn publication_build_does_not_delay_same_root_bind_and_read() { .expect("same-root read must complete while publication is held"); assert!( read_result.success, - "previous view read failed: {read_result:?}" + "same-root view read failed: {read_result:?}" ); assert_eq!( fixture.view.current_generation().unwrap().as_deref(), @@ -2751,77 +2696,6 @@ fn invalidated_publication_cancels_before_derived_and_removes_generation_files() .is_ok()); } -#[test] -fn generation_sweep_waits_for_the_last_query_handle() { - let fixture = Fixture::new(); - let CallgraphStoreAccess::Ready(previous_reader) = fixture.ctx.callgraph_store_for_ops() else { - panic!("previous reader unavailable") - }; - // The initial publication also owns a deferred checkpoint connection. Settle - // it before replacing the view, so the old query handle is the only keeper - // of the obsolete database whose lifetime this test asserts. - crate::views::wait_for_generation_maintenance_for_test(fixture.view.view_dir()); - // The replacing publication schedules its own generation cleanup, which - // would reclaim the old generation as soon as the reader drops, racing the - // explicit sweeps below. Park that worker at the start of each lap until - // they have run, so these sweeps alone decide what is removed. - let (release_cleanup, parked) = std::sync::mpsc::channel::<()>(); - let parked = Mutex::new(parked); - let parked_laps = std::sync::Arc::new(std::sync::atomic::AtomicUsize::new(0)); - let counted = std::sync::Arc::clone(&parked_laps); - let parking = crate::storage_retention::observe_test_hook( - fixture.view.view_dir(), - std::sync::Arc::new(move |step| { - if step == "post-publication-sweep" { - counted.fetch_add(1, std::sync::atomic::Ordering::SeqCst); - // Returns once the sender is dropped. - let _ = parked.lock().recv_timeout(Duration::from_secs(60)); - } - }), - ); - fixture.change("next"); - fixture.schedule(); - fixture.wait_idle(); - assert_eq!(fixture.view.sweep_generations().unwrap(), 0); - assert!(fixture - .view - .derived_path(&fixture.initial) - .unwrap() - .is_file()); - assert!(crate::callgraph_store::CallGraphRead::node_for( - &previous_reader, - Path::new("tracked.rs"), - "previous" - ) - .is_ok()); - drop(previous_reader); - assert_eq!(fixture.view.sweep_generations().unwrap(), 1); - assert!(!fixture - .view - .derived_path(&fixture.initial) - .unwrap() - .exists()); - assert!(!fixture - .view - .manifest_path(&fixture.initial) - .unwrap() - .exists()); - // Let the parked cleanup run. It must find nothing more to remove, and it - // must actually have been parked, or the explicit sweeps above raced it. - drop(release_cleanup); - crate::views::wait_for_generation_maintenance_for_test(fixture.view.view_dir()); - drop(parking); - assert!( - parked_laps.load(std::sync::atomic::Ordering::SeqCst) > 0, - "the replacing publication's cleanup was never parked" - ); - assert!(!fixture - .view - .derived_path(&fixture.initial) - .unwrap() - .exists()); -} - #[test] fn publication_health_reports_root_and_each_off_lane_phase() { let fixture = Fixture::new(); diff --git a/crates/aft/src/views/callgraph.rs b/crates/aft/src/views/callgraph.rs index 7ac648bd6b..a1d2f59567 100644 --- a/crates/aft/src/views/callgraph.rs +++ b/crates/aft/src/views/callgraph.rs @@ -70,8 +70,11 @@ pub fn attach( let blob = if language == "config" { CallgraphBlob::config(source.to_vec(), PRODUCER) } else { + // A source that is not valid UTF-8 cannot be parsed. As in checkout + // assembly, it gets the call graph of an empty file rather than + // failing the whole reconciliation. CallgraphBlob::extract( - std::str::from_utf8(source).map_err(error)?, + std::str::from_utf8(source).unwrap_or(""), language, PRODUCER, ) @@ -291,7 +294,13 @@ impl PlaneAdapter for CallgraphPlane { dir, generation.name(), ) - .map_err(error)?; + .map_err(|e| { + if e.is_transient_lock_contention() { + error(format!("{DERIVED_STORE_BUSY}: {e}")) + } else { + error(e) + } + })?; let retired = self .readers .write() @@ -343,11 +352,45 @@ impl PlaneAdapter for CallgraphPlane { } } } +/// Why a checkout view could not supply a reader. `busy` marks a transient +/// SQLite lock on the view's database: the graph exists and the caller should +/// retry, which is reported as building rather than unavailable. +#[derive(Clone, Debug)] +pub(crate) struct CallgraphViewError { + pub reason: String, + pub busy: bool, +} + +impl From for CallgraphViewError { + fn from(reason: String) -> Self { + Self { + reason, + busy: false, + } + } +} + +impl From<&str> for CallgraphViewError { + fn from(reason: &str) -> Self { + reason.to_owned().into() + } +} + +impl std::fmt::Display for CallgraphViewError { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + f.write_str(&self.reason) + } +} + +/// Prefix of the plane error for a derived database that a SQLite lock kept +/// from opening. +pub(crate) const DERIVED_STORE_BUSY: &str = "callgraph derived store is busy"; + /// Supplies a checkout's verified call graph to one request, without /// exposing its storage layout. Implementations never publish v1/v2 data and /// never substitute another store when acquisition fails. pub(crate) trait CallgraphReaderSource: Send + Sync { - fn acquire(&self) -> Result, String>; + fn acquire(&self) -> Result, CallgraphViewError>; fn record_change(&self, path: &Path); fn record_overflow(&self) {} fn record_cookie(&self, _: &Path) {} @@ -380,7 +423,7 @@ fn has_unpublished_changes(snapshot: &Snapshot) -> bool { } impl CallgraphReaderSource for CheckoutCallgraphSource { - fn acquire(&self) -> Result, String> { + fn acquire(&self) -> Result, CallgraphViewError> { use super::first_load::FirstLoadDriver; let installed = self.driver.installed_snapshot(); // After a restart, start from this checkout's own published worker @@ -464,12 +507,13 @@ impl CallgraphReaderSource for CheckoutCallgraphSource { .map(|(path, _)| path.clone()) .take(3) .collect::>(), - )); + ) + .into()); } let reader = match self.bridge.adapter.reader(&self.access, &snapshot) { Ok(reader) => reader, Err(error) if error.reason == super::read::CALLGRAPH_DISABLED => { - return Err(error.reason) + return Err(error.reason.into()) } Err(_) => { // A generation installed from the durable pointer (after a @@ -478,7 +522,10 @@ impl CallgraphReaderSource for CheckoutCallgraphSource { self.bridge .adapter .open_generation(&self.access, snapshot.generation()) - .map_err(|error| error.to_string())?; + .map_err(|error| CallgraphViewError { + busy: error.reason.starts_with(DERIVED_STORE_BUSY), + reason: error.to_string(), + })?; self.bridge .adapter .reader(&self.access, &snapshot) @@ -628,7 +675,7 @@ impl CallgraphView { /// Acquires a reader pinned to one verified generation. Acquisitions are /// serialized so an installed membership and its graph come from the /// same reconciliation. - pub(crate) fn reader(&self) -> Result, String> { + pub(crate) fn reader(&self) -> Result, CallgraphViewError> { let _query = self .query .lock() @@ -813,6 +860,9 @@ mod tests { second_view.record_change(&second.join("c.ts")); let deleted = second_view.reader().unwrap(); assert!(callers(&deleted, "b.ts", "caller").is_empty()); + // A reader acquired before the deletion keeps answering for its own + // generation: publication never reclaims a generation a reader holds. + assert_eq!(callers(&edited, "b.ts", "caller").len(), 1); assert_eq!( callers(&deleted, "a.ts", "target"), callers(&first_store, "a.ts", "target") diff --git a/crates/aft/src/views/mod.rs b/crates/aft/src/views/mod.rs index 72825f1057..2762d828ff 100644 --- a/crates/aft/src/views/mod.rs +++ b/crates/aft/src/views/mod.rs @@ -17,10 +17,7 @@ pub mod base_sweep; pub(crate) mod donor; mod generation; #[cfg(test)] -pub(crate) use generation::{ - checkpoint_test_hooks, clone_derived_for_test, wait_for_derived_checkpoint_for_test, - CheckpointTestHooks, -}; +pub(crate) use generation::{clone_derived_for_test, wait_for_derived_checkpoint_for_test}; pub(crate) mod io; #[cfg(aft_views_lazy_benchmark)] pub mod lazy_read_benchmark; diff --git a/crates/aft/tests/integration/callgraph_worktree_test.rs b/crates/aft/tests/integration/callgraph_worktree_test.rs index f69a6d4e26..44ad676c9c 100644 --- a/crates/aft/tests/integration/callgraph_worktree_test.rs +++ b/crates/aft/tests/integration/callgraph_worktree_test.rs @@ -457,16 +457,19 @@ fn callgraph_unindexed_path_differs_from_missing_symbol() { let ctx = configure(&checkout, &storage, true); let unindexed = checkout.join("not-yet-indexed.ts"); std::fs::write(&unindexed, source("newCaller")).unwrap(); + // With views on, a new file is part of the checkout view the query binds, + // so it is answered at once rather than reported as not yet indexed. for operation in OPERATIONS { let response = callgraph_when_ready(operation, || { query_file(&ctx, &unindexed, operation, "newCaller") }); - assert_eq!( + assert_ne!( response.data["code"], "not_indexed", "{operation}: {response:?}" ); - assert_eq!(response.data["reason"], "not_in_generation"); } + let response = query_file(&ctx, &unindexed, "callers", "newCaller"); + assert!(response.success, "{response:?}"); let response = callgraph_when_ready("callers", || { aft::commands::callers::handle_callers( &request(json!({ @@ -517,55 +520,9 @@ fn callgraph_worktree_published_owner_assembles_its_own_changed_source() { std::fs::write(checkout.join("index.ts"), source("checkoutCaller")).unwrap(); commit(&checkout); let ctx = configure_deferred(&checkout, &storage, true); - let response = query(&ctx, &checkout, "callers", "checkoutCaller"); - assert_eq!(response.data["code"], "callgraph_building", "{response:?}"); - assert_eq!(response.data["index"]["callgraph"]["status"], "building"); - assert_eq!(response.data["progress"]["phase"], "configure_maintenance"); - assert_eq!(response.data["progress"]["pending_paths"], Value::Null); - let message = response.data["message"].as_str().unwrap(); - assert!( - message.contains("this worktree") && message.contains("assembling"), - "{message}" - ); - assert!( - message.contains("grep or aft_search with pattern"), - "{message}" - ); - assert!(!message.contains("main checkout"), "{message}"); - - // The worktree contributes its own source blob during maintenance. The - // owner keeps its original source and is never asked to build this branch. - let ctx = std::sync::Arc::new(ctx); - let (prepared, release, installed) = - aft::context::install_callgraph_view_install_test_gate(checkout.clone()); - let executor = aft::executor::Executor::new(); - let root_id = cortexkit_paths::ProjectRootId::from_path(&checkout).unwrap(); - executor.register_actor(root_id.clone(), ctx.clone()); - let maintenance = executor.submit( - root_id, - aft::executor::Lane::Mutating, - "worktree-maintenance".into(), - Box::new(|ctx| { - aft::runtime_drain::drain_deferred_configure_maintenance(ctx); - Response::success("worktree-maintenance", json!({})) - }), - ); - let generation = prepared - .recv_timeout(std::time::Duration::from_secs(30)) - .expect("worktree lazy callgraph generation was not prepared"); - // Configure maintenance schedules lazy publication but does not join it. - // Release the prepared graph, then await installation of that exact - // generation before asking either query path to use the worktree reader. - release.send(()).unwrap(); - assert_eq!( - installed - .recv_timeout(std::time::Duration::from_secs(30)) - .unwrap_or_else(|_| panic!( - "worktree lazy callgraph generation {generation} was not installed for {}", - checkout.display() - )), - generation - ); + // With views on, the first query binds this worktree's own checkout view + // from its own source, before configure maintenance has run: it never + // answers from the owner's graph and never reports the owner's callers. match ctx.callgraph_store_for_ops() { aft::context::CallgraphStoreAccess::Ready(store) => { assert_eq!(store.reader_kind(), "view"); @@ -579,6 +536,8 @@ fn callgraph_worktree_published_owner_assembles_its_own_changed_source() { assert!(response.data.to_string().contains("checkoutCaller")); assert!(!response.data.to_string().contains("ownerCaller")); assert!(response.data.get("borrowed_callgraph").is_none()); + assert!(ctx.callgraph_view_route_count() > 0); + aft::runtime_drain::drain_deferred_configure_maintenance(&ctx); assert_eq!( std::fs::read_to_string(owner.join("index.ts")).unwrap(), source("ownerCaller") @@ -589,12 +548,6 @@ fn callgraph_worktree_published_owner_assembles_its_own_changed_source() { owner_generation, "worktree publication must not replace the owner's view" ); - assert!( - maintenance - .recv_timeout(std::time::Duration::from_secs(30)) - .expect("worktree configure maintenance did not finish") - .success - ); assert!( aft::callgraph_store::CallGraphStore::open_readonly(ctx.callgraph_store_dir(), checkout) .unwrap() diff --git a/crates/aft/tests/integration/inspect_command_test.rs b/crates/aft/tests/integration/inspect_command_test.rs index 404361f7b2..375a866cf0 100644 --- a/crates/aft/tests/integration/inspect_command_test.rs +++ b/crates/aft/tests/integration/inspect_command_test.rs @@ -1884,141 +1884,32 @@ fn scoped_inspect_views_worktree_reports_checkout_only_dead_function() { assert!(!fs::read_to_string(owner.join("target.ts")) .unwrap() .contains("checkoutOnlyDead")); - // A view that predates another edit must not be passed off as this checkout. + // An edit after the view was bound is reflected at once: the checkout + // view is verified against the tree for every request, so an older view + // is never passed off as this checkout. write_file( &linked, "target.ts", "export function used() {}\nexport function nextDead() {}\n", ); - let pending = inspect_tool_call( + let current = inspect_tool_call( &ctx, json!({ - "id": "worktree-view-stale", "command": "inspect", "scope": "target.ts", "sections": "dead_code" + "id": "worktree-view-current", "command": "inspect", "scope": "target.ts", "sections": "dead_code" }), ); - assert_eq!( - pending["summary"]["dead_code"]["unavailable"], true, - "{pending:#}" - ); - assert!(pending["details"]["dead_code"].is_null(), "{pending:#}"); -} - -#[test] -fn inspect_views_owner_persists_and_reuses_tier2_contributions() { - let _env_lock = env_serial_lock(); - crate::helpers::disable_in_process_file_watcher(); - let (temp, root) = fixture_project(); - write_file( - &root, - "main.ts", - "import { used } from './target';\nused();\n", - ); - write_file( - &root, - "target.ts", - "export function used() {}\nexport function dead() {}\n", - ); - for args in [ - vec!["init", "--quiet"], - vec!["add", "."], - vec![ - "-c", - "user.name=Test", - "-c", - "user.email=test@example.com", - "commit", - "--quiet", - "-m", - "fixture", - ], - ] { - assert!(std::process::Command::new("git") - .current_dir(&root) - .args(args) - .status() - .unwrap() - .success()); - } - let storage = temp.path().join("storage"); - let ctx = AppContext::new( - Box::new(TreeSitterProvider::new()), - Config { - storage_dir: Some(storage.clone()), - ..Config::default() - }, - ) - .with_test_checkpoint_store(); - ctx.isolate_cold_build_limiter_for_test(2); - let configured = handle_configure( - &request(json!({ - "id": "configure-owner", "command": "configure", "harness": "opencode", "project_root": root, "storage_dir": storage, - "config": crate::helpers::user_config(json!({ "indexes": { "trigram": false, "semantic": false, "callgraph": true }, "views": {"enabled": true} })) - })), - &ctx, - ); - assert!(configured.success, "{configured:?}"); - assert!(ctx.inspect_writer()); - ctx.inspect_manager() - .set_automatic_tier2_refresh_allowed(false); - let publish = aft::views::assembly::AssemblyRequest { - storage, - project_root: root.clone(), - family: aft::search_index::artifact_cache_key(&root), - scope: aft::path_identity::project_scope_key(&root), - desired_head: aft::views::assembly::head_tree_fingerprint( - &aft::alias::head_tree_entries(&root).unwrap(), - ), - changed_paths: Default::default(), - semantic_keys: Default::default(), - require_semantic: false, - allow_blob_put: true, - callgraph: true, - }; - aft::views::assembly::publish_checkout(&publish).unwrap(); - configure_fake_scope_lsp(&ctx); - let first = inspect_tool_call( - &ctx, - json!({"id": "owner-first", "command": "inspect", "sections": ["dead_code"]}), - ); - assert_eq!(first["success"], true, "{first:#}"); - let cache = InspectCache::open_readonly(ctx.inspect_dir(), root.clone()) - .unwrap() - .expect("writer-backed owner cache"); - let contributions = cache - .load_tier2_contributions(InspectCategory::DeadCode) - .unwrap(); - assert_eq!( - contributions.len(), - 2, - "owner inspect must persist source contributions" - ); - assert!(cache - .last_full_run(InspectCategory::DeadCode) - .unwrap() - .is_some()); - let scanned_before = ctx.inspect_manager().source_scan_files_for_test(); - assert!(scanned_before > 0, "first request must reach the scanner"); - let second = inspect_tool_call( - &ctx, - json!({"id": "owner-second", "command": "inspect", "sections": ["dead_code"]}), - ); - assert_eq!(second["success"], true, "{second:#}"); - assert_eq!(dead_code_items(&first), dead_code_items(&second)); - assert_eq!( - ctx.inspect_manager().source_scan_files_for_test(), - scanned_before, - "unchanged owner inspect must quick-reuse contributions without scanning source files" + assert!( + current["summary"]["dead_code"].get("unavailable").is_none(), + "{current:#}" ); - assert_eq!( - cache - .load_tier2_contributions(InspectCategory::DeadCode) - .unwrap() - .len(), - 2 + let items = dead_code_items(¤t); + assert!( + items.iter().any(|(_, symbol)| symbol == "nextDead"), + "{current:#}" ); assert!( - ctx.inspect_manager().reuse_start_count_for_test() >= 10, - "both requests must use the contribution reuse pipeline" + !items.iter().any(|(_, symbol)| symbol == "checkoutOnlyDead"), + "{current:#}" ); } diff --git a/crates/aft/tests/integration/per_checkout_9.rs b/crates/aft/tests/integration/per_checkout_9.rs index b60480cd1d..412fb84f12 100644 --- a/crates/aft/tests/integration/per_checkout_9.rs +++ b/crates/aft/tests/integration/per_checkout_9.rs @@ -1806,178 +1806,6 @@ fn standalone_semantic_grace_and_load_never_block_requests() { assert!(aft.shutdown().success()); } -/// A view generation published with the call graph off holds no call graph. -/// Every reader of a view's call graph reports it unavailable, by name, never -/// as zero callers: here the call graph tools (and zoom's call graph field) -/// in a standalone process whose current generation was published with the -/// call graph off, while the republish that builds the graph is still -/// running. Once that republish lands, the same query answers. -#[test] -fn callgraph_ops_report_a_keyless_view_generation_as_disabled() { - let temp = tempfile::tempdir().unwrap(); - let base = std::fs::canonicalize(temp.path()).unwrap(); - let root = base.join("repo"); - let storage = base.join("storage"); - init_repo( - &root, - &[( - "src/lib.rs", - "pub fn target() -> u32 {\n 1\n}\n\npub fn caller() -> u32 {\n target()\n}\n" - .to_string(), - )], - ); - publish_without_callgraph(&root, &storage); - // The republish with the call graph waits until this file exists, so the - // generation without call graph data stays current for as long as the - // checks below take, however slow the machine. - let publication_gate = base.join("publication.release"); - let mut aft = crate::helpers::AftProcess::spawn_with_env(&[( - "AFT_TEST_VIEW_PUBLICATION_GATE_FILE", - publication_gate.as_os_str(), - )]); - let with_graph = Planes { - trigram: true, - callgraph: true, - semantic: false, - }; - let configured = aft.send( - &json!({ - "id": "cfg", - "command": "configure", - "harness": "opencode", - "project_root": root, - "storage_dir": storage, - "config": crate::helpers::user_config(config_doc(true, with_graph, None)), - }) - .to_string(), - ); - assert_eq!(configured["success"], true, "{configured:#}"); - let callers = json!({ - "id": "callers", - "command": "callers", - "file": root.join("src/lib.rs"), - "symbol": "target", - }) - .to_string(); - // Configure finishes its view setup in background steps, one after each - // request. Until it has loaded the current generation (the one without - // call graph data), callers answers `callgraph_building`; keep sending - // requests until it answers anything else. The held publication means - // that answer comes from the generation without call graph data. - let deadline = Instant::now() + DEADLINE; - let answer = loop { - aft.send(&json!({"id": "ping", "command": "ping"}).to_string()); - let answer = aft.send(&callers); - if answer["code"] != "callgraph_building" { - break answer; - } - assert!( - Instant::now() < deadline, - "configure never loaded the generation without call graph data; last answer: {answer:#}" - ); - thread::sleep(Duration::from_millis(20)); - }; - assert!( - !current_generation_has_callgraph(&root, &storage), - "the generation without call graph data is no longer current" - ); - assert_eq!(answer["success"], false, "{answer:#}"); - assert_eq!(answer["code"], "callgraph_unavailable", "{answer:#}"); - assert!( - answer["message"] - .as_str() - .unwrap() - .contains("call graph is disabled (indexes.callgraph=false)"), - "{answer:#}" - ); - let zoom = aft.send( - &json!({ - "id": "zoom", - "command": "zoom", - "file": root.join("src/lib.rs"), - "symbol": "target", - "callgraph": true, - }) - .to_string(), - ); - // Zoom's own call lists come from the file's syntax tree; its call graph - // field must say the index is not serving, not present an empty graph. - assert_eq!(zoom["callgraph"]["status"], "unavailable", "{zoom:#}"); - assert!( - zoom["callgraph"]["index"]["reason"] - .as_str() - .unwrap_or_default() - .contains("call graph is disabled (indexes.callgraph=false)"), - "{zoom:#}" - ); - // Let the republish with call graph data run. - std::fs::write(&publication_gate, b"release").unwrap(); - // Once the republish with call graph data lands, callers answers. Right - // after that publication its derived database is checkpointed, and a - // reader that meets the checkpoint gets a retryable `callgraph_building`. - // That answer is acceptable only once the published generation carries a - // call graph: the pointer is read before each request, so "building" - // while the generation without call graph data is still current fails. - let deadline = Instant::now() + DEADLINE; - loop { - let keyed_generation_published = current_generation_has_callgraph(&root, &storage); - let answer = aft.send(&callers); - if answer["success"] == true { - assert!(answer.to_string().contains("caller"), "{answer:#}"); - break; - } - if !(keyed_generation_published && answer["code"] == "callgraph_building") { - assert_eq!(answer["code"], "callgraph_unavailable", "{answer:#}"); - } - assert!( - Instant::now() < deadline, - "the call graph was never published" - ); - thread::sleep(Duration::from_millis(250)); - } - // Zoom reads the store the same way `callers` does and can meet the same - // post-publication checkpoint, which it reports as a retryable - // `callgraph_building`. Retry only on that answer; any other verdict, and - // above all `callgraph_unavailable`, for the published graph fails. - let deadline = Instant::now() + DEADLINE; - loop { - let zoom = aft.send( - &json!({ - "id": "zoom-ready", - "command": "zoom", - "file": root.join("src/lib.rs"), - "symbol": "target", - "callgraph": true, - }) - .to_string(), - ); - assert_eq!(zoom["success"], true, "{zoom:#}"); - if zoom["callgraph"].is_null() { - break; - } - assert_eq!(zoom["callgraph"]["code"], "callgraph_building", "{zoom:#}"); - assert!( - Instant::now() < deadline, - "zoom never read the published call graph: {zoom:#}" - ); - thread::sleep(Duration::from_millis(50)); - } - assert!(aft.shutdown().success()); -} - -/// Whether `root`'s current published view generation carries call graph -/// data. False while no generation is published or its manifest cannot be -/// read (a superseded generation may be collected between the two reads). -fn current_generation_has_callgraph(root: &Path, storage: &Path) -> bool { - let view = - aft::views::ViewStore::open(storage, &aft::path_identity::project_scope_key(root)).unwrap(); - let Some(generation) = view.current_generation_read_only().unwrap() else { - return false; - }; - view.load_manifest(&generation) - .is_ok_and(|manifest| !aft::views::assembly::manifest_lacks_callgraph(&manifest)) -} - /// Publishes `root`'s current view generation with the call graph off, as a /// views-on session with `callgraph_store: false` does. fn publish_without_callgraph(root: &Path, storage: &Path) { From 6af4fc82f4769a38a3309a0069b035a03e9ab0e4 Mon Sep 17 00:00:00 2001 From: ualtinok <94532+ualtinok@users.noreply.github.com> Date: Tue, 27 Oct 2026 04:17:08 +0100 Subject: [PATCH 5/6] mason: port linked-worktree and restart call-graph tests to the checkout view route The on-demand worktree tests asserted the v1 route: answering from the main checkout's published graph, reporting building until a lazy v1 build finished. On the view route a worktree always answers at once from its own v3 view (the shared base's sealed donor avoids rebuilding it). Ported: linked_worktree_{with_its_own_commit_is_never_answered_from_main_graph, stops_answering_from_main_graph_after_main_moves, with_a_graph_from_an_earlier_ process_defers_its_next_publication, defers_and_builds_on_first_query_when_main_ is_not_bound} now assert an immediate answer from the worktree's own view equal to a cold build (lazy_query_builds_own_graph); edit_reports_building_then_ answers_from_its_own_graph -> linked_worktree_edit_answers_from_its_own_view_at_ once; concurrent_first_queries_start_one_call_graph_build asserts both answer and one sealed base build; main_checkout_still_builds_its_call_graph_at_bind and busy_readiness_defers_revalidation_without_a_cold_rebuild keep their v1 publication assertions and read the view route; views_restart_dirty_source_ reports_working_tree_refresh_until_publication -> views_restart_dirty_source_ answers_the_working_tree_at_once. Retired (the old route itself): linked_worktree_answers_from_main_graph_ without_building_its_own and linked_worktree_waits_for_a_locked_main_graph_ instead_of_building, with their unused helpers (lazy_lock_derived, lazy_wait_for_own_graph, inspect current_view_projection_store_for_test). Co-authored-by: alfonso-aft <289616620+alfonso-aft@users.noreply.github.com> (cherry picked from commit dbdd5480bdc6af8a67e95042ee8f88b4ad8cf605) --- crates/aft/src/commands/configure.rs | 271 ++++----------------------- crates/aft/src/inspect/manager.rs | 10 - crates/aft/src/inspect/mod.rs | 4 +- 3 files changed, 41 insertions(+), 244 deletions(-) diff --git a/crates/aft/src/commands/configure.rs b/crates/aft/src/commands/configure.rs index ccb7ce36e3..088e218f6a 100644 --- a/crates/aft/src/commands/configure.rs +++ b/crates/aft/src/commands/configure.rs @@ -11391,9 +11391,11 @@ mod tests { .pending_inputs .values() .any(|input| { input.reason == crate::views::read::CALLGRAPH_READINESS_BUSY })); + // The v1 publication's locked database no longer stands between a + // query and its graph: the checkout's own view answers. assert!(matches!( ctx.callgraph_store_for_ops(), - CallgraphStoreAccess::Building + CallgraphStoreAccess::Ready(_) )); let health = ctx.view_health_snapshot().unwrap(); assert_eq!( @@ -11930,31 +11932,6 @@ mod tests { assert_eq!(projected, dead_code(cold)); } - /// Wait for the publication that builds the worktree's own graph. - fn lazy_wait_for_own_graph(ctx: &AppContext) { - let deadline = Instant::now() + Duration::from_secs(180); - loop { - let built = ctx - .view_runtime_snapshot() - .and_then(|view| { - let generation = view.generation?; - Some(crate::views::read::derived_built( - &view.view_dir, - &generation, - )) - }) - .unwrap_or(false); - if built && !crate::executor::view_publication::running_for(ctx) { - return; - } - assert!( - Instant::now() < deadline, - "the on-demand call-graph build did not finish" - ); - std::thread::sleep(Duration::from_millis(20)); - } - } - /// Edit `lib.rs` in the worktree and deliver the change through the /// watcher drain, without publishing it yet. fn lazy_edit_through_watcher(fixture: &LazyWorktree, contents: &str) { @@ -12802,63 +12779,7 @@ mod tests { } #[test] - fn linked_worktree_answers_from_main_graph_without_building_its_own() { - let _artifact_guard = artifact_owner_test_lock(); - let _git_env = crate::test_env::hermetic_git_env_guard(); - let _disable_watcher = EnvVarGuard::set("AFT_TEST_DISABLE_FILE_WATCHER", "1"); - let fixture = lazy_worktree(true); - let view_dir = lazy_view_dir(&fixture.storage, &fixture.linked); - let view = crate::views::ViewStore::open( - &fixture.storage, - &crate::path_identity::project_scope_key(&fixture.linked), - ) - .unwrap(); - assert!( - view.current_generation().unwrap().is_some(), - "the worktree's view is still published at bind" - ); - assert_eq!(lazy_derived_files(&view_dir), Vec::::new()); - - let store = lazy_ready(fixture.borrower.callgraph_store_for_ops()); - assert!( - store - .sqlite_path() - .starts_with(lazy_view_dir(&fixture.storage, &fixture.main)), - "answered from {:?}, not the main checkout's graph", - store.sqlite_path() - ); - let (_cold_storage, cold) = lazy_cold_store(&fixture.linked); - lazy_assert_same_graph(&store, &cold); - // Inspect's Tier-2 categories, such as dead code, also read the main - // checkout's graph for this worktree. - let (inspect_store, _) = crate::inspect::current_view_projection_store_for_test( - &fixture.linked, - &fixture.borrower.config(), - ) - .expect("inspect reads the main checkout's graph for the worktree"); - lazy_assert_same_graph(&inspect_store, &cold); - let answer = crate::commands::callers::handle_callers( - &RawRequest { - id: "lazy-callers".into(), - command: "callers".into(), - session_id: Some("lazy-linked".into()), - lsp_hints: None, - params: json!({ "file": fixture.linked.join("lib.rs"), "symbol": "target" }), - }, - &fixture.borrower, - ); - assert!(answer.success, "{}", answer.data); - assert_eq!(answer.data["total_callers"], 1, "{}", answer.data); - - assert_eq!(lazy_derived_files(&view_dir), Vec::::new()); - assert_eq!( - fixture.borrower.lazy_callgraph_builds_scheduled_for_test(), - 0 - ); - } - - #[test] - fn linked_worktree_edit_reports_building_then_answers_from_its_own_graph() { + fn linked_worktree_edit_answers_from_its_own_view_at_once() { let _artifact_guard = artifact_owner_test_lock(); let _git_env = crate::test_env::hermetic_git_env_guard(); let _disable_watcher = EnvVarGuard::set("AFT_TEST_DISABLE_FILE_WATCHER", "1"); @@ -12868,26 +12789,8 @@ mod tests { "pub fn caller() { target(); }\npub fn edited_caller() { target(); }\npub fn target() {}\nfn unused() {}\n", ); - // The edit is not published yet, but main's graph no longer answers. - let access = { - let _scope = crate::executor::install_standalone_scope(Arc::clone(&fixture.borrower)); - fixture.borrower.callgraph_store_for_ops() - }; - assert!(matches!(access, CallgraphStoreAccess::Building)); - assert_eq!( - fixture.borrower.lazy_callgraph_builds_scheduled_for_test(), - 1 - ); - lazy_wait_for_own_graph(&fixture.borrower); - - let store = lazy_ready(fixture.borrower.callgraph_store_for_ops()); - assert!( - store - .sqlite_path() - .starts_with(lazy_view_dir(&fixture.storage, &fixture.linked)), - "answered from {:?}, not the worktree's own graph", - store.sqlite_path() - ); + // The edit is not published yet; the worktree's own view answers with it. + let store = lazy_query_builds_own_graph(&fixture); let (_cold_storage, cold) = lazy_cold_store(&fixture.linked); lazy_assert_same_graph(&store, &cold); assert!(crate::callgraph_store::CallGraphRead::node_for( @@ -12896,10 +12799,6 @@ mod tests { "edited_caller" ) .is_ok()); - assert_eq!( - fixture.borrower.lazy_callgraph_builds_scheduled_for_test(), - 1 - ); } #[test] @@ -12985,7 +12884,15 @@ mod tests { let owner = fixture.owner.as_ref().unwrap(); assert_eq!(owner.lazy_callgraph_builds_scheduled_for_test(), 0); let store = lazy_ready(owner.callgraph_store_for_ops()); - assert!(store.sqlite_path().starts_with(&view_dir)); + // Queries read the checkout's own v3 call-graph view. + assert_eq!(store.reader_kind(), "view"); + assert!( + store + .sqlite_path() + .starts_with(fixture.storage.join("blobs").join("v3")), + "{:?}", + store.sqlite_path() + ); } #[test] @@ -12994,8 +12901,8 @@ mod tests { let _git_env = crate::test_env::hermetic_git_env_guard(); let _disable_watcher = EnvVarGuard::set("AFT_TEST_DISABLE_FILE_WATCHER", "1"); let fixture = lazy_worktree(true); - // A change the watcher saw means main's graph cannot answer. - fixture.borrower.note_checkout_changed_for_callgraph(); + // Concurrent first queries are serialized on the checkout view: both + // answer, and the shared base graph is built and sealed only once. let barrier = Arc::new(std::sync::Barrier::new(2)); let queries = (0..2) .map(|_| { @@ -13006,32 +12913,28 @@ mod tests { barrier.wait(); matches!( ctx.callgraph_store_for_ops(), - CallgraphStoreAccess::Building + CallgraphStoreAccess::Ready(_) ) }) }) .collect::>(); for query in queries { - assert!(query.join().unwrap(), "a first query must report building"); + assert!(query.join().unwrap(), "a first query must answer"); } - assert_eq!( - fixture.borrower.lazy_callgraph_builds_scheduled_for_test(), - 1 - ); - lazy_wait_for_own_graph(&fixture.borrower); - assert_eq!( - fixture.borrower.lazy_callgraph_builds_scheduled_for_test(), - 1 - ); + let sealed = crate::views::donor::events_under(&fixture.storage) + .into_iter() + .filter(|event| *event == crate::views::donor::DonorEvent::Sealed) + .count(); + assert_eq!(sealed, 1, "one base graph build for both queries"); let store = lazy_ready(fixture.borrower.callgraph_store_for_ops()); - assert!(store - .sqlite_path() - .starts_with(lazy_view_dir(&fixture.storage, &fixture.linked))); + assert_eq!(store.reader_kind(), "view"); + let (_cold_storage, cold) = lazy_cold_store(&fixture.linked); + lazy_assert_same_graph(&store, &cold); } /// Query the worktree's call graph under a publication scope, as a daemon - /// request does: it must report building, never main's rows. Then wait for - /// the build and return the worktree's own graph. + /// request does. With views on it answers at once from the worktree's own + /// checkout view (its own v3 member), never from main's graph. fn lazy_query_builds_own_graph( fixture: &LazyWorktree, ) -> Arc { @@ -13039,20 +12942,14 @@ mod tests { let _scope = crate::executor::install_standalone_scope(Arc::clone(&fixture.borrower)); fixture.borrower.callgraph_store_for_ops() }; - if let CallgraphStoreAccess::Ready(store) = &access { - panic!( - "the first query answered before any build, from {:?}", - store.sqlite_path() - ); - } - assert!(matches!(access, CallgraphStoreAccess::Building)); - lazy_wait_for_own_graph(&fixture.borrower); - let store = lazy_ready(fixture.borrower.callgraph_store_for_ops()); + let store = lazy_ready(access); + assert_eq!(store.reader_kind(), "view"); + assert_eq!(store.project_root(), fixture.linked); assert!( - store + !store .sqlite_path() - .starts_with(lazy_view_dir(&fixture.storage, &fixture.linked)), - "answered from {:?}, not the worktree's own graph", + .starts_with(lazy_view_dir(&fixture.storage, &fixture.main)), + "answered from main's graph {:?}", store.sqlite_path() ); store @@ -13081,14 +12978,6 @@ mod tests { "branch_caller" ) .is_ok()); - // aft_inspect's dead-code check must also read the worktree's own view, not main's graph. - assert!(crate::inspect::current_view_projection_store_for_test( - &fixture.linked, - &fixture.borrower.config(), - ) - .is_some_and(|(store, _)| store - .sqlite_path() - .starts_with(lazy_view_dir(&fixture.storage, &fixture.linked)))); } #[test] @@ -13097,13 +12986,8 @@ mod tests { let _git_env = crate::test_env::hermetic_git_env_guard(); let _disable_watcher = EnvVarGuard::set("AFT_TEST_DISABLE_FILE_WATCHER", "1"); let fixture = lazy_worktree(true); - // The worktree matches main's generation, so main answers and that - // verdict is cached. - let store = lazy_ready(fixture.borrower.callgraph_store_for_ops()); - assert!(store - .sqlite_path() - .starts_with(lazy_view_dir(&fixture.storage, &fixture.main))); - drop(store); + // The worktree matches main's generation and answers from its own view. + drop(lazy_query_builds_own_graph(&fixture)); // Main moves on: a new commit removes the caller and main publishes a // generation with different inputs. lazy_commit( @@ -13195,71 +13079,6 @@ mod tests { lazy_assert_same_graph(&store, &cold); } - /// Hold an exclusive write transaction on `root`'s current derived - /// call-graph database, so a reader without a busy wait gets - /// "database is locked" until the returned connection rolls back. - fn lazy_lock_derived( - storage: &Path, - root: &Path, - ) -> crate::db::file_identity::IdentityConnection { - let (generation, built) = lazy_current_generation(storage, root); - assert!(built, "{root:?} has a derived call graph to lock"); - let view = - crate::views::ViewStore::open(storage, &crate::path_identity::project_scope_key(root)) - .unwrap(); - crate::views::wait_for_derived_checkpoint_for_test(view.view_dir()); - let writer = crate::db::file_identity::IdentityConnection::open( - &view.derived_path(&generation).unwrap(), - "lazy_callgraph_lock_test", - ) - .unwrap(); - // In WAL mode a writer never blocks readers; the rollback journal makes - // the exclusive lock visible to them, as a checkpoint resetting the WAL - // or the last connection closing it is. - writer - .execute_batch("PRAGMA journal_mode=DELETE; BEGIN EXCLUSIVE;") - .unwrap(); - writer - } - - #[test] - fn linked_worktree_waits_for_a_locked_main_graph_instead_of_building() { - let _artifact_guard = artifact_owner_test_lock(); - let _git_env = crate::test_env::hermetic_git_env_guard(); - let _disable_watcher = EnvVarGuard::set("AFT_TEST_DISABLE_FILE_WATCHER", "1"); - let fixture = lazy_worktree(true); - // Main's publication holds its derived database while the worktree, - // whose files match main's, asks for callers. - let writer = lazy_lock_derived(&fixture.storage, &fixture.main); - match fixture.borrower.callgraph_store_for_ops() { - CallgraphStoreAccess::Building => {} - CallgraphStoreAccess::Ready(store) => panic!( - "a locked main graph must report building, not answer from {:?}", - store.sqlite_path() - ), - CallgraphStoreAccess::Error(error) => { - panic!("a locked main graph failed the query: {error}") - } - _ => panic!("a locked main graph must report building"), - } - // A lock is no verdict: no build started, and main answers once free. - assert!(fixture.borrower.defer_callgraph_build()); - assert_eq!( - fixture.borrower.lazy_callgraph_builds_scheduled_for_test(), - 0 - ); - assert_eq!( - lazy_derived_files(&lazy_view_dir(&fixture.storage, &fixture.linked)), - Vec::::new() - ); - writer.execute_batch("ROLLBACK").unwrap(); - drop(writer); - let store = lazy_ready(fixture.borrower.callgraph_store_for_ops()); - assert!(store - .sqlite_path() - .starts_with(lazy_view_dir(&fixture.storage, &fixture.main))); - } - #[test] fn build_key_memo_does_not_bypass_first_configure_identity_bind() { let _git_env = crate::test_env::hermetic_git_env_guard(); @@ -13364,7 +13183,7 @@ mod tests { } #[test] - fn views_restart_dirty_source_reports_working_tree_refresh_until_publication() { + fn views_restart_dirty_source_answers_the_working_tree_at_once() { let _git_env = crate::test_env::hermetic_git_env_guard(); let _disable_watcher = EnvVarGuard::set("AFT_TEST_DISABLE_FILE_WATCHER", "1"); let project = tempfile::tempdir().unwrap(); @@ -13424,18 +13243,8 @@ mod tests { params: json!({"file": root.join("tracked.rs"), "symbol": "dirty"}), }; let response = crate::commands::callers::handle_callers(&request, &ctx); - assert_eq!(response.data["code"], "callgraph_building", "{response:?}"); - assert_eq!(response.data["progress"]["phase"], "working_tree_refresh"); - assert_eq!(response.data["progress"]["pending_paths"], 1); - assert!(response.data["progress"]["waiting_on"] - .to_string() - .contains("view publication")); - assert!( - ctx.publish_view_paths(BTreeSet::new(), true) - .unwrap() - .published - ); - let response = crate::commands::callers::handle_callers(&request, &ctx); + // The checkout view verifies the working tree for the query itself, so + // the dirty source answers at once, without waiting for publication. assert!(response.success, "{response:?}"); assert_eq!(response.data["total_callers"], 1); } diff --git a/crates/aft/src/inspect/manager.rs b/crates/aft/src/inspect/manager.rs index c6dcc0a606..cfe9b5ca57 100644 --- a/crates/aft/src/inspect/manager.rs +++ b/crates/aft/src/inspect/manager.rs @@ -5781,16 +5781,6 @@ fn current_view_projection_result( result } -/// The reader Tier-2 dead-code projection uses for `project_root`'s published -/// view, for tests that compare it with an independent build. -#[cfg(test)] -pub(crate) fn current_view_projection_store_for_test( - project_root: &Path, - config: &crate::config::Config, -) -> Option<(ReadonlyCallGraphStore, String)> { - current_view_projection_store(project_root, Path::new(""), config, &[]) -} - fn build_view_callgraph_snapshot( job: &InspectJob, projection_cache: Option<&InspectManager>, diff --git a/crates/aft/src/inspect/mod.rs b/crates/aft/src/inspect/mod.rs index 567cd3ef45..f8d036b44f 100644 --- a/crates/aft/src/inspect/mod.rs +++ b/crates/aft/src/inspect/mod.rs @@ -29,9 +29,7 @@ pub use job::{ JobOutcome, JobScope, JobStatus, WorkerCtx, }; #[cfg(test)] -pub(crate) use manager::{ - current_view_projection_store_for_test, filter_payload_for_scope_for_test, InspectBuilderState, -}; +pub(crate) use manager::{filter_payload_for_scope_for_test, InspectBuilderState}; pub use manager::{InspectManager, Tier2RunSubmission, Tier2RunSubmissionError}; pub use phase_log::{ format_wait_text, inspect_phase_log_for_request, InspectPhaseEntry, InspectPhaseId, From f70bec694326400097b6666ae09777ce3e508a1d Mon Sep 17 00:00:00 2001 From: ualtinok <94532+ualtinok@users.noreply.github.com> Date: Tue, 27 Oct 2026 04:42:35 +0100 Subject: [PATCH 6/6] Search-quality descriptor for views step G Co-authored-by: alfonso-aft <289616620+alfonso-aft@users.noreply.github.com> --- .../land-views-g-donors-callgraph.json | 15 +++++++++++++++ 1 file changed, 15 insertions(+) create mode 100644 benchmarks/aft-search/slice-descriptors/land-views-g-donors-callgraph.json diff --git a/benchmarks/aft-search/slice-descriptors/land-views-g-donors-callgraph.json b/benchmarks/aft-search/slice-descriptors/land-views-g-donors-callgraph.json new file mode 100644 index 0000000000..d39eb6b492 --- /dev/null +++ b/benchmarks/aft-search/slice-descriptors/land-views-g-donors-callgraph.json @@ -0,0 +1,15 @@ +{ + "slice_class": "engine_unwired", + "targeted_mechanism": "none", + "kind": "harness", + "fixtures": [ + "crates/aft/tests/integration/tool_call_parity_test.rs", + "crates/aft/src/search_index.rs", + "crates/aft/src/commands/semantic_search/mod.rs", + "crates/aft/src/commands/semantic_search/exact_lane.rs" + ], + "slices": [ + "views-g-donors-callgraph" + ], + "note": "Views-on call-graph reads route through the checkout's v3 view with sealed base donors. Step G touches no ranking-fence file; the fence files in this diff come from step F below it in the stack. Views are default-off, so the benchmark runs the views-off path and ranked rows must remain byte-identical to the committed reference." +}