fix: prevent branch name duplication in URL rewriting (fixes #85) #209
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
Fixed a critical bug where branch names containing slashes would be
duplicated in GitHub URLs during pull request link validation.
## The Problem
When a branch name like "release-please/branches/main" contained the
base branch name as a substring, the greedy regex pattern would
incorrectly match and duplicate the branch path:
- Bad: /blob/release-please/branches/release-please/branches/main/file.md
- Good: /blob/release-please/branches/main/file.md
The root cause was the regex pattern `(/.*/)(main)/(.*)` which greedily
matched `/blob/release-please/branches/` as the first capture group,
then when replacing with the full head ref, it duplicated the path.
## The Fix
1. **More specific regex**: Changed from `(/.*/)(${GITHUB_BASE_REF})/(.*)`
to `/(blob|tree)/(${escapedBaseRef})/(.*)` to only match `/blob/` or
`/tree/` specifically, preventing greedy matching of branch name parts
2. **Regex escaping**: Added proper escaping for special characters in
branch names (dots, asterisks, etc.) so patterns like `v1.0.0` work
3. **Proper capture groups**: Preserves whether it's a blob or tree URL
in the replacement
## Tests Added (5 new comprehensive tests)
- Branch names with slashes (release-please/branches/main)
- Branch names with multiple slashes (feature/deep/nested/branch)
- URLs already on head branch (regression test for exact bug scenario)
- Branch names with special regex characters (v1.0.0)
- Tree URLs in addition to blob URLs
All 20 tests passing with 98.68% coverage.
Fixes #85
🤖 Generated with [Claude Code](https://claude.com/claude-code)
Co-Authored-By: Claude <[email protected]>
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
Fixes a critical bug where branch names containing slashes would be duplicated in GitHub URLs during pull request link validation.
The Problem
When a branch name like
release-please/branches/maincontained the base branch name as a substring (e.g.,main), the greedy regex pattern would incorrectly match and duplicate the branch path:Before (broken):
After (correct):
Root Cause
The original regex pattern
(/.*/)(main)/(.*)would:/blob/release-please/branches/as capture group $1mainas capture group $2release-please/branches/main→ duplication!The Fix
1. More Specific Regex Pattern
Changed from
(/.*/)(${GITHUB_BASE_REF})/(.*)to/(blob|tree)/(${escapedBaseRef})/(.*)This prevents the greedy
(/.*/)from capturing parts of branch names by only matching/blob/or/tree/specifically.2. Regex Character Escaping
Added proper escaping for special regex characters in branch names (dots, asterisks, etc.) so branch names like
v1.0.0work correctly.3. Proper Capture Groups
Preserves whether the URL uses
blobortreein the replacement.Tests Added
Added 5 comprehensive new tests to prevent regression:
✅ Branch names with slashes (
release-please/branches/main)✅ Branch names with multiple slashes (
feature/deep/nested/branch)✅ URLs already on head branch (exact bug scenario - should not rewrite)
✅ Branch names with special regex characters (
v1.0.0)✅ Tree URLs in addition to blob URLs
Test Results
Changes
src/action.js:78-90- Updated regex pattern with escaping and specific matchingtest/test.js- Added 5 new comprehensive test casestest/fixtures/*- Added test fixtures for slashed branch scenariosdist/*- Rebuilt distribution bundleFixes #85
🤖 Generated with Claude Code