diff --git a/src/grasp06/receive.rs b/src/grasp06/receive.rs index bea8c49..ad3605a 100644 --- a/src/grasp06/receive.rs +++ b/src/grasp06/receive.rs @@ -436,26 +436,51 @@ fn ensure_repo_initialised( fn finish_prs_receive_pack(state: &PrsPathState, repo_path: &Path) { let _g = state.mu.lock().expect("prs path mutex poisoned"); state.in_flight.fetch_sub(1, Ordering::Relaxed); - if state.in_flight.load(Ordering::Relaxed) == 0 { - if let Ok(refs) = list_refs(repo_path) { - if refs.is_empty() { - if let Err(e) = std::fs::remove_dir_all(repo_path) { - warn!( - "/prs/ receive-pack: failed to clean up empty repo {}: {}", - repo_path.display(), - e - ); - } else { - debug!( - "/prs/ receive-pack: removed empty repo {} (no refs after push)", - repo_path.display() - ); - } - } + remove_idle_empty_repo(state, repo_path); +} + +/// Remove a `/prs/` repository that has no refs and no push in flight, so +/// abandoned repositories do not accumulate. Returns whether it was removed. +/// +/// The caller holds `state.mu`. That mutex also gates `in_flight` updates, so +/// a repository is never removed while a push is being received. +pub(crate) fn remove_idle_empty_repo(state: &PrsPathState, repo_path: &Path) -> bool { + if state.in_flight.load(Ordering::Relaxed) != 0 + || !matches!(list_refs(repo_path), Ok(refs) if refs.is_empty()) + { + return false; + } + match std::fs::remove_dir_all(repo_path) { + Ok(()) => { + debug!(repo = %repo_path.display(), "Removed zero-ref /prs/ repository"); + true + } + Err(error) => { + warn!( + repo = %repo_path.display(), + %error, + "Failed to remove zero-ref /prs/ repository" + ); + false } } } +/// [`remove_idle_empty_repo`] for callers that do not already hold the path +/// mutex. Paths outside `/prs/` are left alone. +pub fn remove_prs_repo_if_empty( + locks: &RepoInitLocks, + git_data_path: &Path, + repo_path: &Path, +) -> bool { + if !crate::grasp06::paths::is_prs_repo_path(repo_path, git_data_path) { + return false; + } + let state = path_state(locks, repo_path); + let _g = state.mu.lock().expect("prs path mutex poisoned"); + remove_idle_empty_repo(&state, repo_path) +} + /// Own the live `/prs/` receive-pack after the request handler has returned its /// streaming response. /// diff --git a/src/nostr/lifecycle/deletion/pr_refs.rs b/src/nostr/lifecycle/deletion/pr_refs.rs index 84ca06e..cfba696 100644 --- a/src/nostr/lifecycle/deletion/pr_refs.rs +++ b/src/nostr/lifecycle/deletion/pr_refs.rs @@ -93,34 +93,16 @@ impl DeletionPolicy { .collect() } - fn cleanup_zero_ref_grasp06_repo_if_idle(&self, repo_path: &PathBuf) { - if !crate::grasp06::paths::is_prs_repo_path(repo_path, &self.ctx.git_data_path) { - return; - } - - let state = crate::grasp06::receive::path_state(&self.ctx.repo_init_locks, repo_path); - let _guard = state.mu.lock().expect("prs path mutex poisoned"); - - if state.in_flight.load(std::sync::atomic::Ordering::Relaxed) != 0 { - return; - } - - let is_zero_ref = matches!(git::list_refs(repo_path), Ok(refs) if refs.is_empty()); - if !is_zero_ref { - return; - } - - if let Err(e) = std::fs::remove_dir_all(repo_path) { - tracing::warn!( - repo = %repo_path.display(), - error = %e, - "Failed to remove zero-ref /prs/ repo while processing deletion request" - ); - return; - } - - if let Some(parent) = repo_path.parent() { - let _ = std::fs::remove_dir(parent); + fn cleanup_zero_ref_grasp06_repo_if_idle(&self, repo_path: &std::path::Path) { + let removed = crate::grasp06::receive::remove_prs_repo_if_empty( + &self.ctx.repo_init_locks, + &self.ctx.git_data_path, + repo_path, + ); + if removed { + if let Some(parent) = repo_path.parent() { + let _ = std::fs::remove_dir(parent); + } } } diff --git a/src/nostr/policy/pr_event.rs b/src/nostr/policy/pr_event.rs index ec9e07d..2aee877 100644 --- a/src/nostr/policy/pr_event.rs +++ b/src/nostr/policy/pr_event.rs @@ -179,29 +179,8 @@ impl PrEventPolicy { error = %e, "Failed to delete /prs/ ref while discarding scoped placeholder", ); - } else if state.in_flight.load(std::sync::atomic::Ordering::Relaxed) == 0 - && matches!( - crate::git::list_refs(&prs_repo), - Ok(refs) if refs.is_empty() - ) - { - // No refs left and no push in flight — the - // repo is now an empty husk. Drop the bare - // dir so /prs/ doesn't accumulate abandoned - // repos. Equivalent to the cleanup the - // receive handler does on push completion. - if let Err(e) = std::fs::remove_dir_all(&prs_repo) { - tracing::warn!( - repo = %prs_repo.display(), - error = %e, - "Failed to remove zero-ref /prs/ repo after discarding scoped placeholder", - ); - } else { - tracing::debug!( - repo = %prs_repo.display(), - "Removed zero-ref /prs/ repo after discarding scoped placeholder", - ); - } + } else { + crate::grasp06::receive::remove_idle_empty_repo(&state, &prs_repo); } } self.ctx.purgatory.remove_pr(&event_id); diff --git a/src/purgatory/mod.rs b/src/purgatory/mod.rs index 91386aa..fd6ee08 100644 --- a/src/purgatory/mod.rs +++ b/src/purgatory/mod.rs @@ -1589,24 +1589,8 @@ impl Purgatory { error = %e, "Failed to delete dangling /prs/ ref during purgatory expiry", ); - } else if state.in_flight.load(std::sync::atomic::Ordering::Relaxed) == 0 - && matches!( - crate::git::list_refs(&repo_path), - Ok(refs) if refs.is_empty() - ) - { - if let Err(e) = std::fs::remove_dir_all(&repo_path) { - tracing::warn!( - repo = %repo_path.display(), - error = %e, - "Failed to remove zero-ref /prs/ repo during purgatory expiry", - ); - } else { - tracing::debug!( - repo = %repo_path.display(), - "Removed zero-ref /prs/ repo during purgatory expiry", - ); - } + } else { + crate::grasp06::receive::remove_idle_empty_repo(&state, &repo_path); } } }