parse_git_apply_output mis-attributes paths via BTreeSet::next_back()

Open 💬 3 comments Opened Jun 29, 2026 by syedali5612

What

parse_git_apply_output in codex-rs/git-utils/src/apply.rs classifies each path from git apply into applied / skipped / conflicted BTreeSets and tracks last_seen_path. After inserting a path it recovers "the path just inserted" with <set>.iter().next_back().cloned() (6 sites: lines ~459, 471, 483, 497, 550, 564).

next_back() returns the lexicographically greatest element of the set, not the element just inserted. Once a set holds more than one path, the cross-set remove() reconciliation and last_seen_path are applied to the wrong file — so a later THREE_WAY_FAILED / LACKS_BLOB skip can be charged to the wrong path, and applied/skipped/conflicted classification gets swapped.

Repro

Input (paths chosen so the lexicographically-greatest is inserted first):

Applied patch zzz.txt cleanly.
Applied patch aaa.txt cleanly.
Failed to perform three-way merge...

Expected: applied == ["zzz.txt"], skipped == ["aaa.txt"].
Actual: the three-way-merge failure is attributed to zzz.txt (the set max), so zzz.txt is de-applied and aaa.txt is wrongly kept as applied.

Root cause

The local add(set, raw) helper inserts but returns nothing, and callers reach back into the BTreeSet with next_back() to guess what was added. BTreeSet is ordered, so that guess is the max, not the insertion.

Suggested fix (outline)

Have the add helper return the inserted (unescaped) key, and use that returned value at every call site instead of next_back(). I tried this locally: it is a minimal change, keeps clippy (collapsible_if) and cargo fmt clean, and a regression test built from the repro above goes from failing to passing (cargo test -p codex-git-utils: 23 passed). Happy to open a PR with the fix + test if you would like — flagging via an issue first per the contributing guidelines.

View original on GitHub ↗

This issue has 3 comments on GitHub. Read the full discussion on GitHub ↗