fix: handle loose ref path-prefix collisions#2645
Merged
Sebastian Thiel (Byron) merged 2 commits intoJun 16, 2026
Merged
Conversation
Sebastian Thiel (Byron)
force-pushed
the
try-find-reference-path-prefix-collision
branch
2 times, most recently
from
June 15, 2026 02:47
b7324d7 to
7ea1274
Compare
There was a problem hiding this comment.
Pull request overview
Fixes loose-ref lookup behavior when a reference file exists that is a path prefix of the requested ref (e.g. refs/heads/A exists as a file while looking up refs/heads/A/new), ensuring try_find_reference()/try_find() return Ok(None) and don’t incorrectly fall back to packed-refs.
Changes:
- Update
gix-reffile-store lookup to treatNotADirectoryas “not found” for that lookup and to avoid packed-ref fallback in prefix-collision scenarios. - Add regression tests in
gix-reffor loose + packed-overlay behavior around prefix collisions (with Windows-specific conditional coverage). - Add a
gixregression test assertingrepo.try_find_reference()returnsNonefor the collision case.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
gix-ref/src/store/file/find.rs |
Adjusts loose ref lookup error handling and adds prefix-collision detection to prevent packed fallback. |
gix-ref/tests/refs/file/store/find.rs |
Adds regression coverage for prefix collisions (including packed-overlay behavior on non-Windows). |
gix/tests/gix/repository/reference.rs |
Adds repository-level regression test for try_find_reference() collision behavior. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
The GitButler branch creation flow reported that
`repo.try_find_reference("refs/heads/A/new")` could fail with a low-level
`NotADirectory` error when `refs/heads/A` already exists as a loose ref. That
lookup is asking whether the longer ref exists; the path-prefix collision
matters to creation/update code, but find should report absence for that
candidate.
Git reference: `refs/refs-internal.h` documents `ENOTDIR` as the case where a
ref prefix is not a directory, alongside `ENOENT` for non-existing refs. For
lookup, both mean the requested ref candidate was not found.
Co-authored-by: Sebastian Thiel <[email protected]>
Sebastian Thiel (Byron)
force-pushed
the
try-find-reference-path-prefix-collision
branch
from
June 16, 2026 02:43
7ea1274 to
9f432ef
Compare
Sebastian Thiel (Byron)
marked this pull request as ready for review
June 16, 2026 02:43
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 9f432ef509
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "Codex (@codex) review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "Codex (@codex) address that feedback".
Sebastian Thiel (Byron)
force-pushed
the
try-find-reference-path-prefix-collision
branch
from
June 16, 2026 03:09
c0cf531 to
84400b8
Compare
On Windows, opening a path below an existing loose ref file can surface as NotFound or PermissionDenied and ref_contents() previously normalized those cases to Ok(None). That allowed callers to continue to packed refs even though the loose namespace path was blocked. Teach ref_contents() to detect file prefixes before normalizing those Windows errors, and return NotADirectory for that specific collision so existing lookup handling returns Ok(None) without consulting packed refs. Co-authored-by: Sebastian Thiel <[email protected]>
Sebastian Thiel (Byron)
force-pushed
the
try-find-reference-path-prefix-collision
branch
from
June 16, 2026 04:07
27c0ba6 to
c9191ab
Compare
Sebastian Thiel (Byron)
enabled auto-merge
June 16, 2026 04:08
Sebastian Thiel (Byron)
deleted the
try-find-reference-path-prefix-collision
branch
June 16, 2026 06:51
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.
Tasks
repo.try_find_reference()returnsOk(None)for that case.ref_contents()normalization case without falling through to packed refs.Created by Codex on behalf of Byron. Byron will review before this is ready to merge.
Reported issue
$issue-full-auto Look at the fix in gitbutlerapp/gitbutler#14153 and reproduce the underlying issue in
gixorgix-ref(or both). Then make sure thatrepo.try_find_reference()returnsOk(None)in that moment.Context
GitButler PR: gitbutlerapp/gitbutler#14153
The issue is a loose reference path-prefix collision:
refs/heads/Aexists as a file, and lookup asks forrefs/heads/A/new.Git baseline:
/Users/byron/dev/github.com/git/gitat7760f83b59;refs/refs-internal.hdocumentsENOTDIRas “ref prefix is not a directory”.Changes
NotADirectoryas absence for the current loose ref lookup candidate ingix-reffind.ref_contents()preserve Windows path-prefix collisions asNotADirectoryinstead of normalizing them to ordinary absence.NotFoundorPermissionDenied.gix-refandgixregression tests, including packed-overlay coverage for the collision behavior.NotADirectoryerror now surfaced on Windows too.Validation
cargo fmt --checkcargo check -p gix-ref --target aarch64-pc-windows-msvc --features sha1cargo test -p gix-ref --test refs file::transaction::prepare_and_commit::delete::rename_a_to_a_slash_b_in_one_transaction -- --exactcargo test -p gix-ref --test refs file::store::find::loose::prefix_file_collision_is_not_found -- --exactcargo test -p gix --test gix try_find_reference_with_existing_ref_as_path_prefix_returns_noneHead validated locally:
27c0ba6b4f6ab96300cf5f67764a6f1ac65b6047