Refuse safe checkout while the index has unresolved conflicts - #15691
Refuse safe checkout while the index has unresolved conflicts#15691krlvi wants to merge 1 commit into
Conversation
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
|
@jonathantanmy2 would you mind taking a look at this fix? Is it a reasonable one? |
There was a problem hiding this comment.
Pull request overview
This PR prevents workspace/materialization checkouts from proceeding when the on-disk repository index (.git/index) contains unresolved conflict (stage 1/2/3) entries, returning a typed precondition failure with actionable guidance instead of surfacing libgit2’s raw checkout conflict error.
Changes:
- Add a pre-check in
safe_checkout_from_headthat refuses non-no-op checkouts when the repo index has unresolved conflicts, producing a user-orientedPreconditionFailederror listing conflicted paths. - Add regression tests covering: (1) refusal happens before mutating worktree/index/HEAD, and (2) merge-base-override (“steamroll”) checkouts still succeed and clear stale index conflicts.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| crates/but-core/src/worktree/checkout/function.rs | Adds an index-conflict precondition guard to safe_checkout_from_head and a helper to format a guided error message. |
| crates/but-core/tests/core/worktree/checkout.rs | Adds tests validating refusal-before-mutation behavior and preserving override checkout semantics under unresolved index conflicts. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
4a33e93 to
1a5cc16
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 4a33e93df0
ℹ️ 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".
A checkout that is allowed to conflict with uncommitted changes (or a merge run outside GitButler) leaves stage 1/2/3 entries in .git/index. The next workspace update then died deep inside libgit2's checkout with a raw 'unresolved conflicts exist in the index' error. Check for unresolved index entries before the checkout mutates anything and fail with a typed precondition naming the conflicted files, so the frontend can present it as an actionable warning. Two paths deliberately keep working while conflicts are unresolved: no-op checkouts (the head tree is unchanged, e.g. a reword), and merge-base-override checkouts whose index rewrite clears the stages - snapshot restore always passes an override and is the escape hatch from exactly this state.
1a5cc16 to
66c5958
Compare
Fixes GB-1937
The problem
Updating the workspace while
.git/indexstill holds unresolved stage 1/2/3 entries died deep inside libgit2's checkout with the raw errorunresolved conflicts exist in the index; class=Checkout; code=Conflict— surfaced verbatim to the user with no recovery guidance. Telemetry shows this on both macOS and Windows on stable 0.22.3.How the state arises
libgit2 refuses any non-force checkout while the repository index has conflict entries — even though GitButler passes its own already-safety-merged in-memory index. Two ways users end up there:
.git/index(that is how the conflicted-uncommitted-files state is represented). Until the user marks those files resolved, every subsequent workspace update hit the raw error.git merge/git cherry-pickin the terminal leaves the same stage entries.The fix
safe_checkout_from_headnow checks for unresolved index entries before the checkout mutates anything and fails with a typedPreconditionFailednaming the conflicted files and pointing at the resolution path (but resolve <path>; desktop/Lite show conflicted uncommitted files with a resolve affordance). Since the checkout runs before any ref edits during materialization, refusing there preserves worktree, index, HEAD, and workspace metadata. This covers all checkout-based operations (workspace update, apply, unapply, …), not just upstream integration.Two paths deliberately keep working while conflicts are unresolved, and are pinned by tests: