@sigitcodereview-old / sigit / commits / f007050

Fork 0

A modular local inference LLM coding agent. Core component of siGit Factory.

Keep the co-author trailer last on conflicted merges

Finishing a conflicted merge with `git commit --no-edit` keeps git's "# Conflicts:" list in the message, because git only strips comment lines when an editor runs. The safety net then amended with --no-edit --trailer, and git placed the trailer above that block. The trailer was no longer the last paragraph, so GitHub didn't pick it up. The amend now passes the message explicitly, with any trailing paragraphs made only of # lines removed first, and adds the trailer after that. A commit that already has the trailer but still ends in a comment block gets the same cleanup. A # line anywhere else in the message is left alone, so Markdown headings in a -m body survive. Fixes #74

Seto Elkahfi committed Sep 27, 2026 at 10:56 UTC f007050232737f1605685d35f507d3ba01e14b53

2 files changed +166 -13

CHANGELOG.md

+10

@@ -14,6 +14,16 @@

14 from the first user message — and a request may filter on `cwd` so one

15 project is never offered another's threads. Threads saved before this have no

16 sidecar and are not listed; they still reopen by id through `session/load`

17 +

18 +### Fixed

19 +

20 +- **The co-author trailer ends a conflicted merge commit.** Finishing a

21 + conflicted merge with `git commit --no-edit` keeps git's `# Conflicts:` list

22 + in the message, since no editor runs to strip it. When siGit Code then added

23 + its trailer, the trailer went in above that list, so it was no longer the last

24 + paragraph and GitHub didn't credit the co-author. The amend now drops the

25 + leftover comment block and puts the trailer last. It also fixes a commit

26 + that already has the trailer but still ends in that block.

29 ## 1.5.10

src/tools.rs

+156 -13

@@ -2035,17 +2035,43 @@ fn git_head(cwd: &Path) -> Option<String> {

2035 git_stdout(cwd, &["rev-parse", "HEAD"])

2036 }

2037

2038 +/// Drop the comment block git leaves at the end of a message it wrote itself.

2039 +///

2040 +/// `git commit --no-edit` after a conflicted merge takes `MERGE_MSG` as is,

2041 +/// and with no editor involved git only cleans up whitespace, so the

2042 +/// `# Conflicts:` list stays in the commit. Any trailer after it is no longer

2043 +/// in the last paragraph, and GitHub stops seeing it. Only trailing

2044 +/// paragraphs made entirely of `#` lines go; a `#` line inside the message

2045 +/// (a Markdown heading in a `-m` body, say) is left alone.

2046 +fn strip_trailing_comment_block(message: &str) -> &str {

2047 + let mut rest = message.trim_end();

2048 + loop {

2049 + let (head, last) = match rest.rfind("\n\n") {

2050 + Some(split) => (&rest[..split], &rest[split + 2..]),

2051 + None => return rest,

2052 + };

2053 + if last.lines().all(|line| line.starts_with('#')) {

2054 + rest = head.trim_end();

2055 + } else {

2056 + return rest;

2057 + }

2058 + }

2059 +}

2060 +

2061 /// Deterministic co-author attribution: if the commit at HEAD lacks the

2062 /// siGit Code trailer, amend it in (via `git commit --amend --trailer`, which

2040 -/// places it after a blank line — the format GitHub detects). Never rewrites

2041 -/// a commit that is already on a remote. Returns a note describing the amend

2042 -/// so the model and user can see it happened.

2063 +/// places it after a blank line — the format GitHub detects). A leftover

2064 +/// comment block from git (see [`strip_trailing_comment_block`]) is removed in

2065 +/// the same amend, since it would otherwise sit below the trailer and hide it.

2066 +/// Never rewrites a commit that is already on a remote. Returns a note

2067 +/// describing the amend so the model and user can see it happened.

