Skip to content

ADFA-2885: Handle merge conflicts#1167

Open
dara-abijo-adfa wants to merge 10 commits intostagefrom
ADFA-2885-merge-conflicts-handling
Open

ADFA-2885: Handle merge conflicts#1167
dara-abijo-adfa wants to merge 10 commits intostagefrom
ADFA-2885-merge-conflicts-handling

Conversation

@dara-abijo-adfa
Copy link
Copy Markdown
Contributor

  • Detect merge conflicts
  • Resolve conflicted files in the editor

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai bot commented Apr 8, 2026

📝 Walkthrough

Release Notes

Features

  • Detect merge conflicts during pull operations and surface a dedicated PullUiState.Conflicts state.
  • Open conflicted files directly in the editor and insert visible conflict markers for in-place resolution.
  • Provide in-editor conflict resolution handling during push operations (better error selection for push failures).
  • Implement Abort Merge action to cancel an in-progress merge.
  • Improved error messaging for push rejections (non-fast-forward) and checkout conflicts (includes conflicting file paths).

UI Updates

  • Added "Abort Merge" button to the Git bottom sheet (visible only when a merge is active).
  • New merge-conflicts dialog shown when conflicts are detected during pull operations.
  • Bottom sheet behavior: conflicted files open in the editor (sheet collapses); non-conflicted changes continue to open the diff dialog.

Technical Changes

  • Git API surface extended:
    • Added GitRepository.merge(branchName: String): MergeResult
    • Added GitRepository.abortMerge()
  • JGitRepository:
    • Implemented merge(branchName) and abortMerge() (abort uses a hard reset to ORIG_HEAD and clears merge metadata).
    • getStatus() now reports isMerging based on repository state.
  • GitStatus model: added isMerging: Boolean and updated GitStatus.EMPTY.
  • ViewModel changes:
    • Added PullUiState.Conflicts and extended PullUiState.Error to include optional errorArgs for formatted messages.
    • Added GitBottomSheetViewModel.abortMerge(onSuccess: (() -> Unit)? = null).
    • Pull flow now catches CheckoutConflictException and emits a formatted checkout_conflict_message with conflicting paths.
    • Push error handling maps REJECTED_NONFASTFORWARD to push_rejected_nonfastforward.
  • Editor integration:
    • EditorHandlerActivity.checkForExternalFileChanges() made public and gained force: Boolean = false to force reloads and refresh file timestamp baselines.
    • Git bottom sheet triggers checkForExternalFileChanges(force = true) when opening conflicted files or after aborting a merge (when hosted in EditorHandlerActivity).
  • Layout and resources:
    • Added abort_merge and related strings and added a btnAbortMerge to fragment_git_bottom_sheet.xml.
    • Minor dialog/message selection logic refinements in GitCommitHistoryDialog.

Risks and Best-Practices Violations

  • API surface expansion: checkForExternalFileChanges() changed from private → public with a force flag; this increases the activity's external surface and is a breaking-risk for future changes.
  • Destructive abort: abortMerge() performs a hard reset to ORIG_HEAD and clears merge metadata — this can discard uncommitted or local changes. Ensure UI confirmations and clear user communication.
  • Potential data loss from forced reloads: forcing external file reloads bypasses the editor's "don't override modified text" guard when force = true, risking loss of unsaved user edits if invoked incorrectly.
  • State/message complexity: introducing PullUiState.Conflicts and errorArgs increases branching; inconsistent handling across callers could produce confusing user messages or drop diagnostic context.
  • Immediate state reset risk: emitting Conflicts then resetting pull state may lose contextual error information needed for diagnostics or retry flows.
  • Increased testing surface: multiple new public methods, model fields, UI flows, and editor interactions require thorough testing (merge, abort, UI visibility, message formatting, and editor concurrency).

Walkthrough

Adds merge and abort-merge support across git core, viewmodel, and UI; introduces PullUiState.Conflicts, an Abort Merge button/flow, and exposes EditorHandlerActivity.checkForExternalFileChanges(force) to coordinate external file reloads after merge actions.

