Skip to content

test: preserve user changes across failed pull retries - #1218

Open
LemieuxStrategicSolutions wants to merge 1 commit into
team-reflect:codex/git-sync-reliabilityfrom
LemieuxStrategicSolutions:upstream/test-failed-pull-retries
Open

LemieuxStrategicSolutions wants to merge 1 commit into
team-reflect:codex/git-sync-reliabilityfrom
LemieuxStrategicSolutions:upstream/test-failed-pull-retries

Conversation

@LemieuxStrategicSolutions

@LemieuxStrategicSolutions LemieuxStrategicSolutions commented Sep 5, 2026

Copy link
Copy Markdown

Adds two failure/retry regressions to #1215, following the remaining review cases from #1209:

  • After a failed checkout leaves a binary conflict copy, a user edits that copy. The next snapshot and pull must retain those bytes in the working tree and merge tree, and preserve the remote attachment in a separately named copy.
  • After a failed checkout, a user deletes the conflicted note. The next snapshot must commit that deletion before a new edit/delete merge. The test explicitly preserves the existing keep-the-edited-version policy: the remote note may return as a reported conflict, while the deletion remains in the local merge parent.

This is intentionally based on codex/git-sync-reliability so the diff contains only the added tests. It can be merged into #1215 or cherry-picked; it does not ask to merge the old recovery mechanism from #1209. The in-memory merge in #1215 avoids creating the MERGE_* state that mechanism would recover.

Validation passed: cargo test -p reflect-open git::merge_tests (11 tests, including both additions), cargo fmt --all --check, cargo clippy -p reflect-open --all-targets -- -D warnings, pnpm check, and pnpm build. These tests exercise deterministic checkout failure and retry; they do not claim physical-device kill testing or migration of pre-existing foreign Git merges.

Summary by CodeRabbit

  • Bug Fixes
    • Improved merge recovery after a failed checkout when the worktree contains uncommitted changes.
    • Preserved edits to conflict copies and recorded file deletions before retrying a merge.
    • Ensured retried merges retain local changes while correctly surfacing incoming content conflicts.

@coderabbitai

coderabbitai Bot commented Sep 5, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Team

Run ID: 03c07fa4-d130-47d2-98c5-2bc8c19f8fb2

📥 Commits

Reviewing files that changed from the base of the PR and between 4780481 and e0eaf63.

📒 Files selected for processing (1)
  • apps/desktop/src-tauri/src/git/merge_tests.rs

Included review availability: Your plan provides up to 4 included reviews per hour; 2 remain after this review.


Walkthrough

The change adds two regression tests for merge retries after failed checkouts. The tests cover edited conflict copies and deleted files, then verify preserved first-parent state, remote merge content, clean repository state, and resolved index conflicts.

Changes

Merge retry regressions

Layer / File(s) Summary
Retry merge snapshot behavior
apps/desktop/src-tauri/src/git/merge_tests.rs
Adds tests for preserving an edited conflict copy and snapshotting a deletion before retrying a merge. The tests verify the resulting conflict contents, first-parent trees, clean repository state, and absence of index conflicts.

Estimated code review effort: 2 (Simple) | ~10 minutes

Merge Risk: ⚪ Minimal · up to e0eaf

This adds regression coverage for preserving edited conflict copies and deleted notes across failed pull retries, with no production behavior changes or current merge-readiness risk identified.

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly summarizes the main change: regression tests that verify user changes remain preserved across failed pull retries.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 3 functions across 1 files.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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

@LemieuxStrategicSolutions

Copy link
Copy Markdown
Author

@coderabbitai review

@coderabbitai

coderabbitai Bot commented Sep 5, 2026

Copy link
Copy Markdown
✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

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.

1 participant