Skip to content

Commit 749e95b

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 e3f1d30 commit 749e95b

File tree

6 files changed

+47
-34
lines changed

6 files changed

+47
-34
lines changed

scripts/bash/common.sh

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

297-
_should_skip() {
298-
local name="$1"
299-
for skip in "${skip_dirs[@]}"; do
300-
[ "$name" = "$skip" ] && return 0
301-
done
302-
return 1
303-
}
304-
305-
_scan_dir() {
306-
local dir="$1"
307-
local current_depth="$2"
308-
for child in "$dir"/*/; do
309-
[ -d "$child" ] || continue
310-
child="${child%/}"
311-
local child_name
312-
child_name="$(basename "$child")"
313-
_should_skip "$child_name" && continue
297+
# Run in a subshell to avoid leaking helper functions into global scope
298+
(
299+
_should_skip() {
300+
local name="$1"
301+
local skip
302+
for skip in "${skip_dirs[@]}"; do
303+
[ "$name" = "$skip" ] && return 0
304+
done
305+
return 1
306+
}
314307

315-
if [ -e "$child/.git" ]; then
316-
if git -C "$child" rev-parse --is-inside-work-tree >/dev/null 2>&1; then
317-
echo "$child"
308+
_scan_dir() {
309+
local dir="$1"
310+
local current_depth="$2"
311+
local child
312+
local child_name
313+
for child in "$dir"/*/; do
314+
[ -d "$child" ] || continue
315+
child="${child%/}"
316+
child_name="$(basename "$child")"
317+
_should_skip "$child_name" && continue
318+
319+
if [ -e "$child/.git" ]; then
320+
if git -C "$child" rev-parse --is-inside-work-tree >/dev/null 2>&1; then
321+
echo "$child"
322+
fi
323+
elif [ "$current_depth" -lt "$max_depth" ]; then
324+
_scan_dir "$child" $((current_depth + 1))
318325
fi
319-
elif [ "$current_depth" -lt "$max_depth" ]; then
320-
_scan_dir "$child" $((current_depth + 1))
321-
fi
322-
done
323-
}
326+
done
327+
}
324328

325-
_scan_dir "$repo_root" 1
329+
_scan_dir "$repo_root" 1
330+
)
326331
}
327332

328333
# 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
@@ -232,7 +232,7 @@ function Test-DirHasFiles {
232232
# Searches up to 2 directory levels deep for subdirectories containing .git
233233
# (directory or file, covering worktrees/submodules). Excludes the root repo
234234
# itself and common non-project directories.
235-
# Returns an array of absolute paths.
235+
# Returns an array of absolute paths. Scan depth is configurable (default 2).
236236
function Find-NestedGitRepos {
237237
param(
238238
[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/specify.md

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -106,7 +106,6 @@ Given that feature description, do this:
106106
- You must only create one feature per `/speckit.specify` invocation
107107
- The spec directory name and the git branch name are independent — they may be the same but that is the user's choice
108108
- The spec directory and file are always created by this command, never by the hook
109-
- **Note**: Nested git repository branching is handled during the `/speckit.plan` and `/speckit.tasks` phases, not here. The plan identifies affected repos and tasks generates a setup task for branch creation.
110109
111110
4. Load `templates/spec-template.md` to understand required sections.
112111

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)