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." +} 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/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..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) { @@ -12128,6 +12105,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 +12747,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() @@ -12892,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"); @@ -12958,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( @@ -12986,10 +12799,6 @@ mod tests { "edited_caller" ) .is_ok()); - assert_eq!( - fixture.borrower.lazy_callgraph_builds_scheduled_for_test(), - 1 - ); } #[test] @@ -13075,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] @@ -13084,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(|_| { @@ -13096,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 { @@ -13129,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 @@ -13171,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] @@ -13187,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( @@ -13285,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(); @@ -13454,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(); @@ -13514,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); } @@ -15716,6 +15435,252 @@ 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< + Arc, + crate::views::callgraph::CallgraphViewError, + > { + 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..bdd25298d9 100644 --- a/crates/aft/src/commands/inspect.rs +++ b/crates/aft/src/commands/inspect.rs @@ -708,12 +708,34 @@ 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; + // 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), @@ -758,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 }; - let use_checkout_view = needs_ephemeral_view + // 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 @@ -830,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 a8c9d47d79..5a469ac653 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, crate::views::callgraph::CallgraphViewError>)>> = + 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,36 @@ 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), + // 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(error)) => CallgraphStoreAccess::Error(CallGraphStoreError::Unavailable( + format!("callgraph view unavailable: {error}"), + )), + 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 +12111,105 @@ 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, crate::views::callgraph::CallgraphViewError>> + { + 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.into())), + } + } + 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 { @@ -20292,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", @@ -20312,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 @@ -20387,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(); @@ -20437,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 { @@ -20459,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(); @@ -20605,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 { @@ -20622,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/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/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/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/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, 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/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..a1d2f59567 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( @@ -61,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, ) @@ -234,10 +246,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, @@ -285,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() @@ -337,6 +352,341 @@ 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, CallgraphViewError>; + 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, CallgraphViewError> { + 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::>(), + ) + .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.into()) + } + 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| CallgraphViewError { + busy: error.reason.starts_with(DERIVED_STORE_BUSY), + reason: 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, CallgraphViewError> { + 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 { @@ -359,6 +709,176 @@ 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()); + // 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") + ); + + // 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(); 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/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/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/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, diff --git a/crates/aft/src/views/mod.rs b/crates/aft/src/views/mod.rs index 67e7433e8e..2762d828ff 100644 --- a/crates/aft/src/views/mod.rs +++ b/crates/aft/src/views/mod.rs @@ -14,12 +14,10 @@ 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::{ - 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) {