fix(scripts): validate --number flag against existing branches/specs

[fsilvaortiz]

Summary

Fixes the intermittent branch numbering reset reported in #1744 (see comment by @echarrod).

PR #1757 fixed the specify.md prompt to tell the AI not to pass --number, but AI assistants intermittently ignore that instruction, causing new features to get 001 instead of the next available number.

This PR adds script-level validation as defense in depth:

• When --number is passed, the script checks specs directories and git branches (after fetch) for collisions
• If the number is already in use, it warns on stderr and auto-detects the next available number
• Skips validation when --allow-existing-branch is set (intentional reuse)
• Applied symmetrically to both Bash and PowerShell scripts

Test plan

• All 942 existing tests pass
• Updated test_without_flag_still_errors → test_without_flag_auto_detects_on_collision to reflect new behavior
• Verified collision detection against specs directories
• Verified collision detection against git branches (local + remote)
• Verified --allow-existing-branch bypasses validation correctly
• Bash syntax validated

🤖 Generated with Claude Code

Co-Authored-By: Claude Opus 4.6 (1M context) noreply@anthropic.com

[Copilot]

Pull request overview

Adds defense-in-depth in the feature-creation scripts to prevent branch/spec number collisions when callers manually pass a number, addressing intermittent “reset to 001” behavior when AI assistants ignore guidance and supply --number.

Changes:

• Bash: when --number is provided (and --allow-existing-branch is not), detect collisions against existing spec directories and git branches (after fetch) and auto-select the next available number.
• PowerShell: implement the same collision validation + auto-detect behavior for -Number, with -AllowExistingBranch bypass.
• Tests: update the existing “no allow-existing” test to assert the new auto-detect-on-collision behavior.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.

File	Description
scripts/bash/create-new-feature.sh	Validates manually provided --number against existing spec dirs/branches; auto-detects next number on collision.
scripts/powershell/create-new-feature.ps1	Mirrors the same manual number collision validation and auto-detect logic for PowerShell users.
tests/test_timestamp_branches.py	Updates the relevant regression test to reflect the new collision-handling behavior.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[mnriem]

Please address Copilot feedback. If not applicable, please explain why

[fsilvaortiz]

Addressed Copilot suggestion in e602275. Added a static guard test (test_powershell_has_number_collision_validation) that verifies the PS script contains the collision detection logic — matching the existing pattern used for PowerShell tests in this file, since pwsh availability is not guaranteed in CI.

[Copilot]

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[fsilvaortiz]

Re: Copilot comment about --number 0 — already addressed in 59153f7. The validation was moved before the if/elif block and now rejects 0 explicitly (-lt 1 check). Test coverage added in 6cf7bd1 (test_rejects_zero).

[Copilot]

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[mnriem]

One more small thing according to Copilot

[Copilot]

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[mnriem]

@fsilvaortiz So more Copilot feedback to address

[fsilvaortiz]

Addressed the double-fetch comments in 135163b (bash) and a8e710e (PowerShell): removed git fetch --all --prune from the collision check block. If a collision is detected, check_existing_branches/Get-NextBranchNumber already fetch internally during auto-detect fallback. The collision check uses git branch -a with whatever refs are currently available.

[Copilot]

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

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]

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.

[fsilvaortiz]

Re: double fetch optimization suggestions (comments on bash line 308 and PS line 260):

Acknowledged — the collision-then-auto-detect path does fetch twice. This is intentional and accepted as a trade-off:

1. Correctness first: The collision check needs fresh remote refs to avoid false negatives. The auto-detect fallback (check_existing_branches/Get-NextBranchNumber) fetches internally and we cannot skip that without modifying existing function signatures.

2. Rare path: The double fetch only happens when --number is passed AND collides — the exact scenario this PR defends against. In normal operation (no --number flag), the existing single-fetch path is unchanged.

3. Scope: Introducing a compute_next_branch_number_from_cache() function or adding a -SkipFetch parameter to Get-NextBranchNumber would expand this PR beyond its fix scope and modify stable interfaces. If the maintainer feels this optimization is valuable, it can be a follow-up PR.

The first comment (PS missing fetch) was already addressed in 29302c2.