Skip to content

Commit c87e0bb

Browse files
sakitACopilot
andcommitted
fix: address PR review comments
- Wrap find_nested_git_repos helpers in subshell to prevent bash global namespace pollution (_should_skip, _scan_dir) - Validate --scan-depth is a positive integer in setup-plan.sh - Remove non-existent nested_repo_scan_depth config reference from plan.md template - Fix duplicate step numbering (two items numbered 5) in plan.md - Quote repo paths in tasks.md git command template - Update Find-NestedGitRepos comment to reflect configurable depth Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
1 parent 44b1275 commit c87e0bb

File tree

5 files changed

+47
-33
lines changed

5 files changed

+47
-33
lines changed

scripts/bash/common.sh

Lines changed: 31 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -268,35 +268,40 @@ find_nested_git_repos() {
268268
"__pycache__" ".gradle" "build" "dist" "target" ".idea"
269269
".vscode" "specs")
270270

271-
_should_skip() {
272-
local name="$1"
273-
for skip in "${skip_dirs[@]}"; do
274-
[ "$name" = "$skip" ] && return 0
275-
done
276-
return 1
277-
}
278-
279-
_scan_dir() {
280-
local dir="$1"
281-
local current_depth="$2"
282-
for child in "$dir"/*/; do
283-
[ -d "$child" ] || continue
284-
child="${child%/}"
285-
local child_name
286-
child_name="$(basename "$child")"
287-
_should_skip "$child_name" && continue
271+
# Run in a subshell to avoid leaking helper functions into global scope
272+
(
273+
_should_skip() {
274+
local name="$1"
275+
local skip
276+
for skip in "${skip_dirs[@]}"; do
277+
[ "$name" = "$skip" ] && return 0
278+
done
279+
return 1
280+
}
288281

289-
if [ -e "$child/.git" ]; then
290-
if git -C "$child" rev-parse --is-inside-work-tree >/dev/null 2>&1; then
291-
echo "$child"
282+
_scan_dir() {
283+
local dir="$1"
284+
local current_depth="$2"
285+
local child
286+
local child_name
287+
for child in "$dir"/*/; do
288+
[ -d "$child" ] || continue
289+
child="${child%/}"
290+
child_name="$(basename "$child")"
291+
_should_skip "$child_name" && continue
292+
293+
if [ -e "$child/.git" ]; then
294+
if git -C "$child" rev-parse --is-inside-work-tree >/dev/null 2>&1; then
295+
echo "$child"
296+
fi
297+
elif [ "$current_depth" -lt "$max_depth" ]; then
298+
_scan_dir "$child" $((current_depth + 1))
292299
fi
293-
elif [ "$current_depth" -lt "$max_depth" ]; then
294-
_scan_dir "$child" $((current_depth + 1))
295-
fi
296-
done
297-
}
300+
done
301+
}
298302

299-
_scan_dir "$repo_root" 1
303+
_scan_dir "$repo_root" 1
304+
)
300305
}
301306

302307
# Resolve a template name to a file path using the priority stack:

scripts/bash/setup-plan.sh

Lines changed: 13 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -32,8 +32,19 @@ for arg in "$@"; do
3232
;;
3333
esac
3434
done
35-
# Reset sentinel if --scan-depth was passed without a value
36-
[ "$SCAN_DEPTH" = "__NEXT__" ] && SCAN_DEPTH=""
35+
# Validate --scan-depth argument
36+
if [ "$SCAN_DEPTH" = "__NEXT__" ]; then
37+
echo "ERROR: --scan-depth requires a positive integer value" >&2
38+
exit 1
39+
fi
40+
if [ -n "$SCAN_DEPTH" ]; then
41+
case "$SCAN_DEPTH" in
42+
''|*[!0-9]*|0)
43+
echo "ERROR: --scan-depth must be a positive integer, got '$SCAN_DEPTH'" >&2
44+
exit 1
45+
;;
46+
esac
47+
fi
3748

3849
# Get script directory and load common functions
3950
SCRIPT_DIR="$(CDPATH="" cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)"

scripts/powershell/common.ps1

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -203,7 +203,7 @@ function Test-DirHasFiles {
203203
# Searches up to 2 directory levels deep for subdirectories containing .git
204204
# (directory or file, covering worktrees/submodules). Excludes the root repo
205205
# itself and common non-project directories.
206-
# Returns an array of absolute paths.
206+
# Returns an array of absolute paths. Scan depth is configurable (default 2).
207207
function Find-NestedGitRepos {
208208
param(
209209
[string]$RepoRoot = (Get-RepoRoot),

templates/commands/plan.md

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -62,8 +62,6 @@ You **MUST** consider the user input before proceeding (if not empty).
6262
6363
1. **Setup**: Run `{SCRIPT}` from repo root and parse JSON for FEATURE_SPEC, IMPL_PLAN, SPECS_DIR, BRANCH, and NESTED_REPOS. For single quotes in args like "I'm Groot", use escape syntax: e.g 'I'\''m Groot' (or double-quote if possible: "I'm Groot").
6464
65-
**Nested repo scan depth**: Check `.specify/init-options.json` for `nested_repo_scan_depth`. If present, pass `--scan-depth N` (Bash) or `-ScanDepth N` (PowerShell) to the script.
66-
6765
2. **Load context**: Read FEATURE_SPEC and `/memory/constitution.md`. Load IMPL_PLAN template (already copied).
6866
6967
3. **Identify affected nested repositories**: If NESTED_REPOS is non-empty:
@@ -83,7 +81,7 @@ You **MUST** consider the user input before proceeding (if not empty).
8381
8482
5. **Stop and report**: Command ends after Phase 2 planning. Report branch, IMPL_PLAN path, generated artifacts, and affected nested repos (if any).
8583
86-
5. **Check for extension hooks**: After reporting, check if `.specify/extensions.yml` exists in the project root.
84+
6. **Check for extension hooks**: After reporting, check if `.specify/extensions.yml` exists in the project root.
8785
- If it exists, read it and look for entries under the `hooks.after_plan` key
8886
- If the YAML cannot be parsed or is invalid, skip hook checking silently and continue normally
8987
- Filter out hooks where `enabled` is explicitly `false`. Treat hooks without an `enabled` field as enabled by default.

templates/commands/tasks.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -68,7 +68,7 @@ You **MUST** consider the user input before proceeding (if not empty).
6868
3. **Execute task generation workflow**:
6969
- Load plan.md and extract tech stack, libraries, project structure
7070
- Load spec.md and extract user stories with their priorities (P1, P2, P3, etc.)
71-
- If plan.md contains an "Affected Nested Repositories" section: extract the repo paths and reasons. Generate a setup task (in Phase 1) to create the feature branch in each affected nested repo using `git -C <repo_path> checkout -b <BRANCH_NAME>`. The branch name comes from the current feature branch.
71+
- If plan.md contains an "Affected Nested Repositories" section: extract the repo paths and reasons. Generate a setup task (in Phase 1) to create the feature branch in each affected nested repo using `git -C "<repo_path>" checkout -b "<BRANCH_NAME>"`. The branch name comes from the current feature branch.
7272
- If data-model.md exists: Extract entities and map to user stories
7373
- If contracts/ exists: Map interface contracts to user stories
7474
- If research.md exists: Extract decisions for setup tasks

0 commit comments

Comments
 (0)