2 files changed, 80 insertions, 67 deletions

diff --git a/src/bin/git_remote_nostr/fetch.rs b/src/bin/git_remote_nostr/fetch.rs
index dd80f0f..7847777 100644
--- a/src/bin/git_remote_nostr/fetch.rs
+++ b/src/bin/git_remote_nostr/fetch.rs
@@ -1,17 +1,11 @@
 use core::str;
-use std::{
-    collections::{HashMap, HashSet},
-    io::Stdin,
-};
+use std::{collections::HashMap, io::Stdin};
 
 use anyhow::{Context, Result, bail};
 use ngit::{
     fetch::fetch_from_git_server,
     git::{Repo, RepoActions},
-    git_events::{
-        KIND_PULL_REQUEST, KIND_PULL_REQUEST_UPDATE,
-        identify_clone_urls_for_oids_from_pr_pr_update_events,
-    },
+    git_events::{KIND_PULL_REQUEST, KIND_PULL_REQUEST_UPDATE},
     login::get_curent_user,
     repo_ref::{RepoRef, is_grasp_server_in_list},
     utils::{
@@ -37,56 +31,29 @@ pub async fn run_fetch(
         .map(|(_, oid)| oid.clone())
         .collect::<Vec<String>>();
 
-    let pr_oid_clone_url_map = identify_clone_urls_for_oids_from_pr_pr_update_events(
-        fetch_batch.values().collect::<Vec<&String>>(),
-        git_repo,
-        repo_ref,
-    )
-    .await?;
-
-    let oids_to_fetch_from_git_servers = [
-        oids_from_state.clone(),
-        pr_oid_clone_url_map
-            .keys()
-            .cloned()
-            .collect::<Vec<String>>(),
-    ]
-    .concat();
-
-    let git_servers = {
-        let mut seen: HashSet<String> = HashSet::new();
-        let mut out: Vec<String> = vec![];
-        for server in &repo_ref.git_server {
-            if seen.insert(server.clone()) {
-                out.push(server.clone());
-            }
-        }
-        for url in pr_oid_clone_url_map.values().flatten() {
-            if seen.insert(url.clone()) {
-                out.push(url.clone());
-            }
-        }
-        out
-    };
-
     let mut errors = vec![];
     let term = console::Term::stderr();
 
-    for git_server_url in &git_servers {
-        let oids_to_fetch_from_server = oids_to_fetch_from_git_servers
-            .clone()
-            .into_iter()
+    // Only repo git servers are tried here. PR tip OIDs are guaranteed local by the
+    // list phase (see get_open_and_draft_proposals_state), so run_fetch only needs
+    // to resolve state OIDs that may have been missed by the bulk prefetch.
+    // We intentionally do not fall back to git-server URLs in PR event `clone`
+    // tags; see the note in get_open_and_draft_proposals_state for the
+    // rationale.
+    for git_server_url in &repo_ref.git_server {
+        let missing = oids_from_state
+            .iter()
             .filter(|oid| !git_repo.does_commit_exist(oid).unwrap_or(false))
+            .cloned()
             .collect::<Vec<String>>();
 
-        if oids_to_fetch_from_server.is_empty() {
-            continue;
+        if missing.is_empty() {
+            break;
         }
 
-        let term = console::Term::stderr();
         if let Err(error) = fetch_from_git_server(
             git_repo,
-            &oids_from_state,
+            &missing,
             git_server_url,
             &repo_ref.to_nostr_git_url(&None),
             &term,
@@ -98,7 +65,7 @@ pub async fn run_fetch(
 
     if oids_from_state
         .iter()
-        .any(|oid| !git_repo.does_commit_exist(oid).unwrap())
+        .any(|oid| !git_repo.does_commit_exist(oid).unwrap_or(false))
         && !errors.is_empty()
     {
         bail!(
diff --git a/src/bin/git_remote_nostr/list.rs b/src/bin/git_remote_nostr/list.rs
index a32ed67..23b3b98 100644
--- a/src/bin/git_remote_nostr/list.rs
+++ b/src/bin/git_remote_nostr/list.rs
@@ -32,13 +32,14 @@ pub async fn run_list(
     if is_verbose() {
         term.write_line("git servers: listing refs...")?;
     }
+    let nostr_git_url = repo_ref.to_nostr_git_url(&None);
     // nostr_state is passed to list_from_remotes only for the sync-status
     // display; the actual ref state we advertise is determined below.
     let remote_states = list_from_remotes(
         &term,
         git_repo,
         &repo_ref.git_server,
-        &repo_ref.to_nostr_git_url(&None),
+        &nostr_git_url,
         nostr_state.as_ref(),
     )
     .await;
@@ -134,6 +135,7 @@ pub async fn run_list(
 
 /// fetches branches and tags from git servers so patch parent commits can be
 /// used to build patches with correct commit ids
+#[allow(clippy::too_many_lines)]
 async fn get_open_and_draft_proposals_state(
     term: &console::Term,
     git_repo: &Repo,
@@ -156,8 +158,6 @@ async fn get_open_and_draft_proposals_state(
                 .filter(|v| !v.starts_with("ref: "))
                 .cloned()
                 .collect::<Vec<String>>(),
-            // TODO we could fetch the oids of Pull Requests and Pull Request Updates to prevent
-            // having repeat fetching during the git remote helper fetch phase
             git_server_url,
             &repo_ref.to_nostr_git_url(&None),
             term,
@@ -169,8 +169,62 @@ async fn get_open_and_draft_proposals_state(
         }
     }
 
-    let mut state = HashMap::new();
     let open_and_draft_proposals = get_open_or_draft_proposals(git_repo, repo_ref).await?;
+
+    // Collect PR/PR-update tip OIDs that are still missing after the bulk prefetch.
+    // We borrow proposals here so we can move them in the state-building loop
+    // below.
+    let mut missing_pr_oids: Vec<String> = open_and_draft_proposals
+        .values()
+        .filter_map(|(_, events)| {
+            events
+                .iter()
+                .find(|e| e.kind.eq(&KIND_PULL_REQUEST) || e.kind.eq(&KIND_PULL_REQUEST_UPDATE))
+                .and_then(|e| tag_value(e, "c").ok())
+        })
+        .filter(|tip| !git_repo.does_commit_exist(tip).unwrap_or(false))
+        .collect();
+
+    // For each repo git server, batch-fetch the PR tip OIDs it carries that are
+    // still missing locally. Only OIDs the server has advertised are included in
+    // each batch (avoids all-or-nothing batch-poisoning). We mop up across servers
+    // until all missing OIDs are satisfied or all servers are exhausted.
+    //
+    // NOTE: we intentionally restrict mop-up to the repo's declared git servers
+    // (remote_states) and do NOT try the git-server URL carried in the PR event's
+    // `clone` tag. A PR submitter could include an arbitrary server URL there;
+    // fetching from it unconditionally would let a malicious or slow server
+    // delay every clone/fetch. If we later want to support PR-supplied servers,
+    // it should be opt-in (e.g. an explicit `--include-pr-servers` flag) so
+    // users consciously accept the trust/performance trade-off. PRs whose tip
+    // OID isn't carried by any repo git server will simply not be advertised as
+    // `refs/heads/pr/*` refs; they are still accessible via their patch events.
+    if !missing_pr_oids.is_empty() {
+        for (server_url, (server_state, is_grasp)) in remote_states {
+            let batch: Vec<String> = missing_pr_oids
+                .iter()
+                .filter(|oid| server_state.values().any(|v| v == *oid))
+                .cloned()
+                .collect();
+            if batch.is_empty() {
+                continue;
+            }
+            let _ = fetch_from_git_server(
+                git_repo,
+                &batch,
+                server_url,
+                &repo_ref.to_nostr_git_url(&None),
+                term,
+                *is_grasp,
+            );
+            missing_pr_oids.retain(|oid| !git_repo.does_commit_exist(oid).unwrap_or(false));
+            if missing_pr_oids.is_empty() {
+                break;
+            }
+        }
+    }
+
+    let mut state = HashMap::new();
     let current_user = get_curent_user(git_repo)?;
     for (_, (proposal, events_to_apply)) in open_and_draft_proposals {
         if let Ok(cl) = event_to_cover_letter(&proposal) {
@@ -192,17 +246,9 @@ async fn get_open_and_draft_proposals_state(
                 {
                     match tag_value(pr_or_pr_update, "c") {
                         Ok(tip) => {
-                            // only list Pull Requests as refs/heads/pr/* if data is commit is
-                            // advertised as tip of a ref on a repo git server or
-                            // available locally. Otherwise the standard cmd:
-                            // `git clone nostr://` will fail as it assumes all /refs/heads
-                            // returned by list are accessable
-                            let tip_oid_is_on_a_repo_git_server =
-                                remote_states.iter().any(|(_url, (state, _is_grasp))| {
-                                    state.iter().any(|(_, oid)| tip == *oid)
-                                }) || git_repo.does_commit_exist(&tip).is_ok_and(|r| r);
-
-                            if tip_oid_is_on_a_repo_git_server {
+                            // Only advertise once confirmed locally available — this
+                            // guarantees the subsequent fetch phase can serve the object.
+                            if git_repo.does_commit_exist(&tip).is_ok_and(|r| r) {
                                 state.insert(format!("refs/heads/{branch_name}"), tip);
                             }
                         }
@@ -254,8 +300,8 @@ async fn get_all_proposals_state(
     let mut state = HashMap::new();
     let all_proposals = get_all_proposals(git_repo, repo_ref).await?;
     let current_user = get_curent_user(git_repo)?;
-    for (_, (proposal, events_to_apply)) in all_proposals {
-        if let Ok(cl) = event_to_cover_letter(&proposal) {
+    for (proposal, events_to_apply) in all_proposals.values() {
+        if let Ok(cl) = event_to_cover_letter(proposal) {
             if let Ok(mut branch_name) = cl.get_branch_name_with_pr_prefix_and_shorthand_id() {
                 branch_name = if let Some(public_key) = current_user {
                     if proposal.pubkey.eq(&public_key) {
@@ -275,7 +321,7 @@ async fn get_all_proposals_state(
                         state.insert(format!("refs/pr/{}/head", proposal.id), tip);
                     }
                 } else if let Ok(tip) =
-                    make_commits_for_proposal(git_repo, repo_ref, &events_to_apply)
+                    make_commits_for_proposal(git_repo, repo_ref, events_to_apply)
                 {
                     state.insert(format!("refs/{branch_name}"), tip.clone());
                     state.insert(format!("refs/pr/{}/head", proposal.id), tip);