feat(push): avoid out of sync issues for ngit relay

we need to be careful with git servers with their own permissions so
a ngit user doesn't inadvertantly push changes on top of a another
user who pushed directly to the git server without using the
force flag.

We dont have this problem with ngit-relay so we can always force
push, even if the user didnt as nostr is the authority of state.

2 files changed, 39 insertions, 27 deletions

diff --git a/src/bin/git_remote_nostr/list.rs b/src/bin/git_remote_nostr/list.rs
index bf16f89..ea29531 100644
--- a/src/bin/git_remote_nostr/list.rs
+++ b/src/bin/git_remote_nostr/list.rs
@@ -31,7 +31,7 @@ pub async fn run_list(
     git_repo: &Repo,
     repo_ref: &RepoRef,
     for_push: bool,
-) -> Result<HashMap<String, HashMap<String, String>>> {
+) -> Result<HashMap<String, (HashMap<String, String>, bool)>> {
     let nostr_state = (get_state_from_cache(Some(git_repo.get_path()?), repo_ref).await).ok();
 
     let term = console::Term::stderr();
@@ -46,7 +46,7 @@ pub async fn run_list(
 
     let mut state = if let Some(nostr_state) = nostr_state {
         for (name, value) in &nostr_state.state {
-            for (url, remote_state) in &remote_states {
+            for (url, (remote_state, _is_ngit_relay)) in &remote_states {
                 let remote_name = get_short_git_server_name(git_repo, url);
                 if let Some(remote_value) = remote_state.get(name) {
                     if value.ne(remote_value) {
@@ -74,15 +74,16 @@ pub async fn run_list(
         }
         nostr_state.state
     } else {
-        repo_ref
+        let (state, _is_ngit_relay) = repo_ref
             .git_server
             .iter()
             .filter_map(|server| remote_states.get(server))
             .cloned()
-            .collect::<Vec<HashMap<String, String>>>()
+            .collect::<Vec<(HashMap<String, String>, bool)>>()
             .first()
             .context("failed to get refs from git server")?
-            .clone()
+            .clone();
+        state
     };
 
     state.retain(|k, _| !k.starts_with("refs/heads/pr/"));
@@ -112,7 +113,7 @@ async fn get_open_and_draft_proposals_state(
     term: &console::Term,
     git_repo: &Repo,
     repo_ref: &RepoRef,
-    remote_states: &HashMap<String, HashMap<String, String>>,
+    remote_states: &HashMap<String, (HashMap<String, String>, bool)>,
 ) -> Result<HashMap<String, String>> {
     // we cannot use commit_id in the latest patch in a proposal because:
     // 1) the `commit` tag is optional
@@ -121,7 +122,7 @@ async fn get_open_and_draft_proposals_state(
 
     // without trusting commit_id we must apply each patch which requires the oid of
     // the parent so we much do a fetch
-    for (git_server_url, oids_from_git_servers) in remote_states {
+    for (git_server_url, (oids_from_git_servers, is_ngit_relay)) in remote_states {
         if fetch_from_git_server(
             git_repo,
             &oids_from_git_servers
@@ -132,7 +133,7 @@ async fn get_open_and_draft_proposals_state(
             git_server_url,
             &repo_ref.to_nostr_git_url(&None),
             term,
-            is_ngit_relay(git_server_url, &repo_ref.ngit_relays()),
+            *is_ngit_relay,
         )
         .is_ok()
         {
@@ -178,22 +179,17 @@ pub fn list_from_remotes(
     git_servers: &Vec<String>,
     decoded_nostr_url: &NostrUrlDecoded,
     ngit_relays: &[String],
-) -> HashMap<String, HashMap<String, String>> {
+) -> HashMap<String, (HashMap<String, String>, bool)> {
     let mut remote_states = HashMap::new();
     let mut errors = HashMap::new();
     for url in git_servers {
-        match list_from_remote(
-            term,
-            git_repo,
-            url,
-            decoded_nostr_url,
-            is_ngit_relay(url, ngit_relays),
-        ) {
+        let is_ngit_relay = is_ngit_relay(url, ngit_relays);
+        match list_from_remote(term, git_repo, url, decoded_nostr_url, is_ngit_relay) {
             Err(error) => {
                 errors.insert(url, error);
             }
             Ok(state) => {
-                remote_states.insert(url.to_string(), state);
+                remote_states.insert(url.to_string(), (state, is_ngit_relay));
             }
         }
     }
diff --git a/src/bin/git_remote_nostr/push.rs b/src/bin/git_remote_nostr/push.rs
index a34d729..c4953f8 100644
--- a/src/bin/git_remote_nostr/push.rs
+++ b/src/bin/git_remote_nostr/push.rs
@@ -50,13 +50,14 @@ use crate::{
 };
 
 #[allow(clippy::too_many_lines)]
+#[allow(clippy::type_complexity)]
 pub async fn run_push(
     git_repo: &Repo,
     repo_ref: &RepoRef,
     stdin: &Stdin,
     initial_refspec: &str,
     client: &Client,
-    list_outputs: Option<HashMap<String, HashMap<String, String>>>,
+    list_outputs: Option<HashMap<String, (HashMap<String, String>, bool)>>,
 ) -> Result<()> {
     let refspecs = get_refspecs_from_push_batch(stdin, initial_refspec)?;
 
@@ -93,7 +94,8 @@ pub async fn run_push(
             .iter()
             .find(|&url| list_outputs.contains_key(url))
         {
-            list_outputs.get(url).unwrap().to_owned()
+            let (state, _is_ngit_relay) = list_outputs.get(url).unwrap().to_owned();
+            state
         } else {
             bail!(
                 "failed to connect to git servers: {}",
@@ -706,13 +708,14 @@ fn create_rejected_refspecs_and_remotes_refspecs(
     git_repo: &Repo,
     refspecs: &Vec<String>,
     nostr_state: &HashMap<String, String>,
-    list_outputs: &HashMap<String, HashMap<String, String>>,
+    list_outputs: &HashMap<String, (HashMap<String, String>, bool)>,
 ) -> Result<(HashMapUrlRefspecs, HashMapUrlRefspecs)> {
     let mut refspecs_for_remotes = HashMap::new();
 
     let mut rejected_refspecs: HashMapUrlRefspecs = HashMap::new();
 
-    for (url, remote_state) in list_outputs {
+    for (url, (remote_state, is_ngit_relay)) in list_outputs {
+        let is_ngit_relay = is_ngit_relay.to_owned();
         let short_name = get_short_git_server_name(git_repo, url);
         let mut refspecs_for_remote = vec![];
         for refspec in refspecs {
@@ -747,12 +750,7 @@ fn create_rejected_refspecs_and_remotes_refspecs(
                         if is_remote_tip_ancestor_of_commit {
                             refspecs_for_remote.push(refspec.clone());
                         } else {
-                            // this is a force push so we need to force push to git server too
-                            if refspec.starts_with('+') {
-                                refspecs_for_remote.push(refspec.clone());
-                            } else {
-                                refspecs_for_remote.push(format!("+{refspec}"));
-                            }
+                            refspecs_for_remote.push(ensure_force_push_refspec(refspec));
                         }
                     } else if let Ok(remote_value_tip) =
                         git_repo.get_commit_or_tip_of_reference(remote_value)
@@ -778,6 +776,9 @@ fn create_rejected_refspecs_and_remotes_refspecs(
                             if ahead_of_nostr.is_empty() {
                                 // ancestor of nostr and we are force pushing anyway...
                                 refspecs_for_remote.push(refspec.clone());
+                            } else if is_ngit_relay {
+                                // an ngit-relay can only be pushed to via nostr so can force push
+                                refspecs_for_remote.push(ensure_force_push_refspec(refspec));
                             } else {
                                 rejected_refspecs
                                     .entry(refspec.to_string())
@@ -794,6 +795,8 @@ fn create_rejected_refspecs_and_remotes_refspecs(
                                 )?;
                             }
                         }
+                    } else if is_ngit_relay {
+                        refspecs_for_remote.push(ensure_force_push_refspec(refspec));
                     } else {
                         // remote_value oid is not present locally
                         // TODO can we download the remote reference?
@@ -827,6 +830,8 @@ fn create_rejected_refspecs_and_remotes_refspecs(
                     if ahead.is_empty() {
                         // can soft push
                         refspecs_for_remote.push(refspec.clone());
+                    } else if is_ngit_relay {
+                        refspecs_for_remote.push(ensure_force_push_refspec(refspec));
                     } else {
                         // cant soft push
                         rejected_refspecs
@@ -841,6 +846,8 @@ fn create_rejected_refspecs_and_remotes_refspecs(
                                     ).as_str(),
                                 )?;
                     }
+                } else if is_ngit_relay {
+                    refspecs_for_remote.push(ensure_force_push_refspec(refspec));
                 } else {
                     // havn't fetched oid from remote
                     // TODO fetch oid from remote
@@ -878,6 +885,15 @@ fn create_rejected_refspecs_and_remotes_refspecs(
     Ok((rejected_refspecs, remotes_refspecs_without_rejected))
 }
 
+fn ensure_force_push_refspec(refspec: &str) -> String {
+    // Check if the refspec starts with '+' or ':'
+    if refspec.starts_with('+') || refspec.starts_with(':') {
+        refspec.to_string() // Return as is
+    } else {
+        format!("+{refspec}") // Add '+' prefix
+    }
+}
+
 fn generate_updated_state(
     git_repo: &Repo,
     existing_state: &HashMap<String, String>,