diff --git a/CHANGELOG.md b/CHANGELOG.md index ce524d9..40e0fa4 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,11 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +### Fixed + +- Remove abandoned normal-endpoint PR refs when their pending event expires, + retaining exact repository scopes across restarts and retrying failed cleanup. + ## [3.0.5] - 2026-09-25 Improve live sync startup, bound retries against unhealthy relays, fix Git push diff --git a/docs/explanation/architecture.md b/docs/explanation/architecture.md index 6b633ea..7a08abd 100644 --- a/docs/explanation/architecture.md +++ b/docs/explanation/architecture.md @@ -525,7 +525,13 @@ See [`types.rs`](../../src/purgatory/types.rs) for complete definitions: - Creates `Arc` at startup - Passes purgatory to both write policy and git handlers - Passes `RepositoryLifecycle` directly to HTTP git serving and deletion/recovery -- Spawns background cleanup task (60-second interval) +- Spawns background cleanup task (60-second interval). Normal-endpoint PR + placeholders persist every pushed owner/identifier and commit. After 30 minutes + without the event, expiry takes repository lifecycle write locks, rechecks the + placeholder, and compare-and-deletes only those recorded refs. Failed deletions + keep the placeholder for retry. `/prs/` copies retain their separate cleanup. + Old unscoped placeholders cannot safely identify a repository for online cleanup; + the authorization-integrity checker preserves unmatched refs for inspection. #### Thread Safety diff --git a/docs/explanation/purgatory-design.md b/docs/explanation/purgatory-design.md index 9a671aa..59ae03c 100644 --- a/docs/explanation/purgatory-design.md +++ b/docs/explanation/purgatory-design.md @@ -598,7 +598,7 @@ sequenceDiagram end else No PR event anywhere (git-data-first) GitHandler->>GitProcess: Execute push - accept any commit - GitHandler->>Purgatory: add_pr_placeholder(event_id, commit) + GitHandler->>Purgatory: add_standard_pr_placeholder(event_id, commit, owner, identifier) GitHandler->>GitClient: Push accepted - awaiting PR event end end @@ -606,6 +606,14 @@ sequenceDiagram --- +Normal-endpoint git-first placeholders persist each destination owner, repository +identifier, and pushed commit. The 60-second expiry task acquires those repositories' +lifecycle write locks before checking the record again. If the event is still absent +after 30 minutes, it compare-and-deletes the recorded refs, retaining the placeholder +on failure for retry. A database check protects accepted events whose placeholders +survived in an older checkpoint. Legacy records without destinations remain readable, +but cannot safely drive scoped online ref deletion. + ## Background Sync Purgatory includes a background sync system that fetches git data from remote servers when events arrive before git data. diff --git a/src/git/authorization.rs b/src/git/authorization.rs index 3eed02a..0fb85ec 100644 --- a/src/git/authorization.rs +++ b/src/git/authorization.rs @@ -88,35 +88,38 @@ pub async fn authorize_push( // Standard endpoint passes `None` for prs_url: signer / a-tag // identifier are enforced by the surrounding maintainer-set // authorization, not by the URL. - match pre_validate_refs_nostr_push(database, purgatory, new_oid, ref_name, None).await { - NostrRefPreValidation::Rejected { reason } => { - warn!("refs/nostr/ validation failed: {}", reason); - return Ok(AuthorizationResult::denied(reason)); - } - NostrRefPreValidation::Authorized { - event_from_purgatory, - } => { - if let Some(event) = event_from_purgatory { - debug!("Found matching PR event in purgatory for ref {}", ref_name); - purgatory_events.push(event); - } else { - debug!("Ref {} validated against existing record", ref_name); + let event_id = ref_name + .strip_prefix("refs/nostr/") + .expect("partitioned ref"); + let track_placeholder = + match pre_validate_refs_nostr_push(database, purgatory, new_oid, ref_name, None) + .await + { + NostrRefPreValidation::Rejected { reason } => { + warn!("refs/nostr/ validation failed: {}", reason); + return Ok(AuthorizationResult::denied(reason)); } - } - NostrRefPreValidation::Unknown => { - // No entry in DB or purgatory — create placeholder so - // the 30-minute sweep can clean the ref up if the PR - // event never arrives. Standard-endpoint placeholders - // carry no /prs/ scope. - let event_id_hex = ref_name - .strip_prefix("refs/nostr/") - .expect("shape validated in pre_validate_refs_nostr_push"); - purgatory.add_pr_placeholder(event_id_hex.to_string(), new_oid.clone()); - debug!( - "Created placeholder for {} - awaiting PR event (will expire in 30min if event doesn't arrive)", - event_id_hex - ); - } + NostrRefPreValidation::Authorized { + event_from_purgatory, + } => { + if let Some(event) = event_from_purgatory { + purgatory_events.push(event); + false + } else { + purgatory.find_pr_placeholder(event_id).is_some() + } + } + NostrRefPreValidation::Unknown => true, + }; + if track_placeholder { + // Remember every destination, including subsequent pushes of the + // same pending event, so expiry can delete only those exact refs. + purgatory.add_standard_pr_placeholder( + event_id.to_string(), + new_oid.clone(), + PublicKey::from_hex(selected_pubkey)?, + identifier.to_string(), + ); } } } diff --git a/src/git/authorization_integrity.rs b/src/git/authorization_integrity.rs index 526b72d..0ebea9a 100644 --- a/src/git/authorization_integrity.rs +++ b/src/git/authorization_integrity.rs @@ -601,6 +601,20 @@ fn build_expectation( for (event_id, entry) in pending_prs { let reference = format!("refs/nostr/{event_id}"); + if entry.event.is_none() { + if let ViewIdentity::Owner { pubkey } = identity { + if let Some(record) = entry + .standard_refs + .iter() + .find(|record| record.owner == *pubkey && record.identifier == identifier) + { + expected + .pending_pr_refs + .insert(reference, record.commit.clone()); + continue; + } + } + } match &entry.event { Some(event) if event_applies_to_view( @@ -620,7 +634,10 @@ fn build_expectation( .pending_pr_refs .insert(reference, entry.commit.clone()); } - None if entry.prs_scope.is_none() && matches!(identity, ViewIdentity::Owner { .. }) => { + None if entry.prs_scope.is_none() + && entry.standard_refs.is_empty() + && matches!(identity, ViewIdentity::Owner { .. }) => + { // Legacy standard-endpoint placeholders did not record their // owner/identifier. Preserve a matching ref, but make the // uncertainty visible rather than treating it as authority. @@ -1574,6 +1591,41 @@ mod tests { assert_eq!(expectation.pr_refs.len(), 1); } + #[test] + fn standard_placeholder_authorizes_only_recorded_owner_and_identifier() { + let owner = Keys::generate().public_key(); + let other = Keys::generate().public_key(); + let purgatory = crate::purgatory::Purgatory::new(PathBuf::new()); + let id = "ab".repeat(32); + let commit = "cd".repeat(20); + purgatory.add_standard_pr_placeholder(id.clone(), commit.clone(), owner, "project".into()); + let pending = vec![(id.clone(), purgatory.find_pr(&id).unwrap())]; + let data = RepositoryData { + announcements: vec![], + states: vec![], + }; + for (pubkey, identifier, matches) in [ + (owner, "project", true), + (other, "project", false), + (owner, "other", false), + ] { + let expectation = build_expectation( + &ViewIdentity::Owner { pubkey }, + identifier, + &data, + &[], + &pending, + &[], + None, + ); + assert_eq!( + expectation.pending_pr_refs.get(&format!("refs/nostr/{id}")), + matches.then_some(&commit) + ); + assert!(expectation.ambiguous_placeholders.is_empty()); + } + } + #[test] fn owner_view_accepts_only_its_exact_standard_clone_endpoint() { let source_owner = Keys::generate(); diff --git a/src/purgatory/mod.rs b/src/purgatory/mod.rs index 1671d45..91386aa 100644 --- a/src/purgatory/mod.rs +++ b/src/purgatory/mod.rs @@ -101,6 +101,8 @@ struct SerializablePrPurgatoryEntry { /// deserialisable. #[serde(default)] prs_scope: Option, + #[serde(default)] + standard_refs: Vec, } /// Serializable wrapper for `AnnouncementPurgatoryEntry` with time offsets. @@ -200,7 +202,7 @@ pub struct Purgatory { /// Stored as EventId (hex string) for efficient lookup. expired_events: Arc>, - _git_data_path: PathBuf, + git_data_path: PathBuf, /// Set once at startup by [`Purgatory::set_prs_cleanup_ctx`] to give /// [`Purgatory::cleanup`] enough information to remove abandoned @@ -234,7 +236,7 @@ impl Purgatory { pr_events: Arc::new(DashMap::new()), sync_queue: Arc::new(DashMap::new()), expired_events: Arc::new(DashMap::new()), - _git_data_path: git_data_path.into(), + git_data_path: git_data_path.into(), prs_cleanup_ctx: std::sync::OnceLock::new(), } } @@ -493,6 +495,7 @@ impl Purgatory { created_at: now, expires_at: now + DEFAULT_EXPIRY, source, + standard_refs: Vec::new(), prs_scope: None, }; @@ -521,12 +524,152 @@ impl Purgatory { created_at: now, expires_at: now + DEFAULT_EXPIRY, source: types::EventSource::Direct, // Git pushes are direct user actions + standard_refs: Vec::new(), prs_scope: None, }; self.pr_events.insert(event_id, entry); } + /// Record a normal-endpoint push without forgetting other copies of the ref. + pub fn add_standard_pr_placeholder( + &self, + event_id: String, + commit: String, + owner: PublicKey, + identifier: String, + ) { + let now = Instant::now(); + let mut entry = self + .pr_events + .entry(event_id) + .or_insert_with(|| PrPurgatoryEntry { + event: None, + commit: commit.clone(), + created_at: now, + expires_at: now + DEFAULT_EXPIRY, + source: types::EventSource::Direct, + prs_scope: None, + standard_refs: Vec::new(), + }); + if entry.event.is_some() { + return; + } + entry + .standard_refs + .retain(|r| r.owner != owner || r.identifier != identifier); + entry.standard_refs.push(types::StandardPrRef { + owner, + identifier, + commit, + }); + entry.expires_at = now + DEFAULT_EXPIRY; + } + + /// Expire normal-endpoint placeholders under the same lifecycle locks as pushes. + /// Recheck the entry after waiting: an event or a new push may have arrived. + /// Failed Git operations retain the record for the next sweep. + pub async fn cleanup_standard_pr_refs( + &self, + lifecycle: &crate::nostr::lifecycle::RepositoryLifecycle, + database: &crate::nostr::SharedDatabase, + ) -> usize { + let candidates: Vec<_> = self + .pr_events + .iter() + .filter(|e| { + e.event.is_none() && !e.standard_refs.is_empty() && e.expires_at <= Instant::now() + }) + .map(|e| (e.key().clone(), e.standard_refs.clone())) + .collect(); + let mut removed = 0; + for (id, refs) in candidates { + let _guards = lifecycle + .write_repositories(refs.iter().map(|r| crate::nostr::lifecycle::RecoveryScope { + owner_pubkey_hex: r.owner.to_hex(), + identifier: r.identifier.clone(), + })) + .await; + // A checkpoint can retain a placeholder after its event was accepted. + // Missing metadata must never cause us to delete a now-authorized ref. + let accepted = match EventId::from_hex(&id) { + Ok(event_id) => match database.event_by_id(&event_id).await { + Ok(event) => event.is_some(), + Err(error) => { + tracing::warn!(%id, %error, "Cannot check PR acceptance; deferring ref expiry"); + continue; + } + }, + Err(_) => continue, + }; + // Holding the map entry serializes event arrival/removal and placeholder refresh. + let dashmap::mapref::entry::Entry::Occupied(mut slot) = + self.pr_events.entry(id.clone()) + else { + continue; + }; + let entry = slot.get_mut(); + let should_remove = (|| { + if entry.event.is_some() + || entry.expires_at > Instant::now() + || entry.standard_refs != refs + { + return false; + } + if accepted { + return true; + } + let mut ok = true; + for r in &refs { + let path = self + .git_data_path + .join(r.owner.to_bech32().expect("public key")) + .join(format!("{}.git", r.identifier)); + match path.try_exists() { + Ok(false) => continue, + Err(error) => { + tracing::warn!(repo = %path.display(), %error, "Cannot inspect PR repository; deferring expiry"); + ok = false; + continue; + } + Ok(true) => {} + } + let reference = format!("refs/nostr/{id}"); + // A previous partial sweep or a rejected push may leave no ref. + match crate::git::list_refs(&path) { + Ok(refs) if !refs.iter().any(|(name, _)| name == &reference) => continue, + Err(error) => { + tracing::warn!(repo = %path.display(), %error, "Cannot inspect PR refs; deferring expiry"); + ok = false; + continue; + } + Ok(_) => {} + } + // Compare-and-delete prevents expiry from removing a replaced ref. + let result = std::process::Command::new("git") + .args(["update-ref", "-d", &reference, &r.commit]) + .current_dir(&path) + .output(); + if !matches!(result, Ok(ref output) if output.status.success()) { + tracing::warn!(repo = %path.display(), %reference, + "Failed to expire normal PR ref; retaining placeholder for retry"); + ok = false; + } + } + if ok && entry.prs_scope.is_some() { + entry.standard_refs.clear(); + return false; // The synchronous sweep still owns the /prs/ ref. + } + ok + })(); + if should_remove { + slot.remove(); + removed += 1; + } + } + removed + } + /// Add a PR placeholder created by a push to the GRASP-06 `/prs/` /// endpoint (06.md line 12). /// @@ -552,19 +695,31 @@ impl Purgatory { identifier: String, ) { let now = Instant::now(); - let entry = PrPurgatoryEntry { + let mut slot = self + .pr_events + .entry(event_id) + .or_insert_with(|| PrPurgatoryEntry { + event: None, + commit: commit.clone(), + created_at: now, + expires_at: now + DEFAULT_EXPIRY, + source: types::EventSource::Direct, + prs_scope: None, + standard_refs: Vec::new(), + }); + let standard_refs = std::mem::take(&mut slot.standard_refs); + *slot = PrPurgatoryEntry { event: None, commit, created_at: now, expires_at: now + DEFAULT_EXPIRY, source: types::EventSource::Direct, + standard_refs, prs_scope: Some(types::PrsPlaceholderScope { submitter, identifier, }), }; - - self.pr_events.insert(event_id, entry); } /// Find state events waiting for a specific repository identifier. @@ -977,7 +1132,7 @@ impl Purgatory { if let Some((repo_path, was_soft_expired)) = revival_info { if was_soft_expired { if !repo_path.exists() { - let storage = crate::git::storage::LocalGitStorage::new(&self._git_data_path); + let storage = crate::git::storage::LocalGitStorage::new(&self.git_data_path); let result = crate::git::storage::FamilyKey::sha1(identifier) .and_then(|family| storage.create_thin_view(&family, &repo_path)); match result { @@ -1301,7 +1456,8 @@ impl Purgatory { .iter() .filter(|entry| { let value = entry.value(); - value.expires_at <= now + value.standard_refs.is_empty() + && value.expires_at <= now && !value .event .as_ref() @@ -1658,6 +1814,7 @@ impl Purgatory { expires_at_offset_secs: expires_offset.as_secs(), source: e.source, prs_scope: e.prs_scope.clone(), + standard_refs: e.standard_refs.clone(), }; pr_events.insert(event_id, serializable); } @@ -1830,6 +1987,7 @@ impl Purgatory { expires_at, source: e.source, prs_scope: e.prs_scope, + standard_refs: e.standard_refs, }; self.pr_events.insert(event_id, entry); @@ -3547,3 +3705,6 @@ fn add_prs_pr_placeholder_does_not_overwrite_existing_scoped_placeholder() { assert_eq!(scope.submitter, submitter_b.public_key()); assert_eq!(scope.identifier, "repo-b"); } + +#[cfg(test)] +mod standard_ref_tests; diff --git a/src/purgatory/standard_ref_tests.rs b/src/purgatory/standard_ref_tests.rs new file mode 100644 index 0000000..2ae0a9f --- /dev/null +++ b/src/purgatory/standard_ref_tests.rs @@ -0,0 +1,281 @@ +use super::*; +use crate::nostr::lifecycle::RepositoryLifecycle; +fn database() -> crate::nostr::SharedDatabase { + Arc::new(nostr_memory::MemoryDatabase::unbounded()) +} +fn refs(path: &Path) -> Result, String> { + crate::git::list_refs(path).map(|refs| refs.into_iter().collect()) +} + +fn git(path: &Path, args: &[&str]) -> String { + let output = std::process::Command::new("git") + .current_dir(path) + .env("GIT_AUTHOR_NAME", "Test") + .env("GIT_AUTHOR_EMAIL", "test@example.com") + .env("GIT_COMMITTER_NAME", "Test") + .env("GIT_COMMITTER_EMAIL", "test@example.com") + .args(args) + .output() + .unwrap(); + assert!( + output.status.success(), + "{}", + String::from_utf8_lossy(&output.stderr) + ); + String::from_utf8(output.stdout).unwrap().trim().to_owned() +} + +fn repository(root: &Path, owner: PublicKey, name: &str) -> (PathBuf, String) { + let path = root + .join(owner.to_bech32().unwrap()) + .join(format!("{name}.git")); + std::fs::create_dir_all(&path).unwrap(); + git(&path, &["init", "--bare"]); + let tree = git(&path, &["mktree"]); + let commit = git(&path, &["commit-tree", &tree, "-m", "test"]); + (path, commit) +} + +fn expire(p: &Purgatory, id: &str) { + p.pr_events.get_mut(id).unwrap().expires_at = Instant::now() - Duration::from_secs(1); +} + +#[tokio::test] +async fn standard_refs_expire_after_restart_without_touching_other_refs() { + let root = tempfile::tempdir().unwrap(); + let owner = Keys::generate().public_key(); + let p = Purgatory::new(root.path()); + let id = "ab".repeat(32); + let reference = format!("refs/nostr/{id}"); + let mut paths = Vec::new(); + for name in ["one", "two"] { + let (path, commit) = repository(root.path(), owner, name); + git(&path, &["update-ref", &reference, &commit]); + git(&path, &["update-ref", "refs/heads/main", &commit]); + p.add_standard_pr_placeholder(id.clone(), commit, owner, name.into()); + paths.push(path); + } + let (unrelated, commit) = repository(root.path(), owner, "unrelated"); + git(&unrelated, &["update-ref", &reference, &commit]); + let state = root.path().join("purgatory.json"); + p.save_to_disk(&state).unwrap(); + let restored = Purgatory::new(root.path()); + restored.restore_from_disk(&state).unwrap(); + assert_eq!(restored.find_pr(&id).unwrap().standard_refs.len(), 2); + expire(&restored, &id); + assert_eq!( + restored.cleanup().2, + 0, + "sync sweep must retain cleanup metadata" + ); + assert_eq!( + restored + .cleanup_standard_pr_refs(&RepositoryLifecycle::in_memory(), &database()) + .await, + 1 + ); + assert!(restored.find_pr(&id).is_none()); + for path in paths { + let refs = refs(&path).unwrap(); + assert!(!refs.contains_key(&reference)); + assert!(refs.contains_key("refs/heads/main")); + } + assert!(refs(&unrelated).unwrap().contains_key(&reference)); +} + +#[tokio::test] +async fn standard_ref_expiry_retries_lock_failure_and_preserves_replaced_ref() { + let root = tempfile::tempdir().unwrap(); + let owner = Keys::generate().public_key(); + let (path, commit) = repository(root.path(), owner, "one"); + let p = Purgatory::new(root.path()); + let id = "cd".repeat(32); + let reference = format!("refs/nostr/{id}"); + git(&path, &["update-ref", &reference, &commit]); + p.add_standard_pr_placeholder(id.clone(), commit.clone(), owner, "one".into()); + expire(&p, &id); + let lifecycle = RepositoryLifecycle::in_memory(); + let db = database(); + let lock = path.join(format!("{reference}.lock")); + std::fs::write(&lock, b"").unwrap(); + assert_eq!(p.cleanup_standard_pr_refs(&lifecycle, &db).await, 0); + assert!(p.find_pr(&id).is_some()); + std::fs::remove_file(lock).unwrap(); + let tree = git(&path, &["mktree"]); + let replacement = git(&path, &["commit-tree", &tree, "-m", "replacement"]); + git(&path, &["update-ref", &reference, &replacement]); + assert_eq!(p.cleanup_standard_pr_refs(&lifecycle, &db).await, 0); + assert_eq!(refs(&path).unwrap()[&reference], replacement); + git(&path, &["update-ref", &reference, &commit]); + assert_eq!(p.cleanup_standard_pr_refs(&lifecycle, &db).await, 1); +} + +#[tokio::test] +async fn standard_ref_expiry_rechecks_refresh_and_event_arrival_after_push_lock() { + let root = tempfile::tempdir().unwrap(); + let owner = Keys::generate().public_key(); + let (path, commit) = repository(root.path(), owner, "one"); + let p = Purgatory::new(root.path()); + let id = "ef".repeat(32); + let reference = format!("refs/nostr/{id}"); + git(&path, &["update-ref", &reference, &commit]); + let lifecycle = RepositoryLifecycle::in_memory(); + let db = database(); + for event_arrives in [false, true] { + p.add_standard_pr_placeholder(id.clone(), commit.clone(), owner, "one".into()); + expire(&p, &id); + let guard = lifecycle.read_repository(&owner.to_hex(), "one").await; + let cleanup = p.cleanup_standard_pr_refs(&lifecycle, &db); + tokio::pin!(cleanup); + assert!(futures_util::poll!(&mut cleanup).is_pending()); + if event_arrives { + p.remove_pr(&id); + } else { + p.add_standard_pr_placeholder(id.clone(), commit.clone(), owner, "one".into()); + } + drop(guard); + assert_eq!( + tokio::time::timeout(Duration::from_secs(5), cleanup) + .await + .unwrap(), + 0 + ); + assert!(refs(&path).unwrap().contains_key(&reference)); + } +} + +#[tokio::test] +async fn standard_push_authorization_records_each_repository_and_legacy_state_still_loads() { + let root = tempfile::tempdir().unwrap(); + let owner = Keys::generate().public_key(); + let database: crate::nostr::SharedDatabase = + Arc::new(nostr_memory::MemoryDatabase::unbounded()); + let p = Arc::new(Purgatory::new(root.path())); + let id = "12".repeat(32); + let commit = "34".repeat(20); + let line = format!( + "{} {commit} refs/nostr/{id}\0report-status\n", + "0".repeat(40) + ); + let body = hyper::body::Bytes::from(format!("{:04x}{line}0000", line.len() + 4)); + for name in ["one", "two", "one"] { + let result = crate::git::authorization::authorize_push( + &database, + name, + &owner.to_hex(), + &body, + &p, + root.path(), + ) + .await + .unwrap(); + assert!(result.authorized); + } + assert_eq!(p.find_pr(&id).unwrap().standard_refs.len(), 2); + let state = root.path().join("state.json"); + p.save_to_disk(&state).unwrap(); + let mut json: serde_json::Value = + serde_json::from_str(&std::fs::read_to_string(&state).unwrap()).unwrap(); + for entry in json["pr_events"].as_object_mut().unwrap().values_mut() { + entry.as_object_mut().unwrap().remove("standard_refs"); + } + std::fs::write(&state, serde_json::to_vec(&json).unwrap()).unwrap(); + let restored = Purgatory::new(root.path()); + restored.restore_from_disk(&state).unwrap(); + assert!(restored.find_pr(&id).unwrap().standard_refs.is_empty()); +} + +#[tokio::test] +async fn standard_and_prs_copies_both_expire() { + let root = tempfile::tempdir().unwrap(); + let owner = Keys::generate().public_key(); + let (path, commit) = repository(root.path(), owner, "one"); + let p = Purgatory::new(root.path()); + p.set_prs_cleanup_ctx(PrsCleanupCtx { + git_data_path: root.path().to_path_buf(), + repo_init_locks: Default::default(), + }); + let id = "56".repeat(32); + let reference = format!("refs/nostr/{id}"); + git(&path, &["update-ref", &reference, &commit]); + p.add_standard_pr_placeholder(id.clone(), commit.clone(), owner, "one".into()); + p.add_prs_pr_placeholder(id.clone(), commit.clone(), owner, "one".into()); + let prs = crate::grasp06::paths::prs_repo_path(root.path(), &owner.to_hex(), "one"); + std::fs::create_dir_all(prs.parent().unwrap()).unwrap(); + git( + root.path(), + &[ + "clone", + "--bare", + path.to_str().unwrap(), + prs.to_str().unwrap(), + ], + ); + git(&prs, &["update-ref", &reference, &commit]); + expire(&p, &id); + assert_eq!( + p.cleanup_standard_pr_refs(&RepositoryLifecycle::in_memory(), &database()) + .await, + 0 + ); + assert!(!refs(&path).unwrap().contains_key(&reference)); + assert_eq!(p.cleanup().2, 1); + assert!(!prs.exists()); + assert!(p.find_pr(&id).is_none()); +} + +#[tokio::test] +async fn standard_ref_expiry_preserves_event_accepted_after_last_checkpoint() { + let root = tempfile::tempdir().unwrap(); + let keys = Keys::generate(); + let owner = keys.public_key(); + let (path, commit) = repository(root.path(), owner, "one"); + let event = EventBuilder::new(Kind::GitPullRequest, "") + .tags([Tag::custom("c", [commit.clone()])]) + .finalize(&keys) + .unwrap(); + let id = event.id.to_hex(); + let reference = format!("refs/nostr/{id}"); + git(&path, &["update-ref", &reference, &commit]); + let p = Purgatory::new(root.path()); + p.add_standard_pr_placeholder(id.clone(), commit, owner, "one".into()); + expire(&p, &id); + let db = database(); + db.save_event(&event).await.unwrap(); + assert_eq!( + p.cleanup_standard_pr_refs(&RepositoryLifecycle::in_memory(), &db) + .await, + 1 + ); + assert!(p.find_pr(&id).is_none()); + assert!(refs(&path).unwrap().contains_key(&reference)); +} + +#[tokio::test] +async fn standard_ref_expiry_retries_after_partial_cleanup() { + let root = tempfile::tempdir().unwrap(); + let owner = Keys::generate().public_key(); + let p = Purgatory::new(root.path()); + let id = "78".repeat(32); + let reference = format!("refs/nostr/{id}"); + let mut paths = Vec::new(); + for name in ["one", "two", "rejected-push"] { + let (path, commit) = repository(root.path(), owner, name); + if name != "rejected-push" { + git(&path, &["update-ref", &reference, &commit]); + } + p.add_standard_pr_placeholder(id.clone(), commit, owner, name.into()); + paths.push(path); + } + expire(&p, &id); + let lock = paths[1].join(format!("{reference}.lock")); + std::fs::write(&lock, b"").unwrap(); + let lifecycle = RepositoryLifecycle::in_memory(); + let db = database(); + assert_eq!(p.cleanup_standard_pr_refs(&lifecycle, &db).await, 0); + assert!(!refs(&paths[0]).unwrap().contains_key(&reference)); + assert!(refs(&paths[1]).unwrap().contains_key(&reference)); + std::fs::remove_file(lock).unwrap(); + assert_eq!(p.cleanup_standard_pr_refs(&lifecycle, &db).await, 1); + assert!(p.find_pr(&id).is_none()); +} diff --git a/src/purgatory/types.rs b/src/purgatory/types.rs index dd19d59..ff15b6b 100644 --- a/src/purgatory/types.rs +++ b/src/purgatory/types.rs @@ -145,6 +145,10 @@ pub struct PrPurgatoryEntry { #[serde(default)] pub source: EventSource, + /// Exact normal-endpoint refs awaiting this event; persisted for expiry cleanup. + #[serde(default)] + pub standard_refs: Vec, + /// If set, this placeholder was created by a push to the GRASP-06 /// `/prs//.git` endpoint (06.md line 12). When /// the corresponding PR event arrives, its signer MUST equal @@ -205,3 +209,11 @@ pub struct AnnouncementPurgatoryEntry { /// Whether the bare repo has been deleted (soft expiry) pub soft_expired: bool, } + +/// A pushed normal-endpoint ref, including the value cleanup may safely delete. +#[derive(Debug, Clone, Serialize, Deserialize, PartialEq, Eq)] +pub struct StandardPrRef { + pub owner: PublicKey, + pub identifier: String, + pub commit: String, +} diff --git a/src/server.rs b/src/server.rs index f1ab589..865ccc7 100644 --- a/src/server.rs +++ b/src/server.rs @@ -388,11 +388,15 @@ impl RelayServer { // Spawn background cleanup task for purgatory entries (60s interval) let cleanup_purgatory = purgatory.clone(); + let cleanup_lifecycle = relay_runtime.lifecycle.clone(); + let cleanup_database = relay_runtime.stores.database.clone(); background_tasks.push(tokio::spawn(async move { let mut interval = tokio::time::interval(Duration::from_secs(60)); loop { interval.tick().await; + let standard_removed = cleanup_purgatory.cleanup_standard_pr_refs(&cleanup_lifecycle, &cleanup_database).await; let (announcement_removed, state_removed, pr_removed) = cleanup_purgatory.cleanup(); + let pr_removed = pr_removed + standard_removed; if announcement_removed > 0 || state_removed > 0 || pr_removed > 0 { info!( "Purgatory cleanup: removed {} announcements, {} state events, {} PR events",