Attribute apply conflicts with the target to the target - #15288
Open
mtsgrd wants to merge 1 commit into
Open
Conversation
Applying a branch based behind the workspace target could name innocent - even empty - stacks as the conflict and suggest unapplying them. The workspace merge only knows stack tips, so a hero-vs-target conflict was unrepresentable and blame backtracking pinned it on whatever tips stood before the hero. Pre-flight the branch against the workspace's integration frame (target commit, or lower bound without one) before stack blame runs, and report a new conflictsWithTarget outcome that names the conflicting files instead of blaming any stack. CLI, desktop and lite list those files below an actionable message: update the branch with the target changes instead of unapplying stacks. Non-abort (materializing) apply is unchanged, and genuine stack conflicts are still blamed on the stacks.
github-actions
Bot
added
rust
mtsgrd
marked this pull request as ready for review
August 12, 2026 09:35
Contributor
There was a problem hiding this comment.
Pull request overview
This PR improves branch apply conflict reporting by detecting when an incoming branch conflicts with the workspace target (rather than blaming intervening/empty stacks), and plumbs a new conflictsWithTarget outcome plus conflicting file paths through the Rust API, SDK types, CLI output, and desktop/lite UI toasts.
Changes:
- Add
OutcomeStatus::ConflictsWithTargetandtarget_conflictsreporting to the apply operation, populated by an upfront three-way tree merge against the workspace target. - Surface the new outcome consistently across API/SDK/CLI, including user-facing messaging and file lists.
- Add fixture-backed regression tests (workspace + CLI) covering “branch behind target with empty stacks parked on target”.
Reviewed changes
Copilot reviewed 12 out of 14 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| packages/but-sdk/src/generated/linear/index.d.ts | Extend generated SDK apply outcome types with conflictsWithTarget and targetConflicts. |
| packages/but-sdk/src/generated/graph/index.d.ts | Same SDK type updates for the graph variant. |
| crates/but/tests/fixtures/scenario/apply-hero-behind-conflicting-target.sh | New CLI repro fixture for “behind target + conflict” scenario. |
| crates/but/tests/but/command/branch/apply.rs | Update JSON expectations and add CLI test asserting the target is blamed (and files listed). |
| crates/but/src/command/legacy/apply.rs | Emit human/JSON output for conflictsWithTarget and include targetConflicts in JSON. |
| crates/but-workspace/tests/workspace/branch/apply_unapply.rs | Add workspace-level test ensuring empty stacks aren’t blamed and all target-conflicting paths are reported. |
| crates/but-workspace/tests/fixtures/scenario/hero-behind-target-with-empty-stacks.sh | New workspace fixture with empty stacks on advanced target plus conflicting/clean branches. |
| crates/but-workspace/src/commit/mod.rs | Expose peel_to_tree() for reuse by the new apply preflight merge. |
| crates/but-workspace/src/branch/apply.rs | Core change: add new outcome + early conflict check against target and return conflicting paths. |
| crates/but-api/src/branch.rs | Add target_conflicts to the JSON transport type and conversion. |
| apps/lite/ui/src/components/Toasts.tsx | Allow toast descriptions to preserve newlines (via CSS class). |
| apps/lite/ui/src/components/Toasts.module.css | Add white-space: pre-line for multiline conflict file lists. |
| apps/lite/ui/src/api/mutations.ts | Handle conflictsWithTarget with a dedicated toast and file list. |
| apps/desktop/src/lib/stacks/stack.ts | Handle conflictsWithTarget with a dedicated toast and file list. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Comment on lines
+853
to
+868
| // No fail-fast here - the conflicting paths are reported, so all of them are wanted. | ||
| let merge = repo.merge_trees( | ||
| peel_to_tree(base.id.attach(repo))?, | ||
| peel_to_tree(frame_id.attach(repo))?, | ||
| peel_to_tree(branch_commit.id.attach(repo))?, | ||
| repo.default_merge_labels(), | ||
| repo.tree_merge_options()?, | ||
| )?; | ||
| let conflict_kind = gix::merge::tree::TreatAsUnresolved::git(); | ||
| let conflicting_paths: Vec<_> = merge | ||
| .conflicts | ||
| .iter() | ||
| .filter(|conflict| conflict.is_unresolved(conflict_kind)) | ||
| .map(|conflict| conflict.ours.location().to_owned()) | ||
| .collect(); | ||
| Ok((!conflicting_paths.is_empty()).then_some(conflicting_paths)) |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Applying a branch that was created before the target moved could fail like this:
…where both of those stacks are empty. An empty stack has no content, so it can never conflict with anything — and unapplying them doesn't fix anything either. This PR makes apply blame the real culprit: the target.
The situation
heroconflicts with the target — both changedshared.txtsinceherobranched off. The empty lanes are just parked on the new target commit and have nothing to do with it.Why the wrong suspect got blamed
The workspace commit is a merge of stack tips — no merge tip represents the target itself, so "the branch conflicts with the target" was unrepresentable. When the merge hit that conflict, the blame search skipped stack tips one by one until the branch merged alone, then reported every skipped tip as conflicting. An empty lane's tip is the new target commit, so from the merge's (older) base it looks like it authored the entire target delta.
The fix
Before any stack blame runs, test the branch directly against the target with an ordinary three-way merge from their common ancestor. If that conflicts, stop right there: report a new
conflictsWithTargetoutcome that names the conflicting files and blames no stacks.Desktop and lite show the equivalent message as a toast, with the file list at the bottom.
What doesn't change
Fixture-backed tests at core and CLI level, plus verification against the field repro above (it now reports the target conflict with all three conflicting files).
Follow-up ideas from the same diagnosis: make "empty stacks never conflict" structural in the blame search, and ultimately let apply proceed by rebasing the branch onto the target with conflicted commits (reviving #14919) instead of refusing.