Skip to content

Fix push conflicts for non-UTF-8 paths - #4259

Merged
max-sixty merged 2 commits into
max-sixty:mainfrom
Duang777:fix/non-utf8-push-conflict-paths
Sep 25, 2026
Merged

max-sixty merged 2 commits into
max-sixty:mainfrom
Duang777:fix/non-utf8-push-conflict-paths

Conversation

@Duang777

@Duang777 Duang777 commented Sep 24, 2026 •

Copy link
Copy Markdown
Contributor

Summary

  • Compare target status paths and pushed paths as raw Git bytes in wt step push conflict checks.
  • Use one byte-oriented porcelain parser and changed-files API, with checked raw stdout methods for Git commands.
  • Cover distinct non-UTF-8 filenames with a Linux end-to-end push regression.

Root cause

ensure_no_target_conflicts decoded git status -z and git diff-tree -z output with String::from_utf8_lossy before comparing paths. Distinct names such as collision-\xFE and collision-\xFF both became collision-�, so an unrelated dirty target path could block the push.

Verification

  • cargo run -- hook pre-merge --yes passed locally.
  • Linux CI and codecov/patch passed on the current head.

This was written by Codex on behalf of max-sixty

@worktrunk-bot worktrunk-bot left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

The byte comparison fixes the false collision, and because it only removes collisions that came from lossy decoding, it doesn't weaken the dirty-target guard. Two follow-ups on the shape (inline):

  • After this change, parse_porcelain_z and Repository::changed_files are used only by tests. Nothing in production uses them for display, so the PR body's reason for keeping them doesn't apply. Both are now test-only helpers in library code. parse_porcelain_z also adds a new .expect, and the #[cfg(test)] import in repository_ext.rs exists only to keep it reachable.
  • The run-then-fail-on-non-success block is copied into two places instead of being shared. Neither new failure branch appears to have a test, so codecov/patch, which gates merge here, will probably flag them.

The same lossy decoding also appears outside this diff: list_ignored_entries in src/commands/step/shared.rs decodes ls-files -z output with String::from_utf8_lossy before joining each entry onto the worktree path. With a non-UTF-8 ignored filename, that join builds a path that doesn't exist on disk. That could be a separate follow-up; it doesn't block this PR.

Comment thread src/git/parse.rs Outdated
Comment thread src/git/repository/diff.rs Outdated
Comment thread src/git/repository/diff.rs Outdated
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.

3 participants