diff --git a/crates/signed_git/src/lib.rs b/crates/signed_git/src/lib.rs index b021782..4e72a29 100644 --- a/crates/signed_git/src/lib.rs +++ b/crates/signed_git/src/lib.rs @@ -1,4 +1,4 @@ -use std::collections::HashSet; +use std::collections::{HashMap, HashSet}; use std::io::Write; use std::path::{Path, PathBuf}; use std::process::{Command, Stdio}; @@ -454,6 +454,47 @@ fn push_refspecs( Ok(()) } +/// Whether `url` advertises every ref in `expected` at the given commit. +/// +/// Extra advertised refs are ignored: the question is whether the data this +/// push wanted to land is already there, not whether the remote is an exact mirror. +/// This is the convergence probe for a push that lost the compare-and-swap race +/// to the grasp server's own background ref alignment. +pub fn remote_has_refs(repo_path: &Path, url: &str, expected: &[(String, String)]) -> Result { + if expected.is_empty() { + return Ok(true); + } + + let output = git_output(repo_path, &["ls-remote", url], "git ls-remote")?; + + if !output.status.success() { + bail!( + "git ls-remote {url} failed: {}", + String::from_utf8_lossy(&output.stderr).trim() + ); + } + + let advertised = parse_ls_remote(&output.stdout); + Ok(expected + .iter() + .all(|(name, oid)| advertised.get(name.as_str()) == Some(oid))) +} + +/// Parse `git ls-remote` output into (refname, oid) pairs. +/// +/// Skips the peeled `^{}` lines that follow annotated tag objects. +fn parse_ls_remote(output: &[u8]) -> HashMap { + String::from_utf8_lossy(output) + .lines() + .filter_map(|line| { + let mut fields = line.split_whitespace(); + let oid = fields.next()?; + let name = fields.next()?; + (!name.ends_with("^{}")).then(|| (name.to_owned(), oid.to_owned())) + }) + .collect() +} + /// The earliest unique commit of the repository at `repo_path`. /// Used as the NIP-34 announcement's `euc` marker. /// @@ -2048,6 +2089,76 @@ mod tests { assert!(!refs.contains("refs/heads/")); } + #[test] + fn remote_has_refs_reports_whether_pushed_refs_landed() { + let server = tempfile::tempdir().unwrap(); + let server_repo = server.path().join("npub1test").join("my-repo.git"); + std::fs::create_dir_all(server_repo.parent().unwrap()).unwrap(); + let init_status = Command::new("git") + .args(["init", "--bare", "-q"]) + .arg(&server_repo) + .status() + .expect("spawn git init --bare"); + assert!(init_status.success()); + + let (dir, repo) = fixture(&[("a.txt", b"one")]); + commit_all(&repo, "initial"); + let dir = dir.path(); + let main = git_in(dir, &["rev-parse", "refs/heads/main"]).expect("main oid"); + let url = format!("file://{}/npub1test/my-repo.git", server.path().display()); + let expected = vec![("refs/heads/main".to_owned(), main.clone())]; + + // Nothing pushed yet: the ref is absent. + assert!(!remote_has_refs(dir, &url, &expected).expect("probe")); + + push_all( + dir, + &format!("file://{}", server.path().display()), + "npub1test", + "my-repo", + ) + .expect("push"); + + // The pushed ref is advertised at the expected commit. + assert!(remote_has_refs(dir, &url, &expected).expect("probe")); + + // A stale expectation - the exact race a retry resolves - is false. + let stale = vec![("refs/heads/main".to_owned(), "0".repeat(40))]; + assert!(!remote_has_refs(dir, &url, &stale).expect("probe")); + + // Extra remote refs (e.g. a tag pushed later) do not invalidate the + // refs this push wanted to land. + git_run(dir, &["tag", "v1.0"]); + push_all( + dir, + &format!("file://{}", server.path().display()), + "npub1test", + "my-repo", + ) + .expect("push"); + assert!(remote_has_refs(dir, &url, &expected).expect("probe")); + } + + #[test] + fn parse_ls_remote_reads_oids_and_skips_peeled_lines() { + let oid_a = "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa"; + let oid_b = "bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb"; + let output = format!( + "{oid_a}\trefs/heads/main\n{oid_b}\trefs/tags/v1.0\n{oid_b}\trefs/tags/v1.0^{{}}\n" + ); + + let advertised = parse_ls_remote(output.as_bytes()); + assert_eq!(advertised.len(), 2); + assert_eq!( + advertised.get("refs/heads/main").map(String::as_str), + Some(oid_a) + ); + assert_eq!( + advertised.get("refs/tags/v1.0").map(String::as_str), + Some(oid_b) + ); + } + #[test] fn repo_ref_state_lists_branches_tags_and_head() { let (_dir, repo) = fixture(&[("a.txt", b"hello")]); diff --git a/crates/signed_state/src/backend.rs b/crates/signed_state/src/backend.rs index cfb68e1..cf4a568 100644 --- a/crates/signed_state/src/backend.rs +++ b/crates/signed_state/src/backend.rs @@ -528,53 +528,66 @@ impl Backend { })? .await?; - let state_event = match this - .update(cx, |this, cx| { - let builder = build_state( - &repo_id, - &[("refs/heads/main".to_owned(), commit)], - Some("main"), - ); - this.send(builder, cx) - })? - .await - { - Ok(state_event) => state_event, - Err(e) => { - this.update(cx, |this, cx| { - this.retract_events(std::slice::from_ref(&event), cx); - }) - .ok(); + // The state event is the push authorization. Stage it on each + // grasp server's relay, then push the initial commit. + // Creation fails only when no server accepted the push, the announcement + // is then retracted so the repository is not left announced without content. + let (client, signer) = + this.update(cx, |this, _cx| (this.client.clone(), this.signer.clone()))?; + let refs = vec![("refs/heads/main".to_owned(), commit)]; - return Err(e.context( - "The repository was announced, but its state could not be published. \ - The announcement has been retracted", - )); - } - }; - - // Push to every grasp server. Creation fails only when no server accepted it. let push = cx.background_spawn({ + let client = client.clone(); + let signer = signer.clone(); let path = path.clone(); let owner = owner.clone(); let repo_id = repo_id.clone(); let servers = servers.clone(); - push_to_grasp_servers(path, owner, repo_id, servers, signed_git::push_main) + let refs = refs.clone(); + async move { + push_staged_to_grasps( + &client, + &signer, + &repo_id, + &refs, + Some("main"), + &path, + &owner, + &servers, + signed_git::push_main, + ) + .await + } }); - if let Err(e) = push.await { - // The events are already published. Retract them so the repository is not left announced without content. + let outcome = push.await; + + if outcome.accepted() == 0 { + // The announcement is already published. + // Retract it so the repository is not left announced without content. this.update(cx, |this, cx| { - this.retract_events(&[event.clone(), state_event.clone()], cx); + this.retract_events(std::slice::from_ref(&event), cx); }) .ok(); - return Err(e.context( - "The repository was announced, but the push to every grasp server failed. \ + return Err(anyhow!( + "The repository was announced, but the push to every grasp server failed: {}. \ The announcement has been retracted", + outcome.failure_summary() )); } + // Fan the state out to the relays once a git server holds the objects. + // Staging already stored the event locally, publishing makes it + // visible to the other relays and clients. + if let Some(state_event) = &outcome.state_event { + broadcast_event(&client, state_event).await.ok(); + this.update(cx, |_this, cx| { + cx.emit(BackendEvent::Published(Box::new(state_event.clone()))); + }) + .ok(); + } + let announcement = Announcement::from_event(&event) .ok_or_else(|| anyhow!("failed to parse announcement"))?; @@ -664,48 +677,65 @@ impl Backend { let refs = state.refs.clone(); let head = state.head.clone(); - let state_event = match this - .update(cx, |this, cx| { - let builder = build_state(&repo_id, &refs, head.as_deref()); - this.send(builder, cx) - })? - .await - { - Ok(state_event) => state_event, - Err(e) => { + + // The state event is the push authorization. Stage it on each + // grasp server's relay, then push every branch and tag. The push + // fails only when no server accepted it. The announcement is then + // retracted so the repository is not left announced without content. + // An empty repository has no state to stage and nothing to push. + let (client, signer) = + this.update(cx, |this, _cx| (this.client.clone(), this.signer.clone()))?; + + if !refs.is_empty() { + let push = cx.background_spawn({ + let client = client.clone(); + let signer = signer.clone(); + let path = path.clone(); + let owner = owner.clone(); + let repo_id = repo_id.clone(); + let servers = servers.clone(); + let refs = refs.clone(); + let head = head.clone(); + async move { + push_staged_to_grasps( + &client, + &signer, + &repo_id, + &refs, + head.as_deref(), + &path, + &owner, + &servers, + signed_git::push_all, + ) + .await + } + }); + let outcome = push.await; + + if outcome.accepted() == 0 { + // The announcement is already published. Retract it so + // the repository is not left announced without content. this.update(cx, |this, cx| { this.retract_events(std::slice::from_ref(&event), cx); }) .ok(); - return Err(e.context( - "The repository was announced, but its state could not be published. \ + return Err(anyhow!( + "The repository was announced, but the push to every grasp server failed: {}. \ The announcement has been retracted", + outcome.failure_summary() )); } - }; - // Push every branch and tag to each grasp server. The push fails only when no server accepted it. - // - // An empty repository has nothing to push. - if !refs.is_empty() { - let push = cx.background_spawn({ - let path = path.clone(); - let owner = owner.clone(); - let repo_id = repo_id.clone(); - let servers = servers.clone(); - push_to_grasp_servers(path, owner, repo_id, servers, signed_git::push_all) - }); - if let Err(e) = push.await { - this.update(cx, |this, cx| { - this.retract_events(&[event.clone(), state_event.clone()], cx); + // Fan the state out to the relays once a git server holds the objects. + // Staging already stored the event locally, publishing makes it visible to the other relays and clients. + if let Some(state_event) = &outcome.state_event { + broadcast_event(&client, state_event).await.ok(); + this.update(cx, |_this, cx| { + cx.emit(BackendEvent::Published(Box::new(state_event.clone()))); }) .ok(); - - return Err(e.context( - "The repository was announced, but the push to every grasp server failed. \ - The announcement has been retracted", - )); } } @@ -724,11 +754,14 @@ impl Backend { } /// Re-push the repository's current refs to the grasp servers in its `relays` tag. + /// + /// Errors when no grasp server accepted the push, the outcome reports + /// which servers did when only some accepted it. pub fn push_repository( &mut self, announcement: Announcement, cx: &mut Context, - ) -> Task> { + ) -> Task> { let cache = GitStore::global(cx).cache().clone(); let path = cache.repo_path(&announcement.addr()); self.push_repo_from(announcement, path, None, cx) @@ -744,7 +777,7 @@ impl Backend { checkout: PathBuf, announced_head: Option, cx: &mut Context, - ) -> Task> { + ) -> Task> { self.push_repo_from(announcement, checkout, announced_head, cx) } @@ -755,18 +788,20 @@ impl Backend { path: PathBuf, announced_head: Option, cx: &mut Context, - ) -> Task> { + ) -> Task> { let addr = announcement.addr(); let guard = { let mut pushing = self .pushing_repos .lock() .unwrap_or_else(|poisoned| poisoned.into_inner()); + if !pushing.insert(addr.clone()) { return Task::ready(Err(anyhow!( "A push to this repository is already in progress" ))); } + PushGuard { repos: self.pushing_repos.clone(), addr: addr.clone(), @@ -805,31 +840,64 @@ impl Backend { state.head = Some(head); } - // Grasp servers authorize a push by the state they have seen. + // Grasp servers authorize a push by the state event they hold in purgatory. + // Stage the state event on each server's own relay, then push the git data, + // retrying transient purgatory denials. let refs = state.refs.clone(); let head = state.head.clone(); - this.update(cx, |this, cx| { - let builder = build_state(&repo_id, &refs, head.as_deref()); - this.send(builder, cx) - })? - .await?; + let (client, signer) = + this.update(cx, |this, _cx| (this.client.clone(), this.signer.clone()))?; - if !refs.is_empty() { + let outcome = if refs.is_empty() { + PushOutcome::default() + } else { let push = cx.background_spawn({ + let client = client.clone(); + let signer = signer.clone(); let path = path.clone(); let owner = owner.clone(); let repo_id = repo_id.clone(); let relays = relays.clone(); + let refs = refs.clone(); + let head = head.clone(); async move { - push_to_grasp_servers(path, owner, repo_id, relays, signed_git::push_all) - .await + push_staged_to_grasps( + &client, + &signer, + &repo_id, + &refs, + head.as_deref(), + &path, + &owner, + &relays, + signed_git::push_all, + ) + .await } }); - push.await?; + push.await + }; + + if !refs.is_empty() && outcome.accepted() == 0 { + bail!( + "could not push the repository to any grasp server: {}", + outcome.failure_summary() + ); } - Ok(()) + // Fan the state out to the relays once a git server holds the objects. + // Staging already stored the event locally, publishing notifies + // the repository views and other relays and clients. + if let Some(state_event) = &outcome.state_event { + broadcast_event(&client, state_event).await.ok(); + this.update(cx, |_this, cx| { + cx.emit(BackendEvent::Published(Box::new(state_event.clone()))); + }) + .ok(); + } + + Ok(outcome) }) } @@ -1533,40 +1601,341 @@ pub async fn user_grasp_list_servers( Ok(latest_grasp_list_servers(events)) } -/// Push the repository at `path` to every grasp server. -async fn push_to_grasp_servers( - path: PathBuf, - owner: String, - repo_id: String, - servers: Vec, - push: fn(&Path, &str, &str, &str) -> Result<(), Error>, -) -> Result<(), Error> { - let mut failures = Vec::new(); - let mut pushed = 0; +/// Attempts per grasp server when a git push is denied transiently. +const GRASP_PUSH_ATTEMPTS: usize = 3; - for relay in &servers { - let Some(base_url) = grasp_base_url(relay) else { - failures.push(format!("{relay}: no domain")); - continue; - }; - match push(&path, &base_url, &owner, &repo_id) { - Ok(()) => pushed += 1, - Err(e) => failures.push(format!("{relay}: {e}")), +/// Pause before re-staging a state event after a transient denial. +const GRASP_RETRY_DELAY: Duration = Duration::from_secs(1); + +/// The outcome of pushing to one grasp server. +#[derive(Debug, Clone)] +pub struct GraspServerResult { + /// The grasp server's relay URL, e.g. `wss://relay.ngit.dev`. + pub relay: RelayUrl, + /// The git URL the data was pushed to. + pub git_url: String, + /// `None` when the server accepted the data, the reason otherwise. + pub reason: Option, +} + +impl GraspServerResult { + fn ok(relay: RelayUrl, git_url: String) -> Self { + Self { + relay, + git_url, + reason: None, } } - if pushed == 0 { - bail!( - "could not push the repository to any grasp server: {}", - failures.join("; ") - ); + fn failed(relay: RelayUrl, git_url: String, reason: impl Into) -> Self { + Self { + relay, + git_url, + reason: Some(reason.into()), + } + } +} + +/// The outcome of a staged push across every grasp server of a repository. +#[derive(Debug, Clone, Default)] +pub struct PushOutcome { + /// Per-server results, in the order the servers were listed. + pub servers: Vec, + /// The newest state event a grasp relay accepted for this push, if any. + /// + /// Broadcast to the other relays once a git server holds the data. + pub state_event: Option, +} + +impl PushOutcome { + /// The number of grasp servers that accepted the git data. + pub fn accepted(&self) -> usize { + self.servers + .iter() + .filter(|server| server.reason.is_none()) + .count() } - for failure in failures { - log::warn!("grasp push failed: {failure}"); + /// Servers that did not accept the push. + fn failing(&self) -> impl Iterator { + self.servers.iter().filter(|server| server.reason.is_some()) } - Ok(()) + /// One-line summary of every server failure, for error messages. + pub fn failure_summary(&self) -> String { + self.failing() + .map(|server| { + let reason = + flatten_whitespace(server.reason.as_deref().unwrap_or("unknown error")); + format!("{}: {reason}", server.relay) + }) + .collect::>() + .join("; ") + } + + /// A warning for a push only some grasp servers accepted. + /// + /// `None` when every server accepted the push or nothing was pushed. + pub fn partial_warning(&self) -> Option { + let accepted = self.accepted(); + if self.servers.is_empty() || accepted == self.servers.len() { + return None; + } + Some(format!( + "Pushed to {accepted} of {} grasp servers: {}. Republish to sync.", + self.servers.len(), + self.failure_summary() + )) + } +} + +/// Collapse a multi-line relay or git error into one display line. +fn flatten_whitespace(text: &str) -> String { + const MAX_CHARS: usize = 200; + let flat: String = text.split_whitespace().collect::>().join(" "); + if flat.chars().count() <= MAX_CHARS { + flat + } else { + let mut clipped: String = flat.chars().take(MAX_CHARS).collect(); + clipped.push('…'); + clipped + } +} + +/// Reasons a push attempt should be retried with a freshly staged state +/// event and a fresh git advertisement. +/// +/// Two families are retried: +/// +/// - **Purgatory denials**: the grasp server sends these when the state +/// event for the push has not reached its purgatory yet. Re-staging a +/// fresh event resolves them. +/// - **Stale advertisement races**: `git receive-pack` compares each ref +/// update against the value it advertised when the push started. The grasp +/// server's own background sync can move a ref in between - typically by +/// aligning the repository to a parked state event once the objects of an +/// earlier attempt land - so the compare-and-swap fails with `cannot lock +/// ref` / `incorrect old value provided`. A retry against the fresh +/// advertisement converges, and when the race is lost the pushed data is +/// usually already on the server (see `is_stale_advertisement_race` and +/// the convergence probe in `push_staged_to_grasps`). +/// +/// Other rejections are not retried. +fn is_transient_grasp_denial(stderr: &str) -> bool { + let error = stderr.to_lowercase(); + [ + "no state events in purgatory", + "no matching state event", + "doesn't match push", + "none from authorized publishers", + "no repository announcement found", + "cannot lock ref", + "incorrect old value provided", + ] + .iter() + .any(|marker| error.contains(marker)) +} + +/// A push rejected because `git receive-pack`'s compare-and-swap lost to the +/// grasp server's own background ref alignment: the ref moved between this +/// push's advertisement and its ref transaction (`cannot lock ref ... is at +/// ... but expected ...` / `incorrect old value provided`). The pushed data +/// is usually already on the server by then. +fn is_stale_advertisement_race(stderr: &str) -> bool { + let error = stderr.to_lowercase(); + error.contains("cannot lock ref") || error.contains("incorrect old value provided") +} + +/// Keep `event` as the push's fan-out state event when it is newer than the +/// current one. All staged events carry the same refs; the newest timestamp +/// wins on the relays. +fn keep_newest(state_event: &mut Option, event: Event) { + if state_event + .as_ref() + .is_none_or(|current| event.created_at > current.created_at) + { + *state_event = Some(event); + } +} + +/// Sign a fresh kind `30618` state event for the push. +/// +/// `last_created_at` is the timestamp of the previous event signed for this push. +/// Retries within the same second get the next second: a grasp relay +/// treats a same-id resend as a duplicate and does not re-run its ingest, +/// so an identical resend cannot re-park a state event lost from its purgatory. +async fn sign_state_event( + signer: &UniversalSigner, + repo_id: &str, + refs: &[(String, String)], + head: Option<&str>, + last_created_at: u64, +) -> Result<(Event, u64), String> { + let now = Timestamp::now().as_secs(); + let created_at = if now > last_created_at { + now + } else { + last_created_at + 1 + }; + + let event = build_state(repo_id, refs, head) + .custom_created_at(Timestamp::from_secs(created_at)) + .finalize_async(signer) + .await + .map_err(|e| format!("could not sign the state event: {e}"))?; + + Ok((event, created_at)) +} + +/// Ensure the relay is known and connected, then publish `event` to it. +/// +/// `Ok` only when the relay confirmed the event. +/// On a grasp relay the accept parks the event in purgatory, +/// which authorizes the paired git push. +async fn stage_event_on_relay( + client: &Client, + relay: &RelayUrl, + event: &Event, +) -> Result<(), String> { + client + .add_relay(relay) + .await + .map_err(|e| format!("could not add relay {relay}: {e}"))?; + client.connect().await; + + let output = client + .send_event(event) + .to([relay.clone()]) + .await + .map_err(|e| format!("could not send the state event to {relay}: {e}"))?; + + if output.success.contains_key(relay) { + Ok(()) + } else { + let reason = output + .failed + .get(relay) + .cloned() + .unwrap_or_else(|| "relay did not confirm the event".to_owned()); + Err(reason) + } +} + +/// Push the repository at `path` to every grasp server in `servers`. +#[allow(clippy::too_many_arguments)] +async fn push_staged_to_grasps( + client: &Client, + signer: &UniversalSigner, + repo_id: &str, + refs: &[(String, String)], + head: Option<&str>, + path: &Path, + owner: &str, + servers: &[RelayUrl], + push: fn(&Path, &str, &str, &str) -> Result<(), Error>, +) -> PushOutcome { + let mut outcome = PushOutcome::default(); + + if refs.is_empty() { + return outcome; + } + + for relay in servers { + let Some(base) = grasp_base_url(relay) else { + outcome.servers.push(GraspServerResult::failed( + relay.clone(), + relay.to_string(), + "no domain", + )); + continue; + }; + let git_url = format!("{base}/{owner}/{repo_id}.git"); + + let mut reason = None; + let mut last_created_at = 0; + // The last state event staged on this server, for the convergence + // probe below when every push attempt lost the stale-ref race. + let mut staged_event = None; + + 'server: for attempt in 1..=GRASP_PUSH_ATTEMPTS { + if attempt > 1 { + // Give the server's ingest a moment before re-staging. + std::thread::sleep(GRASP_RETRY_DELAY); + } + + let (event, created_at) = + match sign_state_event(signer, repo_id, refs, head, last_created_at).await { + Ok(signed) => signed, + Err(e) => { + reason = Some(e); + break 'server; + } + }; + + last_created_at = created_at; + + // Stage the state event on this server's own relay. + // A failed stage means the grasp never parked the state, + // so the git push would be denied anyway: skip it (the eligibility gate). + if let Err(e) = stage_event_on_relay(client, relay, &event).await { + // One retry absorbs a relay connect blip, on the first + // attempt only. + if attempt == 1 && stage_event_on_relay(client, relay, &event).await.is_ok() { + // staged on the retry + } else { + reason = Some(e); + break 'server; + } + } + staged_event = Some(event.clone()); + + match push(path, &base, owner, repo_id) { + Ok(()) => { + keep_newest(&mut outcome.state_event, event); + break 'server; + } + Err(e) => { + let text = e.to_string(); + if attempt < GRASP_PUSH_ATTEMPTS && is_transient_grasp_denial(&text) { + reason = Some(text); + continue 'server; + } + reason = Some(text); + break 'server; + } + } + } + + // The grasp's own background sync aligns refs to staged state + // events as soon as the objects land, which can beat every push + // attempt's compare-and-swap (`cannot lock ref ... but expected`). + // When the last denial was that race the sync has usually finished + // by now: verify the advertised refs and accept the server when the + // pushed data is already there. + if let Some(last_reason) = &reason + && is_stale_advertisement_race(last_reason) + && signed_git::remote_has_refs(path, &git_url, refs).unwrap_or(false) + { + if let Some(event) = staged_event { + keep_newest(&mut outcome.state_event, event); + } + reason = None; + } + + match reason { + Some(reason) => { + log::warn!("grasp push failed: {relay}: {reason}"); + outcome + .servers + .push(GraspServerResult::failed(relay.clone(), git_url, reason)); + } + None => outcome + .servers + .push(GraspServerResult::ok(relay.clone(), git_url)), + } + } + + outcome } /// Split a stored bunker credential into the plain URI and the session key. @@ -1696,4 +2065,145 @@ mod tests { // No list at all, empty, so the caller falls back to the defaults. assert!(latest_grasp_list_servers(Vec::new()).is_empty()); } + + #[test] + fn transient_grasp_denials_are_classified() { + // The exact server rejection that started this work: the state event + // had not reached the grasp's purgatory before the git push. + let reported = "remote: ERR authorisation failed: No state events in purgatory\n\ + fatal: the remote end hung up unexpectedly\n\ + error: failed to push some refs to 'https://relay.ngit.dev/...git'"; + assert!(is_transient_grasp_denial(reported)); + + // The other purgatory states a fresh event resolves. + assert!(is_transient_grasp_denial( + "remote: ERR authorisation failed: No matching state event found in purgatory" + )); + assert!(is_transient_grasp_denial( + "remote: ERR authorisation failed: 1 state event in purgatory from authorized \ + publisher but doesn't match push" + )); + assert!(is_transient_grasp_denial( + "remote: ERR authorisation failed: 2 state events in purgatory but none from \ + authorized publishers" + )); + assert!(is_transient_grasp_denial( + "remote: ERR authorisation failed: No repository announcement found" + )); + + // Rejections a fresh state event cannot fix are not retried. + assert!(!is_transient_grasp_denial( + "remote: ERR authorisation failed: not a maintainer of this repository" + )); + assert!(!is_transient_grasp_denial( + "fatal: unable to access 'https://relay.ngit.dev/...': The requested URL returned \ + error: 403" + )); + assert!(!is_transient_grasp_denial( + "fatal: unable to access 'https://relay.ngit.dev/...': Could not resolve host" + )); + } + + #[test] + fn stale_ref_races_are_retried() { + // The grasp's background sync aligned the ref to a parked state event + // between this push's advertisement and its ref transaction. The ref + // is usually already where the push wants it, so a retry converges. + let reported = "remote: error: cannot lock ref 'refs/heads/main': is at \ + cac2ac91b6f5fb8dfcb6962785babc6e65350cb3 but expected \ + bc5e892aa84dc6240a5fbcd59367a4857d26f49b\n\ + To https://relay.ngit.dev/npub1owner/signed-test.git\n\ + ! [remote rejected] main -> main (incorrect old value provided)\n\ + error: failed to push some refs to 'https://relay.ngit.dev/npub1owner/signed-test.git'"; + assert!(is_transient_grasp_denial(reported)); + assert!(is_stale_advertisement_race(reported)); + + // Markers match independently of the surrounding git output. + assert!(is_stale_advertisement_race( + "cannot lock ref 'refs/heads/main'" + )); + assert!(is_stale_advertisement_race( + "! [remote rejected] main -> main (incorrect old value provided)" + )); + + // A purgatory denial is not a stale-advertisement race. + assert!(!is_stale_advertisement_race("No state events in purgatory")); + + // A real divergence is a different error and stays permanent. + assert!(!is_transient_grasp_denial( + " ! [rejected] main -> main (non-fast-forward)" + )); + } + + #[test] + fn transient_denial_markers_match_case_insensitively() { + assert!(is_transient_grasp_denial( + "ERR NO STATE EVENTS IN PURGATORY" + )); + } + + #[test] + fn push_outcome_reports_partial_failures() { + let outcome = PushOutcome { + servers: vec![ + GraspServerResult::ok( + RelayUrl::parse("wss://gitnostr.com").expect("url"), + "https://gitnostr.com/npub1owner/repo.git".to_owned(), + ), + GraspServerResult::failed( + RelayUrl::parse("wss://relay.ngit.dev").expect("url"), + "https://relay.ngit.dev/npub1owner/repo.git".to_owned(), + "remote: ERR authorisation failed: No state events in purgatory\nfatal: ...", + ), + ], + state_event: None, + }; + + assert_eq!(outcome.accepted(), 1); + assert_eq!( + outcome.failure_summary(), + "wss://relay.ngit.dev: remote: ERR authorisation failed: No state events in \ + purgatory fatal: ..." + ); + let warning = outcome.partial_warning().expect("partial push warning"); + assert!(warning.starts_with("Pushed to 1 of 2 grasp servers")); + assert!(warning.contains("Republish to sync")); + // The multi-line server reason is a single display line. + assert_eq!(warning.lines().count(), 1); + } + + #[test] + fn push_outcome_with_every_server_ok_has_no_warning() { + let outcome = PushOutcome { + servers: vec![ + GraspServerResult::ok( + RelayUrl::parse("wss://gitnostr.com").expect("url"), + "https://gitnostr.com/npub1owner/repo.git".to_owned(), + ), + GraspServerResult::ok( + RelayUrl::parse("wss://relay.ngit.dev").expect("url"), + "https://relay.ngit.dev/npub1owner/repo.git".to_owned(), + ), + ], + state_event: None, + }; + + assert_eq!(outcome.accepted(), 2); + assert!(outcome.partial_warning().is_none()); + assert_eq!(outcome.failure_summary(), ""); + } + + #[test] + fn push_outcome_without_servers_or_pushes_has_no_warning() { + assert!(PushOutcome::default().partial_warning().is_none()); + } + + #[test] + fn flatten_whitespace_collapses_and_clips_long_errors() { + assert_eq!(flatten_whitespace("a\n\n b \t c"), "a b c"); + let long = "word ".repeat(100); + let flat = flatten_whitespace(&long); + assert!(flat.ends_with('…')); + assert_eq!(flat.chars().count(), 201); + } } diff --git a/crates/signed_state/src/repo.rs b/crates/signed_state/src/repo.rs index eb12437..e2d2e1d 100644 --- a/crates/signed_state/src/repo.rs +++ b/crates/signed_state/src/repo.rs @@ -59,6 +59,10 @@ pub struct RepoStore { /// /// Example, a PR published without its commit reaching a grasp server. pub last_warning: Option, + /// Warning of the last push that only some grasp servers accepted. + /// + /// The repository is out of sync on the rejected servers until it is republished. + pub last_push_warning: Option, /// A republish or a checkout push is in flight. /// /// Views show a spinner and disable their push triggers while it is set. @@ -137,6 +141,7 @@ impl RepoStore { version: 0, last_error: None, last_warning: None, + last_push_warning: None, pushing: false, cloning: false, repo_relays: HashSet::new(), @@ -1086,6 +1091,7 @@ impl RepoStore { self.pushing = true; self.last_error = None; + self.last_push_warning = None; cx.notify(); let backend = Backend::global(cx); @@ -1097,14 +1103,23 @@ impl RepoStore { this.update(cx, |this, cx| { this.pushing = false; - if let Err(e) = &result { - this.last_error = Some(format!("Push failed: {e}")); + match &result { + Ok(outcome) => { + this.last_error = None; + // A push only some grasp servers accepted is a warning: + // the repo is out of sync on the rest until it is republished. + this.last_push_warning = outcome.partial_warning(); + } + Err(e) => { + this.last_error = Some(format!("Push failed: {e}")); + this.last_push_warning = None; + } } cx.notify(); })?; - result + result.map(|_| ()) }) } @@ -1131,6 +1146,7 @@ impl RepoStore { self.pushing = true; self.last_error = None; + self.last_push_warning = None; cx.notify(); let checkouts = CheckoutsStore::global(cx); @@ -1146,7 +1162,11 @@ impl RepoStore { this.pushing = false; match &result { - Ok(()) => { + Ok(outcome) => { + this.last_error = None; + // A push only some grasp servers accepted is a warning: + // the repo is out of sync on the rest until it is republished. + this.last_push_warning = outcome.partial_warning(); // The remote moved, so recompute the ready-to-push statuses. checkouts.update(cx, |store, cx| { store.checkout_pushed(&addr, &path, cx); @@ -1154,13 +1174,14 @@ impl RepoStore { } Err(e) => { this.last_error = Some(format!("Push failed: {e}")); + this.last_push_warning = None; } } cx.notify(); })?; - result + result.map(|_| ()) }) } diff --git a/crates/workspace/src/views/repo_detail/mod.rs b/crates/workspace/src/views/repo_detail/mod.rs index f955430..0c9e384 100644 --- a/crates/workspace/src/views/repo_detail/mod.rs +++ b/crates/workspace/src/views/repo_detail/mod.rs @@ -1992,10 +1992,75 @@ impl RepoDetailView { ) } + /// Warning after a push that only some grasp servers accepted. + fn render_push_warning_banner(&self, cx: &Context) -> Option { + let store = self.store.as_ref()?; + let store = store.read(cx); + let warning = store.last_push_warning.clone()?; + let pushing = store.pushing; + + Some( + h_flex() + .p_4() + .gap_2() + .w_full() + .items_start() + .justify_between() + .bg(cx.theme().warning.mix_oklab(transparent_white(), 0.08)) + .child( + h_flex() + .gap_2() + .min_w_0() + .flex_1() + .items_start() + .child(Icon::new(IconName::TriangleAlert).small().flex_shrink_0()) + .child( + div() + .flex_1() + .min_w_0() + .text_sm() + .text_color(cx.theme().warning) + .child(SharedString::from(warning)), + ), + ) + .child( + h_flex() + .gap_1() + .flex_shrink_0() + .child( + Button::new("republish-after-partial-push") + .icon(CustomIconName::Init) + .label("Republish") + .small() + .info() + .loading(pushing) + .disabled(pushing) + .on_click(cx.listener(|this, _event, window, cx| { + this.push_repository(window, cx); + })), + ) + .child( + Button::new("dismiss-push-warning") + .icon(IconName::Close) + .tooltip("Dismiss") + .small() + .ghost() + .disabled(pushing) + .on_click(cx.listener(|this, _ev, _window, cx| { + if let Some(store) = this.store.clone() { + store.update(cx, |store, _| { + store.last_push_warning = None; + }); + } + cx.notify(); + })), + ), + ) + .into_any_element(), + ) + } + /// The ready-to-contribute banner of the repository panel. - /// - /// A checkout has commits ahead of its base branch, with a Create action - /// opening the prefilled New PR panel, and a dismiss control. fn render_ready_banner(&self, cx: &Context) -> Option { let status = self.ready_suggestion(cx)?; let key = (status.path.clone(), status.branch.clone()); @@ -2324,6 +2389,9 @@ impl Render for RepoDetailView { .id("repo") .size_full() .when_some(banner, |this, banner| this.child(banner)) + .when_some(self.render_push_warning_banner(cx), |this, banner| { + this.child(banner) + }) .child(self.render_header(cx)) .when_some(error, |this, error| { this.child( diff --git a/docs/GRASP_PUSH_FIX.md b/docs/GRASP_PUSH_FIX.md new file mode 100644 index 0000000..bac9554 --- /dev/null +++ b/docs/GRASP_PUSH_FIX.md @@ -0,0 +1,298 @@ +# Plan: reliable pushes to multiple grasp servers + +Status: implemented (2026-09-07), see the change summary below. + +A push to a repo announced with several grasp servers sometimes reaches only +one of them. The grasp server on `relay.ngit.dev` rejected the git push with +`remote: ERR authorisation failed: No state events in purgatory`, while +`gitnostr.com` accepted it. The push still reported success because the app +counts an operation as pushed when at least one grasp server accepted; +individual failures only hit the log as `WARN grasp push failed: ...`. The +failed server silently stays behind until something pushes to it again. + +## Background: how a grasp push is authorized + +A push to a GRASP server is a two-stage transaction, not a plain git push: + +1. The client publishes a NIP-34 kind-30618 *state event* listing the refs it + is about to push (`refs/heads/main -> ` etc.). +2. The client git-pushes the objects to `https:////.git`. + +The grasp server does not trust the git push on its own. State events it +accepts are held in an in-memory **purgatory** - accepted but not served, +"until git data arrives" - and the git push is authorized **only against +purgatory state events**, never against the database (the database is the +current state; purgatory holds the intended future state). When a push arrives +and purgatory holds no state event for that repo id, the server rejects with +`No state events in purgatory` (ngit-grasp `git/authorization.rs`), surfaced to +the client through the git protocol as the `remote: ERR authorisation failed: +...` line above. Parked events that are never matched by git data are discarded +after ~30 minutes. + +The relay and the git server of a grasp entry share a host: `wss://gitnostr.com` +and `wss://relay.ngit.dev` are relay URLs, and the git URL is derived with +`grasp_base_url` (`https:////.git`). The grasp learns about +state events from its own relay. A relay accept for a state event parks it in +purgatory before the grasp sends its OK, and the nostr-sdk default ack policy +(`AckPolicy::all`) waits for that OK - so a confirmed stage means the state is +in place to authorize the push. + +## Root causes in the app + +| # | Problem | Consequence | +|---|---|---| +| 1 | State event is published to the whole relay pool, then git is pushed to every grasp server - no per-server staging, no confirmation that the target grasp's own relay accepted it | a push can reach a grasp whose purgatory is still empty | +| 2 | No retry for that denial class | a self-resolving race permanently leaves one server behind | +| 3 | `push_to_grasp_servers` returns `Ok` when >= 1 server accepts; failures only `log::warn!` | UI shows success; the failed grasp silently out of sync | +| 4 | When a grasp's relay is down, the client still fires a doomed git push | confusing git-level `ERR` instead of a clear "state not accepted by relay" | + +Relevant code (`crates/signed_state/src/backend.rs`): + +- `push_repo_from` (L752-834): broadcast the state event to the whole pool, + then `push_to_grasp_servers`. +- `push_to_grasp_servers` (L1536-1570): sequential git pushes, `Ok` when at + least one server accepted, `warn!` per failure. +- `broadcast_event` (L1336-1350): whole-pool `client.send_event`, errors only + when no relay accepted. +- `send` / `publish_task` (L1239-1292): sign, broadcast, store locally (the + client saves accepted events), emit `BackendEvent::Published`. +- `create_repository` (L554-575) and `publish_local_repo` (L692-708): direct + `push_to_grasp_servers` calls; retract announcement+state when zero servers + accept. + +The fix mirrors ngit's own client (its `state_transaction.rs` stages the state +event on the grasp relays, gates which servers are pushed, and fans the state +out to other relays only after a git server accepted) and adds retries, because +the app must also cope with a denial after a relay ack. + +## Target flow: staged push with retries + +```mermaid +sequenceDiagram + participant App as signed app + participant R1 as gitnostr.com relay+grasp + participant R2 as relay.ngit.dev relay+grasp + participant O as other relays + + Note over App: build state event S0 from local refs + App->>R1: stage S0 to grasp relay only (.to(url)) + App->>R2: stage S0 to grasp relay only (.to(url)) + R1-->>App: OK (parked in purgatory) -> eligible + R2-->>App: OK (parked in purgatory) -> eligible + App->>R1: git push + R1-->>App: authorized by S0 in purgatory + App->>R2: git push + R2-->>App: ERR ... No state events in purgatory (transient) + Note over App: retry: stage a fresh S1 to R2, wait ~1s, git push again + App->>R2: stage fresh S1 (new event id) + App->>R2: git push + R2-->>App: authorized + Note over App: >= 1 server ok -> fan out state to the pool + App->>O: broadcast state (only after a git server holds it) +``` + +Retries stage a **fresh** state event (new `created_at` -> new id) rather than +resending the same one: a grasp relay treats a same-id event as a duplicate and +will not re-run its policy, so a lost purgatory entry (e.g. a server restart +without its state file) cannot be re-parked by a resend. A fresh id re-runs +validation and re-parks. Superseded parked events expire server-side after 30 +minutes, so the residue is bounded. + +## Changes + +Implemented: + +- `crates/signed_state/src/backend.rs`: `push_to_grasp_servers` replaced by the + staged orchestration `push_staged_to_grasps` plus `stage_event_on_relay`, + `sign_state_event`, `is_transient_grasp_denial`, and the `GraspServerResult` / + `PushOutcome` result types. `create_repository`, `publish_local_repo` and + `push_repo_from` (repo push and checkout push) all push through it and fan the + state out to the relay pool only after at least one git server accepted. +- `crates/signed_state/src/repo.rs`: push outcomes surface partial failures as + `RepoStore::last_push_warning`. +- `crates/workspace/src/views/repo_detail/mod.rs`: a warning banner with a + Republish action shows when a push did not reach every grasp server. +- Unit tests for the denial classifier and the outcome reporting. + +Decisions taken while implementing: + +- State events are no longer broadcast before any git data exists. They are + staged per grasp relay, and only fanned out to the relay pool after a git + server accepted. A total push failure therefore leaves nothing served to + retract except the announcement (create/publish flows retract that, as + before). Staged-but-unmatched state events expire in the grasp's purgatory + after ~30 minutes. +- Servers whose relay rejects the state event are **skipped**, not pushed + (the eligibility gate): a doomed git push would only produce the same + denial. +- **Stale-ref races are retried and verified.** The grasp server runs its own + background sync that aligns repository refs to parked state events as soon + as the git objects are present - including objects an earlier denied attempt + of this same push already uploaded. `git receive-pack` then rejects the ref + update against its stale advertisement with `cannot lock ref ... is at ... + but expected ...` / `incorrect old value provided`. Such rejections are + retried against a fresh advertisement *and* probed for convergence: when + the sync already aligned the refs to this push's target (`git ls-remote` + matches, `signed_git::remote_has_refs`), the server is counted as accepted + even though the push's compare-and-swap never returned `Ok` - because the + data is already there. The sync can land a moment after the retry window, so + the probe is what makes these pushes succeed without a manual republish. +- Create/publish partial failures (some servers accepted, some not) stay + `Ok` and are logged, matching the pre-existing contract; the repo-push + paths additionally set `last_push_warning` so the failure is visible and + one-click republishable. +- The push warning lives in a dedicated `RepoStore::last_push_warning` so it + never collides with the PR-flow `last_warning` on the shared store. + +### Staged orchestration (`crates/signed_state/src/backend.rs`) + +1. **Per-relay publish helper** (`stage_event_on_relay`). Publishes only to + one relay and verifies its acceptance, using the pinned nostr-sdk 0.45 API + (`send_event_to` is deprecated at 0.45): + + ```rust + // client.send_event(event).to([relay.clone()]).await + // -> Ok only when the relay is in the output's success map + ``` + + The relay is added and connected first (reusing the `add_relay` pattern); + a failed connect is a staged failure with a clear reason. + +2. **Transient-denial classifier** (`is_transient_grasp_denial`, pure, + unit-tested). Two retry families: + + - **Purgatory denials** (ngit-grasp `git/authorization.rs`): + - `No state events in purgatory` (the originally reported failure) + - `no matching state event found in purgatory` + - `in purgatory ... doesn't match push` + - `none from authorized publishers` + - `No repository announcement found` (new repos whose announcement is + still propagating) + - **Stale-ref races** (git receive-pack compare-and-swap against the + advertised value, when the grasp's background sync moved the ref): + - `cannot lock ref` + - `incorrect old value provided` + + Stale-ref race rejections are additionally probed for convergence with + `git ls-remote`; a server that already advertises the pushed refs counts as + accepted (`remote_has_refs` in `signed_git`). + + Everything else - HTTP auth rejection, not a maintainer, a genuine + non-fast-forward divergence, network failure - is permanent for that + attempt. Only the transient classes are retried. + +3. **Orchestration** (`push_staged_to_grasps`, replaces `push_to_grasp_servers`): + + ```rust + pub struct GraspServerResult { relay: RelayUrl, git_url: String, reason: Option } + pub struct PushOutcome { servers: Vec, state_event: Option } + + async fn push_staged_to_grasps( + client: &Client, + signer: &UniversalSigner, + repo_id: &str, + refs: &[(String, String)], + head: Option<&str>, + path: &Path, + owner: &str, + servers: &[RelayUrl], + push: fn(&Path, &str, &str, &str) -> Result<(), Error>, + ) -> PushOutcome + ``` + + Per server, in `servers` order: + + - **Stage**: publish the state event to that relay (one retry for a connect + blip). Not accepted => record the reason and skip the git push (the + eligibility gate - no doomed push). + - **Push**: git push; on a transient denial, stage a **fresh** state event + (see below), back off ~1s, and retry. Budget: three attempts total. + - Permanent git error => record it and stop for that server. + + Retries stage a fresh event (new `created_at` -> new id) rather than + resending the same one: a grasp relay treats a same-id event as a duplicate + and will not re-run its policy, so a lost purgatory entry (e.g. a server + restart without its state file) cannot be re-parked by a resend. When a + retry would fall in the same wall-clock second, `sign_state_event` nudges + `created_at` forward by one second so the event id differs. Superseded + parked events expire server-side after 30 minutes. + +4. **Reorder `push_repo_from`**: replace the pre-push whole-pool `send(state)` + with staging + push through `push_staged_to_grasps`, then, after at least + one server accepted, fan the state out to the pool (`broadcast_event`) and + emit `BackendEvent::Published` once. Moving the fan-out after the first git + success also closes today's leak where state is broadcast before any server + holds the objects (ngit made the same change). + +5. **Other callers updated** (`create_repository`, `publish_local_repo`): same + orchestration. Their announcement broadcast stays first - grasps park + brand-new announcements in announcement purgatory until the first git data, + so there is no leak - and "zero servers accepted => retract announcement" is + preserved. An empty repository (no refs) is announced without staging or + pushing anything. + +### Outcome plumbing (`crates/signed_state/src/repo.rs`) + +- `RepoStore::push_repository` / `push_checkout` (L1075-1150) gain + + ```rust + last_push_warning: Option + ``` + + populated from `PushOutcome::partial_warning()` (a one-line summary naming + the rejected servers and reasons). `last_error` (zero-success hard failure) + and the PR-flow `last_warning` are unchanged. + +### UI (`crates/workspace/src/views/repo_detail/mod.rs`) + +- When `store.last_push_warning` is set, `render_push_warning_banner` shows a + warning banner above the header: + + > Pushed to 1 of 2 grasp servers: wss://relay.ngit.dev: remote: ERR + > authorisation failed: No state events in purgatory fatal: ... Republish to + > sync. + +- The banner's Republish action reuses `push_repository` (an idempotent + re-push of every announced server - the simplest catch-up for any drifted + server); a dismiss control clears the warning. +- Zero-success failures continue through the existing error banner (`self.error` + / `store.last_error`, rendered in `render`). + +### Tests + +- Unit (backend tests module): the classifier (both retry families - the + reported purgatory stderr line and the reported `cannot lock ref` race are + transient; HTTP 403, not-a-maintainer and a non-fast-forward divergence are + not), the outcome reporting (partial warnings are single-line and name the + failing servers, an all-ok outcome has no warning) and `flatten_whitespace`. + All `cargo test -p signed_state` tests pass. +- The staged orchestration itself is exercised against live grasps by manual + validation; the pure decision helpers are unit-tested. + +### Manual validation + +- Create and push a repo with both `gitnostr.com` and `relay.ngit.dev`; after + each push, `git ls-remote https:////.git` against **both** + hosts to confirm convergence; repeat over several commits to shake out the + race. Then repeat with `relay.ngit.dev` temporarily unreachable to exercise + the banner and the Retry action. + +## Behavior decisions + +- At least one server accepted => the operation succeeds (existing contract), + but partial failure is explicit and retryable (warning banner + Republish). +- Announcement publishing stays first; state fan-out moves after the first git + success (fixes the state-before-data leak). +- No wire-protocol changes; error text stays user-readable in the banner. + +## Open questions (status) + +1. **Auto-retry** after a partial success: not implemented - retries happen + inside the push (up to 3 attempts per server for transient denials), and a + residual failure stays visible with a manual Republish. A delayed + background re-push of failed servers could be added later. +2. **Post-push verification** (`git ls-remote` per grasp after a successful + push to confirm the refs are advertised): not implemented; could catch + ack-but-not-promoted cases at the cost of one extra round-trip per server. +3. **Scope**: the create, publish-local and repo/checkout push paths were all + converted in one change.