From 3ff840709998553dded70248986d59b2391ff6b2 Mon Sep 17 00:00:00 2001 From: DanConwayDev Date: Tue, 29 Sep 2026 09:19:54 +0000 Subject: [PATCH] fix(purgatory): preserve pending PR refs on graceful shutdown A persisted placeholder is useless if shutdown deletes the ref it tracks. Leave pending refs and their objects on disk alongside the final purgatory checkpoint; expiry remains responsible for removing abandoned uploads. Update the runtime description and add a SIGTERM regression test. Assume normal Git persistence and the existing checkpoint lifecycle. Crash recovery and object compaction remain separate changes. Validation: nix develop -c cargo test --test purgatory graceful_shutdown_keeps_pending_pr_refs passed, observing process exit and the surviving ref and commit without fixed sleeps. Assisted-by: GPT-6 --- docs/explanation/architecture.md | 2 +- src/server.rs | 11 ++----- tests/common/relay.rs | 17 ++++++++++ tests/purgatory.rs | 53 ++++++++++++++++++++++++++++++++ 4 files changed, 73 insertions(+), 10 deletions(-) diff --git a/docs/explanation/architecture.md b/docs/explanation/architecture.md index 7a08abd..e5ae8b5 100644 --- a/docs/explanation/architecture.md +++ b/docs/explanation/architecture.md @@ -113,7 +113,7 @@ runtime: pair in check-only or repair mode - Serve HTTP + WebSocket until a caller-supplied shutdown future resolves, then stop background mutation, persist a final state snapshot - (purgatory and rejected-events cache), and clean up placeholder refs + (purgatory and rejected-events cache), preserving pending PR refs for restart and expiry **Key Dependencies:** diff --git a/src/server.rs b/src/server.rs index 865ccc7..d47e712 100644 --- a/src/server.rs +++ b/src/server.rs @@ -579,15 +579,8 @@ impl RelayServer { info!("Rejected events cache saved to disk"); } - // Cleanup placeholder refs on shutdown - let placeholder_ids = self.purgatory.get_placeholder_event_ids(); - if !placeholder_ids.is_empty() { - info!( - "Cleaning up {} placeholder refs/nostr/ refs on shutdown", - placeholder_ids.len() - ); - git::cleanup_placeholder_refs(&self.git_data_path, &placeholder_ids); - } + // Pending refs and their staged objects must survive shutdown together + // with the checkpoint. Expiry owns their removal after restart. result } diff --git a/tests/common/relay.rs b/tests/common/relay.rs index 3a24c4c..13caed2 100644 --- a/tests/common/relay.rs +++ b/tests/common/relay.rs @@ -1120,6 +1120,23 @@ impl TestRelay { } /// Stop the relay + /// Stop with SIGTERM so the relay runs its shutdown path, then reap it. + pub async fn stop_gracefully(mut self) { + let status = std::process::Command::new("kill") + .args(["-TERM", &self.process.id().to_string()]) + .status() + .expect("send SIGTERM to relay"); + assert!(status.success(), "SIGTERM delivery failed"); + let deadline = std::time::Instant::now() + std::time::Duration::from_secs(30); + while self.process.try_wait().expect("poll relay").is_none() { + assert!( + std::time::Instant::now() < deadline, + "relay did not exit after SIGTERM" + ); + tokio::task::yield_now().await; + } + } + pub async fn stop(mut self) { // kill() sends SIGKILL; reap directly instead of guessing a grace period. let _ = self.process.kill(); diff --git a/tests/purgatory.rs b/tests/purgatory.rs index 73f85ca..21edc03 100644 --- a/tests/purgatory.rs +++ b/tests/purgatory.rs @@ -87,3 +87,56 @@ isolated_purgatory_test!(test_state_event_served_after_git_push); isolated_purgatory_test!(test_pr_event_accepted_into_purgatory_and_isnt_served); isolated_purgatory_test!(test_pr_event_in_purgatory_git_push_accepted); isolated_purgatory_test!(test_pr_event_served_after_git_push); + +#[tokio::test] +async fn graceful_shutdown_keeps_pending_pr_refs() { + use nostr_sdk::prelude::*; + let persistent = tempfile::tempdir().unwrap(); + let git_data = persistent.path().join("git"); + let relay = TestRelay::start_with_existing_lmdb_paths( + git_data.clone(), + persistent.path().join("relay"), + None, + false, + ) + .await; + let client = AuditClient::new(relay.url(), AuditConfig::isolated()) + .await + .unwrap(); + let (announcement, identifier) = + common::publish_served_repo(&client, "shutdown-pending-pr").await; + let local = tempfile::tempdir().unwrap(); + let tip = + common::create_test_repo_with_commit(local.path(), common::CommitVariant::PrTest).unwrap(); + // The event is never sent, so the pushed ref stays a pending placeholder. + let event = common::create_pr_event( + client.keys(), + &common::announcement_coordinate(&announcement, &identifier), + &tip, + "pending across shutdown", + ) + .unwrap(); + let npub = client.public_key().to_bech32().unwrap(); + let reference = format!("refs/nostr/{}", event.id); + common::push_ref_to_relay( + local.path(), + &relay.domain(), + &npub, + &identifier, + &tip, + &reference, + ) + .unwrap(); + let view = git_data.join(&npub).join(format!("{identifier}.git")); + assert_eq!( + ngit_grasp::git::get_ref_commit(&view, &reference).as_deref(), + Some(tip.as_str()) + ); + relay.stop_gracefully().await; + assert_eq!( + ngit_grasp::git::get_ref_commit(&view, &reference).as_deref(), + Some(tip.as_str()), + "shutdown must keep the pending ref for expiry after restart" + ); + assert!(ngit_grasp::git::oid_exists(&view, &tip)); +}