fix: exclusive mkdir so concurrent create-new-feature cannot overwrite spec.md

[BetterAndBetterII]

Problem

create-new-feature picks the next sequential spec number, checks that FEATURE_DIR is absent, then creates it with mkdir -p (bash), New-Item -Force (PowerShell), or Path.mkdir(exist_ok=True) (Python). Those creates are not exclusive.

Two concurrent invocations can therefore scan the same max number, both pass the exists check, both create the same directory, and the second write clobbers spec.md.

Fix

Create FEATURE_DIR exclusively. On EEXIST, rescan specs/ for the next sequential number and retry before writing spec.md. Timestamp mode still errors on collision, matching the previous exists-check behavior.

Fixes #4270

[Copilot]

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

[Copilot]

Review details

• Files reviewed: 4/4 changed files
• Comments generated: 5
• Review effort level: Balanced

[mnriem]

Please address Copilot feedback

[Copilot]

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

[Copilot]

🔵 Needs a closer look

Pre-loop checks can bypass retries, and allow-existing races can still overwrite spec.md.

Review details

Suppressed comments (7)

scripts/python/create_new_feature.py:400

• A sequential collision can still bypass this retry loop entirely. If another invocation creates feature_dir after the number scan but before the existing-directory check at line 370, that check returns an error instead of rescanning. Fold the pre-loop guard into this reservation loop so timestamp collisions remain errors while sequential collisions always retry.

        while True:

scripts/bash/create-new-feature.sh:376

• A sequential collision can still bypass this retry loop entirely. If another invocation creates FEATURE_DIR after the number scan but before the guard at lines 348-355, the script exits instead of rescanning. Fold that guard into this reservation loop so timestamp collisions remain errors while sequential collisions always retry.

    while true; do

tests/test_create_new_feature_python_parity.py:1268

• The forced concurrency regression excludes PowerShell even though this PR adds an independent New-Item/catch retry path. Add a PowerShell-capable barrier and include a conditionally skipped PowerShell variant so all three first-class script implementations exercise the race.

@pytest.mark.parametrize("variant", ["bash", "python"])

scripts/powershell/create-new-feature.ps1:281

• A sequential collision can still bypass this retry loop entirely. If another invocation creates $featureDir after the number scan but before the guard at lines 263-270, the script exits instead of rescanning. Fold that guard into this reservation loop so timestamp collisions remain errors while sequential collisions always retry.

    while ($true) {

scripts/python/create_new_feature.py:401

• This allow-existing path can still overwrite a concurrent creator's spec.md: needs_spec is computed before reservation, so another process can create and populate the directory after that check; this process then breaks here and writes based on the stale needs_spec=True. Track whether this invocation won the reservation, and do not initialize spec.md when reusing a directory that appeared concurrently.

            if args.allow_existing and feature_dir.is_dir():

scripts/bash/create-new-feature.sh:377

• This allow-existing path can still overwrite a concurrent creator's spec.md: NEEDS_SPEC is computed before reservation, so another process can create and populate the directory after that check; this process then breaks here and writes based on stale state. Track whether this invocation won the reservation, and do not initialize spec.md when reusing a directory that appeared concurrently.

        if [ "$ALLOW_EXISTING" = true ] && [ -d "$FEATURE_DIR" ]; then

scripts/powershell/create-new-feature.ps1:282

• This allow-existing path can still overwrite a concurrent creator's spec.md: $needsSpec is computed before reservation, so another process can create and populate the directory after that check; this process then breaks here and writes based on stale state. Track whether this invocation won the reservation, and do not initialize spec.md when reusing a directory that appeared concurrently.

        if ($AllowExistingBranch -and (Test-Path -LiteralPath $featureDir -PathType Container)) {

• Files reviewed: 4/4 changed files
• Comments generated: 0 new
• Review effort level: Balanced

[mnriem]

Thanks for the update. The reservation still occurs after existence and spec.md state checks, leaving TOCTOU windows that can bypass retries, overwrite an existing spec under --allow-existing, or leave a retried directory without a spec.

Please reserve/retry the directory first, then derive all spec.md state from the final path. Also add deterministic PowerShell concurrency coverage and handle Python collisions where the target is a regular file.

Posted on behalf of @mnriem by GitHub Copilot (model: GPT-5.6 Sol).