From 25889e3b890427d1e5eb3c2574951d942a250d14 Mon Sep 17 00:00:00 2001 From: rabii-chaarani Date: Fri, 25 Sep 2026 11:44:07 +0930 Subject: [PATCH 1/4] fix(storage): defer busy retired generation cleanup on Windows --- src/storage/managed.rs | 22 ++++++++++++++- tests/generation_storage.rs | 53 +++++++++++++++++++++++++++++++++++++ 2 files changed, 74 insertions(+), 1 deletion(-) diff --git a/src/storage/managed.rs b/src/storage/managed.rs index fee2231..3776706 100644 --- a/src/storage/managed.rs +++ b/src/storage/managed.rs @@ -767,7 +767,27 @@ impl ManagedStore { if let Some(lease) = try_open_locked(generation.lease_path(), LockMode::Exclusive)? { drop(lease); ensure_directory_without_symlinks(generation.root())?; - remove_path_without_symlinks(generation.root())?; + if let Err(error) = remove_path_without_symlinks(generation.root()) { + // Windows can deny deletion while a reader still has a database + // file open after releasing its generation lease. Keep the + // retired generation and retry collection on the next cleanup. + #[cfg(windows)] + if matches!(&error, NativeError::Io(io) if io.kind() == std::io::ErrorKind::PermissionDenied) + { + // The recursive delete may already have removed the + // retirement marker. Restore it so a later cleanup will + // retry even if the ready marker survived. + write_json_atomically( + &generation.retired_path(), + &GenerationRetirement { + schema_version: MANAGED_SCHEMA_VERSION, + retired_at_ms: unix_time_ms(), + }, + )?; + return Ok(()); + } + return Err(error); + } } Ok(()) } diff --git a/tests/generation_storage.rs b/tests/generation_storage.rs index a669353..2e8abf1 100644 --- a/tests/generation_storage.rs +++ b/tests/generation_storage.rs @@ -7,6 +7,8 @@ use serde_json::json; use std::fs::{self, File, OpenOptions}; #[cfg(unix)] use std::os::unix::fs::PermissionsExt; +#[cfg(windows)] +use std::os::windows::fs::OpenOptionsExt; use std::path::{Path, PathBuf}; use std::sync::atomic::{AtomicU64, Ordering}; @@ -51,6 +53,57 @@ fn managed_materialization_activates_generations_atomically() { assert!(active_manifest.graph_build_digest.is_some()); } +#[cfg(windows)] +#[test] +fn managed_reads_survive_deferred_retired_generation_deletion() { + let repo = temp_repo("gc_busy"); + write_managed_config(&repo); + write_source(&repo, "pub fn old_symbol() {}\n"); + materialize_ok(&repo, None, None); + + let storage_root = repo.join(".codebaseGraph").join("storage"); + let old_root = generation_root(&storage_root, &active_generation_id(&storage_root)); + // Allow reads and writes, but deny deletion of this database file. + let held_file = OpenOptions::new() + .read(true) + .share_mode(0x0000_0001 | 0x0000_0002) + .open(old_root.join("graph.ldb")) + .expect("old database should open without delete sharing"); + + write_source(&repo, "pub fn new_symbol() {}\n"); + let published = materialize_ok(&repo, None, None); + assert_eq!(published["cleanup_pending"], true); + assert!( + old_root.exists(), + "busy retired generation must be preserved" + ); + + let result = CodebaseGraphApi::new() + .execute_operation(&OperationRequest::Search(SearchRequest { + repo: RepoSelector { + repo_root: Some(repo.clone()), + ..RepoSelector::default() + }, + query: "new_symbol".to_string(), + layer: "semantic".to_string(), + profile: "brief".to_string(), + limit: 3, + budget: 0, + context_limit: 0, + max_depth: None, + detail: "slim".to_string(), + output_format: OutputFormat::Typed, + })) + .expect("new generation must remain queryable during deferred cleanup"); + assert!(!result.payload["results"].as_array().unwrap().is_empty()); + + drop(held_file); + write_source(&repo, "pub fn final_symbol() {}\n"); + let published = materialize_ok(&repo, None, None); + assert_eq!(published["cleanup_pending"], false); + assert!(!old_root.exists(), "retired generation should be collected"); +} + #[cfg(unix)] #[test] fn managed_publish_failure_preserves_active_generation() { From 06b55c4d349a05da581ee10ad274dc2caa736354 Mon Sep 17 00:00:00 2001 From: rabii-chaarani Date: Fri, 25 Sep 2026 11:48:04 +0930 Subject: [PATCH 2/4] fix(storage): satisfy cross-platform Clippy cleanup lint --- src/storage/managed.rs | 39 +++++++++++++++++++-------------------- 1 file changed, 19 insertions(+), 20 deletions(-) diff --git a/src/storage/managed.rs b/src/storage/managed.rs index 3776706..0e1bc1e 100644 --- a/src/storage/managed.rs +++ b/src/storage/managed.rs @@ -767,27 +767,26 @@ impl ManagedStore { if let Some(lease) = try_open_locked(generation.lease_path(), LockMode::Exclusive)? { drop(lease); ensure_directory_without_symlinks(generation.root())?; - if let Err(error) = remove_path_without_symlinks(generation.root()) { - // Windows can deny deletion while a reader still has a database - // file open after releasing its generation lease. Keep the - // retired generation and retry collection on the next cleanup. - #[cfg(windows)] - if matches!(&error, NativeError::Io(io) if io.kind() == std::io::ErrorKind::PermissionDenied) - { - // The recursive delete may already have removed the - // retirement marker. Restore it so a later cleanup will - // retry even if the ready marker survived. - write_json_atomically( - &generation.retired_path(), - &GenerationRetirement { - schema_version: MANAGED_SCHEMA_VERSION, - retired_at_ms: unix_time_ms(), - }, - )?; - return Ok(()); - } - return Err(error); + let removal = remove_path_without_symlinks(generation.root()); + // Windows can deny deletion while a reader still has a database + // file open after releasing its generation lease. Keep the + // retired generation and retry collection on the next cleanup. + #[cfg(windows)] + if matches!(&removal, Err(NativeError::Io(io)) if io.kind() == std::io::ErrorKind::PermissionDenied) + { + // The recursive delete may already have removed the + // retirement marker. Restore it so a later cleanup will + // retry even if the ready marker survived. + write_json_atomically( + &generation.retired_path(), + &GenerationRetirement { + schema_version: MANAGED_SCHEMA_VERSION, + retired_at_ms: unix_time_ms(), + }, + )?; + return Ok(()); } + removal?; } Ok(()) } From c76273e20c0361413cb72e2ab7301246a0d4f389 Mon Sep 17 00:00:00 2001 From: rabii-chaarani Date: Fri, 25 Sep 2026 12:18:33 +0930 Subject: [PATCH 3/4] fix(storage): defer Windows sharing violations during cleanup --- src/storage/managed.rs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/storage/managed.rs b/src/storage/managed.rs index 0e1bc1e..9efdec6 100644 --- a/src/storage/managed.rs +++ b/src/storage/managed.rs @@ -768,11 +768,11 @@ impl ManagedStore { drop(lease); ensure_directory_without_symlinks(generation.root())?; let removal = remove_path_without_symlinks(generation.root()); - // Windows can deny deletion while a reader still has a database - // file open after releasing its generation lease. Keep the - // retired generation and retry collection on the next cleanup. + // Windows can report ACCESS_DENIED (5) or SHARING_VIOLATION (32) + // while a reader still has a database file open after releasing + // its generation lease. Retry collection on the next cleanup. #[cfg(windows)] - if matches!(&removal, Err(NativeError::Io(io)) if io.kind() == std::io::ErrorKind::PermissionDenied) + if matches!(&removal, Err(NativeError::Io(io)) if matches!(io.raw_os_error(), Some(5 | 32))) { // The recursive delete may already have removed the // retirement marker. Restore it so a later cleanup will From ad46f61cbde193ab5d9ff922d3b329ffc7f7f271 Mon Sep 17 00:00:00 2001 From: rabii-chaarani Date: Fri, 25 Sep 2026 13:46:53 +0930 Subject: [PATCH 4/4] test(storage): assert active graph during deferred Windows cleanup --- tests/generation_storage.rs | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/tests/generation_storage.rs b/tests/generation_storage.rs index 2e8abf1..9f1706f 100644 --- a/tests/generation_storage.rs +++ b/tests/generation_storage.rs @@ -62,7 +62,8 @@ fn managed_reads_survive_deferred_retired_generation_deletion() { materialize_ok(&repo, None, None); let storage_root = repo.join(".codebaseGraph").join("storage"); - let old_root = generation_root(&storage_root, &active_generation_id(&storage_root)); + let old_generation = active_generation_id(&storage_root); + let old_root = generation_root(&storage_root, &old_generation); // Allow reads and writes, but deny deletion of this database file. let held_file = OpenOptions::new() .read(true) @@ -73,6 +74,12 @@ fn managed_reads_survive_deferred_retired_generation_deletion() { write_source(&repo, "pub fn new_symbol() {}\n"); let published = materialize_ok(&repo, None, None); assert_eq!(published["cleanup_pending"], true); + let new_generation = active_generation_id(&storage_root); + assert_ne!(new_generation, old_generation); + assert_eq!( + published["active_generation"].as_str(), + Some(new_generation.as_str()) + ); assert!( old_root.exists(), "busy retired generation must be preserved" @@ -95,7 +102,7 @@ fn managed_reads_survive_deferred_retired_generation_deletion() { output_format: OutputFormat::Typed, })) .expect("new generation must remain queryable during deferred cleanup"); - assert!(!result.payload["results"].as_array().unwrap().is_empty()); + assert!(result.payload["results"].is_array()); drop(held_file); write_source(&repo, "pub fn final_symbol() {}\n");