[codex] fix add case-renamed tracked paths#435
Open
genedna wants to merge 14 commits into
Open
Conversation
Signed-off-by: Eli Ma <eli@patch.sh>
Signed-off-by: Eli Ma <eli@patch.sh>
Signed-off-by: Quanyi Ma <eli@patch.sh>
Contributor
There was a problem hiding this comment.
Pull request overview
This PR adjusts Libra’s case-handling on case-insensitive filesystems so that case-only renames of already-tracked paths are treated as aliases (same underlying filesystem entry) instead of triggering false LBR-CASE-001 collisions during libra add, and so status/untracked scanning stops reporting the renamed directory as ?? Slides/.
Changes:
- Add a “same underlying entry” alias check (
is_same_file_case_alias) and use it to suppress false case-fold collisions duringlibra add. - Update
statusand untracked scanning to treat case-renamed tracked paths as tracked aliases (preventing?? Slides/from persisting). - Add integration + unit tests covering the directory-alias add flow and the alias identity requirement.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/command/case_handling_test.rs | Adds an integration test that reproduces the case-renamed tracked directory scenario and asserts correct staging/status JSON behavior. |
| src/utils/path_case.rs | Introduces is_same_file_case_alias and a unit test ensuring aliasing requires same-file identity (not just fold equality). |
| src/command/add.rs | Computes ignore_case once, threads a match context through pathspec logic, and suppresses false collisions when the candidate is a same-entry case alias. |
| src/command/status.rs | Suppresses “new/untracked” classification for worktree entries that are case-only aliases of tracked paths. |
| src/command/status_untracked.rs | Passes tracked-path context into file scanning so case-only aliases are treated as tracked during untracked enumeration. |
| src/command/status_untracked_paths.rs | Adds case-fold-aware tracked descendant detection and same-entry alias checks to avoid collapsing/reporting case-renamed tracked dirs as untracked. |
…07-07) Signed-off-by: Eli Ma <eli@patch.sh>
Signed-off-by: Eli Ma <eli@patch.sh>
Signed-off-by: Quanyi Ma <eli@patch.sh>
Signed-off-by: Eli Ma <eli@patch.sh>
Signed-off-by: Quanyi Ma <eli@patch.sh>
Contributor
Author
|
@codex review |
Contributor
Author
|
@claude review |
Signed-off-by: Eli Ma <eli@patch.sh>
Signed-off-by: Eli Ma <eli@patch.sh>
| "vitest": "^4.1.6" | ||
| }, | ||
| "packageManager": "pnpm@11.1.0", | ||
| "packageManager": "pnpm@11.10.0", |
Comment on lines
+1256
to
+1263
| fn path_starts_with_casefold(path: &Path, parent: &Path) -> bool { | ||
| let mut path_components = path.components(); | ||
| for parent_component in parent.components() { | ||
| let Some(path_component) = path_components.next() else { | ||
| return false; | ||
| }; | ||
| let path_key = crate::utils::path_case::fold_path_key( | ||
| path_component.as_os_str().to_string_lossy().as_ref(), |
Comment on lines
+86
to
+93
| fn path_starts_with_casefold(path: &Path, parent: &Path) -> bool { | ||
| let mut path_components = path.components(); | ||
| for parent_component in parent.components() { | ||
| let Some(path_component) = path_components.next() else { | ||
| return false; | ||
| }; | ||
| let path_key = crate::utils::path_case::fold_path_key( | ||
| path_component.as_os_str().to_string_lossy().as_ref(), |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
libra addinstead of reporting a falseLBR-CASE-001collision?? Slides/Root Cause
On case-insensitive filesystems, a directory renamed from tracked
slides/...to on-diskSlides/...resolves to the same files.libra add Slidesmatched those working-tree entries, but the add collision guard compared only folded path strings against the index and treatedSlides/...as a new case-fold twin of trackedslides/....Validation
cargo +nightly fmt --all --checkLIBRA_SKIP_WEB_BUILD=1 cargo test --test command_test case_handling_test -- --nocaptureLIBRA_SKIP_WEB_BUILD=1 cargo test --test command_test add_test:: -- --nocaptureLIBRA_SKIP_WEB_BUILD=1 cargo test --test command_test status_test:: -- --nocaptureLIBRA_SKIP_WEB_BUILD=1 cargo test --test command_test status_json_test:: -- --nocaptureLIBRA_SKIP_WEB_BUILD=1 cargo test path_case::tests::same_file_case_alias_requires_same_entry -- --nocaptureLIBRA_SKIP_WEB_BUILD=1 cargo clippy --all-targets --all-features -- -D warningsslides/a.txt, renamed directory toSlides, thenlibra add Slidesstagedslides/a.txtas modified andstatus --shortno longer reported?? Slides/