Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
39 changes: 38 additions & 1 deletion scripts/bash/create-new-feature.sh
Original file line number Diff line number Diff line change
Expand Up @@ -167,6 +167,21 @@ fit_branch_name() {
printf '%s' "$branch_name"
}

# After a lost exclusive mkdir, pick the next sequential number.
rescan_sequential_feature() {
local highest
highest=$(get_highest_from_specs "$SPECS_DIR")
if [ "$highest" -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=$((highest + 1))
FEATURE_NUM=$(printf "%03d" "$((10#$BRANCH_NUMBER))")
BRANCH_NAME=$(fit_branch_name "$FEATURE_NUM" "$BRANCH_SUFFIX")
FEATURE_DIR="$SPECS_DIR/$BRANCH_NAME"
SPEC_FILE="$FEATURE_DIR/spec.md"
}

# 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).
Expand Down Expand Up @@ -355,7 +370,29 @@ if [ "$DRY_RUN" != true ]; then
fi
fi

mkdir -p "$FEATURE_DIR"
# Exclusive create: plain mkdir fails with EEXIST if another invocation
# reserved the same FEATURE_DIR after the exists check above. Rescan
# and retry before writing spec.md so the loser cannot overwrite it.
while true; do
if [ "$ALLOW_EXISTING" = true ] && [ -d "$FEATURE_DIR" ]; then
break
fi
if mkdir "$FEATURE_DIR" 2>/dev/null; then
break
fi
if [ ! -d "$FEATURE_DIR" ]; then
echo "Error: could not create feature directory '$FEATURE_DIR'" >&2
exit 1
fi
if [ "$ALLOW_EXISTING" = true ]; then
break
fi
if [ "$USE_TIMESTAMP" = true ]; then
>&2 echo "Error: Feature directory '$FEATURE_DIR' already exists. Rerun to get a new timestamp or use a different --short-name."
exit 1
fi
rescan_sequential_feature
done

if [ "$NEEDS_SPEC" = true ]; then
if [ "$SPEC_TEMPLATE_FOUND" = true ]; then
Expand Down
34 changes: 33 additions & 1 deletion scripts/powershell/create-new-feature.ps1
Original file line number Diff line number Diff line change
Expand Up @@ -268,7 +268,39 @@ if (-not $DryRun) {
$content = Resolve-TemplateContent -TemplateName 'spec-template' -RepoRoot $repoRoot
}

New-Item -ItemType Directory -Path $featureDir -Force | Out-Null
# Exclusive create: New-Item without -Force fails if another invocation
# reserved the same FEATURE_DIR after the exists check above. Rescan
# and retry before writing spec.md so the loser cannot overwrite it.
while ($true) {
if ($AllowExistingBranch -and (Test-Path -LiteralPath $featureDir -PathType Container)) {
break
}
try {
New-Item -ItemType Directory -Path $featureDir | Out-Null
break
} catch {
if (-not (Test-Path -LiteralPath $featureDir -PathType Container)) {
throw
}
if ($AllowExistingBranch) {
break
}
if ($Timestamp) {
Write-Error "Error: Feature directory '$featureDir' already exists. Rerun to get a new timestamp or use a different -ShortName."
exit 1
}
$highestNumber = Get-HighestNumberFromSpecs -SpecsDir $specsDir
if ($highestNumber -eq [long]::MaxValue) {
Write-Error "Error: feature number must be between 0 and $([long]::MaxValue), got '9223372036854775808'"
exit 1
}
$resolvedNumber = $highestNumber + 1
$featureNum = ('{0:000}' -f $resolvedNumber)
$branchName = Get-FittedBranchName -FeatureNum $featureNum -BranchSuffix $branchSuffix
$featureDir = Join-Path $specsDir $branchName
$specFile = Join-Path $featureDir 'spec.md'
}
}

if ($needsSpec) {
if ($null -ne $content) {
Expand Down
32 changes: 31 additions & 1 deletion scripts/python/create_new_feature.py
Original file line number Diff line number Diff line change
Expand Up @@ -394,7 +394,37 @@ def main(argv: list[str] | None = None) -> int:
print(f"Error: {exc}", file=sys.stderr)
return 1

feature_dir.mkdir(parents=True, exist_ok=True)
# Exclusive create: mkdir without exist_ok fails if another invocation
# reserved the same FEATURE_DIR after the exists check above. Rescan
# and retry before writing spec.md so the loser cannot overwrite it.
while True:
if args.allow_existing and feature_dir.is_dir():
break
try:
feature_dir.mkdir(parents=True)
break
except FileExistsError:
if args.allow_existing:
break
if args.use_timestamp:
print(
f"Error: Feature directory '{feature_dir}' already exists. "
"Rerun to get a new timestamp or use a different --short-name.",
file=sys.stderr,
)
return 1
number = _get_highest_from_specs(specs_dir) + 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}"
branch_name = _fit_branch_name(feature_num, branch_suffix)
feature_dir = specs_dir / branch_name
spec_file = feature_dir / "spec.md"

if needs_spec:
if template_content is not None:
Expand Down
187 changes: 187 additions & 0 deletions tests/test_create_new_feature_python_parity.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,10 @@

from __future__ import annotations

import os
import re
import shutil
import subprocess
from pathlib import Path

import pytest
Expand All @@ -14,6 +17,7 @@
HAS_POWERSHELL,
bash_cmd,
break_wrap_layer,
clean_env,
install_composition_stack,
install_scripts,
json_stdout,
Expand Down Expand Up @@ -1065,3 +1069,186 @@ def test_all_variants_corrected_prefix_skips_timestamp_collision(repo: Path) ->
assert json_stdout(py)["FEATURE_NUM"] == "20260320"
for result in (bash, ps, py):
assert "using 20260320 instead" in result.stderr


def _shared_specs_repos(tmp_path: Path) -> tuple[Path, Path, Path]:
"""Two project roots that share one specs/ directory via symlink."""
repo_a = _setup_repo(tmp_path, "proj-a")
repo_b = _setup_repo(tmp_path, "proj-b")
shared = tmp_path / "shared-specs"
shared.mkdir()
try:
(repo_a / "specs").symlink_to(shared, target_is_directory=True)
(repo_b / "specs").symlink_to(shared, target_is_directory=True)
except (OSError, NotImplementedError):
pytest.skip("Symlinks are not available in this environment")
(repo_a / ".specify" / "templates" / "spec-template.md").write_text(
"ALPHA-SPEC\n", encoding="utf-8"
)
(repo_b / ".specify" / "templates" / "spec-template.md").write_text(
"BETA-SPEC\n", encoding="utf-8"
)
return repo_a, repo_b, shared


def _mkdir_barrier_env(tmp_path: Path, n_waiters: int = 2) -> dict[str, str]:
"""Pause FEATURE_DIR mkdir until n_waiters arrive so the TOCTOU window is forced.

Two processes that have already scanned the same max spec number then both
call mkdir on the same specs/NNN-name path. mkdir -p / exist_ok=True lets
both succeed and the second write overwrites spec.md.
"""
env = clean_env()
barrier = tmp_path / "mkdir-barrier"
barrier.mkdir()
released = barrier / "released"
real_mkdir = shutil.which("mkdir")
if real_mkdir is None:
pytest.skip("mkdir not available")
bin_dir = tmp_path / "mkdir-bin"
bin_dir.mkdir()
wrapper = bin_dir / "mkdir"
wrapper.write_text(
f"""#!/bin/sh
path_last=""
for arg in "$@"; do
path_last="$arg"
done
base=$(basename -- "$path_last")
parent=$(basename -- "$(dirname -- "$path_last")")
case "$parent/$base" in
specs/[0-9]*-*)
if [ ! -f "{released}" ]; then
i=0
while ! mkdir "{barrier}/w-$i" 2>/dev/null; do
i=$((i + 1))
if [ "$i" -gt 100 ]; then
break
fi
done
n=0
while [ "$n" -lt 200 ]; do
count=$(ls -d "{barrier}"/w-* 2>/dev/null | wc -l)
if [ "$count" -ge {n_waiters} ]; then
touch "{released}"
break
fi
if [ -f "{released}" ]; then
break
fi
sleep 0.05
n=$((n + 1))
done
fi
;;
esac
exec "{real_mkdir}" "$@"
""",
encoding="utf-8",
)
wrapper.chmod(0o755)
env["PATH"] = f"{bin_dir}{os.pathsep}{env.get('PATH', '')}"
env["SPECKIT_MKDIR_BARRIER"] = str(barrier)
env["SPECKIT_MKDIR_BARRIER_N"] = str(n_waiters)
site = tmp_path / "mkdir-site"
site.mkdir()
(site / "sitecustomize.py").write_text(
"""
import os
import time
from pathlib import Path

_orig_mkdir = os.mkdir
_barrier = Path(os.environ["SPECKIT_MKDIR_BARRIER"])
_released = _barrier / "released"
_n_waiters = int(os.environ.get("SPECKIT_MKDIR_BARRIER_N", "2"))


def _is_feature_dir(path):
p = Path(path)
return p.parent.name == "specs" and "-" in p.name and p.name[0:1].isdigit()


def _gated_mkdir(path, mode=0o777, *args, **kwargs):
if _is_feature_dir(path) and not _released.exists():
(_barrier / f"w-py-{os.getpid()}-{time.time_ns()}").mkdir(exist_ok=True)
for _ in range(200):
if _released.exists():
break
waiters = list(_barrier.glob("w-*"))
if len(waiters) >= _n_waiters:
_released.touch()
break
time.sleep(0.05)
return _orig_mkdir(path, mode, *args, **kwargs)


os.mkdir = _gated_mkdir
""",
encoding="utf-8",
)
env["PYTHONPATH"] = f"{site}{os.pathsep}{env.get('PYTHONPATH', '')}"
return env


def _popen(
cmd: list[str], repo: Path, env: dict[str, str]
) -> subprocess.Popen[str]:
return subprocess.Popen(
cmd,
cwd=repo,
env=env,
stdout=subprocess.PIPE,
stderr=subprocess.PIPE,
text=True,
)


def _wait(
proc: subprocess.Popen[str], cmd: list[str]
) -> subprocess.CompletedProcess[str]:
stdout, stderr = proc.communicate(timeout=20)
return subprocess.CompletedProcess(cmd, proc.returncode, stdout, stderr)


@requires_bash
@pytest.mark.parametrize("variant", ["bash", "python"])
def test_concurrent_reservations_do_not_share_spec_directory(
tmp_path: Path, variant: str
) -> None:
"""Two concurrent reservations must not land in the same FEATURE_DIR.

create-new-feature scans max(specs)+1, checks that FEATURE_DIR is absent,
then creates it with mkdir -p (bash) / New-Item -Force (powershell) /
Path.mkdir(exist_ok=True) (python). That create is not exclusive, so two
invocations can share the directory and the second overwrites spec.md.
"""
repo_a, repo_b, shared = _shared_specs_repos(tmp_path)
env = _mkdir_barrier_env(tmp_path)
args = ("--json", "--short-name", "user-auth", "Add user authentication")
if variant == "bash":
cmd_a = bash_cmd(repo_a, SCRIPT, *args)
cmd_b = bash_cmd(repo_b, SCRIPT, *args)
else:
cmd_a = py_cmd(repo_a, SCRIPT, *args)
cmd_b = py_cmd(repo_b, SCRIPT, *args)

proc_a = _popen(cmd_a, repo_a, env)
proc_b = _popen(cmd_b, repo_b, env)
result_a = _wait(proc_a, cmd_a)
result_b = _wait(proc_b, cmd_b)

assert result_a.returncode == 0, result_a.stderr
assert result_b.returncode == 0, result_b.stderr

data_a = json_stdout(result_a)
data_b = json_stdout(result_b)
assert data_a["BRANCH_NAME"] != data_b["BRANCH_NAME"]
assert {data_a["FEATURE_NUM"], data_b["FEATURE_NUM"]} == {"001", "002"}

names = sorted(path.name for path in shared.iterdir() if path.is_dir())
assert names == ["001-user-auth", "002-user-auth"]
contents = {
(shared / name / "spec.md").read_text(encoding="utf-8") for name in names
}
assert contents == {"ALPHA-SPEC\n", "BETA-SPEC\n"}