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/mbox_parser.rs | 62 ++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 62 insertions(+) (limited to 'src/lib/mbox_parser.rs') diff --git a/src/lib/mbox_parser.rs b/src/lib/mbox_parser.rs index fd2f8ed..48190ba 100644 --- a/src/lib/mbox_parser.rs +++ b/src/lib/mbox_parser.rs @@ -1,3 +1,57 @@ +//! Parser for mbox-format git patch content. +//! +//! This module is a **fallback path** used only when nostr patch events are +//! missing optional tags (`author`, `committer`, `description`, +//! `parent-commit`). When those tags are present they always take precedence — +//! see [`crate::git::RepoActions::apply_patch_chain`]. +//! +//! ## Why hand-rolled rather than a library? +//! +//! Neither libgit2 (via the `git2` crate) nor gitoxide (`gix`) exposes a +//! mailinfo-style parser. libgit2's email API is output-only +//! (`git_email_create_from_commit`); there is no `git_mailinfo` equivalent. The +//! gitoxide monorepo has no `gix-patch` crate, not even as a placeholder. No +//! production-quality standalone Rust mbox/git-patch parser crate exists. +//! +//! The genuinely hard parts of RFC 2822 parsing (header folding, RFC 2047 MIME +//! encoded-words for non-ASCII author names and subjects) are delegated to the +//! `mailparse` crate. The git-specific overlay (mbox envelope line, `[PATCH]` +//! prefix stripping, commit-message body extraction up to the `---` diffstat +//! separator) is implemented here, matching the behaviour of `git am`'s +//! `patchbreak()` function in `mailinfo.c`. +//! +//! ## If edge cases are reported +//! +//! If real-world patches produce incorrect metadata through this parser, the +//! escape hatch is to shell out to `git mailinfo` directly: +//! ```text +//! git mailinfo /tmp/msg /tmp/patch < input.patch +//! ``` +//! This prints `Author:`, `Email:`, `Subject:`, `Date:` to stdout and writes +//! the commit body to `/tmp/msg`. Since ngit already requires `git` in PATH (it +//! is a git plugin), this is always available. It is not the primary approach +//! because it requires two temp files and a process spawn per patch, which is +//! acceptable cost but unnecessary given that most patches in the ngit `pr/` +//! flow will have the optional nostr tags and never reach this code. +//! +//! ## Known limitation: `---` in commit message body +//! +//! The `---` line that separates the commit message from the diffstat is +//! ambiguous when the commit message itself contains `---` (e.g. Markdown +//! horizontal rules). This parser stops at the first `---`-only line, matching +//! git am's own behaviour — `git am` has the same limitation and documents it. +//! This is not a bug we can fix without lookahead into the diff structure. +//! +//! ## Commit ID from mbox envelope +//! +//! The SHA1 in the mbox `From ` envelope line is extracted but +//! **must not be assumed correct**. libgit2 generates this ID from the commit +//! object, but if the original commit was GPG-signed, or if the patch was +//! generated by a different tool, the reconstructed commit (applied via +//! `apply_to_tree` + `commit_create_buffer`) will have a different OID. +//! The `commit` nostr tag is the authoritative source for commit identity when +//! present. + use anyhow::{Context, Result, bail}; use chrono::DateTime; use mailparse::{MailHeaderMap, parse_headers}; @@ -34,6 +88,14 @@ pub fn parse_mbox_patch(content: &str) -> Result { }) } +/// Extract the SHA1 from the mbox `From ` envelope line. +/// +/// **This value should not be assumed correct for the reconstructed commit.** +/// If the original commit was GPG-signed, or the patch was generated by a +/// different tool (e.g. `git format-patch` vs libgit2), the commit recreated +/// by applying this patch via `commit_create_buffer` will have a different OID. +/// Use the `commit` nostr event tag as the authoritative commit identity when +/// present. fn extract_commit_id_from_mbox(content: &str) -> Result { if !content.starts_with("From ") { bail!("patch does not start with 'From ' - not a valid mbox format"); -- cgit v1.2.3