From e7d7f933cd7eac19434f09096a311adeb5e60747 Mon Sep 17 00:00:00 2001
From: DanConwayDev <DanConwayDev@protonmail.com>
Date: Fri, 27 Feb 2026 17:17:12 +0000
Subject: fix: skip ^{} peeled-tag refs in sync to prevent invalid refspec
 crash

Regression introduced in 28ad5440: ngit sync crashed with
'invalid refspec refs/remotes/origin/v1.4.4^{}:refs/tags/v1.4.4^{}'
on repos with annotated tags. Fixed by guarding all three iteration
sites in sync.rs and identify_remote_sync_issues in list.rs; also
corrected the always-false logic bug in invalid_nostr_state_ref.
---
 CHANGELOG.md                      |  4 +++
 src/bin/ngit/sub_commands/sync.rs | 72 +++++++++++++++++++++++++++++++++++++--
 src/lib/list.rs                   |  4 +++
 3 files changed, 78 insertions(+), 2 deletions(-)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index a86ceea..d4d0a49 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -7,6 +7,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ## [Unreleased]
 
+### Fixed
+
+- Regression introduced in 28ad5440: `ngit sync` crashed with "invalid refspec refs/remotes/origin/v1.4.4^{}:refs/tags/v1.4.4^{}" on repos with annotated tags; `RepoState::try_from` now retains `^{}` peeled-tag entries in state, but the sync refspec builder did not skip them; fixed by guarding all three iteration sites in sync.rs and `identify_remote_sync_issues` in list.rs; also corrected the always-false logic bug in `invalid_nostr_state_ref`
+
 ## [2.2.2] - 2026-02-27
 
 ### Added
diff --git a/src/bin/ngit/sub_commands/sync.rs b/src/bin/ngit/sub_commands/sync.rs
index 146bcbc..c326817 100644
--- a/src/bin/ngit/sub_commands/sync.rs
+++ b/src/bin/ngit/sub_commands/sync.rs
@@ -228,6 +228,10 @@ pub async fn launch(args: &SubCommandArgs) -> Result<()> {
         // delete ref from remote
         let mut not_deleted = vec![];
         for remote_ref_name in remote_state.keys() {
+            // skip peeled-tag dereference markers — not real refs
+            if remote_ref_name.ends_with("^{}") {
+                continue;
+            }
             // skip unspecified refs
             if let Some(full_ref_name) = &full_ref_name {
                 if remote_ref_name != full_ref_name {
@@ -253,6 +257,11 @@ pub async fn launch(args: &SubCommandArgs) -> Result<()> {
         // add or update ref on remote
         let mut not_updated = vec![];
         for nostr_ref_name in nostr_state.state.keys() {
+            // skip peeled-tag dereference markers (e.g. refs/tags/v1.0.0^{})
+            // — these are not real git refs and cannot appear in refspecs
+            if nostr_ref_name.ends_with("^{}") {
+                continue;
+            }
             // skip unspecified refs
             if let Some(full_ref_name) = &full_ref_name {
                 if nostr_ref_name != full_ref_name {
@@ -391,8 +400,9 @@ pub async fn launch(args: &SubCommandArgs) -> Result<()> {
 }
 
 fn invalid_nostr_state_ref(ref_name: &str) -> bool {
-    ref_name.starts_with("refs/heads/pr/")
-        && !(ref_name.starts_with("refs/heads/") || ref_name.starts_with("refs/tags/"))
+    ref_name.ends_with("^{}")
+        || ref_name.starts_with("refs/heads/pr/")
+        || (!ref_name.starts_with("refs/heads/") && !ref_name.starts_with("refs/tags/"))
 }
 
 fn identify_missing_refs(git_repo: &Repo, state: &HashMap<String, String>) -> Vec<String> {
@@ -490,3 +500,61 @@ fn fetch_missing_refs(
         missing_refs
     }
 }
+
+#[cfg(test)]
+mod tests {
+    use super::invalid_nostr_state_ref;
+
+    // Regression test: annotated-tag peeled refs (ending with ^{}) must be
+    // treated as invalid nostr state refs so they are never used to build
+    // git refspecs.  Before the fix, these passed through and caused git2 to
+    // reject the push with "invalid refspec refs/remotes/origin/v1.4.4^{}:…".
+    #[test]
+    fn annotated_tag_peeled_ref_is_invalid() {
+        assert!(
+            invalid_nostr_state_ref("refs/tags/v1.4.4^{}"),
+            "peeled annotated-tag ref must be invalid"
+        );
+        assert!(
+            invalid_nostr_state_ref("refs/tags/v1.0.0^{}"),
+            "peeled annotated-tag ref must be invalid"
+        );
+    }
+
+    #[test]
+    fn pr_ref_is_invalid() {
+        assert!(
+            invalid_nostr_state_ref("refs/heads/pr/42"),
+            "PR branch refs must be invalid"
+        );
+    }
+
+    #[test]
+    fn arbitrary_non_heads_non_tags_ref_is_invalid() {
+        assert!(
+            invalid_nostr_state_ref("refs/notes/commits"),
+            "refs outside heads/tags must be invalid"
+        );
+        assert!(invalid_nostr_state_ref("HEAD"), "HEAD must be invalid");
+    }
+
+    #[test]
+    fn normal_branch_and_tag_refs_are_valid() {
+        assert!(
+            !invalid_nostr_state_ref("refs/heads/main"),
+            "normal branch must be valid"
+        );
+        assert!(
+            !invalid_nostr_state_ref("refs/heads/feature/foo"),
+            "feature branch must be valid"
+        );
+        assert!(
+            !invalid_nostr_state_ref("refs/tags/v1.4.4"),
+            "lightweight tag must be valid"
+        );
+        assert!(
+            !invalid_nostr_state_ref("refs/tags/v2.0.0-rc1"),
+            "release-candidate tag must be valid"
+        );
+    }
+}
diff --git a/src/lib/list.rs b/src/lib/list.rs
index d8b038e..19d17df 100644
--- a/src/lib/list.rs
+++ b/src/lib/list.rs
@@ -605,6 +605,10 @@ pub fn identify_remote_sync_issues(
     let mut remote_issues: HashMap<String, RemoteIssues> = HashMap::new();
 
     for (name, value) in &nostr_state.state {
+        // skip peeled-tag dereference markers — not real refs
+        if name.ends_with("^{}") {
+            continue;
+        }
         for (url, (remote_state, _is_grasp_server)) in remote_states {
             let remote_name = get_short_git_server_name(url);
             let issues = remote_issues.entry(remote_name.clone()).or_default();
-- 
cgit v1.2.3