Changes

Cohort / File(s) Summary
Git core API & impl
git-core/src/main/java/com/itsaky/androidide/git/core/GitRepository.kt, git-core/src/main/java/com/itsaky/androidide/git/core/JGitRepository.kt
Added merge(branchName: String): MergeResult and abortMerge() to the repository API; JGitRepository implements merge and abort (hard reset + clear merge metadata) and surfaces isMerging in status.
Git status model
git-core/src/main/java/com/itsaky/androidide/git/core/models/GitStatus.kt
Added isMerging: Boolean to GitStatus and updated GitStatus.EMPTY.
ViewModel (pull/abort logic)
app/src/main/java/com/itsaky/androidide/viewmodel/GitBottomSheetViewModel.kt
Added abortMerge(onSuccess?). Extended pull flow to catch CheckoutConflictException, emit PullUiState.Conflicts for merge conflicts, and updated PullUiState.Error to include errorArgs. Refined push-error mapping.
Bottom sheet UI & interactions
app/src/main/java/com/itsaky/androidide/fragments/git/GitBottomSheetFragment.kt, app/src/main/res/layout/fragment_git_bottom_sheet.xml
Added btnAbortMerge (hidden by default) and adjusted constraints; show/hide based on status.isMerging and repo state; clicking conflicted file forces external-reload and opens file; abort flow shows confirmation, calls viewModel.abortMerge, and forces editor reload.
Commit history dialog
app/src/main/java/com/itsaky/androidide/fragments/git/GitCommitHistoryDialog.kt
Adjusted selection of push-failure message: prefer resolved errorResId only when meaningful, else fall back to state message or default string.
Editor activity
app/src/main/java/com/itsaky/androidide/activities/editor/EditorHandlerActivity.kt
Made checkForExternalFileChanges public with force: Boolean = false; when forced, bypasses modified-text guard, treats missing timestamps as 0, reloads file and updates timestamp baseline.
Strings
resources/src/main/res/values/strings.xml
Added strings for checkout conflict, push non-fast-forward, merge-conflict info, abort/confirm labels, and related messages.

Sequence Diagram

sequenceDiagram
    actor User
    participant GitFragment as "Git Bottom Sheet\nFragment"
    participant ViewModel as "GitBottomSheet\nViewModel"
    participant GitRepo as "GitRepository\n(JGitRepository)"
    participant Editor as "EditorHandler\nActivity"

    User->>GitFragment: Initiate Pull
    GitFragment->>ViewModel: pull()
    ViewModel->>GitRepo: perform pull/merge
    alt MergeStatus == CONFLICTING
        GitRepo-->>ViewModel: MergeStatus.CONFLICTING
        ViewModel-->>GitFragment: emit PullUiState.Conflicts
        GitFragment->>User: show merge-conflicts dialog
        User->>GitFragment: Click "Abort Merge"
        GitFragment->>User: show confirm dialog
        User->>GitFragment: Confirm
        GitFragment->>ViewModel: abortMerge(onSuccess)
        ViewModel->>GitRepo: abortMerge()
        GitRepo-->>ViewModel: success
        ViewModel-->>GitFragment: refreshStatus / callback
        GitFragment->>Editor: checkForExternalFileChanges(force=true)
        Editor-->>GitFragment: external files reloaded
    else CheckoutConflictException
        GitRepo-->>ViewModel: CheckoutConflictException (paths)
        ViewModel-->>GitFragment: emit PullUiState.Error(errorResId, errorArgs)
        GitFragment->>User: display checkout conflict message with paths
    end
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Possibly related PRs

  • PR #1038 — Modifies the same Git UI (GitBottomSheetFragment/ViewModel); likely predecessor for bottom-sheet and diff-related flows updated here.
  • PR #1128 — Related updates to pull/push handling and UI state in GitBottomSheetViewModel that this PR extends.
  • PR #962 — Prior work on git-core that introduced repository operations which this PR builds upon.

Suggested reviewers

  • jomen-adfa
  • itsaky-adfa
  • Daniel-ADFA

Poem

