Skip to content

Move conflicted commit detection from headers to commit message markers#13247

Merged
Byron merged 3 commits intomasterfrom
is-conflicted-refactor
Apr 15, 2026
Merged

Move conflicted commit detection from headers to commit message markers#13247
Byron merged 3 commits intomasterfrom
is-conflicted-refactor

Conversation

@mtsgrd
Copy link
Copy Markdown
Contributor

@mtsgrd mtsgrd commented Apr 9, 2026

Summary

The gitbutler-conflicted extra header is stripped when commits are rebased outside of GitButler, silently losing conflict state. This replaces it with commit message markers that survive rebasing and are visible to developers in git log:

  • A [conflict] prefix on the subject line for fast visual detection
  • A GitButler-Conflict: multi-line git trailer whose value explains the conflict tree layout

Using a standard git trailer means the description is embedded as the trailer value with indented continuation lines, so git interpret-trailers and other standard tools can parse and manipulate it. The resulting commit message looks like:

[conflict] <original subject>

<original body>

<existing trailers, if any>
GitButler-Conflict: This is a GitButler-managed conflicted
   commit. Files are auto-resolved using the "ours" side.
   The commit tree contains additional directories:
     .conflict-side-0 — our tree
     .conflict-side-1 — their tree
     ...
   To manually resolve, check out this commit, remove the
   directories listed above, resolve the conflicts, and
   amend the commit.

The trailer is appended after any existing trailers in the message, preserving Change-Id, Signed-off-by, and similar lines. When stripping conflict markers (on resolution, display, or commit matching), the prefix, trailer, and all continuation lines are removed to restore the original message.

New commits only write message markers (no header). Reading checks the message first, then falls back to the header for backward compatibility with existing conflicted commits.

The CONFLICT-README.txt tree blob is removed since the trailer now serves the same explanatory purpose with better visibility.

The conflicted file count (Option<u64>) stored in the old header was only ever used as a boolean, so the new approach uses a simple presence check with no count.

Test plan

  • All Rust unit/integration tests pass (including 10 new conflict marker round-trip tests)
  • All 45 Playwright e2e tests pass
  • cargo clippy --workspace --all-targets -- -D warnings clean
  • cargo fmt --all clean
  • Conflicted commits survive rebase outside GitButler (message markers preserved)
  • Old header-based conflicted commits still detected (backward compat)
  • Existing trailers (Change-Id, Signed-off-by) preserved in correct order

🤖 Generated with Claude Code

Copilot AI review requested due to automatic review settings April 9, 2026 15:42
@mtsgrd mtsgrd marked this pull request as draft April 9, 2026 15:42
@github-actions github-actions bot added rust Pull requests that update Rust code CLI The command-line program `but` labels Apr 9, 2026
@vercel
Copy link
Copy Markdown

vercel bot commented Apr 9, 2026

The latest updates on your projects. Learn more about Vercel for GitHub.

1 Skipped Deployment
Project Deployment Actions Updated (UTC)
gitbutler-web Ignored Ignored Preview Apr 14, 2026 0:50am

Request Review

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR changes GitButler’s “conflicted commit” detection from relying on a custom commit header (gitbutler-conflicted, which can be lost during external rebases) to commit-message markers ([conflict] prefix + GitButler-Conflict: true footer), while keeping legacy header fallback for existing commits.

Changes:

  • Add/strip/check conflict markers in commit messages and update conflict detection to prefer message markers with legacy-header fallback.
  • Update rebase/cherry-pick/edit-mode flows to add markers when creating conflicted commits and strip markers when conflicts are cleared.
  • Adjust UI/workspace projections and snapshots/tests to hide markers in displayed messages while preserving has_conflicts.

Reviewed changes

Copilot reviewed 14 out of 14 changed files in this pull request and generated 3 comments.

Show a summary per file
File Description
crates/gitbutler-edit-mode/src/lib.rs Strips conflict markers when saving edited commits back to workspace.
crates/gitbutler-commit/src/commit_ext.rs Updates is_conflicted() to check message markers first, then legacy headers.
crates/gitbutler-branch-actions/tests/branch-actions/reorder.rs Normalizes commit-message comparisons by stripping markers in tests.
crates/but/tests/but/command/rub.rs Updates snapshot due to changed CLI IDs in output.
crates/but-workspace/tests/workspace/commit/squash_commits.rs Updates snapshots to reflect [conflict] prefix on conflicted commits.
crates/but-workspace/src/ref_info.rs Strips markers before exposing commit messages to UI models when conflicted.
crates/but-workspace/src/branch_details.rs Strips markers from local commit messages while keeping has_conflicts.
crates/but-rebase/tests/rebase/main.rs Updates snapshots for new message-marker format and removal of CONFLICT-README.txt.
crates/but-rebase/tests/rebase/graph_rebase/conflictable_restriction.rs Updates snapshots for new message-marker format.
crates/but-rebase/tests/rebase/graph_rebase/cherry_pick.rs Updates snapshots for new message-marker format and tree layout.
crates/but-rebase/src/lib.rs Preserves markers on reword/squash operations when commit remains conflicted.
crates/but-rebase/src/graph_rebase/cherry_pick.rs Adds markers on conflicted commit creation; strips markers/legacy header when unconflicting.
crates/but-rebase/src/cherry_pick.rs Same as graph_rebase path; removes legacy conflicted-count writing helpers.
crates/but-core/src/commit.rs Implements marker constants + add/strip/detect helpers; updates Commit::is_conflicted().

