fix: detect and warn about spec directory mismatch #736
Conversation
Fixes github#733 When users delete a branch and create a new one, the spec directory name no longer matches the branch name, causing spec-kit to fail. This fix adds automatic detection of mismatched spec directories and provides clear instructions to fix the issue using git mv. Changes: - Add check_and_fix_spec_directory_mismatch() function to common.sh - Integrate mismatch check into setup-plan.sh and check-prerequisites.sh - Handle three scenarios: single orphaned dir, multiple orphaned dirs, no dir - Provide actionable git mv commands in warning messages
There was a problem hiding this comment.
Pull Request Overview
Adds automatic detection and warning for spec directory mismatches when branch names don't align with spec directory names, typically occurring after branch deletion and recreation. This prevents spec-kit failures by providing clear remediation instructions.
- Implements
check_and_fix_spec_directory_mismatch()function to detect orphaned spec directories - Integrates mismatch detection into both setup and prerequisite checking workflows
- Provides actionable
git mvcommands to resolve directory naming conflicts
Reviewed Changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| scripts/bash/common.sh | Adds core logic to detect spec directory mismatches and generate appropriate warning messages |
| scripts/bash/setup-plan.sh | Integrates mismatch check into the setup workflow |
| scripts/bash/check-prerequisites.sh | Adds mismatch check to prerequisite validation with paths-only mode support |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
robustness - Add variable validation and nullglob protection - Skip symlinks and hidden directories - Optimize git branch lookups - Add comprehensive function documentation
There was a problem hiding this comment.
Pull Request Overview
Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Pull Request Overview
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Pull Request Overview
Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| # Check if branch and spec directory are in sync | ||
| check_and_fix_spec_directory_mismatch | ||
| result=$? | ||
| if [[ $result -ne 0 && $result -ne 1 ]]; then |
There was a problem hiding this comment.
The return code handling logic is unclear. According to the function documentation (lines 167-172 in common.sh), the function only returns 0 (success/no mismatch) or 1 (mismatch detected). The condition if [[ $result -ne 0 && $result -ne 1 ]] suggests there are other return codes, but none are documented or implemented. Either simplify this to handle only the documented return codes, or update the function documentation to specify what other return codes mean.
| if [[ $result -ne 0 && $result -ne 1 ]]; then | |
| if [[ $result -eq 1 ]]; then |
| check_and_fix_spec_directory_mismatch | ||
| result=$? | ||
| if [[ $result -ne 0 && $result -ne 1 ]]; then | ||
| exit 1 | ||
| fi |
There was a problem hiding this comment.
Inconsistent error handling between setup-plan.sh and check-prerequisites.sh. In check-prerequisites.sh (line 87), the function is called with || exit 1, treating any non-zero return as an error. Here, return code 1 is explicitly allowed to continue. This inconsistency suggests different intentions that should be clarified with comments or unified.
|
|
||
| if [[ -d "$specs_dir" ]] && has_git; then | ||
| # Get all existing branch names for efficient lookup (Bash 3.2 compatible) | ||
| local existing_branches=$(git for-each-ref refs/heads/ --format='%(refname:short)' 2>/dev/null | tr '\n' '|') |
There was a problem hiding this comment.
The existing_branches variable will have a trailing pipe character when branches exist, but will be empty when no branches exist. Line 226's check echo \"|${existing_branches}\" | grep -qF \"|${dirname}|\" will fail for the first branch name when existing_branches is non-empty because of the missing leading pipe in existing_branches. The variable should be |branch1|branch2| format, but tr '\\n' '|' produces branch1|branch2|. This will cause false positives for orphan detection.
| local existing_branches=$(git for-each-ref refs/heads/ --format='%(refname:short)' 2>/dev/null | tr '\n' '|') | |
| local existing_branches="|$(git for-each-ref refs/heads/ --format='%(refname:short)' 2>/dev/null | tr '\n' '|')" |
1 participant
Fixes #733
When users delete a branch and create a new one, the spec directory
name no longer matches the branch name, causing spec-kit to fail.
This fix adds automatic detection of mismatched spec directories and
provides clear instructions to fix the issue using git mv.
Changes: