clean up

crates/signed_git/src/repo.rs

@@ -1,4 +1,4 @@
-use std::collections::{BTreeSet, HashMap, HashSet};
+use std::collections::{BTreeSet, HashSet};
 use std::io::Write;
 use std::path::{Path, PathBuf};
 use std::process::Stdio;
@@ -1262,55 +1262,6 @@ impl Repo {
         Ok(())
     }
 
-    /// Checks that `url` advertises exactly the expected ref oids.
-    pub fn remote_has_refs(&self, url: &str, expected: &[(String, String)]) -> Result<bool> {
-        if expected.is_empty() {
-            return Ok(true);
-        }
-
-        let url = Self::transport_url(url);
-
-        let refspecs = expected
-            .iter()
-            .map(|(name, _)| {
-                gix::refspec::parse(
-                    gix::bstr::BStr::new(format!("+{name}:{name}").as_bytes()),
-                    gix::refspec::parse::Operation::Fetch,
-                )
-                .map(|spec| spec.to_owned())
-            })
-            .collect::<Result<Vec<_>, _>>()
-            .into_anyhow()
-            .context("invalid refspec")?;
-
-        let options = gix::remote::ref_map::Options {
-            extra_refspecs: refspecs,
-            ..Default::default()
-        };
-
-        let (refs, _) = self
-            .inner
-            .remote_at(url.as_str())
-            .with_context(|| format!("cannot use remote {url}"))?
-            .connect(gix::remote::Direction::Fetch)
-            .with_context(|| format!("cannot connect to {url}"))?
-            .ref_map(Discard, options)
-            .with_context(|| format!("listing refs of {url} failed"))?;
-
-        let advertised: HashMap<String, String> = refs
-            .remote_refs
-            .iter()
-            .filter_map(|reference| {
-                let (name, object, _peeled) = reference.unpack();
-                object.map(|oid| (String::from_utf8_lossy(name).into_owned(), oid.to_string()))
-            })
-            .collect();
-
-        Ok(expected
-            .iter()
-            .all(|(name, oid)| advertised.get(name.as_str()) == Some(oid)))
-    }
-
     /// Sets the `origin` URL when the repository has no remote yet.
     pub fn ensure_origin(&self, url: &str) -> Result<()> {
         if self.inner.find_remote("origin").is_ok() {

crates/signed_git/src/tests.rs

@@ -113,42 +113,6 @@ fn ref_state_matches_the_git_cli_ref_listing() {
     );
 }
 
-#[test]
-fn remote_has_refs_reports_whether_pushed_refs_landed() {
-    let server = tempfile::tempdir().unwrap();
-    bare_server(server.path(), "npub1test", "my-repo");
-    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())];
-    let repo = Repo::open(dir).expect("open");
-
-    assert!(!repo.remote_has_refs(&url, &expected).expect("probe"));
-
-    repo.push_all(
-        &format!("file://{}", server.path().display()),
-        "npub1test",
-        "my-repo",
-    )
-    .expect("push");
-
-    assert!(repo.remote_has_refs(&url, &expected).expect("probe"));
-
-    let stale = vec![("refs/heads/main".to_owned(), "0".repeat(40))];
-    assert!(!repo.remote_has_refs(&url, &stale).expect("probe"));
-
-    git_run(dir, &["tag", "v1.0"]);
-    repo.push_all(
-        &format!("file://{}", server.path().display()),
-        "npub1test",
-        "my-repo",
-    )
-    .expect("push");
-    assert!(repo.remote_has_refs(&url, &expected).expect("probe"));
-}
-
 #[test]
 fn repo_ref_state_lists_branches_tags_and_head() {
     let (_dir, repo) = fixture(&[("a.txt", b"hello")]);

crates/signed_state/src/backend.rs

@@ -5,9 +5,7 @@ use std::time::{Duration, Instant};
 
 use anyhow::{Error, anyhow, bail};
 use bitcoin_hashes::sha1::Hash as Sha1Hash;
-use gpui::{
+use gpui::{App, AppContext, AsyncApp, Context, Entity, EventEmitter, Global, Task, TaskExt};
-    App, AppContext, AsyncApp, Context, Entity, EventEmitter, Global, Task, TaskExt, WeakEntity,
-};
 use nostr::event::IntoEventBuilder;
 use nostr::nips::nip19::Nip19Coordinate;
 use nostr_connect::prelude::*;
@@ -194,7 +192,7 @@ impl Backend {
                         this.set_signer(keys, cx);
                     })?;
                 } else if content.starts_with("bunker://") {
-                    let (base, keys) = extract_master_key(&content);
+                    let (base, keys) = extract_master_key(&content)?;
                     let uri = NostrConnectUri::parse(base)?;
                     let mut signer = NostrConnect::new(
                         uri,
@@ -463,7 +461,7 @@ impl Backend {
                         let event = builder.finalize_async(&signer).await?;
                         let output = client.send_event(&event).broadcast().await?;
 
-                        if output.success.is_empty() && !output.failed.is_empty() {
+                        if output.success.is_empty() {
                             let reasons = output
                                 .failed
                                 .values()
@@ -539,7 +537,7 @@ impl Backend {
             folder.join(dir_name)
         };
 
-        cx.spawn(async move |this, cx| {
+        cx.spawn(async move |_this, cx| {
             let work = cx.background_spawn({
                 let destination = destination.clone();
                 let name = name.clone();
@@ -577,7 +575,6 @@ impl Backend {
             );
 
             let event = announce_repository_and_push(
-                &this,
                 &client,
                 &signer,
                 announcement,
@@ -643,7 +640,7 @@ impl Backend {
         let client = self.client.clone();
         let signer = self.signer.clone();
 
-        cx.spawn(async move |this, cx| {
+        cx.spawn(async move |_this, cx| {
             let (mut state, euc) = cx
                 .background_spawn({
                     let path = path.clone();
@@ -672,7 +669,6 @@ impl Backend {
             let ann = build_announcement(&repo_id, &name, &description, &owner, &servers, euc);
 
             let event = announce_repository_and_push(
-                &this,
                 &client,
                 &signer,
                 ann,
@@ -1155,11 +1151,15 @@ impl Backend {
             cx.background_spawn(async move {
                 let builder = EventDeletionRequest::new().id(event.id);
                 let deletion = builder.finalize_async(&signer).await?;
-                client.send_event(&deletion).broadcast().await?;
+                let output = client.send_event(&deletion).broadcast().await?;
 
-                Ok::<(), anyhow::Error>(())
+                if output.success.is_empty() {
+                    bail!("deletion event not accepted by any relay");
+                }
+
+                Ok::<(), Error>(())
             })
-            .detach();
+            .detach_and_log_err(cx);
         }
     }
 }
@@ -1190,7 +1190,6 @@ fn build_announcement(
 // Announce the repository, then stage the state event and push.
 #[allow(clippy::too_many_arguments)]
 async fn announce_repository_and_push(
-    backend: &WeakEntity<Backend>,
     client: &Client,
     signer: &UniversalSigner,
     announcement: GitRepositoryAnnouncement,
@@ -1204,13 +1203,15 @@ async fn announce_repository_and_push(
     push: impl Fn(&Path, &str, &str, &str) -> Result<(), Error> + Send + 'static,
 ) -> Result<Event, Error> {
     for url in servers {
-        client.add_relay(url).and_connect().await.ok();
+        if let Err(e) = client.add_relay(url).and_connect().await {
+            log::warn!("failed to connect grasp relay {url}: {e}");
+        }
     }
 
     let event = announcement.finalize_async(signer).await?;
     let output = client.send_event(&event).broadcast().await?;
 
-    if output.success.is_empty() && !output.failed.is_empty() {
+    if output.success.is_empty() {
         let reasons = output
             .failed
             .values()
@@ -1252,17 +1253,34 @@ async fn announce_repository_and_push(
 
     if outcome.accepted() == 0 {
         // Retract the announcement so the repository is not left announced without content.
-        backend
+        let retracted = async {
-            .update(cx, |backend, cx| {
+            let deletion = EventDeletionRequest::new()
-                backend.retract_events(std::slice::from_ref(&event), cx);
+                .id(event.id)
-            })
+                .finalize_async(signer)
-            .ok();
+                .await?;
+            let output = client.send_event(&deletion).broadcast().await?;
 
-        return Err(anyhow!(
+            // Without relay acceptance the deletion request stays live.
+            if output.success.is_empty() {
+                bail!("deletion event not accepted by any relay");
+            }
+
+            Ok::<(), Error>(())
+        }
+        .await;
+
+        return Err(match retracted {
+            Ok(()) => anyhow!(
                 "The repository was announced, but the push to every grasp server failed: {}. \
                  The announcement has been retracted",
                 outcome.failure_summary()
-        ));
+            ),
+            Err(e) => anyhow!(
+                "The repository was announced, but the push to every grasp server failed: {}. \
+                 Retracting the announcement failed: {e}; delete the repository to clean it up",
+                outcome.failure_summary()
+            ),
+        });
     }
 
     // Fan the state out to the relays once a git server holds the objects.
@@ -1313,15 +1331,13 @@ impl UpdateEvent {
     }
 }
 
-fn extract_master_key(credential: &str) -> (&str, Keys) {
+fn extract_master_key(credential: &str) -> Result<(&str, Keys), Error> {
     match credential.split_once("master=") {
         Some((base, nsec)) => {
-            let keys = SecretKey::parse(nsec)
+            let keys = Keys::new(SecretKey::parse(nsec)?);
-                .map(Keys::new)
+            Ok((base.trim_end_matches(['?', '&']), keys))
-                .unwrap_or_else(|_| Keys::generate());
-            (base.trim_end_matches(['?', '&']), keys)
         }
-        None => (credential, Keys::generate()),
+        None => Ok((credential, Keys::generate())),
     }
 }