diff --git a/src/storage/managed.rs b/src/storage/managed.rs index fee2231..9efdec6 100644 --- a/src/storage/managed.rs +++ b/src/storage/managed.rs @@ -767,7 +767,26 @@ 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())?; + let removal = remove_path_without_symlinks(generation.root()); + // 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 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 + // 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(()) } diff --git a/tests/generation_storage.rs b/tests/generation_storage.rs index a669353..9f1706f 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,64 @@ 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_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) + .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); + 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" + ); + + 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"].is_array()); + + 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() {