fix: use global maximum for branch numbering to prevent collisions #1237
Conversation
The check_existing_branches (bash) and Get-NextBranchNumber (PowerShell) functions were only looking for branches/specs matching the SAME short name when determining the next feature number. This caused collisions where multiple features could be assigned the same number if they had different short names. For example, if feature 023-ci-optimization existed, creating a new feature with a different short name would incorrectly use 001 instead of 024. This fix changes both functions to: 1. Use get_highest_from_branches() / Get-HighestNumberFromBranches to find the highest number across ALL branches globally 2. Use get_highest_from_specs() / Get-HighestNumberFromSpecs to find the highest number across ALL spec directories globally 3. Return the maximum of both + 1 The helper functions already existed but were not being used. This fix properly utilizes them to ensure features are numbered sequentially regardless of their short names. Issue: Branch number collisions when creating features with different names Impact: Prevents multiple features from sharing the same number prefix
There was a problem hiding this comment.
Pull request overview
This PR fixes a critical branch numbering collision bug by changing the Get-NextBranchNumber (PowerShell) and check_existing_branches (Bash) functions to use global maximum branch numbers instead of only checking branches matching the same short name. Previously, creating features with different short names could result in duplicate branch numbers (e.g., if 023-ci-optimization existed, a new feature with a different name would incorrectly start at 001 instead of 024).
Key changes:
- Replace local pattern matching with calls to existing helper functions (
Get-HighestNumberFromBranches/get_highest_from_branchesandGet-HighestNumberFromSpecs/get_highest_from_specs) - Calculate the next number as the global maximum across all branches and specs + 1
- Simplify the logic by removing ~40 lines of redundant pattern matching code per file
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| scripts/powershell/create-new-feature.ps1 | Replaced local branch matching logic with calls to global helper functions Get-HighestNumberFromBranches and Get-HighestNumberFromSpecs, using [Math]::Max() to find the overall maximum |
| scripts/bash/create-new-feature.sh | Replaced local branch matching logic with calls to global helper functions get_highest_from_branches and get_highest_from_specs, using conditional logic to find the overall maximum |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
The check_existing_branches (bash) and Get-NextBranchNumber (PowerShell) functions no longer use the short_name parameter since they now find the global maximum across ALL features. This commit: 1. Removes the unused parameter from function signatures 2. Updates all call sites to not pass the parameter This prevents the scripts from failing when the function is called with the wrong number of arguments.
|
@localden I've been running into this bug quite often |
When --number 027 was passed, printf '%03d' interpreted it as octal, converting 027 (octal) to 23 (decimal). Now forces base-10 with 10# prefix. Bug: User passes --number 027, gets feature 023 instead of 027 Root cause: printf %03d treats leading zeros as octal notation Fix: Use $((10#$BRANCH_NUMBER)) to force decimal interpretation Example: - Before: --number 027 → octal 027 → decimal 23 → feature 023 - After: --number 027 → decimal 27 → feature 027 Note: PowerShell version does not have this bug because [int] type conversion in PowerShell does not treat leading zeros as octal.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 2 out of 2 changed files in this pull request and generated no new comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
1 participant
The check_existing_branches (bash) and Get-NextBranchNumber (PowerShell) functions were only looking for branches/specs matching the SAME short name when determining the next feature number. This caused collisions where multiple features could be assigned the same number if they had different short names.
For example, if feature 023-ci-optimization existed, creating a new feature with a different short name would incorrectly use 001 instead of 024.
This fix changes both functions to:
The helper functions already existed but were not being used. This fix properly utilizes them to ensure features are numbered sequentially regardless of their short names.
Additional Fix: Octal Interpretation Bug
Bug: When
--number 027was passed to the bash script,printf '%03d'interpreted it as octal, converting 027 (octal) to 23 (decimal).Root Cause: The bash
printf %03dcommand treats strings with leading zeros as octal notation.Fix: Added
$((10#$BRANCH_NUMBER))to force base-10 interpretation before formatting.Example:
--number 027→ octal 027 → decimal 23 → feature 023--number 027→ decimal 27 → feature 027Note: The PowerShell version does not have this bug because
[int]type conversion in PowerShell does not treat leading zeros as octal.Issues Fixed:
--numberwith leading zerosImpact: Ensures features are numbered correctly and sequentially