2068 fn ensure_commit_co_author(cwd: &Path) -> Option<String> {

2069 let message = git_stdout(cwd, &["log", "-1", "--format=%B"])?;

2045 - if message

2070 + let cleaned = strip_trailing_comment_block(&message);

2071 + let has_trailer = cleaned

2072 .to_lowercase()

2047 - .contains("co-authored-by: sigit code")

2048 - {

2073 + .contains("co-authored-by: sigit code");

2074 + if has_trailer && cleaned == message {

2075 return None;

2076 }

2077 // Amending changes the commit id; a commit that any remote ref already

@@ -2054,9 +2080,17 @@ fn ensure_commit_co_author(cwd: &Path) -> Option<String> {

2080 Some(remotes) if remotes.is_empty() => {}

2081 _ => return None,

2082 }

2057 - let amend = Command::new("git")

2058 - .args(["commit", "--amend", "--no-edit", "--trailer"])

2059 - .arg(COMMIT_CO_AUTHOR_TRAILER)

2083 + // The message is passed explicitly rather than with --no-edit so the

2084 + // comment block is gone before git decides where the trailer goes.

2085 + // `whitespace` is what git already applied to a message it didn't edit.

2086 + let mut amend = Command::new("git");

2087 + amend

2088 + .args(["commit", "--amend", "--cleanup=whitespace", "-m"])

2089 + .arg(cleaned);

2090 + if !has_trailer {

2091 + amend.arg("--trailer").arg(COMMIT_CO_AUTHOR_TRAILER);

2092 + }

2093 + let amend = amend

2094 .stdin(std::process::Stdio::null())

2095 .current_dir(cwd)

2096 .output()

@@ -2066,10 +2100,16 @@ fn ensure_commit_co_author(cwd: &Path) -> Option<String> {

2100 "appended co-author trailer to the new commit in {}",

2101 cwd.display()

2102 );

2069 - Some(format!(

2070 - "[siGit Code] The new commit was amended to append the co-author trailer \

2071 - \"{COMMIT_CO_AUTHOR_TRAILER}\" (its hash changed)."

2072 - ))

2103 + Some(if has_trailer {

2104 + "[siGit Code] The new commit was amended to drop git's leftover comment lines, \

2105 + which sat below the co-author trailer and hid it (its hash changed)."

2106 + .to_string()

2107 + } else {

2108 + format!(

2109 + "[siGit Code] The new commit was amended to append the co-author trailer \

2110 + \"{COMMIT_CO_AUTHOR_TRAILER}\" (its hash changed)."

2111 + )

2112 + })

2113 } else {

2114 log::warn!(

2115 "could not append co-author trailer in {}: {}",

@@ -3258,6 +3298,109 @@ mod tests {

3298 assert!(result.contains("no output"), "unexpected result: {result}");

3299 }

3300

3301 + #[test]

3302 + fn strip_trailing_comment_block_only_drops_trailing_comment_paragraphs() {

3303 + assert_eq!(

3304 + strip_trailing_comment_block("Merge branch 'side'\n\n# Conflicts:\n#\tfile.txt\n"),

3305 + "Merge branch 'side'"

3306 + );

3307 + assert_eq!(

3308 + strip_trailing_comment_block(

3309 + "Merge\n\nCo-Authored-By: siGit Code <noreply@sigit.si>\n\n# Conflicts:\n#\tf"

3310 + ),

3311 + "Merge\n\nCo-Authored-By: siGit Code <noreply@sigit.si>"

3312 + );

3313 + // A heading inside the body, or a paragraph that only starts with

3314 + // one, is the author's text.

3315 + let body = "Title\n\n## Why\n\nBecause\n\n# Note\nnot a comment";

3316 + assert_eq!(strip_trailing_comment_block(body), body);

3317 + assert_eq!(strip_trailing_comment_block("# only"), "# only");

3318 + }

3319 +

3320 + /// Start a merge of `side` into `main` that conflicts on file.txt and

3321 + /// resolve it, leaving the merge ready to commit.

3322 + fn stage_resolved_merge_conflict(dir: &Path) {

3323 + test_git(dir, &["switch", "-q", "-c", "side"]);

3324 + fs::write(dir.join("file.txt"), "side\n").unwrap();

3325 + test_git(dir, &["commit", "-q", "-am", "Side"]);

3326 + test_git(dir, &["switch", "-q", "main"]);

3327 + fs::write(dir.join("file.txt"), "main\n").unwrap();

3328 + test_git(dir, &["commit", "-q", "-am", "Main"]);

3329 + let merge = Command::new("git")

3330 + .args(["merge", "-q", "side"])

3331 + .current_dir(dir)

3332 + .output()

3333 + .unwrap();

3334 + assert!(!merge.status.success(), "the merge should conflict");

3335 + fs::write(dir.join("file.txt"), "both\n").unwrap();

3336 + test_git(dir, &["add", "file.txt"]);

3337 + }

3338 +

3339 + /// Issue #74's screenshot: the model finished a conflicted merge with

3340 + /// `git commit --no-edit`, git kept its `# Conflicts:` block, and the

3341 + /// amended trailer landed above it where GitHub doesn't look.

3342 + #[test]

3343 + fn run_command_puts_trailer_last_on_a_conflicted_merge() {

3344 + let dir = init_test_repo("coauthor_merge_conflict");

3345 + stage_resolved_merge_conflict(&dir);

3346 + let args = serde_json::json!({

3347 + "command": "git commit --no-edit",

3348 + "cwd": dir.display().to_string(),

3349 + })

3350 + .to_string();

3351 +

3352 + let result = exec_run_command(&args, None);

3353 + assert!(result.contains("co-author trailer"), "got: {result}");

3354 +

3355 + let message = git_stdout(&dir, &["log", "-1", "--format=%B"]).unwrap();

3356 + assert_eq!(

3357 + message,

3358 + format!("Merge branch 'side'\n\n{COMMIT_CO_AUTHOR_TRAILER}")

3359 + );

3360 + let parents = git_stdout(&dir, &["log", "-1", "--format=%P"]).unwrap();

3361 + assert_eq!(parents.split(' ').count(), 2, "still a merge: {parents}");

3362 + let _ = fs::remove_dir_all(&dir);

3363 + }

3364 +

3365 + #[test]

3366 + fn run_command_drops_comment_block_below_an_existing_trailer() {

3367 + let dir = init_test_repo("coauthor_merge_trailer_hidden");

3368 + stage_resolved_merge_conflict(&dir);

3369 + // The layout from the issue, trailer present but not last. Written

3370 + // with direct git args because cmd /C can't pass the trailer through.

3371 + test_git(

3372 + &dir,

3373 + &[

3374 + "commit",

3375 + "-q",

3376 + "--cleanup=verbatim",

3377 + "-m",

3378 + &format!(

3379 + "Merge branch 'side'\n\n{COMMIT_CO_AUTHOR_TRAILER}\n\n# Conflicts:\n#\tfile.txt"

3380 + ),

3381 + ],

3382 + );

3383 + // Change the tree too, or an amend in the same second can reproduce

3384 + // the same commit id and HEAD wouldn't move.

3385 + fs::write(dir.join("file.txt"), "both, fixed\n").unwrap();

3386 + test_git(&dir, &["add", "file.txt"]);

3387 + let args = serde_json::json!({

3388 + "command": "git commit --amend --no-edit",

3389 + "cwd": dir.display().to_string(),

3390 + })

3391 + .to_string();

3392 +

3393 + let result = exec_run_command(&args, None);

3394 + assert!(result.contains("leftover comment lines"), "got: {result}");

3395 +

3396 + let message = git_stdout(&dir, &["log", "-1", "--format=%B"]).unwrap();

3397 + assert_eq!(

3398 + message,

3399 + format!("Merge branch 'side'\n\n{COMMIT_CO_AUTHOR_TRAILER}")

3400 + );

3401 + let _ = fs::remove_dir_all(&dir);

3402 + }

3403 +

3404 #[test]

3405 fn run_command_appends_co_author_trailer_to_new_commits() {

3406 let dir = init_test_repo("coauthor_append");