Skip to content
Open
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
44 changes: 44 additions & 0 deletions scripts/bash/create-new-feature.sh
Original file line number Diff line number Diff line change
Expand Up @@ -251,6 +251,7 @@ if [ "$USE_TIMESTAMP" = true ]; then
else
# Determine branch number
if [ -z "$BRANCH_NUMBER" ]; then
# No manual number provided -- auto-detect
if [ "$HAS_GIT" = true ]; then
# Check existing branches on remotes
BRANCH_NUMBER=$(check_existing_branches "$SPECS_DIR")
Expand All @@ -259,6 +260,49 @@ else
HIGHEST=$(get_highest_from_specs "$SPECS_DIR")
BRANCH_NUMBER=$((HIGHEST + 1))
fi
elif [ "$ALLOW_EXISTING" = false ]; then
# Manual number provided -- validate it is not already in use
# (skip when --allow-existing-branch, as the caller intentionally targets an existing branch)
MANUAL_NUM=$((10#$BRANCH_NUMBER))
MANUAL_NUM_PADDED=$(printf "%03d" "$MANUAL_NUM")
NUMBER_IN_USE=false

# Check specs directory for collision
if [ -d "$SPECS_DIR" ]; then
for dir in "$SPECS_DIR"/*/; do
[ -d "$dir" ] || continue
dirname=$(basename "$dir")
if echo "$dirname" | grep -q "^${MANUAL_NUM_PADDED}-"; then
NUMBER_IN_USE=true
break
fi
done
fi

# Check git branches for collision (fetch first to catch remote-only branches)
if [ "$NUMBER_IN_USE" = false ] && [ "$HAS_GIT" = true ]; then
git fetch --all --prune 2>/dev/null || true
branches=$(git branch -a 2>/dev/null || echo "")
if [ -n "$branches" ]; then
while IFS= read -r branch; do
clean_branch=$(echo "$branch" | sed 's/^[* ]*//; s|^remotes/[^/]*/||')
if echo "$clean_branch" | grep -q "^${MANUAL_NUM_PADDED}-"; then
NUMBER_IN_USE=true
break
fi
done <<< "$branches"
fi
fi

if [ "$NUMBER_IN_USE" = true ]; then
>&2 echo "Warning: --number $BRANCH_NUMBER conflicts with existing branch/spec (${MANUAL_NUM_PADDED}-*). Auto-detecting next available number."
if [ "$HAS_GIT" = true ]; then
BRANCH_NUMBER=$(check_existing_branches "$SPECS_DIR")
Comment on lines +305 to +308
Copy link

Copilot AI Mar 31, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When a collision is detected, this branch re-calls check_existing_branches, which itself runs git fetch --all --prune. Since the collision check above already fetched and scanned git branch -a, this can cause a redundant second fetch (extra latency / potential network hang) in the exact failure mode this PR is trying to harden. Consider refactoring so the conflict path reuses the already-fetched branch/spec info (or add a way for check_existing_branches to skip fetching when the caller already fetched).

Suggested change
if [ "$NUMBER_IN_USE" = true ]; then
>&2 echo "Warning: --number $BRANCH_NUMBER conflicts with existing branch/spec (${MANUAL_NUM_PADDED}-*). Auto-detecting next available number."
if [ "$HAS_GIT" = true ]; then
BRANCH_NUMBER=$(check_existing_branches "$SPECS_DIR")
compute_next_branch_number_from_cache() {
# Compute the next available branch number using existing specs and an
# already-fetched list of git branches (to avoid a redundant git fetch).
local specs_dir="$1"
local branches_input="$2"
local highest max_num
# Start from the highest number found in the specs directory.
highest=$(get_highest_from_specs "$specs_dir")
max_num="$highest"
# If we have a cached branch list, incorporate it to find the true max.
if [ -n "$branches_input" ]; then
while IFS= read -r branch; do
# Normalize branch name: strip leading markers and remote prefix.
local clean_branch
clean_branch=$(echo "$branch" | sed 's/^[* ]*//; s|^remotes/[^/]*/||')
# Extract numeric prefix (e.g., "012" from "012-something").
local num_part
num_part=${clean_branch%%-*}
# Ensure prefix is numeric before comparing.
if echo "$num_part" | grep -Eq '^[0-9]+$'; then
local num_value
num_value=$((10#$num_part))
if [ "$num_value" -gt "$max_num" ]; then
max_num="$num_value"
fi
fi
done <<< "$branches_input"
fi
echo $((max_num + 1))
}
if [ "$NUMBER_IN_USE" = true ]; then
>&2 echo "Warning: --number $BRANCH_NUMBER conflicts with existing branch/spec (${MANUAL_NUM_PADDED}-*). Auto-detecting next available number."
if [ "$HAS_GIT" = true ]; then
if [ -n "${branches:-}" ]; then
# Reuse the already-fetched branch list to avoid a redundant git fetch.
BRANCH_NUMBER=$(compute_next_branch_number_from_cache "$SPECS_DIR" "$branches")
else
# No cached branches available; fall back to the existing behavior.
BRANCH_NUMBER=$(check_existing_branches "$SPECS_DIR")
fi

Copilot uses AI. Check for mistakes.
else
HIGHEST=$(get_highest_from_specs "$SPECS_DIR")
BRANCH_NUMBER=$((HIGHEST + 1))
fi
fi
fi

# Force base-10 interpretation to prevent octal conversion (e.g., 010 → 8 in octal, but should be 10 in decimal)
Expand Down
44 changes: 44 additions & 0 deletions scripts/powershell/create-new-feature.ps1
Original file line number Diff line number Diff line change
Expand Up @@ -203,13 +203,57 @@ if ($Timestamp) {
} else {
# Determine branch number
if ($Number -eq 0) {
# No manual number provided -- auto-detect
if ($hasGit) {
# Check existing branches on remotes
$Number = Get-NextBranchNumber -SpecsDir $specsDir
} else {
# Fall back to local directory check
$Number = (Get-HighestNumberFromSpecs -SpecsDir $specsDir) + 1
}
} elseif (-not $AllowExistingBranch) {
# Manual number provided -- validate it is not already in use
# (skip when -AllowExistingBranch, as the caller intentionally targets an existing branch)
$manualNumPadded = '{0:000}' -f $Number
$numberInUse = $false

# Check specs directory for collision
if (Test-Path $specsDir) {
foreach ($dir in (Get-ChildItem -Path $specsDir -Directory)) {
if ($dir.Name -match "^$manualNumPadded-") {
$numberInUse = $true
break
}
}
}

# Check git branches for collision (fetch first to catch remote-only branches)
if (-not $numberInUse -and $hasGit) {
try {
git fetch --all --prune 2>$null | Out-Null
$branches = git branch -a 2>$null
if ($LASTEXITCODE -eq 0 -and $branches) {
foreach ($branch in $branches) {
$cleanBranch = $branch.Trim() -replace '^\*?\s*', '' -replace '^remotes/[^/]+/', ''
if ($cleanBranch -match "^$manualNumPadded-") {
$numberInUse = $true
break
}
}
}
} catch {
Write-Verbose "Could not check Git branches: $_"
}
}

if ($numberInUse) {
Write-Warning "-Number $Number conflicts with existing branch/spec ($manualNumPadded-*). Auto-detecting next available number."
Copy link

Copilot AI Apr 1, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

New collision-warning message lacks the [specify] prefix used by other warnings in this script (e.g., -Number ignored with -Timestamp, branch-name truncation). For consistent output and easier grepping, consider including [specify] Warning: here too.

Suggested change
Write-Warning "-Number $Number conflicts with existing branch/spec ($manualNumPadded-*). Auto-detecting next available number."
Write-Warning "[specify] Warning: -Number $Number conflicts with existing branch/spec ($manualNumPadded-*). Auto-detecting next available number."

Copilot uses AI. Check for mistakes.
if ($hasGit) {
$Number = Get-NextBranchNumber -SpecsDir $specsDir
} else {
$Number = (Get-HighestNumberFromSpecs -SpecsDir $specsDir) + 1
Comment on lines +255 to +260
Copy link

Copilot AI Mar 31, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In the collision path, the script has already executed git fetch --all --prune (above) to validate $Number, but then calls Get-NextBranchNumber, which performs another git fetch --all --prune. This duplicates network work and can add noticeable latency/hangs when the user hits a collision (the scenario this validation is targeting). Consider reusing the already-fetched branch/spec scan results or adding a -SkipFetch option to Get-NextBranchNumber for callers that already fetched.

Copilot uses AI. Check for mistakes.
}
}
}

$featureNum = ('{0:000}' -f $Number)
Expand Down
9 changes: 5 additions & 4 deletions tests/test_timestamp_branches.py
Original file line number Diff line number Diff line change
Expand Up @@ -326,8 +326,8 @@ def test_allow_existing_creates_spec_dir(self, git_repo: Path):
assert (git_repo / "specs" / "006-spec-dir").is_dir()
assert (git_repo / "specs" / "006-spec-dir" / "spec.md").exists()

def test_without_flag_still_errors(self, git_repo: Path):
"""T009: Verify backwards compatibility (error without flag)."""
def test_without_flag_auto_detects_on_collision(self, git_repo: Path):
"""T009: Without --allow-existing-branch, a conflicting --number auto-detects next available."""
subprocess.run(
["git", "checkout", "-b", "007-no-flag"],
cwd=git_repo, check=True, capture_output=True,
Expand All @@ -339,8 +339,9 @@ def test_without_flag_still_errors(self, git_repo: Path):
result = run_script(
git_repo, "--short-name", "no-flag", "--number", "7", "No flag feature",
)
assert result.returncode != 0, "should fail without --allow-existing-branch"
assert "already exists" in result.stderr
assert result.returncode == 0, result.stderr
assert "conflicts with existing branch/spec" in result.stderr
assert "008-no-flag" in result.stdout

def test_allow_existing_no_overwrite_spec(self, git_repo: Path):
"""T010: Pre-create spec.md with content, verify it is preserved."""
Expand Down
Loading