Skip to content

Commit 2f7d8f3

Browse files
fsilvaortizclaude
andcommitted
fix: address Copilot review — input validation and flexible test assertion
- Bash: reject non-numeric --number with clear error before arithmetic - PowerShell: reject negative -Number values (< 1) with clear error - Test: use regex for elseif assertion to tolerate whitespace changes Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
1 parent e602275 commit 2f7d8f3

3 files changed

Lines changed: 12 additions & 2 deletions

File tree

scripts/bash/create-new-feature.sh

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -263,6 +263,10 @@ else
263263
elif [ "$ALLOW_EXISTING" = false ]; then
264264
# Manual number provided -- validate it is not already in use
265265
# (skip when --allow-existing-branch, as the caller intentionally targets an existing branch)
266+
if ! echo "$BRANCH_NUMBER" | grep -q '^[0-9]\+$'; then
267+
>&2 echo "Error: --number requires a positive integer, got '$BRANCH_NUMBER'"
268+
exit 1
269+
fi
266270
MANUAL_NUM=$((10#$BRANCH_NUMBER))
267271
MANUAL_NUM_PADDED=$(printf "%03d" "$MANUAL_NUM")
268272
NUMBER_IN_USE=false

scripts/powershell/create-new-feature.ps1

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -214,6 +214,10 @@ if ($Timestamp) {
214214
} elseif (-not $AllowExistingBranch) {
215215
# Manual number provided -- validate it is not already in use
216216
# (skip when -AllowExistingBranch, as the caller intentionally targets an existing branch)
217+
if ($Number -lt 1) {
218+
Write-Error "-Number requires a positive integer, got $Number"
219+
exit 1
220+
}
217221
$manualNumPadded = '{0:000}' -f $Number
218222
$numberInUse = $false
219223

tests/test_timestamp_branches.py

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -423,5 +423,7 @@ def test_powershell_has_number_collision_validation(self):
423423
assert "git fetch --all --prune" in contents
424424
# Must warn and auto-detect on conflict
425425
assert "conflicts with existing branch/spec" in contents
426-
# Must skip validation when -AllowExistingBranch is set
427-
assert "elseif (-not $AllowExistingBranch)" in contents
426+
# Must skip validation when -AllowExistingBranch is set; allow flexible whitespace
427+
assert re.search(r"elseif\s*\(\s*-not\s+\$AllowExistingBranch\s*\)", contents), (
428+
"Expected an elseif guard that negates $AllowExistingBranch"
429+
)

0 commit comments

Comments
 (0)