From 73d829b916d87626f33ea2adead0c48f1d9d737d Mon Sep 17 00:00:00 2001 From: DanConwayDev Date: Wed, 18 Feb 2026 21:04:59 +0000 Subject: fix: harden apply_patch_chain when optional patch tags absent - Replace .unwrap() calls in filter closure with pattern-let guards so a missing/invalid mbox commit id conservatively includes the patch - Use the OID returned by create_commit_from_patch as branch tip instead of the tag commit id, which may differ for GPG-signed commits - Add module-level doc comment to mbox_parser explaining design rationale and known limitations around the mbox envelope SHA1 --- src/lib/git/mod.rs | 72 ++++++++++++++++++++++++++++++++++++++---------------- 1 file changed, 51 insertions(+), 21 deletions(-) (limited to 'src/lib/git/mod.rs') diff --git a/src/lib/git/mod.rs b/src/lib/git/mod.rs index 57e8403..9f86b5f 100644 --- a/src/lib/git/mod.rs +++ b/src/lib/git/mod.rs @@ -576,15 +576,23 @@ impl RepoActions for Repo { let mut patches_to_apply: Vec = patch_and_ancestors .into_iter() .filter(|e| { - let commit_id = get_commit_id_from_patch(e).unwrap(); - if let Ok(branch_tip) = branch_tip_result { - !branch_tip.to_string().eq(&commit_id) - && !self - .ancestor_of(&branch_tip, &str_to_sha1(&commit_id).unwrap()) - .unwrap() - } else { - true - } + // When the commit tag is absent, the commit id from the mbox envelope + // may not match the reconstructed commit's OID (e.g. GPG-signed commits). + // In that case we conservatively include the patch for application — + // create_commit_from_patch handles idempotency via content-addressed storage. + let Ok(commit_id) = get_commit_id_from_patch(e) else { + return true; + }; + let Ok(branch_tip) = branch_tip_result else { + return true; + }; + let Ok(commit_sha1) = str_to_sha1(&commit_id) else { + // Commit id is not a valid SHA1 (e.g. placeholder from mbox envelope). + // Include conservatively. + return true; + }; + !branch_tip.to_string().eq(&commit_id) + && !self.ancestor_of(&branch_tip, &commit_sha1).unwrap_or(false) }) .collect(); @@ -612,12 +620,26 @@ impl RepoActions for Repo { patches_to_apply.reverse(); for patch in &patches_to_apply { - let commit_id = get_commit_id_from_patch(patch)?; - // only create new commits - otherwise make them the tip - if !self.does_commit_exist(&commit_id)? { - self.create_commit_from_patch(patch, None)?; - } - self.create_branch_at_commit(branch_name, &commit_id)?; + // The commit id from the tag (or mbox envelope) is the authoritative id + // when the optional `commit` nostr tag is present. When it is absent the + // mbox envelope SHA1 is used as a best-effort value — it will often differ + // from the reconstructed commit's actual OID (e.g. GPG-signed commits). + // We therefore always use the OID returned by create_commit_from_patch as + // the branch tip, falling back to the tag commit id only when the commit + // already exists in the repo (meaning it was previously applied correctly). + let tag_commit_id = get_commit_id_from_patch(patch).ok(); + let applied_oid = if let Some(ref id) = tag_commit_id { + if self.does_commit_exist(id)? { + // Commit already exists (e.g. previously fetched), use it directly. + id.clone() + } else { + self.create_commit_from_patch(patch, None)?.to_string() + } + } else { + // No commit id available at all — apply and use the resulting OID. + self.create_commit_from_patch(patch, None)?.to_string() + }; + self.create_branch_at_commit(branch_name, &applied_oid)?; self.checkout(branch_name)?; } Ok(patches_to_apply) @@ -641,8 +663,10 @@ impl RepoActions for Repo { } else { let metadata = crate::mbox_parser::parse_mbox_patch(&patch.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); + let best_guess = self .find_best_guess_parent_commit(timestamp) .context("failed to find best guess parent commit")?; @@ -684,8 +708,10 @@ impl RepoActions for Repo { None }; - let author_data = extract_signature_data_with_fallback(&patch.tags, "author", &patch.content)?; - let committer_data = extract_signature_data_with_fallback(&patch.tags, "committer", &patch.content)?; + let author_data = + extract_signature_data_with_fallback(&patch.tags, "author", &patch.content)?; + let committer_data = + extract_signature_data_with_fallback(&patch.tags, "committer", &patch.content)?; let author_sig = author_data.to_signature()?; let committer_sig = committer_data.to_signature()?; @@ -983,7 +1009,9 @@ fn extract_signature_data_from_tags(tags: &Tags, tag_name: &str) -> Result