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.

[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.

[Copilot]

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."

[mnriem]

Please resolve conflicts

[github-actions]

This pull request has been automatically marked as stale because it has not had any activity for 150 days. It will be closed in 30 days if no further activity occurs.