Skip to content

fix(git): preserve POSIX filenames during bulk discard - #27134

Closed
BichengWang wants to merge 1 commit into
stablyai:mainfrom
BichengWang:BichengWang/contribution-orca-pr-20261009
Closed

BichengWang wants to merge 1 commit into
stablyai:mainfrom
BichengWang:BichengWang/contribution-orca-pr-20261009

Conversation

@BichengWang

Copy link
Copy Markdown

ELI5

On macOS and Linux, docs\file.txt and docs/file.txt are different files. Discarding a selection containing both now restores the tracked file and removes the selected untracked file instead of failing with a Git path error.

What Changed

The shared tracked-path comparison keeps backslashes literal on POSIX execution hosts. It retains separator normalization on Windows and for the existing WSL input conversion. Native and relay bulk discard use the same matcher.

Why

Correcting the existing matcher makes the tracked/untracked decision agree with the filenames Git receives. Separate platform-specific discard implementations would duplicate the restore, clean, and safety logic; this approach also retains the existing lazy comparison cache.

Linked Issue

Fixes #27133

Visual Proof

N/A — backend filename comparison only; no UI components, layout, or interaction flows are changed. The real-Git regression cases demonstrate the behavior before and after:

Before: upstream 8ba3cf46ab09
  native: 3 failures; relay: 3 failures
  git restore rejects the selected untracked path
After: this patch
  native: 3 passes; relay: 3 passes
  tracked file restored to staged bytes, selected untracked path removed
  staged index and unselected leave.txt preserved

Testing

  • I manually tested these changes locally
  • Automated tests added/updated

Tested on macOS with Node 24.19.0, Bun 1.4.2, and real Git 2.54.0. All six new cases fail against unmodified upstream and pass with this patch; 121 tests across 12 discard, staging, path-safety, pathspec, and relay suites pass. Reproduce the new coverage with:

pnpm test src/main/git/source-control/discard-backslash-paths.test.ts

pnpm lint, pnpm tc, the changed-code quality gate, formatting, build:relay, and build:electron-vite pass. Full-suite and packaging checks remain for CI. No manual GUI check or live Linux, Windows, WSL, or SSH session was run; Windows comparison and WSL conversion have automated coverage.

Review

Reviewed execution-host path semantics, native/relay consistency, staged-content preservation, unselected-file and symlink safety, and comparison cost. Git options and remote messages are unchanged. The fix applies to repository-backed folder workspaces as well as Git worktrees and adds no provider or integration assumptions.

Skill upstream boundary

  • Not applicable; no upstream skill source or related artifacts are copied or translated.

Notes

No UI, mobile, keyboard-shortcut, or integration code changes.

Author

GitHub: @BichengWang. X handle not provided.

Checklist

  • This PR is small and focused
  • I explained what changed and why
  • Before/after screenshots or videos attached for UI changes, or N/A with reason
  • Self-reviewed for correctness, security, and performance
  • Cross-platform, SSH/remote, and path/shortcut impact considered
  • Local checks listed above pass; CI will cover full-suite and packaging checks

@coderabbitai

coderabbitai Bot commented Oct 10, 2026

Copy link
Copy Markdown
Contributor

Review in Change Stack →Review in Change Stack →

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration
  • Configuration used: Repository UI
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: 25b9e296-4bee-401d-84fa-027896d2ff91

📥 Commits

Reviewing files that changed from the base of the PR and between 63b3f15 and c549415.


📒 Files selected for processing (6)
  • src/main/git/source-control/discard-backslash-paths.test.ts
  • src/main/git/source-control/discard-changes.ts
  • src/main/git/status-discard-and-bulk-staging.test.ts
  • src/relay/git-handler-working-tree-changes.test.ts
  • src/shared/git-tracked-pathspecs.test.ts
  • src/shared/git-tracked-pathspecs.ts

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 9 remain after this review.



📝 Walkthrough

Walkthrough

Tracked-path matching now converts backslashes to forward slashes only when Windows path handling is enabled. The matching functions default that setting based on the current platform. Bulk discard enables Windows-style matching on Windows and when a WSL distro is configured. Tests cover matching behavior and bulk-discard classification on POSIX and Windows.


Priority: ➖ Normal

Severity of issue fixed: Medium

Merge Risk: ⚪ Minimal · up to c5494

The change appears ready to merge after normal CI checks; no actionable risk was identified in the supplied evidence.

Pre-merge checks | Passed 4 | Failed 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage Warning Docstring coverage is 16.67% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 6 functions across 6 files. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check Passed The title clearly and concisely describes the main change: preserving POSIX filenames during Git bulk discard.
Description check Passed The description is mostly complete. It explains the user impact, implementation, rationale, linked issue, testing, platform scope, review notes, and checklist status. Visual proof is correctly marked …
Linked Issues check Passed Issue #27133 requires POSIX path identity during bulk discard. normalizeGitPathForCompare now treats backslashes as literal characters when windowsPaths is false. bulkDiscardChanges enables sepa…
Out of Scope Changes check Passed The changes stay within issue #27133. Production changes adjust the shared tracked-path matcher and pass the execution-host path mode to bulk discard. Test changes cover the matcher, native discard, r…

  • Fix all pre-merge checks with AI
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

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.

@BichengWang

Copy link
Copy Markdown
Author

Follow-up check: this PR now conflicts with main. The legacy relay implementation and its test harness have been removed upstream; the current execution-host runtime path also normalizes POSIX backslashes before discard. I am preparing a replacement against current main for #27133, with real-Git regression coverage through the native and current runtime entry points. I will link the replacement here once verified.

@BichengWang

Copy link
Copy Markdown
Author

Superseded by #27757, which ports the fix to current main and also preserves POSIX filenames in the current execution-host runtime validator. Seven real-Git regressions fail before and pass afterward; 128 focused tests, full lint, full typecheck, the execution-host build, and a standalone native/runtime smoke pass. Closing this conflicting legacy-relay version to keep review in one place.

@BichengWang
BichengWang deleted the BichengWang/contribution-orca-pr-20261009 branch October 15, 2026 00:07
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.

[Bug]: Bulk discard confuses literal backslashes with POSIX separators

1 participant