2 files changed, 143 insertions, 66 deletions

diff --git a/src/bin/git_remote_nostr/fetch.rs b/src/bin/git_remote_nostr/fetch.rs
index 2e16297..46e7ad3 100644
--- a/src/bin/git_remote_nostr/fetch.rs
+++ b/src/bin/git_remote_nostr/fetch.rs
@@ -1,11 +1,12 @@
 use core::str;
 use std::{
+    collections::HashMap,
     io::Stdin,
     sync::{Arc, Mutex},
     time::Instant,
 };
 
-use anyhow::{anyhow, bail, Result};
+use anyhow::{anyhow, bail, Context, Result};
 use auth_git2::GitAuthenticator;
 use git2::{Progress, Repository};
 use ngit::{
@@ -19,7 +20,7 @@ use ngit::{
     repo_ref::RepoRef,
 };
 use nostr::nips::nip19;
-use nostr_sdk::ToBech32;
+use nostr_sdk::{Event, ToBech32};
 
 use crate::utils::{
     count_lines_per_msg_vec, fetch_or_list_error_is_not_authentication_failure,
@@ -78,65 +79,97 @@ pub async fn run_fetch(
 
     fetch_batch.retain(|refstr, _| refstr.contains("refs/heads/pr/"));
 
-    if !fetch_batch.is_empty() {
+    fetch_proposals(git_repo, &term, repo_ref, &fetch_batch).await?;
+    term.flush()?;
+    println!();
+    Ok(())
+}
+
+pub fn make_commits_for_proposal(
+    git_repo: &Repo,
+    repo_ref: &RepoRef,
+    patches_ancestor_last: &[Event],
+) -> Result<String> {
+    let patches_ancestor_first: Vec<&Event> = patches_ancestor_last.iter().rev().collect();
+    let mut tip_commit_id = if let Ok(parent_commit) = tag_value(
+        patches_ancestor_first
+            .first()
+            .context("proposal should have at least one patch")?,
+        "parent-commit",
+    ) {
+        parent_commit
+    } else {
+        // TODO choose most recent commit on master before patch timestamp so it doesnt
+        // constantly get rebased
+        let (_, hash) = git_repo.get_main_or_master_branch()?;
+        hash.to_string()
+    };
+
+    for patch in &patches_ancestor_first {
+        let commit_id = git_repo
+            .create_commit_from_patch(patch, Some(tip_commit_id.clone()))
+            .context(format!(
+                "cannot create commit for patch {}",
+                nip19::Nip19Event {
+                    event_id: patch.id(),
+                    author: Some(patch.author()),
+                    kind: Some(patch.kind()),
+                    relays: if let Some(relay) = repo_ref.relays.first() {
+                        vec![relay.to_string()]
+                    } else {
+                        vec![]
+                    },
+                }
+                .to_bech32()
+                .unwrap_or_default()
+            ))?;
+        tip_commit_id = commit_id.to_string();
+    }
+    Ok(tip_commit_id)
+}
+
+async fn fetch_proposals(
+    git_repo: &Repo,
+    term: &console::Term,
+    repo_ref: &RepoRef,
+    proposal_refs: &HashMap<String, String>,
+) -> Result<()> {
+    if !proposal_refs.is_empty() {
         let open_proposals = get_open_proposals(git_repo, repo_ref).await?;
 
         let current_user = get_curent_user(git_repo)?;
 
-        for (refstr, oid) in fetch_batch {
+        for refstr in proposal_refs.keys() {
             if let Some((_, (_, patches))) =
-                find_proposal_and_patches_by_branch_name(&refstr, &open_proposals, &current_user)
+                find_proposal_and_patches_by_branch_name(refstr, &open_proposals, &current_user)
             {
-                if !git_repo.does_commit_exist(&oid)? {
-                    let mut patches_ancestor_first = patches.clone();
-                    patches_ancestor_first.reverse();
-                    if git_repo.does_commit_exist(&tag_value(
-                        patches_ancestor_first.first().unwrap(),
-                        "parent-commit",
-                    )?)? {
-                        for patch in &patches_ancestor_first {
-                            if let Err(error) = git_repo.create_commit_from_patch(patch) {
-                                term.write_line(
-                                    format!(
-                                        "WARNING: cannot create branch for {refstr}, error: {error} for patch {}",
-                                        nip19::Nip19Event {
-                                            event_id: patch.id(),
-                                            author: Some(patch.author()),
-                                            kind: Some(patch.kind()),
-                                            relays: if let Some(relay) = repo_ref.relays.first() {
-                                                vec![relay.to_string()]
-                                            } else { vec![]},
-                                        }.to_bech32().unwrap_or_default()
-                                    )
-                                    .as_str(),
-                                )?;
-                                break;
-                            }
-                        }
-                    } else {
-                        term.write_line(
-                            format!("WARNING: cannot find parent commit for {refstr}").as_str(),
-                        )?;
-                    }
+                if let Err(error) = make_commits_for_proposal(git_repo, repo_ref, patches) {
+                    term.write_line(
+                        format!("WARNING: cannot create branch for {refstr}, error: {error}",)
+                            .as_str(),
+                    )?;
+                    break;
                 }
-            } else {
-                term.write_line(format!("WARNING: cannot find proposal for {refstr}").as_str())?;
             }
         }
     }
-
-    term.flush()?;
-    println!();
     Ok(())
 }
 
-fn fetch_from_git_server(
+pub fn fetch_from_git_server(
     git_repo: &Repo,
     oids: &[String],
     git_server_url: &str,
     decoded_nostr_url: &NostrUrlDecoded,
     term: &console::Term,
 ) -> Result<()> {
+    let already_have_oids = oids
+        .iter()
+        .all(|oid| git_repo.does_commit_exist(oid).is_ok_and(|outcome| outcome));
+    if already_have_oids {
+        return Ok(());
+    }
+
     let server_url = git_server_url.parse::<CloneUrl>()?;
 
     let protocols_to_attempt = get_read_protocols_to_try(git_repo, &server_url, decoded_nostr_url);
diff --git a/src/bin/git_remote_nostr/list.rs b/src/bin/git_remote_nostr/list.rs
index 959b8c8..378a124 100644
--- a/src/bin/git_remote_nostr/list.rs
+++ b/src/bin/git_remote_nostr/list.rs
@@ -5,14 +5,13 @@ use anyhow::{anyhow, Context, Result};
 use auth_git2::GitAuthenticator;
 use client::get_state_from_cache;
 use git::RepoActions;
-use git_events::{event_to_cover_letter, get_commit_id_from_patch};
 use ngit::{
     client,
     git::{
         self,
         nostr_url::{CloneUrl, NostrUrlDecoded, ServerProtocol},
     },
-    git_events,
+    git_events::event_to_cover_letter,
     login::get_curent_user,
     repo_ref,
 };
@@ -20,6 +19,7 @@ use nostr_sdk::hashes::sha1::Hash as Sha1Hash;
 use repo_ref::RepoRef;
 
 use crate::{
+    fetch::{fetch_from_git_server, make_commits_for_proposal},
     git::Repo,
     utils::{
         fetch_or_list_error_is_not_authentication_failure, get_open_proposals,
@@ -88,6 +88,61 @@ pub async fn run_list(
 
     state.retain(|k, _| !k.starts_with("refs/heads/pr/"));
 
+    let proposals_state =
+        get_open_proposals_state(&term, git_repo, repo_ref, decoded_nostr_url, &remote_states)
+            .await?;
+
+    state.extend(proposals_state);
+
+    // TODO 'for push' should we check with the git servers to see if any of them
+    // allow push from the user?
+    for (name, value) in state {
+        if value.starts_with("ref: ") {
+            if !for_push {
+                println!("{} {name}", value.replace("ref: ", "@"));
+            }
+        } else {
+            println!("{value} {name}");
+        }
+    }
+
+    println!();
+    Ok(remote_states)
+}
+
+async fn get_open_proposals_state(
+    term: &console::Term,
+    git_repo: &Repo,
+    repo_ref: &RepoRef,
+    decoded_nostr_url: &NostrUrlDecoded,
+    remote_states: &HashMap<String, HashMap<String, String>>,
+) -> Result<HashMap<String, String>> {
+    // we cannot use commit_id in the latest patch in a proposal because:
+    // 1) the `commit` tag is optional
+    // 2) if the commit tag is wrong, it will cause errors which stop clone from
+    //    working
+
+    // 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 {
+        if fetch_from_git_server(
+            git_repo,
+            &oids_from_git_servers
+                .values()
+                .filter(|v| !v.starts_with("ref: "))
+                .cloned()
+                .collect::<Vec<String>>(),
+            git_server_url,
+            decoded_nostr_url,
+            term,
+        )
+        .is_ok()
+        {
+            break;
+        }
+    }
+
+    let mut state = HashMap::new();
     let open_proposals = get_open_proposals(git_repo, repo_ref).await?;
     let current_user = get_curent_user(git_repo)?;
     for (_, (proposal, patches)) in open_proposals {
@@ -102,32 +157,21 @@ pub async fn run_list(
                 } else {
                     branch_name
                 };
-                if let Some(patch) = patches.first() {
-                    // TODO this isn't resilient because the commit id stated may not be correct
-                    // we will need to check whether the commit id exists in the repo or apply the
-                    // proposal and each patch to check
-                    if let Ok(commit_id) = get_commit_id_from_patch(patch) {
-                        state.insert(format!("refs/heads/{branch_name}"), commit_id);
+                match make_commits_for_proposal(git_repo, repo_ref, &patches) {
+                    Ok(tip) => {
+                        state.insert(format!("refs/heads/{branch_name}"), tip);
                     }
-                }
-            }
-        }
-    }
-
-    // TODO 'for push' should we check with the git servers to see if any of them
-    // allow push from the user?
-    for (name, value) in state {
-        if value.starts_with("ref: ") {
-            if !for_push {
-                println!("{} {name}", value.replace("ref: ", "@"));
+                    Err(error) => {
+                        let _ = term.write_line(
+                            format!("WARNING: cannot fetch branch {branch_name} error: {error}")
+                                .as_str(),
+                        );
+                    }
+                };
             }
-        } else {
-            println!("{value} {name}");
         }
     }
-
-    println!();
-    Ok(remote_states)
+    Ok(state)
 }
 
 pub fn list_from_remotes(