github/github-spec-kit
1
0
Fork 0
mirror of https://github.com/github/spec-kit.git synced 2026-08-03 06:26:30 +08:00
Code Issues Packages Projects Releases Wiki Activity

fix: auto-correct conflicting feature prefixes (#1829)

Browse Source 
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)
This commit is contained in:
Daniel Graham
2026-07-24 10:13:53 -04:00
committed by GitHub
parent c0ba81190b
commit bfe4772b79
5 changed files with 462 additions and 58 deletions
Download Patch File Download Diff File Expand all files Collapse all files

160
tests/test_create_new_feature_python_parity.py
View File

@@ -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 '<REPO>/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

117
tests/test_timestamp_branches.py
View File

@@ -288,6 +288,50 @@ class TestSequentialBranch:
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 @@ class TestSequentialBranchPowerShell:
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 @@ class TestAllowExistingBranch:
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 @@ class TestAllowExistingBranchPowerShell:
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,