Fix but pr new for repository names containing ".git" (#15302) - #15311
Open
zkasuran wants to merge 1 commit into
Open
Fix but pr new for repository names containing ".git" (#15302)#15311zkasuran wants to merge 1 commit into
zkasuran wants to merge 1 commit into
Conversation
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.
🧢 Changes
but pr newfailed with HTTP 404 ("Failed to list open pull requests") for anyGitHub repo whose name contains the substring
.git, for example a*.github.iopages repo cloned over its normal.gitURL.This strips a single trailing
.gitfrom the remote URL inderive_forge_repo_info(crates/but-forge/src/lib.rs) before it is parsed, sothe repository name is kept intact. One line of code plus a comment and a
table-driven regression test.
☕️ Reasoning
derive_forge_repo_infoparses the remote URL into(owner, repo)withgit-url-parse0.6.0. When the URL ends in.git, that crate reads the reposegment with
take_until(".git"), which stops at the first.gitsubstring, not the terminal suffix. For the path
org/org.github.io.gititstops inside
.githuband returnsrepo = "org". The value then flows throughlist_forge_reviewstobut_github'slist_open_pulls, which issuesGET /repos/{owner}/{repo}/pulls. With the corrupted pair that becomesGET /repos/org/org/pulls, which 404s.The bug is inside the dependency and cannot be edited there, so the fix
normalizes the input at our boundary. Once the URL no longer ends in
.git, theparser takes its other branch (
is_not("/")), which reads the repo name up tothe next slash and keeps
org.github.iowhole.Why
strip_suffixspecifically:.gitclone suffix GitHub appends.
str::replace(".git", "")would delete the.gitinside.githuband recreate the bug..git,so a trailing
.gitis always the clone suffix.string before parsing.
The same
(owner, repo)feeds the web base URL and the commit, PR and comparelinks, so those are repaired too.
🎫 Affected issues
Fixes: #15302
✅ Verification
Run in
crates/but-forge:cargo test -p but-forgewith the fix: 104 passed, 0 failed.103 passed, 1 failed, failing exactly on
repo for git@github.com:org/org.github.io.git(left: "org",right: "org.github.io"). This confirms the test detects the real defect andthe one line is what fixes it.
cargo clippy -p but-forge --all-targets: 0 warnings.cargo fmt -- --check: clean, no diff.The new test
repo_name_containing_dotgit_is_parsed_correctlyis table-drivenover nine URL cases: SSH and HTTPS, each plain and with a trailing
.git, plusthe
*.github.iofamily that carries.gitas a substring, with and withoutthe trailing suffix.
AI assistance (Claude, Anthropic) was used in developing this change. The
diagnosis, design, review and verification were done by the author. Verified
locally before submitting:
cargo test -p but-forge(104 passed, plus the newtest fails 1/104 with the fix reverted),
cargo clippy -p but-forge --all-targets(0 warnings) andcargo fmt -- --check(clean).