fix(remote): add resilience to `prs`

make poorly formatted patches fail silently. we stop trusting that the
`commit` tag in the latest patch can be produced by apply the patches.
to achieve this we must recreate the commit during the list command,
which require fetching the parent oids.

support patches without optional `commit` and `parent-commit` tags.

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(