🐰 In tangled branches the rabbit hops light,

"Merge or abort?" it hums through the night.
Timestamps refreshed, conflicts in view,
A button to fix — then files renew.
Hoppity code, now merges take flight.

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 10.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Title check ✅ Passed The title 'ADFA-2885: Handle merge conflicts' directly and clearly summarizes the main change—adding merge conflict detection and handling functionality across the codebase.
Description check ✅ Passed The description 'Detect merge conflicts' and 'Resolve conflicted files in the editor' is directly related to the changeset, which implements merge conflict detection, conflict display, and editor-based resolution.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch ADFA-2885-merge-conflicts-handling

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 5

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
app/src/main/java/com/itsaky/androidide/activities/editor/EditorHandlerActivity.kt (1)

283-301: ⚠️ Potential issue | 🟠 Major

Seed fileTimestamps before using this reload path.

checkForExternalFileChanges() only refreshes files that already have an entry in fileTimestamps, and Line 289 skips everything else. Right now that map is filled on save, onPause(), or a prior reload, but not when a tab is first opened. So if a pull writes conflict markers into a file that was already open in this session, reopening it from the Git sheet can still show the stale in-memory buffer instead of the conflicted disk content. Initializing the timestamp when a file is opened, or lazily backfilling missing entries before the comparison, would close that gap.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In
`@app/src/main/java/com/itsaky/androidide/activities/editor/EditorHandlerActivity.kt`
around lines 283 - 301, checkForExternalFileChanges() only processes files
present in the fileTimestamps map; seed missing entries to avoid ignoring newly
opened tabs—either initialize fileTimestamps[file.absolutePath] =
file.lastModified() when a tab is opened (where the editor adds opened files in
editorViewModel.getOpenedFiles()/the tab-opening routine), or add a quick
backfill at the start of checkForExternalFileChanges(): for each file from
editorViewModel.getOpenedFiles() if fileTimestamps[file.absolutePath] is null
then set it to file.lastModified(); this ensures the later comparison
(currentTimestamp > lastKnownTimestamp) will detect real on-disk changes.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In
`@app/src/main/java/com/itsaky/androidide/fragments/git/GitBottomSheetFragment.kt`:
- Around line 170-175: The abort-merge callback currently only calls
EditorHandlerActivity.checkForExternalFileChanges(), which intentionally ignores
modified tabs and therefore leaves stale conflict edits; update the
viewModel.abortMerge completion handler to force-refresh conflicted editor tabs
instead of the normal external-change check: identify the conflicted file paths
from the aborted merge result (or from viewModel state) and then call
EditorHandlerActivity methods to either forceReloadTab(path) or
closeAndReopenTab(path) (implement these helpers if missing) so modified tabs
showing conflict resolution are replaced with the repository state; ensure to
call these force-refresh helpers on the UI thread after viewModel.abortMerge
completes.

In
`@app/src/main/java/com/itsaky/androidide/viewmodel/GitBottomSheetViewModel.kt`:
- Around line 288-297: The Conflicts state emitted by handlePullError should be
treated as sticky: in pull()'s finally block skip scheduling the delayed reset
when _pullState.value is PullUiState.Conflicts (e.g. wrap the existing
pullResetJob/viewModelScope.launch/delay logic in a guard like if
(_pullState.value !is PullUiState.Conflicts) { ... }), and ensure that
repository merge resolution paths (the abort/resolve handlers) explicitly set
_pullState.value = PullUiState.Idle after successful abort or resolve so the UI
is cleared only when the merge is completed.
- Around line 360-365: The abortMerge method currently calls
currentRepository?.abortMerge() and still invokes onSuccess even when
currentRepository is null; update abortMerge to check currentRepository at the
start (e.g., if (currentRepository == null) ...) and return early or invoke a
failure callback instead of calling onSuccess, ensuring you only call
currentRepository.abortMerge(), refreshStatus(), and onSuccess after confirming
currentRepository is non-null; reference the abortMerge function,
currentRepository property, refreshStatus(), and onSuccess to locate and fix the
logic.

