From 4665a00ea9d64102ff5206b483863d3e6a6cda84 Mon Sep 17 00:00:00 2001 From: DanConwayDev Date: Fri, 15 May 2026 18:49:45 +0000 Subject: [PATCH] fix(grasp06): inline zero-ref /prs/ cleanup under per-path lock MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Replaces the periodic /prs/ cleanup sweep (reverted in the previous commit) with inline zero-ref cleanups at the three sites that can leave a /prs//.git bare repo empty: 1. The /prs/ receive handler at the end of a push, already in place prior to the revert. 2. The PR-event policy when it discards a scoped placeholder whose incoming event fails the (signer, identifier, commit) check — deletes refs/nostr/ and, if that empties the repo, removes the bare directory in the same step. 3. The standard 30-minute purgatory expiry sweep when a scoped placeholder times out without a matching PR event arriving — same shape as (2), but reached from the synchronous cleanup loop, so the per-path lock is taken with try_lock and the filesystem cleanup is skipped (leaving a harmless dangling ref) if a push is currently in flight to the same path. All three sites share a single Arc>>> of per-`(submitter, identifier)` locks. The receive handler now holds its entry for the entire pipeline — `git init --bare` → `git-receive-pack` → per-ref validation → zero-ref cleanup — instead of only for the init step, so a concurrent push or off-push cleanup cannot remove the bare repo while it is still being written. The same lock map is plumbed into PolicyContext (used by pr_event.rs) and Purgatory (via a one-shot `set_prs_cleanup_ctx` setter wired in main, so tests can leave it unset and get the previous behaviour for in-memory entries). This delivers what the reverted commit was reaching for without the 370-line periodic walker, the mtime heuristic, or the `has_prs_scope`/`DEFAULT_EXPIRY` API surface area on Purgatory: the last ref always implies an immediate (or, under lock contention, a next-cycle) repo removal, and the only code path that ever deletes a /prs/ repo dir is one that already holds the per-path lock. Docs: - CHANGELOG.md, docs/how-to/enable-grasp-06.md: describe the three inline cleanup sites; drop the "periodic 10-minute sweep" line. - docs/explanation/architecture.md: drop the src/grasp06/cleanup.rs bullet; document the lock as held for the whole pipeline and shared with off-push cleanup paths. - docs/explanation/grasp-06-contributor-pr-submission.md: replace the "Periodic /prs/ cleanup" subsection with a "Zero-ref /prs/ cleanup" subsection enumerating the three sites and the shared lock map; update "On-demand bare repo creation" to reflect the wider lock scope. --- CHANGELOG.md | 2 +- docs/explanation/architecture.md | 3 +- .../grasp-06-contributor-pr-submission.md | 20 ++-- docs/how-to/enable-grasp-06.md | 12 +- src/grasp06/receive.rs | 66 ++++++----- src/http/mod.rs | 10 +- src/main.rs | 26 ++++- src/nostr/builder.rs | 12 +- src/nostr/policy/deletion.rs | 2 +- src/nostr/policy/mod.rs | 30 +++++ src/nostr/policy/pr_event.rs | 39 ++++++- src/purgatory/mod.rs | 103 +++++++++++++++++- 12 files changed, 269 insertions(+), 56 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 46dc339..6f248fd 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,7 +9,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Added -- **GRASP-06 contributor PR submission endpoint** (`NGIT_GRASP06_ENABLE`, default off). When enabled, the relay accepts unauthenticated `git push` of `refs/nostr/` to `/prs//.git` from any contributor, even for repositories this relay has no accepted announcement for. The corresponding PR (kind 1618) or PR Update (kind 1619) event is accepted into purgatory when its `clone` tag names this relay's `/prs//.git` endpoint and its `a` tag's d-tag matches the URL identifier. When the event and the push match (signer, d-tag, c-tag commit) the event is released from purgatory and the ref is mirrored into any accepted-announcement repos on this relay. Empty `/prs/` repos (probe pushes, mismatched events) are garbage-collected by the receive handler and by a periodic 10-minute sweep. GRASP-06 is advertised in NIP-11 `supported_grasps` when enabled. See [how-to/enable-grasp-06.md](docs/how-to/enable-grasp-06.md) and [explanation/grasp-06-contributor-pr-submission.md](docs/explanation/grasp-06-contributor-pr-submission.md). +- **GRASP-06 contributor PR submission endpoint** (`NGIT_GRASP06_ENABLE`, default off). When enabled, the relay accepts unauthenticated `git push` of `refs/nostr/` to `/prs//.git` from any contributor, even for repositories this relay has no accepted announcement for. The corresponding PR (kind 1618) or PR Update (kind 1619) event is accepted into purgatory when its `clone` tag names this relay's `/prs//.git` endpoint and its `a` tag's d-tag matches the URL identifier. When the event and the push match (signer, d-tag, c-tag commit) the event is released from purgatory and the ref is mirrored into any accepted-announcement repos on this relay. Empty `/prs/` repos (probe pushes, mismatched events) are garbage-collected inline at the three sites that can leave them empty: the receive handler at the end of a push, the PR-event policy when discarding a mismatched scoped placeholder, and the purgatory sweep when a scoped placeholder expires without a matching event. GRASP-06 is advertised in NIP-11 `supported_grasps` when enabled. See [how-to/enable-grasp-06.md](docs/how-to/enable-grasp-06.md) and [explanation/grasp-06-contributor-pr-submission.md](docs/explanation/grasp-06-contributor-pr-submission.md). ## [1.0.2] - 2026-04-10 diff --git a/docs/explanation/architecture.md b/docs/explanation/architecture.md index 2464038..d740bf3 100644 --- a/docs/explanation/architecture.md +++ b/docs/explanation/architecture.md @@ -554,9 +554,8 @@ Optional endpoint at `/prs//.git`, gated on `NGIT_GRASP06_ENAB - [`src/grasp06/endpoint.rs`](../../src/grasp06/endpoint.rs) — URL parsing. - [`src/grasp06/paths.rs`](../../src/grasp06/paths.rs) — on-disk path conventions under `/prs//.git`. - [`src/grasp06/fetch.rs`](../../src/grasp06/fetch.rs) — empty-repo synthesis for `info/refs` and `git-upload-pack` against repos that don't yet exist on disk. -- [`src/grasp06/receive.rs`](../../src/grasp06/receive.rs) — `git-receive-pack` with init-on-push, strict `refs/nostr/` ref-name validation, and per-ref post-push validation against the database and purgatory. +- [`src/grasp06/receive.rs`](../../src/grasp06/receive.rs) — `git-receive-pack` with init-on-push, strict `refs/nostr/` ref-name validation, and per-ref post-push validation against the database and purgatory. Holds a per-`(submitter, identifier)` lock for the whole pipeline (init → receive-pack → validation → zero-ref cleanup) so a concurrent push to the same path cannot delete the bare repo mid-receive. The same lock is taken (with `try_lock` in synchronous contexts) by the PR-event policy and the purgatory expiry sweep before either of them deletes a `/prs/` ref or removes a zero-ref bare repo. - [`src/grasp06/policy.rs`](../../src/grasp06/policy.rs) — strict clone-tag URL comparator used by the PR-event acceptance relaxation. -- [`src/grasp06/cleanup.rs`](../../src/grasp06/cleanup.rs) — periodic sweep that removes zero-ref `/prs/` repos older than the purgatory TTL. `/prs/` repos are intentionally isolated from other subsystems: empty-repo cleanup skips the `/prs/` subtree, the proactive-sync subsystem never discovers them because subscriptions are built from DB-resident announcements, and the standard repo landing page guards against ever matching a `/prs/` path. Full design: [GRASP-06 Contributor Pull Request Submission](grasp-06-contributor-pr-submission.md). Operator how-to: [Enable GRASP-06](../how-to/enable-grasp-06.md). diff --git a/docs/explanation/grasp-06-contributor-pr-submission.md b/docs/explanation/grasp-06-contributor-pr-submission.md index a0d2f0e..13e52b4 100644 --- a/docs/explanation/grasp-06-contributor-pr-submission.md +++ b/docs/explanation/grasp-06-contributor-pr-submission.md @@ -107,7 +107,7 @@ The flow mirrors the existing `refs/nostr/` path at the standard endpo #### On-demand bare repo creation -The first push to `/prs//.git` creates the bare repo on disk. A per-`(submitter, identifier)` `tokio::sync::Mutex` (kept in a `DashMap` on the `HttpService`, see [`crate::grasp06::receive::RepoInitLocks`](../../src/grasp06/receive.rs)) serialises only the `git init --bare` step. Once init has succeeded, git's own ref locking handles intra-push concurrency, so simultaneous pushes to the same path proceed in parallel. +The first push to `/prs//.git` creates the bare repo on disk. A per-`(submitter, identifier)` `tokio::sync::Mutex` (kept in a `DashMap` on the `HttpService`, see [`crate::grasp06::receive::RepoInitLocks`](../../src/grasp06/receive.rs)) is held for the entire push pipeline — on-demand `git init --bare`, `git-receive-pack`, per-ref validation, and the zero-ref cleanup at the end. Pushes to different `(submitter, identifier)` paths still run in parallel. The same lock map is also taken (via `try_lock` from synchronous contexts) by the PR-event policy and the purgatory expiry sweep before either of them inspects or removes a `/prs/` repo, so no off-push code path can delete the bare repo while a push is in flight. ### Event acceptance relaxation @@ -154,15 +154,21 @@ Placeholder entries created at `/prs/` should be validated against `(submitter = - **Proactive sync** (GRASP-02): `/prs/` repos are not replicated between relays. The proactive-sync subsystem derives every subscription from the DB-resident announcement set; `/prs/` repos have no announcement and so are excluded by construction. No filesystem walk discovers them. A GRASP-06 relay is authoritative for the PRs it accepts. Clients that need a PR should fetch it from the relay the event's `clone` tag names. - **Repo listings / NIP-11**: `/prs/` repos are not advertised as repositories. They are a submission side-channel, not first-class hosted repos. GRASP-06 itself is advertised in the relay's NIP-11 `supported_grasps` list when the flag is on. -### Periodic `/prs/` cleanup +### Zero-ref `/prs/` cleanup -In addition to the receive-handler's post-push zero-ref cleanup, a periodic sweep ([`src/grasp06/cleanup.rs`](../../src/grasp06/cleanup.rs)) walks `/prs/` every ten minutes (one second under `NGIT_TEST=1`) and removes any `/.git` directory that: +A `/prs//.git` bare repo can become zero-ref through three independent paths. Each is cleaned up inline at the site where the last ref is removed; there is no separate periodic sweep over `/prs/`: -1. has zero refs, -2. has no active `PrPurgatoryEntry` scoped to `(submitter=, identifier=)`, and -3. has a directory mtime older than the purgatory TTL ([`purgatory::DEFAULT_EXPIRY`](../../src/purgatory/mod.rs), 30 minutes). +1. **Probe push leaves no valid refs.** The `/prs/` receive handler validates each pushed `refs/nostr/` against the database and purgatory. If every ref fails validation, the bare repo is empty at the end of the push. The handler removes it before returning, under the per-path lock it has been holding since `git init --bare`. +2. **Scoped placeholder fails validation against a later-arriving event.** When a PR event arrives whose `(signer, identifier, commit)` does not match the placeholder a `/prs/` push registered, the PR-event policy ([`src/nostr/policy/pr_event.rs`](../../src/nostr/policy/pr_event.rs)) deletes the corresponding `refs/nostr/` ref and discards the placeholder. If that leaves the bare repo with zero refs, the policy also removes the directory. It takes the same per-path lock as the receive handler first, so it can never race with an in-flight push that is still writing other refs to the same path. +3. **Scoped placeholder expires without a matching event.** The standard purgatory sweep ([`src/purgatory/mod.rs`](../../src/purgatory/mod.rs), every 60 seconds) walks expiring `PrPurgatoryEntry` rows. For entries with a `prs_scope`, the sweep best-effort deletes the dangling `refs/nostr/` ref and, if that leaves the repo with zero refs, the bare repo itself. The sweep is synchronous, so it uses `try_lock` on the per-path mutex: if a push is currently in flight to that path the cleanup is skipped this cycle. In the worst case (sustained lock contention from rapid pushes) a dangling ref is left on disk; this is harmless because the repo is never zero-ref while the contended push is making progress, and any future push to the same path simply ignores the dangling ref. -The mtime check defends against deleting a repo out from under an in-flight push. Empty submitter directories are removed when the sweep empties them. +The shared lock map is wired through: + +- [`HttpService`](../../src/http/mod.rs) holds it as `repo_init_locks` and passes it into every `/prs/` request. +- [`PolicyContext`](../../src/nostr/policy/mod.rs) holds the same `Arc>` so the PR-event policy can lock without going through HTTP. +- [`Purgatory::set_prs_cleanup_ctx`](../../src/purgatory/mod.rs) wires the lock map (plus `git_data_path`) into purgatory at startup so the expiry sweep has everything it needs to act on a scoped placeholder. + +Off-push deletion paths and the receive handler therefore share a single source of truth for "is a push currently in flight to this `(submitter, identifier)`". ## Flow examples diff --git a/docs/how-to/enable-grasp-06.md b/docs/how-to/enable-grasp-06.md index d1c5888..4bec8b3 100644 --- a/docs/how-to/enable-grasp-06.md +++ b/docs/how-to/enable-grasp-06.md @@ -59,10 +59,13 @@ The push succeeds, the relay creates `/prs//.git` ## Storage cost -One bare repo per `(submitter, identifier)` combination under `/prs//.git`. Repos are garbage-collected automatically: +One bare repo per `(submitter, identifier)` combination under `/prs//.git`. Repos are garbage-collected inline at the three sites that can leave one empty — there is no separate periodic sweep over `/prs/`: - After receive-pack, the repo is removed immediately if it has zero refs left (probe pushes that produced no valid state). -- A periodic sweep (every 10 minutes) removes any zero-ref `/prs/` repo with no active purgatory entry, older than the purgatory TTL. +- When a PR event arrives that fails validation against a scoped `/prs/` placeholder, the corresponding `refs/nostr/` ref is deleted; if that leaves the repo with zero refs the directory is removed in the same step. +- When a scoped placeholder expires from purgatory without a matching PR event (default 30 minutes), the standard purgatory sweep deletes the dangling ref and, if it leaves the repo zero-ref, the bare repo itself. + +All three sites share the same per-`(submitter, identifier)` mutex so they cannot race with an in-flight push. There is no quota in this release. Disk consumption is bounded only by the rate at which contributors push valid PR refs. @@ -72,9 +75,8 @@ The current release relies entirely on: - the signed PR / PR Update event (no NIP-98 or other HTTP auth on push), - the requirement that the event's `clone` tag names this relay's `/prs//.git` endpoint, -- the standard 30-minute purgatory TTL, -- post-push zero-ref cleanup, and -- the periodic `/prs/` sweep. +- the standard 30-minute purgatory TTL, and +- the inline zero-ref cleanups described under "Storage cost". The following knobs are **future** additions and are not yet wired: diff --git a/src/grasp06/receive.rs b/src/grasp06/receive.rs index 222ff46..56d3f59 100644 --- a/src/grasp06/receive.rs +++ b/src/grasp06/receive.rs @@ -12,10 +12,12 @@ //! exact shape `refs/nostr/<64-lowercase-hex>`. The repo on disk is not //! touched in this path — probe pushes that would have been rejected //! leave no trace. -//! 2. Creates the bare repo on demand at the GRASP-06 path. A -//! per-`(submitter, identifier)` mutex (lazily inserted into a `DashMap`) -//! serialises only the `git init --bare` step; ref locking inside the -//! repo is left to git itself for the actual push. +//! 2. Acquires the per-path lock from [`RepoInitLocks`] for the rest of +//! the request, then creates the bare repo on demand at the GRASP-06 +//! path. Holding the lock for the whole pipeline guarantees a +//! concurrent push to the same `(submitter, identifier)` cannot delete +//! the bare repo out from under this one in step 5. Different paths +//! still run in parallel. //! 3. Runs `git-receive-pack` against the repo, mirroring the subprocess //! plumbing in [`crate::git::handlers::handle_receive_pack`]. //! 4. For each accepted `refs/nostr/` ref, validates against the @@ -26,7 +28,8 @@ //! standard 30-minute purgatory sweep can clean it up if the event never //! arrives. //! 5. After validation, the repo is removed if it has zero refs left — -//! discarding probe pushes that produced no valid state. +//! discarding probe pushes that produced no valid state. Safe to do +//! under the lock acquired in step 2. //! 6. Triggers the standard purgatory-release path via //! [`crate::git::sync::process_newly_available_git_data`] so PR events //! already in purgatory waiting for these commits get promoted. @@ -57,13 +60,19 @@ use crate::nostr::builder::{Nip34WritePolicy, SharedDatabase}; use crate::purgatory::Purgatory; use crate::sync::rejected_index::RejectedEventsIndex; -/// Shared lock map ensuring a single `git init --bare` runs per -/// `/prs//.git` path at a time. +/// Shared lock map serialising the full `/prs//.git` +/// receive-pack pipeline. /// -/// The lock is only held across the directory-creation + `git init` step. -/// Once the repo exists, git's own ref locking handles intra-push -/// concurrency, so simultaneous pushes to the same path proceed in parallel -/// after init. +/// The lock is held for the entire request — `git init --bare`, +/// `git-receive-pack`, per-ref validation, and the post-validation +/// "remove if zero refs" cleanup — so that two simultaneous pushes to the +/// same path cannot have one push delete the bare repo while the other is +/// mid-receive. Pushes to *different* paths still proceed in parallel. +/// +/// The same lock is taken by code paths outside the receive handler +/// (purgatory expiry, scoped-placeholder mismatch handling) before they +/// inspect or delete the bare repo, so all callers serialise against +/// in-flight pushes. pub type RepoInitLocks = Arc>>>; /// Create a fresh, empty [`RepoInitLocks`] for use as an [`HttpService`] @@ -126,15 +135,25 @@ pub async fn handle_prs_receive_pack( } } - // 2. Create the bare repo on demand under a per-path mutex. Only the - // init step is serialised; the push itself relies on git's ref - // locking. + // 2. Acquire the per-path lock for the rest of the request. The lock + // covers `git init --bare`, `git-receive-pack`, per-ref validation + // and the zero-ref cleanup so a concurrent push to the same + // `(submitter, identifier)` cannot have one push delete the bare + // repo while the other is mid-receive. Different paths still run + // in parallel — the DashMap entry is per `repo_path`. let repo_path = prs_repo_path( Path::new(git_data_path), &prs.submitter.to_hex(), &prs.identifier, ); - if let Err(e) = ensure_repo_initialised(&repo_path, &repo_init_locks).await { + let path_lock = repo_init_locks + .entry(repo_path.clone()) + .or_insert_with(|| Arc::new(Mutex::new(()))) + .value() + .clone(); + let _path_guard = path_lock.lock().await; + + if let Err(e) = ensure_repo_initialised(&repo_path).await { error!( "/prs/ receive-pack: failed to initialise repo at {}: {}", repo_path.display(), @@ -258,17 +277,12 @@ fn invalid_ref_reason(ref_name: &str) -> Option { None } -/// Acquire the per-path init mutex, then `mkdir -p` the parent and -/// `git init --bare --initial-branch=main --quiet` into `repo_path` if it -/// does not already exist. -async fn ensure_repo_initialised(repo_path: &Path, locks: &RepoInitLocks) -> Result<(), GitError> { - let lock = locks - .entry(repo_path.to_path_buf()) - .or_insert_with(|| Arc::new(Mutex::new(()))) - .value() - .clone(); - let _guard = lock.lock().await; - +/// `mkdir -p` the parent and `git init --bare --initial-branch=main +/// --quiet` into `repo_path` if it does not already exist. +/// +/// The caller must hold the per-path lock from [`RepoInitLocks`] for +/// `repo_path` before invoking this function. +async fn ensure_repo_initialised(repo_path: &Path) -> Result<(), GitError> { if repo_path.exists() { return Ok(()); } diff --git a/src/http/mod.rs b/src/http/mod.rs index 92aab29..15a5a60 100644 --- a/src/http/mod.rs +++ b/src/http/mod.rs @@ -25,7 +25,7 @@ use tokio::net::TcpListener; use crate::config::Config; use crate::git; -use crate::grasp06::receive::{new_repo_init_locks, RepoInitLocks}; +use crate::grasp06::receive::RepoInitLocks; use crate::metrics::Metrics; use crate::nostr::builder::{Nip34WritePolicy, SharedDatabase}; use crate::purgatory::Purgatory; @@ -757,6 +757,7 @@ fn derive_accept_key(request_key: &[u8]) -> String { /// * `purgatory` - Purgatory for event/git coordination /// * `write_policy` - Write policy for re-processing hot-cache events after git push promotion /// * `rejected_events_index` - Rejected events index for hot-cache re-processing +#[allow(clippy::too_many_arguments)] pub async fn run_server( config: Config, relay: LocalRelay, @@ -765,6 +766,7 @@ pub async fn run_server( purgatory: Arc, write_policy: Arc, rejected_events_index: Arc, + repo_init_locks: RepoInitLocks, ) -> anyhow::Result<()> { let bind_addr: SocketAddr = config.bind_address.parse()?; @@ -774,12 +776,6 @@ pub async fn run_server( let listener = TcpListener::bind(&bind_addr).await?; - // Shared per-path init mutexes for the GRASP-06 `/prs/` endpoint. Lives - // for the lifetime of the server so concurrent pushes to the same - // `/prs//.git` path serialise their on-demand - // `git init --bare` step. - let repo_init_locks = new_repo_init_locks(); - loop { let (socket, addr) = listener.accept().await?; let io = TokioIo::new(socket); diff --git a/src/main.rs b/src/main.rs index 3487060..ab080ba 100644 --- a/src/main.rs +++ b/src/main.rs @@ -10,7 +10,7 @@ use tracing_subscriber::{EnvFilter, FmtSubscriber}; use ngit_grasp::{ audit_cleanup, cleanup_empty_repos, config::{Config, DatabaseBackend}, - git, http, + git, grasp06, http, metrics::Metrics, nostr, purgatory::{sync::RealSyncContext, sync::ThrottleManager, Purgatory}, @@ -131,9 +131,18 @@ async fn run_relay(config: Config) -> Result<()> { } } + // Shared per-path init mutexes for the GRASP-06 `/prs/` endpoint. Lives + // for the lifetime of the process so concurrent pushes to the same + // `/prs//.git` path — and policy / purgatory code paths + // that may delete a `/prs/` bare repo — serialise against in-flight + // pushes via the same DashMap. + let repo_init_locks = grasp06::receive::new_repo_init_locks(); + // Create Nostr relay with NIP-34 validation // Returns both the relay and database for direct queries in handlers - if let Ok(relay_with_db) = nostr::builder::create_relay(&config, purgatory.clone()).await { + if let Ok(relay_with_db) = + nostr::builder::create_relay(&config, purgatory.clone(), repo_init_locks.clone()).await + { info!( "Relay created with NIP-34 validation for domain: {}", config.domain @@ -145,6 +154,17 @@ async fn run_relay(config: Config) -> Result<()> { .write_policy .set_local_relay(relay_with_db.relay.clone()); + // Wire the GRASP-06 `/prs/` filesystem cleanup context into + // purgatory so the standard expiry sweep can delete dangling + // refs/nostr/ refs (and zero-ref bare repos) when a + // scoped placeholder expires without a matching PR event. Held + // under the same per-path lock the receive handler uses so the + // cleanup can never race with an in-flight push. + purgatory.set_prs_cleanup_ctx(ngit_grasp::purgatory::PrsCleanupCtx { + git_data_path: PathBuf::from(config.effective_git_data_path()), + repo_init_locks: repo_init_locks.clone(), + }); + // Start SyncManager for proactive sync (Phase 2: multi-relay support, Phase 3: health tracking) // Even without bootstrap relay, SyncManager discovers relays from stored announcements // Pass the already-registered sync metrics from Metrics to avoid duplicate registration @@ -285,6 +305,7 @@ async fn run_relay(config: Config) -> Result<()> { purgatory, http_write_policy, http_rejected_index, + repo_init_locks, ) => { result? } @@ -308,6 +329,7 @@ async fn run_relay(config: Config) -> Result<()> { purgatory, http_write_policy, http_rejected_index, + repo_init_locks, ) => { result? } diff --git a/src/nostr/builder.rs b/src/nostr/builder.rs index 41f449f..e244907 100644 --- a/src/nostr/builder.rs +++ b/src/nostr/builder.rs @@ -57,6 +57,7 @@ impl Nip34WritePolicy { git_data_path: impl Into, purgatory: std::sync::Arc, config: crate::config::Config, + repo_init_locks: crate::grasp06::receive::RepoInitLocks, ) -> Self { let ctx = PolicyContext::new( &config.domain, @@ -64,6 +65,7 @@ impl Nip34WritePolicy { git_data_path, purgatory, config.clone(), + repo_init_locks, ); Self { announcement_policy: AnnouncementPolicy::new(ctx.clone(), config.clone()), @@ -692,6 +694,7 @@ pub struct RelayWithDatabase { pub async fn create_relay( config: &Config, purgatory: Arc, + repo_init_locks: crate::grasp06::receive::RepoInitLocks, ) -> Result { tracing::info!("Configuring nostr relay with GRASP-01 validation..."); @@ -752,8 +755,13 @@ pub async fn create_relay( } // Create write policy with purgatory integration - let write_policy = - Nip34WritePolicy::new(database.clone(), &git_data_path, purgatory, config.clone()); + let write_policy = Nip34WritePolicy::new( + database.clone(), + &git_data_path, + purgatory, + config.clone(), + repo_init_locks, + ); let mut builder = LocalRelayBuilder::default() .database(database.clone()) diff --git a/src/nostr/policy/deletion.rs b/src/nostr/policy/deletion.rs index c5a52d4..b5f6499 100644 --- a/src/nostr/policy/deletion.rs +++ b/src/nostr/policy/deletion.rs @@ -303,7 +303,7 @@ mod tests { })); let purgatory = Arc::new(Purgatory::new(PathBuf::new())); let config = crate::config::Config::for_testing(); - PolicyContext::new("test.example.com", db, PathBuf::new(), purgatory, config) + PolicyContext::new_for_test("test.example.com", db, PathBuf::new(), purgatory, config) } fn make_announcement_event(keys: &Keys, identifier: &str) -> Event { diff --git a/src/nostr/policy/mod.rs b/src/nostr/policy/mod.rs index f5b981a..9f2e6f8 100644 --- a/src/nostr/policy/mod.rs +++ b/src/nostr/policy/mod.rs @@ -21,6 +21,9 @@ pub use state::{StatePolicy, StateResult}; pub use crate::git::sync::AlignmentResult; use super::SharedDatabase; +use crate::grasp06::receive::RepoInitLocks; +#[cfg(test)] +use crate::grasp06::receive::new_repo_init_locks; use crate::purgatory::Purgatory; use nostr_relay_builder::LocalRelay; use std::sync::Arc; @@ -36,6 +39,10 @@ pub struct PolicyContext { pub local_relay: Arc>>, /// Configuration reference for policy settings (includes blacklists) pub config: crate::config::Config, + /// Per-path locks shared with the GRASP-06 `/prs/` receive handler so + /// validation paths that may delete a `/prs/` bare repo serialise + /// against in-flight pushes to the same `(submitter, identifier)`. + pub repo_init_locks: RepoInitLocks, } impl PolicyContext { @@ -45,6 +52,7 @@ impl PolicyContext { git_data_path: impl Into, purgatory: Arc, config: crate::config::Config, + repo_init_locks: RepoInitLocks, ) -> Self { Self { domain: domain.into(), @@ -53,9 +61,31 @@ impl PolicyContext { purgatory, local_relay: Arc::new(std::sync::RwLock::new(None)), config, + repo_init_locks, } } + /// Construct a [`PolicyContext`] with a fresh, isolated [`RepoInitLocks`] + /// map. Intended for unit tests that exercise policy logic without a + /// running HTTP server. + #[cfg(test)] + pub fn new_for_test( + domain: impl Into, + database: SharedDatabase, + git_data_path: impl Into, + purgatory: Arc, + config: crate::config::Config, + ) -> Self { + Self::new( + domain, + database, + git_data_path, + purgatory, + config, + new_repo_init_locks(), + ) + } + /// Set the local relay after it's been created. /// /// This is called after the relay is built since the relay depends on the policy diff --git a/src/nostr/policy/pr_event.rs b/src/nostr/policy/pr_event.rs index b2b4dbf..e7bca57 100644 --- a/src/nostr/policy/pr_event.rs +++ b/src/nostr/policy/pr_event.rs @@ -110,7 +110,12 @@ impl PrEventPolicy { "Discarding scoped /prs/ PR placeholder — incoming event does not match URL submitter + identifier", ); - // Delete the ref the original /prs/ push wrote. + // Delete the ref the original /prs/ push wrote, and if + // that leaves the bare repo with zero refs, delete the + // directory too. Held under the same per-path lock the + // /prs/ receive handler uses so a concurrent push to + // this `(submitter, identifier)` cannot have its work + // wiped mid-receive. let prs_repo = crate::grasp06::paths::prs_repo_path( &self.ctx.git_data_path, &scope.submitter.to_hex(), @@ -118,6 +123,17 @@ impl PrEventPolicy { ); let ref_name = format!("refs/nostr/{}", event_id); if prs_repo.exists() { + let lock = self + .ctx + .repo_init_locks + .entry(prs_repo.clone()) + .or_insert_with(|| { + std::sync::Arc::new(tokio::sync::Mutex::new(())) + }) + .value() + .clone(); + let _guard = lock.lock().await; + if let Err(e) = crate::git::delete_ref(&prs_repo, &ref_name) { tracing::warn!( event_id = %event_id, @@ -125,6 +141,27 @@ impl PrEventPolicy { error = %e, "Failed to delete /prs/ ref while discarding scoped placeholder", ); + } else if matches!( + crate::git::list_refs(&prs_repo), + Ok(refs) if refs.is_empty() + ) { + // No refs left — 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 already + // 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", + ); + } } } self.ctx.purgatory.remove_pr(&event_id); diff --git a/src/purgatory/mod.rs b/src/purgatory/mod.rs index 790b4bc..efa1c81 100644 --- a/src/purgatory/mod.rs +++ b/src/purgatory/mod.rs @@ -199,6 +199,26 @@ pub struct Purgatory { expired_events: Arc>, _git_data_path: PathBuf, + + /// Set once at startup by [`Purgatory::set_prs_cleanup_ctx`] to give + /// [`Purgatory::cleanup`] enough information to remove abandoned + /// `/prs//.git` refs and bare repos when their + /// scoped placeholders expire without an arriving PR event. Left + /// `None` in tests and any caller that does not need this behaviour. + prs_cleanup_ctx: std::sync::OnceLock, +} + +/// Context shared with [`Purgatory::cleanup`] so it can perform the same +/// best-effort empty-repo cleanup the `/prs/` receive handler does, for +/// placeholders that expire because no matching PR event ever arrived. +/// +/// The lock map is the same one held by the receive handler, so a sync +/// `try_lock` here is enough to know whether a push is in flight; if it +/// is, the cleanup is deferred to the next [`Purgatory::cleanup`] cycle. +#[derive(Clone)] +pub struct PrsCleanupCtx { + pub git_data_path: PathBuf, + pub repo_init_locks: crate::grasp06::receive::RepoInitLocks, } impl Purgatory { @@ -211,9 +231,20 @@ impl Purgatory { sync_queue: Arc::new(DashMap::new()), expired_events: Arc::new(DashMap::new()), _git_data_path: git_data_path.into(), + prs_cleanup_ctx: std::sync::OnceLock::new(), } } + /// Wire the cleanup-time `/prs/` filesystem context. Called once at + /// startup by `main`; tests and code paths that do not exercise + /// scoped `/prs/` placeholders leave it unset and the cleanup loop + /// then only removes the in-memory entry, leaving any on-disk refs + /// in place. + pub fn set_prs_cleanup_ctx(&self, ctx: PrsCleanupCtx) { + // Ignore a second set — there is exactly one server-wide context. + let _ = self.prs_cleanup_ctx.set(ctx); + } + /// Enqueue an identifier for background git data sync. /// /// This method is called when a state or PR event is added to purgatory. @@ -1171,12 +1202,13 @@ impl Purgatory { let event_opt = pr_entry.event.clone(); let commit = pr_entry.commit.clone(); let source = pr_entry.source; - (event_id_str, event_opt, commit, source) + let prs_scope = pr_entry.prs_scope.clone(); + (event_id_str, event_opt, commit, source, prs_scope) }) .collect(); let pr_removed = expired_prs.len(); - for (event_id_str, event_opt, commit, source) in expired_prs { + for (event_id_str, event_opt, commit, source, prs_scope) in expired_prs { // Log structured entry for PR events (not placeholders) if let Some(ref event) = event_opt { let npub = event @@ -1259,6 +1291,73 @@ impl Purgatory { &commit[..commit.len().min(12)] ); } + + // For GRASP-06 `/prs/`-scoped placeholders, best-effort + // filesystem cleanup of the dangling refs/nostr/ + // ref and (if it leaves the bare repo with zero refs) the + // repo directory itself. The cleanup is gated on a + // `try_lock` of the same per-path mutex the receive handler + // uses, so it can never race with an in-flight push: if a + // push to this `(submitter, identifier)` is currently + // holding the lock, the cleanup is skipped this cycle and + // the dangling ref is simply left on disk. The receive + // handler will re-evaluate cleanup at the end of its + // current push, and any future push to the same path will + // ignore the dangling ref. + if let (Some(scope), Some(ctx)) = + (prs_scope.as_ref(), self.prs_cleanup_ctx.get()) + { + let repo_path = crate::grasp06::paths::prs_repo_path( + &ctx.git_data_path, + &scope.submitter.to_hex(), + &scope.identifier, + ); + if repo_path.exists() { + let lock = ctx + .repo_init_locks + .entry(repo_path.clone()) + .or_insert_with(|| Arc::new(tokio::sync::Mutex::new(()))) + .value() + .clone(); + let guard = lock.try_lock(); + match guard { + Ok(_guard) => { + let ref_name = format!("refs/nostr/{}", event_id_str); + if let Err(e) = crate::git::delete_ref(&repo_path, &ref_name) { + tracing::warn!( + repo = %repo_path.display(), + ref_name = %ref_name, + error = %e, + "Failed to delete dangling /prs/ ref during purgatory expiry", + ); + } else if 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", + ); + } + } + } + Err(_) => { + tracing::debug!( + repo = %repo_path.display(), + "/prs/ repo lock contended during purgatory expiry — skipping filesystem cleanup this cycle", + ); + } + } + } + } + self.pr_events.remove(&event_id_str); }