diff --git a/crates/tracedecay-code-index-runtime/src/project_reads/ignored_dependency_admission_tests.rs b/crates/tracedecay-code-index-runtime/src/project_reads/ignored_dependency_admission_tests.rs index 35ab833191..9135cf4f90 100644 --- a/crates/tracedecay-code-index-runtime/src/project_reads/ignored_dependency_admission_tests.rs +++ b/crates/tracedecay-code-index-runtime/src/project_reads/ignored_dependency_admission_tests.rs @@ -21,7 +21,6 @@ use tracedecay_tool_catalog::{CapabilityId, UseCaseId}; use crate::code_index_scheduler::{CodeIndexSchedulerRegistryV1, LatestCompleteCodeIndexV1}; use crate::project_reads::project_code_index_ignored_dependency_admission_port; -use tracedecay_runtime_core::path_safety::canonical_existing_identity; const PROJECT_ID: &str = "project.project-open-ignored-dependency"; @@ -70,7 +69,6 @@ export function generationAnchor() { return 1; } .mount_worktree(project_id(), root.path(), store.path().to_path_buf()) .await .expect("mount code-index scheduler"); - wait_for_initial_generation(®istry, root.path()).await; let baseline = latest(®istry, root.path()).await; let generation = baseline.generation(); let snapshot = generation.snapshot(); @@ -242,31 +240,11 @@ fn request_context(scope: ResolvedScope, suffix: &str) -> RequestContext { .expect("request context") } -async fn wait_for_initial_generation(registry: &CodeIndexSchedulerRegistryV1, project_root: &Path) { - if registry.latest_generation_id(project_root).await.is_some() { - return; - } - let canonical_root = canonical_existing_identity(project_root).expect("canonical fixture root"); - let mut publications = registry.subscribe_generation_publications(); - tokio::time::timeout(Duration::from_secs(5), async { - loop { - let event = publications.recv().await.expect("generation publication"); - if event.project_root == canonical_root { - break; - } - } - }) - .await - .expect("initial generation publication"); -} - /// Wait for the complete serving generation, not merely its publication. /// -/// The generation-publication broadcast and `latest_generation_id` both fire -/// on the reconcile pass, ahead of the bind and the serving swap that fill -/// `serving_generation`. Reading the complete arm the instant -/// `wait_for_initial_generation` returns therefore raced the seat and reported -/// a missing generation for a store that had one. +/// Publication and `latest_generation_id` precede the serving swap. Wait +/// for the actual fresh serving state before using imports as admission +/// evidence; no earlier broadcast establishes readiness. async fn latest( registry: &CodeIndexSchedulerRegistryV1, project_root: &Path, diff --git a/crates/tracedecay-graph-db/src/generation_runtime.rs b/crates/tracedecay-graph-db/src/generation_runtime.rs index c8accc16b4..d2fdd35dd5 100644 --- a/crates/tracedecay-graph-db/src/generation_runtime.rs +++ b/crates/tracedecay-graph-db/src/generation_runtime.rs @@ -1837,7 +1837,7 @@ impl GraphDb { // and nothing serves it, so its directory leaves the disk now // instead of waiting, possibly forever, on a project that never // publishes again, behind the deferred row delete. - self.retire_sealed_generation_store(locator); + self.retire_sealed_generation_store(locator)?; return Ok(GenerationContentsDeletion::RetentionPending); } let mut state = self.wait_verified_generations_write()?; @@ -1858,7 +1858,7 @@ impl GraphDb { state.heads.remove(&locator.projection); } drop(state); - self.retire_sealed_generation_store(locator); + self.retire_sealed_generation_store(locator)?; Ok(GenerationContentsDeletion::Deleted) } @@ -2270,7 +2270,7 @@ impl GraphDb { drop(state); // A quarantined generation's rows are in doubt; its derived sealed // artifact must not outlive that doubt. - self.retire_sealed_generation_store(&locator); + self.retire_sealed_generation_store(&locator)?; Ok(()) } } @@ -2799,8 +2799,8 @@ mod tests { }; use super::{ - GenerationLocator, GenerationStageOutcome, StagedGenerationRowsDeletion, - generation_stage_pages, + GenerationContentsDeletion, GenerationLocator, GenerationStageOutcome, + StagedGenerationRowsDeletion, generation_stage_pages, }; fn manifest(source: &str, watermark: &str) -> GraphGenerationManifest { @@ -3977,4 +3977,49 @@ mod tests { ); owner.close().unwrap(); } + + #[test] + fn sealed_generation_retirement_keeps_unlink_errors_retryable() { + let temp = TempDir::new().unwrap(); + let (owner, database) = persistent_database(&temp); + let generation = manifest("source:one", "watermark:one"); + let locator = + GenerationLocator::new(generation.projection.clone(), generation.generation.clone()); + let namespace = generation.identity().physical_namespace().unwrap(); + let directory = temp.path().join("commit-metadata.sealed").join( + namespace + .as_str() + .strip_prefix("generation:") + .unwrap_or(namespace.as_str()), + ); + std::fs::create_dir_all(directory.parent().unwrap()).unwrap(); + // A regular file in the exact artifact slot deterministically refuses + // remove_dir_all even on hosts that run the test with full privileges. + std::fs::write(&directory, b"not a directory").unwrap(); + + let error = database + .delete_generation_contents(&locator, &|| Ok(())) + .expect_err("a failed sealed unlink must not complete retention"); + assert!( + error + .to_string() + .contains("sealed directory removal failed") + ); + assert_eq!(std::fs::read(&directory).unwrap(), b"not a directory"); + + std::fs::remove_file(&directory).unwrap(); + std::fs::create_dir(&directory).unwrap(); + std::fs::write(directory.join("derived-rows"), b"retired").unwrap(); + assert!(matches!( + database.delete_generation_contents(&locator, &|| Ok(())), + Ok(GenerationContentsDeletion::Deleted) + )); + assert!(!directory.exists()); + // Idempotent replay after the directory is gone still succeeds. + assert!(matches!( + database.delete_generation_contents(&locator, &|| Ok(())), + Ok(GenerationContentsDeletion::Deleted) + )); + owner.close().unwrap(); + } } diff --git a/crates/tracedecay-graph-db/src/sealed_store.rs b/crates/tracedecay-graph-db/src/sealed_store.rs index a6e140d089..e644148982 100644 --- a/crates/tracedecay-graph-db/src/sealed_store.rs +++ b/crates/tracedecay-graph-db/src/sealed_store.rs @@ -472,12 +472,12 @@ impl SealedGenerationStore { ) } - /// Best-effort teardown used only when the generation is quarantined or - /// retired, or before staging rows become releasable. A sealed-only head - /// is never discarded through this path while it remains serveable. - fn discard(&self) { + /// Retirement tears down the reader and requires a successful artifact + /// unlink. A failed unlink remains a typed error for the durable cleanup + /// owner to retry; a sealed-only live head never enters this path. + fn discard(&self) -> Result<(), GraphDbError> { let _ = self.close(); - remove_sealed_directory(&self.directory); + remove_sealed_directory_checked(&self.directory) } } @@ -751,15 +751,28 @@ pub(crate) fn sweep_abandoned_row_spills(database_path: &Path) { } } -fn remove_sealed_directory(directory: &Path) { +fn remove_sealed_directory_checked(directory: &Path) -> Result<(), GraphDbError> { match std::fs::remove_dir_all(directory) { - Ok(()) => {} - Err(error) if error.kind() == std::io::ErrorKind::NotFound => {} - Err(_) => { - // Retirement re-runs are idempotent; a leaked directory is - // re-collected on the next retirement or rebuild of this - // generation and never serves reads without a matching receipt. - } + Ok(()) => Ok(()), + Err(error) if error.kind() == std::io::ErrorKind::NotFound => Ok(()), + Err(error) => Err(sealed_store_io_failure( + "sealed directory removal failed", + error, + )), + } +} + +/// Staging and invalid-artifact cleanup are best effort. Retiring an accepted +/// replay instead uses the checked path, so a failed unlink cannot be reported +/// as a completed retirement and lose its durable retry ownership. +fn remove_sealed_directory(directory: &Path) { + if let Err(error) = remove_sealed_directory_checked(directory) { + tracing::warn!( + event = "sealed_artifact_cleanup_deferred", + path = %directory.display(), + %error, + "sealed artifact cleanup did not complete" + ); } } @@ -1657,30 +1670,31 @@ impl GraphDb { /// Retires the sealed artifact for `locator`: uninstalls the reader and /// deletes its directory. Idempotent, and never touches staging rows. #[tracing::instrument(name = "graph_db.sealed_store.retire", level = "trace", skip_all)] - pub(crate) fn retire_sealed_generation_store(&self, locator: &GenerationLocator) { + pub(crate) fn retire_sealed_generation_store( + &self, + locator: &GenerationLocator, + ) -> Result<(), GraphDbError> { let removed = self .inner .sealed_generations .write() - .ok() - .and_then(|mut sealed| sealed.remove(locator)); + .map_err(|_| GraphDbError::unavailable("sealed generation registry lock poisoned"))? + .remove(locator); if let Some(store) = removed { - store.discard(); - return; + return store.discard(); } - // No installed reader: still delete any on-disk artifact so a - // retired generation leaves nothing behind. + // A removed in-memory reader is not necessary for the artifact to + // exist on disk. Unlink by exact locator even after a prior failed + // retirement; a durable tombstone can then retry without the reader. let Some(reopen) = self.inner.reopen.as_ref() else { - return; + return Ok(()); }; let Some(database_path) = reopen.config.path.as_ref() else { - return; - }; - let Ok(physical_namespace) = locator.physical_namespace() else { - return; + return Ok(()); }; + let physical_namespace = locator.physical_namespace()?; let root = sealed_store_root(database_path); - remove_sealed_directory(&sealed_generation_directory(&root, &physical_namespace)); + remove_sealed_directory_checked(&sealed_generation_directory(&root, &physical_namespace)) } }