Comment thread crates/but-core/src/commit.rs Outdated
Comment thread crates/but-core/src/commit.rs Outdated
Comment thread crates/gitbutler-edit-mode/src/lib.rs Outdated
@mtsgrd mtsgrd force-pushed the is-conflicted-refactor branch from 71220ae to 102930a Compare April 9, 2026 16:01
@mtsgrd mtsgrd marked this pull request as ready for review April 9, 2026 16:07
Copilot AI review requested due to automatic review settings April 9, 2026 16:07
@mtsgrd
Copy link
Copy Markdown
Contributor Author

mtsgrd commented Apr 9, 2026

@krlvi this is what we spoke about earlier, let me know what you think?

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 17 out of 17 changed files in this pull request and generated 3 comments.

Comment thread crates/but-core/src/commit.rs Outdated
Comment thread crates/but-rebase/src/lib.rs Outdated
Comment thread crates/but-rebase/src/lib.rs Outdated
@mtsgrd mtsgrd force-pushed the is-conflicted-refactor branch from 102930a to fce7789 Compare April 9, 2026 16:33
Copilot AI review requested due to automatic review settings April 9, 2026 20:39
@mtsgrd mtsgrd force-pushed the is-conflicted-refactor branch from fce7789 to e23f921 Compare April 9, 2026 20:39
@mtsgrd mtsgrd force-pushed the is-conflicted-refactor branch from e23f921 to 303e711 Compare April 9, 2026 20:41
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 20 out of 20 changed files in this pull request and generated 2 comments.

Comment thread crates/but-core/src/commit.rs Outdated
Comment thread crates/but-workspace/src/changeset.rs Outdated
@mtsgrd mtsgrd marked this pull request as draft April 9, 2026 20:50
@mtsgrd mtsgrd force-pushed the is-conflicted-refactor branch from 303e711 to 91e3de1 Compare April 9, 2026 20:59
@Byron
Copy link
Copy Markdown
Collaborator

Byron commented Apr 10, 2026

I like it!

Just a question regarding the format:

GitButler-Conflict: true
This is a GitButler-managed conflicted commit. Files are auto-resolved
using the "ours" side. The commit tree contains additional directories:
  .conflict-side-0  — our tree
  .conflict-side-1  — their tree
  .conflict-base-0  — the merge base tree
  .auto-resolution  — the auto-resolved tree
  .conflict-files   — metadata about conflicted files
To manually resolve, check out this commit, remove the directories
listed above, resolve the conflicts, and amend the commit.

GitButler-Conflict: true looks like a trailer, but what follows doesn't look like it will be part of the value. While this might be intentional, I'd prefer it if trailers aren't interleaved with non-trailers. You can also try if gitoxide can parse these correctly in its interleaved form, should be commit.message().trailers().

@mtsgrd mtsgrd force-pushed the is-conflicted-refactor branch 2 times, most recently from 559abb4 to 486271d Compare April 10, 2026 04:57
@mtsgrd
Copy link
Copy Markdown
Contributor Author

mtsgrd commented Apr 10, 2026

Cool cool, I'm afk for most of the day but I'll pick this back up sometime soon.

@mtsgrd mtsgrd force-pushed the is-conflicted-refactor branch 2 times, most recently from 53c95aa to 632c2c1 Compare April 10, 2026 13:33
Byron added a commit to Byron/gitbutler that referenced this pull request Apr 14, 2026
@mtsgrd
Copy link
Copy Markdown
Contributor Author

mtsgrd commented Apr 14, 2026

Actually, I think I am happy with it in principle, but feel the need to refactor. Can I take it from here?

Absolutely! Thank you.

@mtsgrd mtsgrd force-pushed the is-conflicted-refactor branch from d116a18 to d081a27 Compare April 14, 2026 09:47
@Byron Byron marked this pull request as draft April 14, 2026 12:10
@Byron Byron self-assigned this Apr 14, 2026
@Byron
Copy link
Copy Markdown
Collaborator

Byron commented Apr 14, 2026

Tasks

  • first review
  • structure in but-core
  • use latest gix trailer parsing facilities
    • don't normalize line-endings!
  • can unconditionally strip in ref_info.rs
  • try to unify is_conflicted() checks, it taking out the conflicted trailer in many places now. Check for but_core::commit::message_is_conflicted to find them.
  • add_markers_if_needed()
  • persist synthetic change-ids when it's obvious enough. doesn't belong here and just increases complexity

