Skip to content

Commit 0966b43

Browse files
committed
fix: address PR review feedback for allow-existing-branch
- Make checkout failure fatal instead of suppressing with || true (bash) - Check $LASTEXITCODE after git checkout in PowerShell - Use Test-Path -PathType Leaf for spec file existence check (PS) - Add PowerShell static assertion test for -AllowExistingBranch flag Assisted-By: 🤖 Claude Code
1 parent bcdd421 commit 0966b43

3 files changed

Lines changed: 18 additions & 2 deletions

File tree

scripts/bash/create-new-feature.sh

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -294,7 +294,10 @@ if [ "$HAS_GIT" = true ]; then
294294
if git branch --list "$BRANCH_NAME" | grep -q .; then
295295
if [ "$ALLOW_EXISTING" = true ]; then
296296
# Switch to the existing branch instead of failing
297-
git checkout "$BRANCH_NAME" 2>/dev/null || true
297+
if ! git checkout "$BRANCH_NAME" 2>/dev/null; then
298+
>&2 echo "Error: Failed to switch to existing branch '$BRANCH_NAME'. Please resolve any local changes or conflicts and try again."
299+
exit 1
300+
fi
298301
elif [ "$USE_TIMESTAMP" = true ]; then
299302
>&2 echo "Error: Branch '$BRANCH_NAME' already exists. Rerun to get a new timestamp or use a different --short-name."
300303
exit 1

scripts/powershell/create-new-feature.ps1

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -256,6 +256,10 @@ if ($hasGit) {
256256
if ($AllowExistingBranch) {
257257
# Switch to the existing branch instead of failing
258258
git checkout -q $branchName 2>$null | Out-Null
259+
if ($LASTEXITCODE -ne 0) {
260+
Write-Error "Error: Branch '$branchName' exists but could not be checked out. Resolve any uncommitted changes or conflicts and try again."
261+
exit 1
262+
}
259263
} elseif ($Timestamp) {
260264
Write-Error "Error: Branch '$branchName' already exists. Rerun to get a new timestamp or use a different -ShortName."
261265
exit 1
@@ -276,7 +280,7 @@ $featureDir = Join-Path $specsDir $branchName
276280
New-Item -ItemType Directory -Path $featureDir -Force | Out-Null
277281

278282
$specFile = Join-Path $featureDir 'spec.md'
279-
if (-not (Test-Path $specFile)) {
283+
if (-not (Test-Path -PathType Leaf $specFile)) {
280284
$template = Resolve-Template -TemplateName 'spec-template' -RepoRoot $repoRoot
281285
if ($template -and (Test-Path $template)) {
282286
Copy-Item $template $specFile -Force

tests/test_timestamp_branches.py

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -403,3 +403,12 @@ def test_allow_existing_no_git(self, no_git_dir: Path):
403403
"No git feature",
404404
)
405405
assert result.returncode == 0, result.stderr
406+
407+
408+
class TestAllowExistingBranchPowerShell:
409+
def test_powershell_supports_allow_existing_branch_flag(self):
410+
"""Static guard: PS script exposes and uses -AllowExistingBranch."""
411+
contents = CREATE_FEATURE_PS.read_text(encoding="utf-8")
412+
assert "-AllowExistingBranch" in contents
413+
# Ensure the flag is referenced in script logic, not just declared
414+
assert "AllowExistingBranch" in contents.replace("-AllowExistingBranch", "")

0 commit comments

Comments
 (0)