In `@git-core/src/main/java/com/itsaky/androidide/git/core/JGitRepository.kt`:
- Around line 82-87: The isMerging check in JGitRepository.kt currently only
tests repository.repositoryState == RepositoryState.MERGING, which misses the
MERGING_RESOLVED state; update the isMerging logic (the val isMerging assignment
used to populate GitStatus) to treat RepositoryState.MERGING_RESOLVED as merging
as well (i.e., check for either MERGING or MERGING_RESOLVED) so merge UI remains
visible until the merge commit is created.
- Around line 284-287: abortMerge() currently only does a hard reset and does
not remove merge-state files; update the JGitRepository.abortMerge()
implementation to explicitly remove .git/MERGE_HEAD and .git/MERGE_MSG (and any
other MERGE_* files) from the repository directory, then perform a reset to the
pre-merge commit: attempt to resolve "ORIG_HEAD" and reset to it if present,
otherwise reset to "HEAD"; keep using git.reset().setMode(ResetType.HARD).call()
for the reset step but replace the single-step behavior with this two-step
cleanup+reset flow and ensure you reference repository.repositoryState,
abortMerge(), git.reset(), ResetType.HARD, and the ORIG_HEAD/MERGE_HEAD file
names when locating the code to change.

---

Outside diff comments:
In
`@app/src/main/java/com/itsaky/androidide/activities/editor/EditorHandlerActivity.kt`:
- Around line 283-301: checkForExternalFileChanges() only processes files
present in the fileTimestamps map; seed missing entries to avoid ignoring newly
opened tabs—either initialize fileTimestamps[file.absolutePath] =
file.lastModified() when a tab is opened (where the editor adds opened files in
editorViewModel.getOpenedFiles()/the tab-opening routine), or add a quick
backfill at the start of checkForExternalFileChanges(): for each file from
editorViewModel.getOpenedFiles() if fileTimestamps[file.absolutePath] is null
then set it to file.lastModified(); this ensures the later comparison
(currentTimestamp > lastKnownTimestamp) will detect real on-disk changes.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: a6f88884-bce8-4c2e-ba3e-7f5d8b2e694a

📥 Commits

Reviewing files that changed from the base of the PR and between d300cff and c44b4dc.

📒 Files selected for processing (9)
  • app/src/main/java/com/itsaky/androidide/activities/editor/EditorHandlerActivity.kt
  • app/src/main/java/com/itsaky/androidide/fragments/git/GitBottomSheetFragment.kt
  • app/src/main/java/com/itsaky/androidide/fragments/git/GitCommitHistoryDialog.kt
  • app/src/main/java/com/itsaky/androidide/viewmodel/GitBottomSheetViewModel.kt
  • app/src/main/res/layout/fragment_git_bottom_sheet.xml
  • git-core/src/main/java/com/itsaky/androidide/git/core/GitRepository.kt
  • git-core/src/main/java/com/itsaky/androidide/git/core/JGitRepository.kt
  • git-core/src/main/java/com/itsaky/androidide/git/core/models/GitStatus.kt
  • resources/src/main/res/values/strings.xml

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In
`@app/src/main/java/com/itsaky/androidide/fragments/git/GitBottomSheetFragment.kt`:
- Around line 53-63: The call to
EditorHandlerActivity.checkForExternalFileChanges in GitBottomSheetFragment
(inside the viewLifecycleOwner.lifecycleScope.launch before openFile and
bottomSheetViewModel.setSheetState) uses the default force=false which can skip
reloading a modified editor and cause stale content to be opened; change the
invocation to pass force=true when calling checkForExternalFileChanges so the
editor is forced to refresh before activity.openFile(File(repo.rootDir,
change.path)) executes.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: c153b13b-d977-421e-8b70-7567df013f73

📥 Commits

Reviewing files that changed from the base of the PR and between c44b4dc and cf2b44d.

