Skip to content

Commit 8a9ce68

Browse files
iamaeroplaneclaude
andcommitted
fix: address code review feedback
- Normalize paths in find_specify_root to prevent infinite loop with relative paths - Use -PathType Container in PowerShell to only match .specify directories - Improve has_git/Test-HasGit to check git command availability and validate work tree - Handle git worktrees/submodules where .git can be a file - Remove dead fallback code in create-new-feature scripts Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
1 parent 00647fd commit 8a9ce68

4 files changed

Lines changed: 35 additions & 23 deletions

File tree

scripts/bash/common.sh

Lines changed: 13 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -5,11 +5,15 @@
55
# This is the primary marker for spec-kit projects
66
find_specify_root() {
77
local dir="${1:-$(pwd)}"
8-
while [ "$dir" != "/" ]; do
8+
# Normalize to absolute path to prevent infinite loop with relative paths
9+
dir="$(cd "$dir" 2>/dev/null && pwd)" || return 1
10+
local prev_dir=""
11+
while [ "$dir" != "/" ] && [ "$dir" != "$prev_dir" ]; do
912
if [ -d "$dir/.specify" ]; then
1013
echo "$dir"
1114
return 0
1215
fi
16+
prev_dir="$dir"
1317
dir="$(dirname "$dir")"
1418
done
1519
return 1
@@ -82,10 +86,16 @@ get_current_branch() {
8286
}
8387

8488
# Check if we have git available at the spec-kit root level
85-
# Returns true only if the .specify root has its own .git directory
89+
# Returns true only if git is installed and the repo root is inside a git work tree
90+
# Handles both regular repos (.git directory) and worktrees/submodules (.git file)
8691
has_git() {
8792
local repo_root=$(get_repo_root)
88-
[ -d "$repo_root/.git" ]
93+
# First check if git command is available
94+
command -v git >/dev/null 2>&1 || return 1
95+
# Check if .git exists (directory or file for worktrees/submodules)
96+
[ -e "$repo_root/.git" ] || return 1
97+
# Verify it's actually a valid git work tree
98+
git -C "$repo_root" rev-parse --is-inside-work-tree >/dev/null 2>&1
8999
}
90100

91101
check_feature_branch() {

scripts/bash/create-new-feature.sh

Lines changed: 0 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -167,14 +167,6 @@ SCRIPT_DIR="$(CDPATH="" cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)"
167167
source "$SCRIPT_DIR/common.sh"
168168

169169
REPO_ROOT=$(get_repo_root)
170-
if [ -z "$REPO_ROOT" ]; then
171-
# Fallback to local find_repo_root if common.sh couldn't find it
172-
REPO_ROOT="$(find_repo_root "$SCRIPT_DIR")"
173-
if [ -z "$REPO_ROOT" ]; then
174-
echo "Error: Could not determine repository root. Please run this script from within the repository." >&2
175-
exit 1
176-
fi
177-
fi
178170

179171
# Check if git is available at this repo root (not a parent)
180172
if has_git; then

scripts/powershell/common.ps1

Lines changed: 22 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -6,9 +6,12 @@
66
function Find-SpecifyRoot {
77
param([string]$StartDir = (Get-Location).Path)
88

9-
$current = $StartDir
9+
# Normalize to absolute path to prevent issues with relative paths
10+
$current = (Resolve-Path $StartDir -ErrorAction SilentlyContinue)?.Path
11+
if (-not $current) { return $null }
12+
1013
while ($true) {
11-
if (Test-Path (Join-Path $current ".specify")) {
14+
if (Test-Path -Path (Join-Path $current ".specify") -PathType Container) {
1215
return $current
1316
}
1417
$parent = Split-Path $current -Parent
@@ -86,10 +89,25 @@ function Get-CurrentBranch {
8689
}
8790

8891
# Check if we have git available at the spec-kit root level
89-
# Returns true only if the .specify root has its own .git directory
92+
# Returns true only if git is installed and the repo root is inside a git work tree
93+
# Handles both regular repos (.git directory) and worktrees/submodules (.git file)
9094
function Test-HasGit {
9195
$repoRoot = Get-RepoRoot
92-
return (Test-Path (Join-Path $repoRoot ".git"))
96+
# First check if git command is available
97+
if (-not (Get-Command git -ErrorAction SilentlyContinue)) {
98+
return $false
99+
}
100+
# Check if .git exists (directory or file for worktrees/submodules)
101+
if (-not (Test-Path (Join-Path $repoRoot ".git"))) {
102+
return $false
103+
}
104+
# Verify it's actually a valid git work tree
105+
try {
106+
$null = git -C $repoRoot rev-parse --is-inside-work-tree 2>$null
107+
return ($LASTEXITCODE -eq 0)
108+
} catch {
109+
return $false
110+
}
93111
}
94112

95113
function Test-FeatureBranch {

scripts/powershell/create-new-feature.ps1

Lines changed: 0 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -140,14 +140,6 @@ function ConvertTo-CleanBranchName {
140140

141141
# Use common.ps1 functions which prioritize .specify over git
142142
$repoRoot = Get-RepoRoot
143-
if (-not $repoRoot) {
144-
# Fallback to local Find-RepositoryRoot if common.ps1 couldn't find it
145-
$repoRoot = (Find-RepositoryRoot -StartDir $PSScriptRoot)
146-
if (-not $repoRoot) {
147-
Write-Error "Error: Could not determine repository root. Please run this script from within the repository."
148-
exit 1
149-
}
150-
}
151143

152144
# Check if git is available at this repo root (not a parent)
153145
$hasGit = Test-HasGit

0 commit comments

Comments
 (0)