fix(core): make case-only renames stick on case-insensitive filesystems - #1362
Merged
Conversation
After #1002 a case-only move (config.md -> Config.md) is accepted and the row updated, but materialization writes through temp.replace(target), which on APFS/NTFS keeps the existing directory entry's old casing. The next scan misses the new spelling byte-wise, treats the old one as a move back, and rewrites the row — the tool reports success and nothing changes, while the permalink churns config -> config-1 -> config because the move's collision check counted the entity itself. - In the local cleanup path that already recognizes the same-inode alias, rename the directory entry to the accepted casing so disk agrees with the row and the scan is a no-op. - resolve_permalink takes the row being re-resolved (current_file_path) and no longer suffixes a permalink that row already owns; moves and scan-detected moves pass it. Fixes #1281 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017STCpbNsYjZgUdftxgEAZ4 Signed-off-by: phernandez <paul@basicmachines.co>
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
Member
Author
|
@codex review |
…ure uses Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017STCpbNsYjZgUdftxgEAZ4 Signed-off-by: phernandez <paul@basicmachines.co>
Member
Author
|
@codex review |
|
Codex Review: Didn't find any major issues. 👍 Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
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
1 participant
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.
Fixes #1281.
What actually happens today (re-checked on
main)The 0.22.1 "already exists" rejection is gone since #1002's samefile guards, but the rename still doesn't land:
move_note docs/config.md → docs/Config.mdreturns 202 and the row becomesdocs/Config.md.temp_path.replace(target); on APFS/NTFS that replaces the bytes through the existing case-variant entry and keeps the old casing — disk staysconfig.md.config.mdbyte-wise, checksum move-detection rewrites the row back todocs/config.md, and the permalink churnsconfig → config-1 → configbecause the move's collision check counts the entity itself.No duplicate rows anymore (those came from the pre-#1002 flow); the reporter's duplicates were on 0.22.1.
Fix
InlineNoteFileDeleteEnqueueralready recognizes the same-inode alias; when the two spellings differ only by case it now renames the directory entry to the accepted casing (FileService.move_file→Path.rename). On a case-sensitive filesystem the alias check only passes for hard links, where a rename onto itself is a POSIX no-op — the existing hard-link P0 test still passes.resolve_permalinktakescurrent_file_path(the row being re-resolved) and does not suffix a permalink that row already owns.prepare_move_entity_contentand the scan-detected move planner pass it; create paths are unchanged.Tests
test_inline_delete_adopts_accepted_casing_for_case_only_rename— on a case-insensitive filesystem the entry ends up spelledConfig.mdwith the bytes intact (skips on case-sensitive FS).test_resolve_permalink_keeps_the_permalink_the_entity_already_owns—docs/Config.mdresolves todocs/config-1without the hint and todocs/configwith it.ruff/tyclean.Out of scope, as discussed on the issue: directory case-only renames and empty-dir cleanup.
🤖 Generated with Claude Code
https://claude.ai/code/session_017STCpbNsYjZgUdftxgEAZ4