5 files changed, 77 insertions, 185 deletions

diff --git a/src/bin/git_remote_nostr/fetch.rs b/src/bin/git_remote_nostr/fetch.rs
index 7ee264b..dd80f0f 100644
--- a/src/bin/git_remote_nostr/fetch.rs
+++ b/src/bin/git_remote_nostr/fetch.rs
@@ -10,7 +10,7 @@ use ngit::{
     git::{Repo, RepoActions},
     git_events::{
         KIND_PULL_REQUEST, KIND_PULL_REQUEST_UPDATE,
-        identify_clone_urls_for_oids_from_pr_pr_update_events, tag_value,
+        identify_clone_urls_for_oids_from_pr_pr_update_events,
     },
     login::get_curent_user,
     repo_ref::{RepoRef, is_grasp_server_in_list},
@@ -126,19 +126,13 @@ pub fn make_commits_for_proposal(
     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()
-    };
+    let first_patch = patches_ancestor_first
+        .first()
+        .context("proposal should have at least one patch")?;
+
+    let mut tip_commit_id =
+        ngit::git_events::get_parent_commit_from_patch(first_patch, Some(git_repo))
+            .context("failed to determine parent commit for proposal")?;
 
     for patch in &patches_ancestor_first {
         let commit_id = git_repo
diff --git a/src/bin/ngit/sub_commands/checkout.rs b/src/bin/ngit/sub_commands/checkout.rs
index 87f1ff2..67447ae 100644
--- a/src/bin/ngit/sub_commands/checkout.rs
+++ b/src/bin/ngit/sub_commands/checkout.rs
@@ -24,7 +24,7 @@ use nostr_sdk::{EventId, FromBech32};
 use crate::{
     client::{Client, Connect, fetching_with_report, get_repo_ref_from_cache},
     git::{Repo, RepoActions, str_to_sha1},
-    git_events::{event_to_cover_letter, get_parent_commit_from_patch, patch_supports_commit_ids},
+    git_events::event_to_cover_letter,
     repo_ref::get_repo_coordinates_when_remote_unknown,
 };
 
@@ -261,32 +261,7 @@ fn checkout_patch(
     most_recent_proposal_patch_chain_or_pr_or_pr_update: &[nostr::Event],
     nostr_remote_name: Option<&str>,
 ) -> Result<()> {
-    let no_support_for_patches_as_branch = most_recent_proposal_patch_chain_or_pr_or_pr_update
-        .iter()
-        .any(|event| !patch_supports_commit_ids(event));
-
-    if no_support_for_patches_as_branch {
-        bail!(
-            "this proposal cannot be checked out as a branch because some patches do not have a parent commit.\n\
-             Try `ngit apply --stdout` to apply patches to the current branch, or use `ngit list` for interactive options."
-        );
-    }
-
-    let last_patch = most_recent_proposal_patch_chain_or_pr_or_pr_update
-        .last()
-        .context("there should be at least one patch")?;
-
-    let proposal_base_commit = str_to_sha1(&get_parent_commit_from_patch(last_patch, Some(git_repo))?)
-        .context("failed to get valid parent commit id from patch")?;
-
-    let (main_branch_name, _master_tip) = git_repo.get_main_or_master_branch()?;
-
-    if !git_repo.does_commit_exist(&proposal_base_commit.to_string())? {
-        bail!(
-            "the proposal parent commit doesn't exist in your local repository.\n\
-             Try running `git pull` on '{main_branch_name}' first, or use `ngit apply --stdout` to apply patches to the current branch."
-        );
-    }
+    let (_, _master_tip) = git_repo.get_main_or_master_branch()?;
 
     if git_repo.has_outstanding_changes()? {
         bail!("working directory is not clean. Discard or stash (un)staged changes and try again.");
@@ -322,23 +297,25 @@ fn checkout_patch(
 
     let local_branch_tip = git_repo.get_tip_of_branch(&branch_name)?;
 
-    let proposal_tip = str_to_sha1(
-        &get_commit_id_from_patch(
-            most_recent_proposal_patch_chain_or_pr_or_pr_update
-                .first()
-                .context("there should be at least one patch")?,
-        )
-        .context("failed to get valid commit_id from patch")?,
-    )
-    .context("failed to get valid commit_id from patch")?;
-
-    if proposal_tip.eq(&local_branch_tip) {
-        git_repo.checkout(&branch_name)?;
-        println!("branch '{branch_name}' checked out and up-to-date");
-        return Ok(());
+    // If we can reliably determine the proposal tip commit, use it to skip
+    // re-applying when already up-to-date. If the commit tag is absent or
+    // unreliable, skip this check and let apply_patch_chain handle idempotency.
+    if let Ok(proposal_tip_str) = get_commit_id_from_patch(
+        most_recent_proposal_patch_chain_or_pr_or_pr_update
+            .first()
+            .context("there should be at least one patch")?,
+    ) {
+        if let Ok(proposal_tip) = str_to_sha1(&proposal_tip_str) {
+            if proposal_tip.eq(&local_branch_tip) {
+                git_repo.checkout(&branch_name)?;
+                println!("branch '{branch_name}' checked out and up-to-date");
+                return Ok(());
+            }
+        }
     }
 
-    git_repo.create_branch_at_commit(&branch_name, &proposal_base_commit.to_string())?;
+    // Branch exists but may need updating — re-apply the chain.
+    // apply_patch_chain handles already-applied commits idempotently.
     git_repo.checkout(&branch_name)?;
     let _ = git_repo
         .apply_patch_chain(
diff --git a/src/bin/ngit/sub_commands/init.rs b/src/bin/ngit/sub_commands/init.rs
index 75306d1..5dd6157 100644
--- a/src/bin/ngit/sub_commands/init.rs
+++ b/src/bin/ngit/sub_commands/init.rs
@@ -1724,10 +1724,7 @@ struct PollContext {
     reveal_state: Arc<ServerRevealState>,
 }
 
-fn create_server_bars(
-    clone_urls: &[String],
-    detail_multi: &MultiProgress,
-) -> Vec<ProgressBar> {
+fn create_server_bars(clone_urls: &[String], detail_multi: &MultiProgress) -> Vec<ProgressBar> {
     let waiting_style = ProgressStyle::with_template("  {spinner} {msg}")
         .unwrap()
         .tick_chars("⠁⠂⠄⡀⢀⠠⠐⠈");
@@ -1779,12 +1776,7 @@ fn spawn_expand_timer(
     })
 }
 
-fn finalize_spinner(
-    all_ready: bool,
-    spinner_pb: &ProgressBar,
-    final_ready: u64,
-    total: u64,
-) {
+fn finalize_spinner(all_ready: bool, spinner_pb: &ProgressBar, final_ready: u64, total: u64) {
     if all_ready {
         spinner_pb.finish_and_clear();
     } else {
diff --git a/src/bin/ngit/sub_commands/list.rs b/src/bin/ngit/sub_commands/list.rs
index 133ac83..3d5e876 100644
--- a/src/bin/ngit/sub_commands/list.rs
+++ b/src/bin/ngit/sub_commands/list.rs
@@ -35,7 +35,7 @@ use crate::{
     git::{Repo, RepoActions, str_to_sha1},
     git_events::{
         commit_msg_from_patch_oneliner, event_is_revision_root, event_to_cover_letter,
-        get_parent_commit_from_patch, patch_supports_commit_ids,
+        get_parent_commit_from_patch,
     },
     repo_ref::get_repo_coordinates_when_remote_unknown,
 };
@@ -623,72 +623,6 @@ async fn launch_interactive() -> Result<()> {
             }
         }
 
-        let binding_patch_text_ref = format!(
-            "{} commits",
-            most_recent_proposal_patch_chain_or_pr_or_pr_update.len()
-        );
-        let patch_text_ref = if most_recent_proposal_patch_chain_or_pr_or_pr_update
-            .len()
-            .gt(&1)
-        {
-            binding_patch_text_ref.as_str()
-        } else {
-            "1 commit"
-        };
-
-        let no_support_for_patches_as_branch = most_recent_proposal_patch_chain_or_pr_or_pr_update
-            .iter()
-            .any(|event| !patch_supports_commit_ids(event));
-
-        if no_support_for_patches_as_branch {
-            println!("{patch_text_ref}");
-            return match Interactor::default().choice(
-                PromptChoiceParms::default()
-                    .with_default(0)
-                    .with_choices(vec![
-                        "learn why this proposals can't be checked out".to_string(),
-                        format!("apply to current branch with `git am`"),
-                        format!("download to ./patches"),
-                        "back".to_string(),
-                    ]),
-            )? {
-                0 => {
-                    println!(
-                        "Some proposals are posted as patch without listing a parent commit\n"
-                    );
-                    println!(
-                        "they are not anchored against a particular state of the code base like a standard patch or a pull request can be\n"
-                    );
-                    println!(
-                        "they are designed to reviewed by studying the diff (in a tool like gitworkshop.dev) and if acceptable by a maintainer, applied to the latest version of master with any conflicts resolved as the do so\n"
-                    );
-                    println!(
-                        "this has proven to be a smoother workflow for large scale projects with a high frequency of changes, even when patches are exchanged via email\n"
-                    );
-                    println!(
-                        "by default ngit posts proposals with a parent commit so either workflow can be used"
-                    );
-                    Interactor::default().choice(
-                        PromptChoiceParms::default()
-                            .with_default(0)
-                            .with_choices(vec!["back".to_string()]),
-                    )?;
-                    continue;
-                }
-                1 => {
-                    launch_git_am_with_patches(most_recent_proposal_patch_chain_or_pr_or_pr_update)
-                }
-                2 => save_patches_to_dir(
-                    most_recent_proposal_patch_chain_or_pr_or_pr_update,
-                    &git_repo,
-                ),
-                3 => continue,
-                _ => {
-                    bail!("unexpected choice")
-                }
-            };
-        }
-
         let branch_exists = git_repo
             .get_local_branch_names()
             .context("gitlib2 will not show a list of local branch names")?
@@ -705,35 +639,36 @@ async fn launch_interactive() -> Result<()> {
 
         let last_patch = most_recent_proposal_patch_chain_or_pr_or_pr_update
             .last()
-            .context(
-                "there should be at least one patch as we have already checked for this",
-            )?;
+            .context("there should be at least one patch as we have already checked for this")?;
 
-        let proposal_base_commit = str_to_sha1(&get_parent_commit_from_patch(last_patch, Some(&git_repo))?)
-            .context("failed to get valid parent commit id from patch")?;
+        let proposal_base_commit = get_parent_commit_from_patch(last_patch, Some(&git_repo))
+            .ok()
+            .and_then(|s| str_to_sha1(&s).ok());
 
         let (main_branch_name, master_tip) = git_repo.get_main_or_master_branch()?;
 
-        if !git_repo.does_commit_exist(&proposal_base_commit.to_string())? {
-            println!("your '{main_branch_name}' branch may not be up-to-date.");
-            println!("the proposal parent commit doesnt exist in your local repository.");
-            return match Interactor::default().choice(PromptChoiceParms::default().with_default(0).with_choices(
-                vec![
-                    format!(
-                        "manually run `git pull` on '{main_branch_name}' and select proposal again"
-                    ),
-                    format!("apply to current branch with `git am`"),
-                    format!("download to ./patches"),
-                    "back".to_string(),
-                ],
-            ))? {
-                0 | 3 => continue,
-                1 => launch_git_am_with_patches(most_recent_proposal_patch_chain_or_pr_or_pr_update),
-                2 => save_patches_to_dir(most_recent_proposal_patch_chain_or_pr_or_pr_update, &git_repo),
-                _ => {
-                    bail!("unexpected choice")
-                }
-            };
+        if let Some(ref base_commit) = proposal_base_commit {
+            if !git_repo.does_commit_exist(&base_commit.to_string())? {
+                println!("your '{main_branch_name}' branch may not be up-to-date.");
+                println!("the proposal parent commit doesnt exist in your local repository.");
+                return match Interactor::default().choice(PromptChoiceParms::default().with_default(0).with_choices(
+                    vec![
+                        format!(
+                            "manually run `git pull` on '{main_branch_name}' and select proposal again"
+                        ),
+                        format!("apply to current branch with `git am`"),
+                        format!("download to ./patches"),
+                        "back".to_string(),
+                    ],
+                ))? {
+                    0 | 3 => continue,
+                    1 => launch_git_am_with_patches(most_recent_proposal_patch_chain_or_pr_or_pr_update),
+                    2 => save_patches_to_dir(most_recent_proposal_patch_chain_or_pr_or_pr_update, &git_repo),
+                    _ => {
+                        bail!("unexpected choice")
+                    }
+                };
+            }
         }
 
         let proposal_tip = str_to_sha1(
@@ -748,8 +683,14 @@ async fn launch_interactive() -> Result<()> {
         )
         .context("failed to get valid commit_id from patch")?;
 
-        let (_, proposal_behind_main) =
-            git_repo.get_commits_ahead_behind(&master_tip, &proposal_base_commit)?;
+        let proposal_behind_main_len = if let Some(ref base_commit) = proposal_base_commit {
+            git_repo
+                .get_commits_ahead_behind(&master_tip, base_commit)
+                .map(|(_, behind)| behind.len())
+                .unwrap_or(0)
+        } else {
+            0
+        };
 
         // branch doesnt exist
         if !branch_exists {
@@ -758,7 +699,7 @@ async fn launch_interactive() -> Result<()> {
                 format!(
                     "create and checkout proposal branch ({} ahead {} behind '{main_branch_name}')",
                     most_recent_proposal_patch_chain_or_pr_or_pr_update.len(),
-                    proposal_behind_main.len(),
+                    proposal_behind_main_len,
                 ),
                 format!("apply to current branch with `git am`"),
                 format!("download to ./patches"),
@@ -815,7 +756,7 @@ async fn launch_interactive() -> Result<()> {
                         format!(
                             "checkout proposal branch ({} ahead {} behind '{main_branch_name}')",
                             most_recent_proposal_patch_chain_or_pr_or_pr_update.len(),
-                            proposal_behind_main.len(),
+                            proposal_behind_main_len,
                         ),
                         format!("apply to current branch with `git am`"),
                         format!("download to ./patches"),
@@ -913,7 +854,7 @@ async fn launch_interactive() -> Result<()> {
             println!(
                 "updated proposal available ({} ahead {} behind '{main_branch_name}'). existing version is {} ahead {} behind '{main_branch_name}'",
                 most_recent_proposal_patch_chain_or_pr_or_pr_update.len(),
-                proposal_behind_main.len(),
+                proposal_behind_main_len,
                 local_ahead_of_main.len(),
                 local_beind_main.len(),
             );
@@ -930,13 +871,6 @@ async fn launch_interactive() -> Result<()> {
             )? {
                 0 => {
                     check_clean(&git_repo)?;
-                    git_repo.create_branch_at_commit(
-                        &cover_letter.get_branch_name_with_pr_prefix_and_shorthand_id()?,
-                        &proposal_base_commit.to_string(),
-                    )?;
-                    git_repo.checkout(
-                        &cover_letter.get_branch_name_with_pr_prefix_and_shorthand_id()?,
-                    )?;
                     let chain_length = most_recent_proposal_patch_chain_or_pr_or_pr_update.len();
                     let _ = git_repo
                         .apply_patch_chain(
@@ -947,7 +881,7 @@ async fn launch_interactive() -> Result<()> {
                     println!(
                         "checked out new version of proposal ({} ahead {} behind '{main_branch_name}'), replacing old version ({} ahead {} behind '{main_branch_name}')",
                         chain_length,
-                        proposal_behind_main.len(),
+                        proposal_behind_main_len,
                         local_ahead_of_main.len(),
                         local_beind_main.len(),
                     );
@@ -991,7 +925,7 @@ async fn launch_interactive() -> Result<()> {
                 "local proposal branch exists with {} unpublished commits on top of the most up-to-date version of the proposal ({} ahead {} behind '{main_branch_name}')",
                 local_ahead_of_proposal.len(),
                 local_ahead_of_main.len(),
-                proposal_behind_main.len(),
+                proposal_behind_main_len,
             );
             return match Interactor::default().choice(
                 PromptChoiceParms::default()
@@ -1012,7 +946,7 @@ async fn launch_interactive() -> Result<()> {
                         "checked out proposal branch with {} unpublished commits ({} ahead {} behind '{main_branch_name}')",
                         local_ahead_of_proposal.len(),
                         local_ahead_of_main.len(),
-                        proposal_behind_main.len(),
+                        proposal_behind_main_len,
                     );
                     Ok(())
                 }
@@ -1032,7 +966,7 @@ async fn launch_interactive() -> Result<()> {
             println!(
                 "you have previously applied the latest version of the proposal ({} ahead {} behind '{main_branch_name}') but your local proposal branch has amended or rebased it ({} ahead {} behind '{main_branch_name}')",
                 most_recent_proposal_patch_chain_or_pr_or_pr_update.len(),
-                proposal_behind_main.len(),
+                proposal_behind_main_len,
                 local_ahead_of_main.len(),
                 local_beind_main.len(),
             );
@@ -1045,7 +979,7 @@ async fn launch_interactive() -> Result<()> {
                 local_ahead_of_main.len(),
                 local_beind_main.len(),
                 most_recent_proposal_patch_chain_or_pr_or_pr_update.len(),
-                proposal_behind_main.len(),
+                proposal_behind_main_len,
             );
 
             println!(
@@ -1085,10 +1019,6 @@ async fn launch_interactive() -> Result<()> {
             }
             1 => {
                 check_clean(&git_repo)?;
-                git_repo.create_branch_at_commit(
-                    &cover_letter.get_branch_name_with_pr_prefix_and_shorthand_id()?,
-                    &proposal_base_commit.to_string(),
-                )?;
                 let chain_length = most_recent_proposal_patch_chain_or_pr_or_pr_update.len();
                 let _ = git_repo
                     .apply_patch_chain(
@@ -1102,7 +1032,7 @@ async fn launch_interactive() -> Result<()> {
                 println!(
                     "checked out latest version of proposal ({} ahead {} behind '{main_branch_name}'), replacing unpublished version ({} ahead {} behind '{main_branch_name}')",
                     chain_length,
-                    proposal_behind_main.len(),
+                    proposal_behind_main_len,
                     local_ahead_of_main.len(),
                     local_beind_main.len(),
                 );
diff --git a/src/lib/git_events.rs b/src/lib/git_events.rs
index b39e797..32c23ac 100644
--- a/src/lib/git_events.rs
+++ b/src/lib/git_events.rs
@@ -40,17 +40,16 @@ pub fn get_commit_id_from_patch(event: &Event) -> Result<String> {
     }
 }
 
-pub fn get_parent_commit_from_patch(
-    event: &Event,
-    git_repo: Option<&Repo>,
-) -> Result<String> {
+pub fn get_parent_commit_from_patch(event: &Event, git_repo: Option<&Repo>) -> Result<String> {
     if let Ok(parent) = tag_value(event, "parent-commit") {
         return Ok(parent);
     }
 
     let metadata = crate::mbox_parser::parse_mbox_patch(&event.content)
         .context("failed to parse patch for timestamp")?;
-    let timestamp = metadata.committer_timestamp.unwrap_or(metadata.author_timestamp);
+    let timestamp = metadata
+        .committer_timestamp
+        .unwrap_or(metadata.author_timestamp);
 
     if let Some(repo) = git_repo {
         if let Some(best_guess) = repo