Copilot feedback

Author: amweiss

scripts/bash/common.sh

@@ -133,7 +133,7 @@ find_feature_dir_by_prefix() {
     else
         # Multiple matches - this shouldn't happen with proper naming convention
         echo "ERROR: Multiple spec directories found with prefix '$prefix': ${matches[*]}" >&2
-        echo "Please ensure only one spec directory exists per numeric prefix." >&2
+        echo "Please ensure only one spec directory exists per prefix." >&2
         return 1
     fi
 }

scripts/bash/create-new-feature.sh

@@ -69,7 +69,7 @@ done
 
 FEATURE_DESCRIPTION="${ARGS[*]}"
 if [ -z "$FEATURE_DESCRIPTION" ]; then
-    echo "Usage: $0 [--json] [--short-name <name>] [--number N] <feature_description>" >&2
+    echo "Usage: $0 [--json] [--short-name <name>] [--number N] [--timestamp] <feature_description>" >&2
     exit 1
 fi
 

scripts/powershell/create-new-feature.ps1

@@ -33,7 +33,7 @@ if ($Help) {
 
 # Check if feature description provided
 if (-not $FeatureDescription -or $FeatureDescription.Count -eq 0) {
-    Write-Error "Usage: ./create-new-feature.ps1 [-Json] [-ShortName <name>] <feature description>"
+    Write-Error "Usage: ./create-new-feature.ps1 [-Json] [-ShortName <name>] [-Number N] [-Timestamp] <feature description>"
     exit 1
 }
 
@@ -96,7 +96,7 @@ function Get-HighestNumberFromBranches {
                 $cleanBranch = $branch.Trim() -replace '^\*?\s+', '' -replace '^remotes/[^/]+/', ''
 
                 # Extract feature number if branch matches pattern ###-*
-                if ($cleanBranch -match '^(\d+)-') {
+                if ($cleanBranch -match '^(\d{3})-') {
                     $num = [int]$matches[1]
                     if ($num -gt $highest) { $highest = $num }
                 }

tests/test_branch_numbering.py

@@ -51,20 +51,24 @@ def test_invalid_branch_numbering_rejected(self, tmp_path: Path):
         assert result.exit_code == 1
         assert "Invalid --branch-numbering" in result.output
 
-    def test_valid_branch_numbering_sequential(self, tmp_path: Path):
+    def test_valid_branch_numbering_sequential(self, tmp_path: Path, monkeypatch):
         from typer.testing import CliRunner
         from specify_cli import app
 
+        monkeypatch.setattr("specify_cli.download_and_extract_template", lambda *args, **kwargs: None)
+
         runner = CliRunner()
         result = runner.invoke(app, ["init", str(tmp_path / "proj"), "--ai", "claude", "--branch-numbering", "sequential"])
-        # Should not fail on validation (may fail later due to network/template fetch)
+        assert result.exit_code == 0
         assert "Invalid --branch-numbering" not in (result.output or "")
 
-    def test_valid_branch_numbering_timestamp(self, tmp_path: Path):
+    def test_valid_branch_numbering_timestamp(self, tmp_path: Path, monkeypatch):
         from typer.testing import CliRunner
         from specify_cli import app
 
+        monkeypatch.setattr("specify_cli.download_and_extract_template", lambda *args, **kwargs: None)
+
         runner = CliRunner()
         result = runner.invoke(app, ["init", str(tmp_path / "proj"), "--ai", "claude", "--branch-numbering", "timestamp"])
-        # Should not fail on validation (may fail later due to network/template fetch)
+        assert result.exit_code == 0
         assert "Invalid --branch-numbering" not in (result.output or "")

tests/test_timestamp_branches.py

@@ -51,7 +51,7 @@ def no_git_dir(tmp_path: Path) -> Path:
     return tmp_path
 
 
-def run_script(cwd: Path, *args: str, capture_stderr: bool = False) -> subprocess.CompletedProcess:
+def run_script(cwd: Path, *args: str) -> subprocess.CompletedProcess:
     """Run create-new-feature.sh with given args."""
     cmd = ["bash", "scripts/bash/create-new-feature.sh", *args]
     return subprocess.run(
@@ -91,6 +91,7 @@ def test_timestamp_creates_branch(self, git_repo: Path):
     def test_number_and_timestamp_warns(self, git_repo: Path):
         """Test 3: --number + --timestamp warns and uses timestamp."""
         result = run_script(git_repo, "--timestamp", "--number", "42", "--short-name", "feat", "Feature")
+        assert result.returncode == 0, result.stderr
         assert "Warning" in result.stderr and "--number" in result.stderr
 
     def test_json_output_keys(self, git_repo: Path):
@@ -209,6 +210,7 @@ class TestNoGitTimestamp:
     def test_no_git_timestamp(self, no_git_dir: Path):
         """Test 13: No-git repo + timestamp creates spec dir with warning."""
         result = run_script(no_git_dir, "--timestamp", "--short-name", "no-git-feat", "No git feature")
+        assert result.returncode == 0, result.stderr
         spec_dirs = list((no_git_dir / "specs").iterdir()) if (no_git_dir / "specs").exists() else []
         assert len(spec_dirs) > 0, "spec dir not created"
         assert "git" in result.stderr.lower() or "warning" in result.stderr.lower()