diff --git a/CHANGELOG.md b/CHANGELOG.md index 74fa6ad..e73ed26 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -13,7 +13,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 to `refs/nostr/` whose PR event is not yet known is staged in the repository that received it, promoted into shared storage when the event is accepted, and reclaimed after its pending ref expires. Pushes backed by a - signed State or PR event are stored as before. + signed State or PR event are stored as before. Empty staged `/prs/` + repositories are removed after their staging metadata is reclaimed. - Accept a GRASP-06 `/prs/` PR event whose pushed ref survived a crash that lost its purgatory placeholder, matching it by the event's service-local diff --git a/docs/explanation/git-family-object-storage.md b/docs/explanation/git-family-object-storage.md index 8389bbb..53e5022 100644 --- a/docs/explanation/git-family-object-storage.md +++ b/docs/explanation/git-family-object-storage.md @@ -290,6 +290,11 @@ This closes the window between receive-pack and the periodic purgatory checkpoint. A view whose remaining staged objects belong to pending refs is parked until the next request rather than polled. +Runtime cleanup of an empty staged `/prs/` view also requests maintenance +instead of deleting the directory directly. The worker reclaims its objects +and unregisters staging before removing the empty view, so expiry and failed +pushes cannot leave orphaned registry records. + ### Limits - Staging is not a storage quota. It bounds how long an unsigned upload is diff --git a/src/git/staging/recovery.rs b/src/git/staging/recovery.rs index 62d921c..9fd71e3 100644 --- a/src/git/staging/recovery.rs +++ b/src/git/staging/recovery.rs @@ -216,9 +216,10 @@ mod tests { } } let purgatory = Purgatory::new(fixture.storage.git_data_path()); + let locks = crate::grasp06::receive::new_repo_init_locks(); purgatory.set_prs_cleanup_ctx(crate::purgatory::PrsCleanupCtx { git_data_path: fixture.storage.git_data_path().to_owned(), - repo_init_locks: Default::default(), + repo_init_locks: locks.clone(), }); recover_placeholders(&fixture.storage, &purgatory, &db) .await @@ -231,11 +232,15 @@ mod tests { .await; purgatory.cleanup(); if prs { + super::super::worker::maintain(&fixture.view, &locks) + .await + .unwrap(); assert!(!fixture.view.exists()); } else { compact(&fixture.view).unwrap(); assert!(!object_exists(&fixture.view, &oid).unwrap()); } + assert!(!view.record.exists()); } } diff --git a/src/git/staging/worker.rs b/src/git/staging/worker.rs index b186419..d03cc52 100644 --- a/src/git/staging/worker.rs +++ b/src/git/staging/worker.rs @@ -263,4 +263,52 @@ mod tests { assert_eq!(outcome.unwrap(), Compaction::Done); assert!(!fixture.view.exists()); } + + #[tokio::test] + async fn prs_expiry_reclaims_the_registry_before_removing_the_view() { + use crate::purgatory::{PrsCleanupCtx, Purgatory}; + use nostr_sdk::prelude::Keys; + + let submitter = Keys::generate().public_key(); + let fixture = Fixture::with_view(&format!("prs/{}", submitter.to_hex())); + stage(&fixture.view, &[]).unwrap(); + let abandoned = commit(&fixture.view, "abandoned", None); + super::super::stage_upload( + &fixture.view, + &[], + &[super::super::Tip::new(PENDING, &abandoned)], + ) + .unwrap(); + reference(&fixture.view, PENDING, &abandoned); + let record = View::resolve(&fixture.view).unwrap().unwrap().record; + let locks = new_repo_init_locks(); + let purgatory = Purgatory::new(fixture.storage.git_data_path()); + purgatory.set_prs_cleanup_ctx(PrsCleanupCtx { + git_data_path: fixture.storage.git_data_path().to_owned(), + repo_init_locks: locks.clone(), + }); + purgatory.recover_upload_placeholder( + PENDING.strip_prefix("refs/nostr/").unwrap().to_owned(), + abandoned.clone(), + (submitter, fixture.key.identifier.clone()), + true, + std::time::SystemTime::UNIX_EPOCH, + ); + + assert_eq!(purgatory.cleanup(), (0, 0, 1)); + assert!( + fixture.view.exists(), + "expiry must leave the view for staging maintenance" + ); + assert!(record.exists()); + + let worker = tokio::spawn(run_worker(fixture.storage.clone(), locks)); + wait_until( + "expired PRS view and staging record to be reclaimed", + || !fixture.view.exists() && !record.exists(), + ) + .await; + assert!(!object_exists(&fixture.family(), &abandoned).unwrap()); + worker.abort(); + } } diff --git a/src/grasp06/receive.rs b/src/grasp06/receive.rs index 231d102..16b1de0 100644 --- a/src/grasp06/receive.rs +++ b/src/grasp06/receive.rs @@ -459,6 +459,8 @@ fn finish_prs_receive_pack(state: &PrsPathState, repo_path: &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. +/// Staged views are queued for maintenance, which unregisters staging before +/// calling back here to remove the directory. /// /// The caller holds `state.mu`. That mutex also gates `in_flight` updates, so /// a repository is never removed while a push is being received. @@ -469,6 +471,12 @@ pub(crate) fn remove_idle_empty_repo(state: &PrsPathState, repo_path: &Path) -> { return false; } + if crate::git::staging::is_staged(repo_path) { + // Deleting the view now would strand its registry record: maintenance + // cannot resolve or unregister a view after its directory is gone. + crate::git::staging::request_maintenance(repo_path); + return false; + } match std::fs::remove_dir_all(repo_path) { Ok(()) => { debug!(repo = %repo_path.display(), "Removed zero-ref /prs/ repository");