@Byron Byron force-pushed the is-conflicted-refactor branch from d081a27 to c7477ed Compare April 14, 2026 12:50
@mtsgrd mtsgrd force-pushed the is-conflicted-refactor branch from c7477ed to d081a27 Compare April 14, 2026 13:55
mtsgrd and others added 2 commits April 15, 2026 07:33
The `gitbutler-conflicted` extra header is stripped when commits are
rebased outside of GitButler, silently losing conflict state. Replace
it with commit message markers that survive rebasing and are visible
to developers in `git log`:

  - A `[conflict] ` prefix on the subject line for fast visual
    detection
  - A `GitButler-Conflict:` multi-line git trailer whose value
    explains the conflict tree layout

Using a standard git trailer means the description is embedded as the
trailer value with indented continuation lines, so
`git interpret-trailers` and other standard tools can parse and
manipulate it. The resulting commit message looks like:

    [conflict] <original subject>

    <original body>

    <existing trailers, if any>
    GitButler-Conflict: This is a GitButler-managed conflicted
       commit. Files are auto-resolved using the "ours" side.
       The commit tree contains additional directories:
         .conflict-side-0 — our tree
         .conflict-side-1 — their tree
         ...
       To manually resolve, check out this commit, remove the
       directories listed above, resolve the conflicts, and
       amend the commit.

The trailer is appended after any existing trailers in the message,
preserving Change-Id, Signed-off-by, and similar lines. When stripping
conflict markers (on resolution, display, or commit matching), the
prefix, trailer, and all continuation lines are removed to restore the
original message.

New commits only write message markers (no header). Reading checks the
message first, then falls back to the header for backward compatibility
with existing conflicted commits.

The CONFLICT-README.txt tree blob is removed since the trailer now
serves the same explanatory purpose with better visibility.

The conflicted file count (`Option<u64>`) stored in the old header was
only ever used as a boolean, so the new approach uses a simple presence
check with no count.
- more structured `but-core` addition
- idempotent `add_conflict_markers`
- simplify marker removal

Co-authored-by: GPT 5.4 <codex@openai.com>
@Byron Byron force-pushed the is-conflicted-refactor branch from d081a27 to 4bc0630 Compare April 15, 2026 01:35
Copy link
Copy Markdown
Collaborator

@Byron Byron left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am force-pushing based on the most recent changes that were , erm, force-pushed on top of mine.
The changes in @mtsgrd commit are only the whitespace that was added to Cargo.toml for no reason. Maybe it's a formatter thing.

My refactors are on top and I will merge (if I can) in fear of another force-push 😅.

@Caleb-T-Owens To my mind, the changes to but-rebase are Ok but you might want to take another look nonetheless. It would also be good to find a way to tackle the 'persist synthetic change-ids' story that is still lingering.

@Byron Byron marked this pull request as ready for review April 15, 2026 01:35
Copilot AI review requested due to automatic review settings April 15, 2026 01:35
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 21 out of 21 changed files in this pull request and generated 4 comments.

Comment thread crates/but-core/src/commit/conflict.rs
Comment thread crates/but-core/src/commit/conflict.rs
Comment thread crates/but-core/src/commit/conflict.rs
Comment thread crates/but-workspace/src/legacy/stacks.rs Outdated
Copy link
Copy Markdown
Contributor

@chatgpt-codex-connector chatgpt-codex-connector bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 4bc063068d

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread crates/gitbutler-edit-mode/src/lib.rs
Comment thread crates/but-workspace/src/legacy/stacks.rs Outdated
@Byron Byron force-pushed the is-conflicted-refactor branch from 4bc0630 to 3c42d96 Compare April 15, 2026 06:38
@Byron
Copy link
Copy Markdown
Collaborator

Byron commented Apr 15, 2026

Hitting auto-merge as I spent some time with it and think it's good.

It's notable that for GUI purposes, we strip the [conflict] prefix from commit messages in types that also have a has_conflict field. This introduces a mismatch between the titles people see in git log and friends, and in our clients. So that's definitely to watch out for and maybe we change it.

@Byron Byron enabled auto-merge April 15, 2026 06:40
Copilot AI review requested due to automatic review settings April 15, 2026 06:41
@Byron Byron force-pushed the is-conflicted-refactor branch from 3c42d96 to 9514792 Compare April 15, 2026 06:41
@Byron Byron force-pushed the is-conflicted-refactor branch from 9514792 to 6c8301c Compare April 15, 2026 06:42
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 23 out of 23 changed files in this pull request and generated 1 comment.

Comment thread crates/but-core/src/commit/conflict.rs Outdated
Co-authored-by: Sebastian Thiel <sebastian.thiel@icloud.com>
@Byron Byron force-pushed the is-conflicted-refactor branch from 6c8301c to 79ac145 Compare April 15, 2026 06:49
@Byron Byron merged commit 8b6b03c into master Apr 15, 2026
35 checks passed
@Byron Byron deleted the is-conflicted-refactor branch April 15, 2026 07:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

CLI The command-line program `but` rust Pull requests that update Rust code

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants