From 12756f56f0c3720f4931e5fcac8b23633d530b17 Mon Sep 17 00:00:00 2001 From: Daniel Graham Date: Fri, 13 Mar 2026 12:54:21 -0400 Subject: [PATCH] fix: auto-correct conflicting feature prefixes Treat an explicit feature number as a preference when an existing spec directory already uses that prefix. Advance to the next available spec prefix and warn without fetching or scanning git branches. Keep Bash, PowerShell, and Python variants aligned. Preserve 64-bit numbering, timestamp mode, dry-run output, matching-file behavior, and exact-directory reuse through the allow-existing option. Assisted-by: Codex (model: GPT-5, autonomous) --- scripts/bash/create-new-feature.sh | 82 +++++++-- scripts/powershell/create-new-feature.ps1 | 82 +++++++-- scripts/python/create_new_feature.py | 79 ++++++++- .../test_create_new_feature_python_parity.py | 160 ++++++++++++++++-- tests/test_timestamp_branches.py | 117 ++++++++++++- 5 files changed, 462 insertions(+), 58 deletions(-) diff --git a/scripts/bash/create-new-feature.sh b/scripts/bash/create-new-feature.sh index e40e8b257b..c1b189dc08 100644 --- a/scripts/bash/create-new-feature.sh +++ b/scripts/bash/create-new-feature.sh @@ -8,6 +8,7 @@ ALLOW_EXISTING=false SHORT_NAME="" BRANCH_NUMBER="" USE_TIMESTAMP=false +NUMBER_EXPLICIT=false ARGS=() i=1 while [ $i -le $# ]; do @@ -48,6 +49,9 @@ while [ $i -le $# ]; do exit 1 fi BRANCH_NUMBER="$next_arg" + if [ -n "$BRANCH_NUMBER" ]; then + NUMBER_EXPLICIT=true + fi ;; --timestamp) USE_TIMESTAMP=true @@ -60,7 +64,7 @@ while [ $i -le $# ]; do echo " --dry-run Compute feature name and paths without creating directories or files" echo " --allow-existing-branch Reuse an existing feature directory if it already exists" echo " --short-name Provide a custom short name (2-4 words) for the feature" - echo " --number N Specify branch number manually (overrides auto-detection)" + echo " --number N Prefer a feature number (auto-corrected if its specs prefix exists)" echo " --timestamp Use timestamp prefix (YYYYMMDD-HHMMSS) instead of sequential numbering" echo " --help, -h Show this help message" echo "" @@ -91,6 +95,7 @@ if [ -z "$FEATURE_DESCRIPTION" ]; then fi MAX_FEATURE_NUMBER=9223372036854775807 +MAX_BRANCH_LENGTH=244 is_feature_number_in_range() { local value="$1" @@ -128,12 +133,40 @@ get_highest_from_specs() { echo "$highest" } +# Return success when a spec directory owns the given numeric prefix. +spec_prefix_exists() { + local specs_dir="$1" + local feature_num="$2" + + for spec_path in "$specs_dir/${feature_num}-"*; do + [ -d "$spec_path" ] && return 0 + done + return 1 +} + # Function to clean and format a branch name clean_branch_name() { local name="$1" echo "$name" | tr '[:upper:]' '[:lower:]' | sed 's/[^a-z0-9]/-/g' | sed 's/-\+/-/g' | sed 's/^-//' | sed 's/-$//' } +# Fit a feature prefix and suffix within GitHub's branch-name limit. +fit_branch_name() { + local feature_num="$1" + local branch_suffix="$2" + local branch_name="${feature_num}-${branch_suffix}" + + if [ ${#branch_name} -gt $MAX_BRANCH_LENGTH ]; then + local prefix_length=$(( ${#feature_num} + 1 )) + local max_suffix_length=$((MAX_BRANCH_LENGTH - prefix_length)) + local truncated_suffix + truncated_suffix=$(printf '%s' "$branch_suffix" | cut -c "1-$max_suffix_length" | sed 's/-$//') + branch_name="${feature_num}-${truncated_suffix}" + fi + + printf '%s' "$branch_name" +} + # Quote a value for POSIX shell reuse, byte-identical to Python's shlex.quote # so the persistence hints match the Python variant exactly (printf %q output # differs between bash versions and from shlex.quote for spaces/metachars). @@ -253,26 +286,41 @@ else # Force base-10 interpretation to prevent octal conversion (e.g., 010 → 8 in octal, but should be 10 in decimal) FEATURE_NUM=$(printf "%03d" "$((10#$BRANCH_NUMBER))") - BRANCH_NAME="${FEATURE_NUM}-${BRANCH_SUFFIX}" -fi -# GitHub enforces a 244-byte limit on branch names -# Validate and truncate if necessary -MAX_BRANCH_LENGTH=244 -if [ ${#BRANCH_NAME} -gt $MAX_BRANCH_LENGTH ]; then - # Calculate how much we need to trim from suffix - # Account for prefix length: timestamp (15) + hyphen (1) = 16, or sequential (3) + hyphen (1) = 4 - PREFIX_LENGTH=$(( ${#FEATURE_NUM} + 1 )) - MAX_SUFFIX_LENGTH=$((MAX_BRANCH_LENGTH - PREFIX_LENGTH)) + # Treat an explicit number as a preference when its prefix is already used + # by a feature directory. Auto-detected numbers are already conflict-free. + if [ "$NUMBER_EXPLICIT" = true ]; then + SPEC_CONFLICT=false + REQUESTED_BRANCH_NAME=$(fit_branch_name "$FEATURE_NUM" "$BRANCH_SUFFIX") + REQUESTED_DIR="$SPECS_DIR/$REQUESTED_BRANCH_NAME" + if [ "$ALLOW_EXISTING" != true ] || [ ! -d "$REQUESTED_DIR" ]; then + spec_prefix_exists "$SPECS_DIR" "$FEATURE_NUM" && SPEC_CONFLICT=true + fi - # Truncate suffix at word boundary if possible - TRUNCATED_SUFFIX=$(echo "$BRANCH_SUFFIX" | cut -c1-$MAX_SUFFIX_LENGTH) - # Remove trailing hyphen if truncation created one - TRUNCATED_SUFFIX=$(echo "$TRUNCATED_SUFFIX" | sed 's/-$//') + if [ "$SPEC_CONFLICT" = true ]; then + REQUESTED_NUM="$FEATURE_NUM" + HIGHEST=$(get_highest_from_specs "$SPECS_DIR") + BRANCH_NUMBER=$HIGHEST + while true; do + if [ "$BRANCH_NUMBER" -eq "$MAX_FEATURE_NUMBER" ]; then + echo "Error: feature number must be between 0 and $MAX_FEATURE_NUMBER, got '9223372036854775808'" >&2 + exit 1 + fi + BRANCH_NUMBER=$((BRANCH_NUMBER + 1)) + FEATURE_NUM=$(printf "%03d" "$((10#$BRANCH_NUMBER))") + spec_prefix_exists "$SPECS_DIR" "$FEATURE_NUM" || break + done + >&2 echo "[specify] Warning: --number $REQUESTED_NUM conflicts with an existing spec directory; using $FEATURE_NUM instead" + fi + fi - ORIGINAL_BRANCH_NAME="$BRANCH_NAME" - BRANCH_NAME="${FEATURE_NUM}-${TRUNCATED_SUFFIX}" +fi +# GitHub enforces a 244-byte limit on branch names +# Validate and truncate if necessary +ORIGINAL_BRANCH_NAME="${FEATURE_NUM}-${BRANCH_SUFFIX}" +BRANCH_NAME=$(fit_branch_name "$FEATURE_NUM" "$BRANCH_SUFFIX") +if [ "$BRANCH_NAME" != "$ORIGINAL_BRANCH_NAME" ]; then >&2 echo "[specify] Warning: Branch name exceeded GitHub's 244-byte limit" >&2 echo "[specify] Original: $ORIGINAL_BRANCH_NAME (${#ORIGINAL_BRANCH_NAME} bytes)" >&2 echo "[specify] Truncated to: $BRANCH_NAME (${#BRANCH_NAME} bytes)" diff --git a/scripts/powershell/create-new-feature.ps1 b/scripts/powershell/create-new-feature.ps1 index 5e9aebb291..abe70f65ed 100644 --- a/scripts/powershell/create-new-feature.ps1 +++ b/scripts/powershell/create-new-feature.ps1 @@ -14,6 +14,7 @@ param( [string[]]$FeatureDescription ) $ErrorActionPreference = 'Stop' +$maxBranchLength = 244 # Show help if requested if ($Help) { @@ -24,7 +25,7 @@ if ($Help) { Write-Host " -DryRun Compute feature name and paths without creating directories or files" Write-Host " -AllowExistingBranch Reuse an existing feature directory if it already exists" Write-Host " -ShortName Provide a custom short name (2-4 words) for the feature" - Write-Host " -Number N Specify branch number manually (overrides auto-detection)" + Write-Host " -Number N Prefer a feature number (auto-corrected if its specs prefix exists)" Write-Host " -Timestamp Use timestamp prefix (YYYYMMDD-HHMMSS) instead of sequential numbering" Write-Host " -Help Show this help message" Write-Host "" @@ -67,11 +68,44 @@ function Get-HighestNumberFromSpecs { return $highest } +function Test-SpecPrefixInUse { + param( + [string]$SpecsDir, + [string]$FeatureNum + ) + + if (-not (Test-Path -LiteralPath $SpecsDir -PathType Container)) { + return $false + } + + return $null -ne (Get-ChildItem -LiteralPath $SpecsDir -Directory -ErrorAction SilentlyContinue | + Where-Object { $_.Name -like "$FeatureNum-*" } | + Select-Object -First 1) +} + function ConvertTo-CleanBranchName { param([string]$Name) return $Name.ToLower() -replace '[^a-z0-9]', '-' -replace '-{2,}', '-' -replace '^-', '' -replace '-$', '' } + +function Get-FittedBranchName { + param( + [string]$FeatureNum, + [string]$BranchSuffix + ) + + $fittedName = "$FeatureNum-$BranchSuffix" + if ($fittedName.Length -gt $maxBranchLength) { + $prefixLength = $FeatureNum.Length + 1 + $maxSuffixLength = $maxBranchLength - $prefixLength + $truncatedSuffix = $BranchSuffix.Substring(0, [Math]::Min($BranchSuffix.Length, $maxSuffixLength)) + $truncatedSuffix = $truncatedSuffix -replace '-$', '' + $fittedName = "$FeatureNum-$truncatedSuffix" + } + + return $fittedName +} # Load common functions (includes Get-RepoRoot and Resolve-Template) . "$PSScriptRoot/common.ps1" @@ -176,26 +210,40 @@ if ($Timestamp) { } $featureNum = ('{0:000}' -f $resolvedNumber) - $branchName = "$featureNum-$branchSuffix" -} -# GitHub enforces a 244-byte limit on branch names -# Validate and truncate if necessary -$maxBranchLength = 244 -if ($branchName.Length -gt $maxBranchLength) { - # Calculate how much we need to trim from suffix - # Account for prefix length: timestamp (15) + hyphen (1) = 16, or sequential (3) + hyphen (1) = 4 - $prefixLength = $featureNum.Length + 1 - $maxSuffixLength = $maxBranchLength - $prefixLength + # Treat an explicit number as a preference when its prefix is already used + # by a feature directory. Auto-detected numbers are already conflict-free. + $specConflict = $false + if ($hasNumber -and (Test-Path -LiteralPath $specsDir -PathType Container)) { + $requestedBranchName = Get-FittedBranchName -FeatureNum $featureNum -BranchSuffix $branchSuffix + $requestedDir = Join-Path $specsDir $requestedBranchName + if (-not $AllowExistingBranch -or -not (Test-Path -LiteralPath $requestedDir -PathType Container)) { + $specConflict = Test-SpecPrefixInUse -SpecsDir $specsDir -FeatureNum $featureNum + } + } - # Truncate suffix - $truncatedSuffix = $branchSuffix.Substring(0, [Math]::Min($branchSuffix.Length, $maxSuffixLength)) - # Remove trailing hyphen if truncation created one - $truncatedSuffix = $truncatedSuffix -replace '-$', '' + if ($specConflict) { + $requestedNum = $featureNum + $highestNumber = Get-HighestNumberFromSpecs -SpecsDir $specsDir + $resolvedNumber = $highestNumber + do { + if ($resolvedNumber -eq [long]::MaxValue) { + Write-Error "Error: feature number must be between 0 and $([long]::MaxValue), got '9223372036854775808'" + exit 1 + } + $resolvedNumber++ + $featureNum = ('{0:000}' -f $resolvedNumber) + } while (Test-SpecPrefixInUse -SpecsDir $specsDir -FeatureNum $featureNum) + [Console]::Error.WriteLine("[specify] Warning: -Number $requestedNum conflicts with an existing spec directory; using $featureNum instead") + } - $originalBranchName = $branchName - $branchName = "$featureNum-$truncatedSuffix" +} +# GitHub enforces a 244-byte limit on branch names +# Validate and truncate if necessary +$originalBranchName = "$featureNum-$branchSuffix" +$branchName = Get-FittedBranchName -FeatureNum $featureNum -BranchSuffix $branchSuffix +if ($branchName -ne $originalBranchName) { [Console]::Error.WriteLine("[specify] Warning: Branch name exceeded GitHub's 244-byte limit") [Console]::Error.WriteLine("[specify] Original: $originalBranchName ($($originalBranchName.Length) bytes)") [Console]::Error.WriteLine("[specify] Truncated to: $branchName ($($branchName.Length) bytes)") diff --git a/scripts/python/create_new_feature.py b/scripts/python/create_new_feature.py index ea856bf594..c46837d9d5 100644 --- a/scripts/python/create_new_feature.py +++ b/scripts/python/create_new_feature.py @@ -76,7 +76,7 @@ def _help_text(argv0: str) -> str: --dry-run Compute feature name and paths without creating directories or files --allow-existing-branch Reuse an existing feature directory if it already exists --short-name Provide a custom short name (2-4 words) for the feature - --number N Specify branch number manually (overrides auto-detection) + --number N Prefer a feature number (auto-corrected if its specs prefix exists) --timestamp Use timestamp prefix (YYYYMMDD-HHMMSS) instead of sequential numbering --help, -h Show this help message @@ -204,6 +204,43 @@ def _get_highest_from_specs(specs_dir: Path) -> int: return highest +def _fit_branch_name(feature_num: str, branch_suffix: str) -> str: + """Fit a feature prefix and suffix within GitHub's branch-name limit.""" + branch_name = f"{feature_num}-{branch_suffix}" + if len(branch_name) <= _MAX_BRANCH_LENGTH: + return branch_name + + max_suffix_length = _MAX_BRANCH_LENGTH - (len(feature_num) + 1) + truncated_suffix = re.sub(r"-$", "", branch_suffix[:max_suffix_length]) + return f"{feature_num}-{truncated_suffix}" + + +def _spec_prefix_exists(specs_dir: Path, feature_num: str) -> bool: + """Return whether a spec directory owns the given numeric prefix.""" + try: + return any( + entry.is_dir() and entry.name.startswith(f"{feature_num}-") + for entry in specs_dir.iterdir() + ) + except OSError: + # Match Bash globbing and PowerShell's ErrorAction=SilentlyContinue. + return False + + +def _has_spec_prefix_conflict( + specs_dir: Path, + feature_num: str, + requested_dir: Path, + *, + allow_existing: bool, +) -> bool: + """Return whether another spec directory owns the requested prefix.""" + if allow_existing and requested_dir.is_dir(): + return False + + return _spec_prefix_exists(specs_dir, feature_num) + + def main(argv: list[str] | None = None) -> int: argv0 = sys.argv[0] args = _parse_args(list(argv if argv is not None else sys.argv[1:]), argv0) @@ -261,18 +298,48 @@ def main(argv: list[str] | None = None) -> int: return 1 feature_num = f"{number:03d}" + # Treat an explicit number as a preference when its prefix is already used + # by a feature directory. Auto-detected numbers are already conflict-free. + if branch_number: + requested_branch_name = _fit_branch_name(feature_num, branch_suffix) + requested_dir = specs_dir / requested_branch_name + spec_conflict = _has_spec_prefix_conflict( + specs_dir, + feature_num, + requested_dir, + allow_existing=args.allow_existing, + ) + if spec_conflict: + requested_num = feature_num + number = _get_highest_from_specs(specs_dir) + while True: + number += 1 + if number > _MAX_FEATURE_NUMBER: + print( + f"Error: feature number must be between 0 and " + f"{_MAX_FEATURE_NUMBER}, got '{number}'", + file=sys.stderr, + ) + return 1 + feature_num = f"{number:03d}" + if not _spec_prefix_exists(specs_dir, feature_num): + break + print( + f"[specify] Warning: --number {requested_num} conflicts with " + f"an existing spec directory; using {feature_num} instead", + file=sys.stderr, + ) + max_suffix_length = _MAX_BRANCH_LENGTH - (len(feature_num) + 1) if max_suffix_length <= 0: print("Error: feature number is too long for a branch name", file=sys.stderr) return 1 - branch_name = f"{feature_num}-{branch_suffix}" + original_branch_name = f"{feature_num}-{branch_suffix}" + branch_name = _fit_branch_name(feature_num, branch_suffix) # GitHub enforces a 244-byte limit on branch names. - if len(branch_name) > _MAX_BRANCH_LENGTH: - truncated_suffix = re.sub(r"-$", "", branch_suffix[:max_suffix_length]) - original_branch_name = branch_name - branch_name = f"{feature_num}-{truncated_suffix}" + if branch_name != original_branch_name: print( "[specify] Warning: Branch name exceeded GitHub's 244-byte limit", file=sys.stderr, diff --git a/tests/test_create_new_feature_python_parity.py b/tests/test_create_new_feature_python_parity.py index 8e440c6a47..7c2c0e5622 100644 --- a/tests/test_create_new_feature_python_parity.py +++ b/tests/test_create_new_feature_python_parity.py @@ -53,6 +53,60 @@ def repo_pair(tmp_path: Path) -> tuple[Path, Path]: return _setup_repo(tmp_path, "proj-a"), _setup_repo(tmp_path, "proj-b") +def _run_all_variants_allow_existing( + repo: Path, *, number: str, short_name: str +): + """Run each create-new-feature variant with the allow-existing options.""" + common_args = ( + "--json", + "--dry-run", + "--number", + number, + "--allow-existing-branch", + "--short-name", + short_name, + "x", + ) + bash = run(bash_cmd(repo, SCRIPT, *common_args), repo) + py = run(py_cmd(repo, SCRIPT, *common_args), repo) + ps = run( + ps_cmd( + repo, + SCRIPT, + "-Json", + "-DryRun", + "-Number", + number, + "-AllowExistingBranch", + "-ShortName", + short_name, + "x", + ), + repo, + ) + return bash, ps, py + + +def test_python_prefix_scan_tolerates_permission_error( + tmp_path: Path, monkeypatch: pytest.MonkeyPatch +) -> None: + """Python matches shell variants when a spec directory cannot be listed.""" + specs_dir = tmp_path / "specs" + specs_dir.mkdir() + + def deny_listing(_path: Path): + raise PermissionError("denied") + + monkeypatch.setattr(Path, "iterdir", deny_listing) + + assert not create_new_feature._has_spec_prefix_conflict( + specs_dir, + "001", + specs_dir / "001-x", + allow_existing=False, + ) + + @requires_bash @pytest.mark.parametrize( "description", @@ -355,7 +409,7 @@ def test_python_missing_template_warning_matches_bash( @requires_bash -def test_python_existing_directory_error_matches_bash( +def test_python_existing_prefix_auto_correct_matches_bash( repo_pair: tuple[Path, Path], ) -> None: repo_a, repo_b = repo_pair @@ -377,8 +431,8 @@ def test_python_existing_directory_error_matches_bash( bash = run(bash_cmd(repo_a, SCRIPT, "--json", "--number", "1", description), repo_a) py = run(py_cmd(repo_b, SCRIPT, "--json", "--number", "1", description), repo_b) - assert py.returncode == bash.returncode == 1 - assert py.stdout == bash.stdout == "" + assert py.returncode == bash.returncode == 0 + assert json_stdout(py)["FEATURE_NUM"] == json_stdout(bash)["FEATURE_NUM"] == "002" assert normalize_repo_paths(py.stderr, repo_b) == normalize_repo_paths( bash.stderr, repo_a ) @@ -531,6 +585,11 @@ def test_all_variants_reject_signed_number(repo: Path, number: str) -> None: def test_all_variants_treat_empty_number_as_omitted( repo: Path, timestamp: bool ) -> None: + if not timestamp: + specs_dir = repo / "specs" + (specs_dir / "20260318-sequential").mkdir(parents=True) + (specs_dir / "20260319-143022-timestamp").mkdir() + bash_args = ["--json", "--dry-run", "--number", ""] ps_args = ["-Json", "-DryRun", "-Number", ""] py_args = ["--json", "--dry-run", "--number", ""] @@ -819,19 +878,92 @@ def test_all_variants_allow_existing_branch(repo: Path) -> None: @requires_bash @pytest.mark.skipif(not HAS_POWERSHELL, reason="no PowerShell available") -def test_all_variants_existing_directory_failure_diagnostics(repo: Path) -> None: - (repo / "specs" / "001-x").mkdir(parents=True) - expected = ( - "Error: Feature directory '/specs/001-x' already exists. " - "Please use a different feature name or specify a different number " - "with --number." +def test_all_variants_allow_existing_prefers_exact_dir_over_sibling( + repo: Path, +) -> None: + """Allow-existing preserves exact reuse even when a sibling shares its prefix.""" + (repo / "specs" / "004-pre-exist").mkdir(parents=True) + (repo / "specs" / "004-other").mkdir() + + bash, ps, py = _run_all_variants_allow_existing( + repo, number="4", short_name="pre-exist" ) - bash = run(bash_cmd(repo, SCRIPT, "--json", "--number", "1", "x"), repo) - ps = run(ps_cmd(repo, SCRIPT, "-Json", "-Number", "1", "x"), repo) - py = run(py_cmd(repo, SCRIPT, "--json", "--number", "1", "x"), repo) + assert bash.returncode == ps.returncode == py.returncode == 0 + assert json_stdout(bash) == json_stdout(ps) == json_stdout(py) + assert json_stdout(py)["BRANCH_NAME"] == "004-pre-exist" + for result in (bash, ps, py): + assert "conflicts with an existing spec directory" not in result.stderr - assert bash.returncode == ps.returncode == py.returncode == 1 - assert bash.stdout == ps.stdout == py.stdout == "" + +@requires_bash +@pytest.mark.skipif(not HAS_POWERSHELL, reason="no PowerShell available") +def test_all_variants_allow_existing_reuses_truncated_exact_dir(repo: Path) -> None: + """Allow-existing compares the canonical truncated feature directory name.""" + short_name = "a" * 300 + expected_branch = f"001-{'a' * 240}" + (repo / "specs" / expected_branch).mkdir(parents=True) + + bash, ps, py = _run_all_variants_allow_existing( + repo, number="1", short_name=short_name + ) + + assert bash.returncode == ps.returncode == py.returncode == 0 + assert json_stdout(bash) == json_stdout(ps) == json_stdout(py) + assert json_stdout(py)["BRANCH_NAME"] == expected_branch + for result in (bash, ps, py): + assert "conflicts with an existing spec directory" not in result.stderr + + +@requires_bash +@pytest.mark.skipif(not HAS_POWERSHELL, reason="no PowerShell available") +def test_all_variants_existing_prefix_auto_correct_diagnostics(repo: Path) -> None: + (repo / "specs" / "001-x").mkdir(parents=True) + expected = "conflicts with an existing spec directory; using 002 instead" + + bash = run( + bash_cmd(repo, SCRIPT, "--json", "--dry-run", "--number", "1", "x"), + repo, + ) + ps = run( + ps_cmd(repo, SCRIPT, "-Json", "-DryRun", "-Number", "1", "x"), + repo, + ) + py = run( + py_cmd(repo, SCRIPT, "--json", "--dry-run", "--number", "1", "x"), + repo, + ) + + assert bash.returncode == ps.returncode == py.returncode == 0 + assert json_stdout(bash) == json_stdout(ps) == json_stdout(py) for result in (bash, ps, py): assert expected in _normalized_error_text(result.stderr, repo) + + +@requires_bash +@pytest.mark.skipif(not HAS_POWERSHELL, reason="no PowerShell available") +def test_all_variants_corrected_prefix_skips_timestamp_collision(repo: Path) -> None: + """Auto-correction skips candidates owned by timestamp directories.""" + specs_dir = repo / "specs" + (specs_dir / "001-existing").mkdir(parents=True) + (specs_dir / "20260318-sequential").mkdir() + (specs_dir / "20260319-143022-timestamp").mkdir() + + bash = run( + bash_cmd(repo, SCRIPT, "--json", "--dry-run", "--number", "1", "x"), + repo, + ) + ps = run( + ps_cmd(repo, SCRIPT, "-Json", "-DryRun", "-Number", "1", "x"), + repo, + ) + py = run( + py_cmd(repo, SCRIPT, "--json", "--dry-run", "--number", "1", "x"), + repo, + ) + + assert bash.returncode == ps.returncode == py.returncode == 0 + assert json_stdout(bash) == json_stdout(ps) == json_stdout(py) + assert json_stdout(py)["FEATURE_NUM"] == "20260320" + for result in (bash, ps, py): + assert "using 20260320 instead" in result.stderr diff --git a/tests/test_timestamp_branches.py b/tests/test_timestamp_branches.py index 44c85ca798..c9adf14424 100644 --- a/tests/test_timestamp_branches.py +++ b/tests/test_timestamp_branches.py @@ -288,6 +288,50 @@ def test_explicit_number_zero_is_honored(self, git_repo: Path): assert data["FEATURE_NUM"] == "000" assert data["BRANCH_NAME"] == "000-zero" + def test_explicit_conflicting_number_uses_next_spec_prefix(self, git_repo: Path): + """An explicit number is advanced when its spec prefix already exists.""" + (git_repo / "specs" / "001-existing").mkdir(parents=True) + (git_repo / "specs" / "1000-latest").mkdir() + + result = run_script( + git_repo, + "--json", + "--dry-run", + "--number", + "1", + "--short-name", + "test", + "Test feature", + ) + + assert result.returncode == 0, result.stderr + data = json.loads(result.stdout) + assert data["FEATURE_NUM"] == "1001" + assert data["BRANCH_NAME"] == "1001-test" + assert "--number 001 conflicts with an existing spec directory" in result.stderr + assert "using 1001 instead" in result.stderr + + def test_explicit_number_ignores_matching_file(self, git_repo: Path): + """A matching file does not count as a conflicting spec directory.""" + specs_dir = git_repo / "specs" + specs_dir.mkdir() + (specs_dir / "001-placeholder").write_text("not a directory", encoding="utf-8") + + result = run_script( + git_repo, + "--json", + "--dry-run", + "--number", + "1", + "--short-name", + "test", + "Test feature", + ) + + assert result.returncode == 0, result.stderr + assert json.loads(result.stdout)["FEATURE_NUM"] == "001" + assert "conflicts with an existing spec directory" not in result.stderr + class TestSequentialBranchPowerShell: def test_powershell_scanner_uses_long_tryparse_for_large_prefixes(self): @@ -332,6 +376,50 @@ def test_explicit_number_zero_is_honored_matching_bash(self, ps_git_repo: Path): assert data["FEATURE_NUM"] == "000" assert data["BRANCH_NAME"] == "000-zero" + @pytest.mark.skipif(not _has_pwsh(), reason="pwsh not installed") + def test_explicit_conflicting_number_uses_next_spec_prefix( + self, ps_git_repo: Path + ): + """PowerShell advances an explicit number when its spec prefix exists.""" + script = ps_git_repo / "scripts" / "powershell" / "create-new-feature.ps1" + (ps_git_repo / "specs" / "001-existing").mkdir(parents=True) + (ps_git_repo / "specs" / "1000-latest").mkdir() + + result = subprocess.run( + [ + "pwsh", "-NoProfile", "-File", str(script), "-Json", "-DryRun", + "-Number", "1", "-ShortName", "test", "Test feature", + ], + cwd=ps_git_repo, capture_output=True, text=True, + ) + + assert result.returncode == 0, result.stderr + data = json.loads(result.stdout) + assert data["FEATURE_NUM"] == "1001" + assert data["BRANCH_NAME"] == "1001-test" + assert "-Number 001 conflicts with an existing spec directory" in result.stderr + assert "using 1001 instead" in result.stderr + + @pytest.mark.skipif(not _has_pwsh(), reason="pwsh not installed") + def test_explicit_number_ignores_matching_file(self, ps_git_repo: Path): + """PowerShell ignores files that resemble numbered spec directories.""" + script = ps_git_repo / "scripts" / "powershell" / "create-new-feature.ps1" + specs_dir = ps_git_repo / "specs" + specs_dir.mkdir() + (specs_dir / "001-placeholder").write_text("not a directory", encoding="utf-8") + + result = subprocess.run( + [ + "pwsh", "-NoProfile", "-File", str(script), "-Json", "-DryRun", + "-Number", "1", "-ShortName", "test", "Test feature", + ], + cwd=ps_git_repo, capture_output=True, text=True, + ) + + assert result.returncode == 0, result.stderr + assert json.loads(result.stdout)["FEATURE_NUM"] == "001" + assert "conflicts with an existing spec directory" not in result.stderr + @pytest.mark.skipif(not _has_pwsh(), reason="pwsh not installed") def test_missing_spec_template_warns_matching_bash(self, ps_git_repo: Path): """When no spec template can be resolved, create-new-feature.ps1 must warn on @@ -531,14 +619,15 @@ def test_allow_existing_reuses_existing_feature_dir(self, git_repo: Path): assert feature_dir.is_dir() assert (feature_dir / "spec.md").exists() - def test_without_flag_still_errors(self, git_repo: Path): - """T009: Existing feature directories still fail without the flag.""" + def test_without_flag_auto_corrects_existing_prefix(self, git_repo: Path): + """T009: Existing prefix advances when exact reuse is not allowed.""" (git_repo / "specs" / "007-no-flag").mkdir(parents=True) 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 (git_repo / "specs" / "008-no-flag").is_dir() + assert "using 008 instead" in result.stderr def test_allow_existing_no_overwrite_spec(self, git_repo: Path): """T010: Pre-create spec.md with content, verify it is preserved.""" @@ -598,6 +687,26 @@ def test_powershell_reuses_existing_feature_dir(self): assert "Feature directory '$featureDir' already exists" in contents assert "-not $AllowExistingBranch" in contents + @pytest.mark.skipif(not _has_pwsh(), reason="pwsh not installed") + def test_powershell_reuses_exact_feature_dir(self, ps_git_repo: Path): + """PowerShell exact-directory reuse bypasses prefix auto-correction.""" + script = ps_git_repo / "scripts" / "powershell" / "create-new-feature.ps1" + feature_dir = ps_git_repo / "specs" / "004-pre-exist" + feature_dir.mkdir(parents=True) + + result = subprocess.run( + [ + "pwsh", "-NoProfile", "-File", str(script), "-Json", + "-AllowExistingBranch", "-Number", "4", "-ShortName", + "pre-exist", "Pre-existing feature", + ], + cwd=ps_git_repo, capture_output=True, text=True, + ) + + assert result.returncode == 0, result.stderr + assert json.loads(result.stdout)["BRANCH_NAME"] == "004-pre-exist" + assert (feature_dir / "spec.md").is_file() + @pytest.mark.skipif(not _has_pwsh(), reason="pwsh not installed") @pytest.mark.skipif( os.name != "nt" or shutil.which("powershell.exe") is None,