📒 Files selected for processing (4)
  • app/src/main/java/com/itsaky/androidide/activities/editor/EditorHandlerActivity.kt
  • app/src/main/java/com/itsaky/androidide/fragments/git/GitBottomSheetFragment.kt
  • git-core/src/main/java/com/itsaky/androidide/git/core/JGitRepository.kt
  • resources/src/main/res/values/strings.xml
✅ Files skipped from review due to trivial changes (1)
  • resources/src/main/res/values/strings.xml
🚧 Files skipped from review as they are similar to previous changes (2)
  • git-core/src/main/java/com/itsaky/androidide/git/core/JGitRepository.kt
  • app/src/main/java/com/itsaky/androidide/activities/editor/EditorHandlerActivity.kt

@dara-abijo-adfa dara-abijo-adfa force-pushed the ADFA-2885-merge-conflicts-handling branch from cf2b44d to 0bcc3e5 Compare April 10, 2026 15:44
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@git-core/src/main/java/com/itsaky/androidide/git/core/JGitRepository.kt`:
- Around line 284-286: abortMerge currently always performs a hard reset to
Constants.ORIG_HEAD via git.reset() which can discard work when no merge is in
progress; update the abortMerge() implementation to first check
repository.repositoryState and only proceed if it equals RepositoryState.MERGING
(otherwise return immediately or throw a clear exception), and then perform the
reset/cleanup
(git.reset().setMode(ResetType.HARD).setRef(Constants.ORIG_HEAD).call()) — use
repository.repositoryState == RepositoryState.MERGING to gate the operation to
avoid touching refs/worktree when not merging.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 200bb195-eefb-4306-b768-feb45d2be22d

📥 Commits

Reviewing files that changed from the base of the PR and between cf2b44d and 0bcc3e5.

📒 Files selected for processing (2)
  • app/src/main/java/com/itsaky/androidide/fragments/git/GitBottomSheetFragment.kt
  • git-core/src/main/java/com/itsaky/androidide/git/core/JGitRepository.kt

Comment on lines +284 to +286
override suspend fun abortMerge(): Unit = withContext(Dispatchers.IO) {
// Reset working tree and index to HEAD
git.reset().setMode(ResetType.HARD).setRef(Constants.ORIG_HEAD).call()
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🔴 Critical

Guard abortMerge() behind the actual merge state.

This hard-resets to ORIG_HEAD even when no merge is in progress. A stray call here can roll the repo back to an unrelated previous head and discard work. Please gate the reset/cleanup on repository.repositoryState == RepositoryState.MERGING (or fail fast) before touching refs or the worktree.

Suggested fix
 override suspend fun abortMerge(): Unit = withContext(Dispatchers.IO) {
+    if (repository.repositoryState != RepositoryState.MERGING) {
+        return@withContext
+    }
+
     // Reset working tree and index to HEAD
     git.reset().setMode(ResetType.HARD).setRef(Constants.ORIG_HEAD).call()
     
     // Explicitly clear merge-related files to exit the MERGING state
     repository.apply {
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
override suspend fun abortMerge(): Unit = withContext(Dispatchers.IO) {
// Reset working tree and index to HEAD
git.reset().setMode(ResetType.HARD).setRef(Constants.ORIG_HEAD).call()
override suspend fun abortMerge(): Unit = withContext(Dispatchers.IO) {
if (repository.repositoryState != RepositoryState.MERGING) {
return@withContext
}
// Reset working tree and index to HEAD
git.reset().setMode(ResetType.HARD).setRef(Constants.ORIG_HEAD).call()
}
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@git-core/src/main/java/com/itsaky/androidide/git/core/JGitRepository.kt`
around lines 284 - 286, abortMerge currently always performs a hard reset to
Constants.ORIG_HEAD via git.reset() which can discard work when no merge is in
progress; update the abortMerge() implementation to first check
repository.repositoryState and only proceed if it equals RepositoryState.MERGING
(otherwise return immediately or throw a clear exception), and then perform the
reset/cleanup
(git.reset().setMode(ResetType.HARD).setRef(Constants.ORIG_HEAD).call()) — use
repository.repositoryState == RepositoryState.MERGING to gate the operation to
avoid touching refs/worktree when not merging.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants