-
Notifications
You must be signed in to change notification settings - Fork 1.9k
feat(hooks): add support for custom hooks #345
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR adds comprehensive hook support to the specify CLI tool, allowing users to customize the specification workflow with pre-processing, feature number generation, and post-processing hooks.
- Introduces Git-style hook system with four hook types: pre-specify, prepare-feature-num, post-checkout, and post-specify
- Adds hook execution logic to the specify command workflow with cross-platform support
- Updates script permissions handling to include hooks alongside existing script management
Reviewed Changes
Copilot reviewed 14 out of 14 changed files in this pull request and generated 2 comments.
Show a summary per file
File | Description |
---|---|
templates/commands/specify.md | Updates workflow to integrate hook execution at appropriate stages |
src/specify_cli/init.py | Extends script permission handling to include hooks in the .specify/hooks directory |
scripts/powershell/create-new-feature.ps1 | Adds --FeatureNum parameter support for custom feature numbering |
scripts/bash/create-new-feature.sh | Adds --feature-num parameter support for custom feature numbering |
hooks/*.sample | Provides sample hook implementations for all four hook types in both bash and PowerShell |
hooks/README.md | Comprehensive documentation for hook system usage and examples |
.github/workflows/scripts/create-release-packages.sh | Updates release packaging to include hooks directory |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
Copilot reviewed 14 out of 14 changed files in this pull request and generated 2 comments.
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
Copilot reviewed 14 out of 14 changed files in this pull request and generated 1 comment.
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
Copilot reviewed 14 out of 14 changed files in this pull request and generated 3 comments.
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
Copilot reviewed 14 out of 14 changed files in this pull request and generated 1 comment.
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
templates/commands/specify.md
Outdated
3. Write the specification to SPEC_FILE using the template structure, replacing placeholders with concrete details derived from the feature description (arguments) while preserving section order and headings. | ||
4. Report completion with branch name, spec file path, and readiness for the next phase. | ||
1. Run pre-specify hook if available (ignore errors): | ||
- Windows: Try `.specify/hooks/pre-specify.ps1 "{ARGS}"` then `.specify/hooks/pre-specify "{ARGS}"` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am still on the fence about how we want to handle multi-script scenarios, that's why you see {SCRIPT}
above that dynamically swaps the right script at package build time.
Thoughts?
templates/commands/specify.md
Outdated
- Unix/Linux: Try `.specify/hooks/prepare-feature-num "{ARGS}"` | ||
- If hook returns a number, use `--feature-num $FEATURE_NUM` with the script | ||
3. Run the script `{SCRIPT}` from repo root (with optional --feature-num parameter) and parse its JSON output for BRANCH_NAME, SPEC_FILE, and FEATURE_NUM. All file paths must be absolute. | ||
4. Export environment variables: `export BRANCH_NAME SPEC_FILE FEATURE_NUM` (Unix) or `$env:BRANCH_NAME = $BRANCH_NAME; $env:SPEC_FILE = $SPEC_FILE; $env:FEATURE_NUM = $FEATURE_NUM` (Windows) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same here - I've seen in my observations that LLMs are typically terrible at determining the current OS. We might need to encode some step here that allows checking for the OS from the terminal before doing anything else.
Co-authored-by: Copilot <[email protected]>
Co-authored-by: Copilot <[email protected]>
Co-authored-by: Copilot <[email protected]>
…ditional arguments
Co-authored-by: Copilot <[email protected]>
3beaaf4
to
486da6b
Compare
…ctive hook copying
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
Copilot reviewed 14 out of 14 changed files in this pull request and generated 2 comments.
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
@localden Thanks for the feedback! I've addressed your comments with the following changes: Changes Made
Regarding OS Detection You're absolutely right about LLMs being unreliable at OS detection. I've kept the current approach (script type determined at build time) while simplifying so each package only handles its own script type. If runtime OS detection becomes necessary, we should handle it in a separate PR. Let me know if you have any additional feedback! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
Copilot reviewed 14 out of 14 changed files in this pull request and generated 2 comments.
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
Co-authored-by: Copilot <[email protected]>
Co-authored-by: Copilot <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
Copilot reviewed 14 out of 14 changed files in this pull request and generated 3 comments.
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
Co-authored-by: Copilot <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
Copilot reviewed 14 out of 14 changed files in this pull request and generated 3 comments.
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
Co-authored-by: Copilot <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
Copilot reviewed 14 out of 14 changed files in this pull request and generated 3 comments.
Comments suppressed due to low confidence (1)
scripts/powershell/create-new-feature.ps1:1
- Add tests for -FeatureNum to cover: accepted values (e.g., 1 -> 001, 42 -> 042), validation failures (0, 1000), and that the auto-increment path is used when the parameter is omitted.
#!/usr/bin/env pwsh
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
os.chmod(hook, new_mode) | ||
updated += 1 | ||
except Exception as e: | ||
failures.append(f"hooks/{hook.name}: {e}") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Failure reporting for hooks uses only the basename while scripts use a path relative to their root, which is inconsistent. Consider aligning the formats to improve diagnostics, e.g., use hook.relative_to(hooks_root) similar to scripts: failures.append(f"hooks/{hook.relative_to(hooks_root)}: {e}").
failures.append(f"hooks/{hook.name}: {e}") | |
failures.append(f"hooks/{hook.relative_to(hooks_root)}: {e}") |
Copilot uses AI. Check for mistakes.
find hooks -maxdepth 1 -type f -name "*.sample" ! -name "*.ps1.sample" -exec cp {} "$SPEC_DIR/hooks/" \; | ||
;; | ||
ps) | ||
# Copy PowerShell hook samples (.ps1.sample) | ||
find hooks -maxdepth 1 -type f -name "*.ps1.sample" -exec cp {} "$SPEC_DIR/hooks/" \; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The use of find -maxdepth is a GNU extension and will fail on BSD/macOS find. For portability, replace these with simple shell globs or a portable loop, e.g., for f in hooks/*.sample; do [[ -f "$f" && "$f" != *.ps1.sample ]] && cp "$f" "$SPEC_DIR/hooks/"; done and similarly for *.ps1.sample.
find hooks -maxdepth 1 -type f -name "*.sample" ! -name "*.ps1.sample" -exec cp {} "$SPEC_DIR/hooks/" \; | |
;; | |
ps) | |
# Copy PowerShell hook samples (.ps1.sample) | |
find hooks -maxdepth 1 -type f -name "*.ps1.sample" -exec cp {} "$SPEC_DIR/hooks/" \; | |
for f in hooks/*.sample; do | |
[[ -f "$f" && "$f" != *.ps1.sample ]] && cp "$f" "$SPEC_DIR/hooks/" | |
done | |
;; | |
ps) | |
# Copy PowerShell hook samples (.ps1.sample) | |
for f in hooks/*.ps1.sample; do | |
[[ -f "$f" ]] && cp "$f" "$SPEC_DIR/hooks/" | |
done |
Copilot uses AI. Check for mistakes.
### Hook Execution | ||
- Hooks are called with multiple arguments: feature description, feature number, branch name, spec file path | ||
- The `prepare-feature-num` hook should output only the number to stdout | ||
- All hooks have access to environment variables (BRANCH_NAME, SPEC_FILE, FEATURE_NUM) in addition to explicit arguments | ||
- Failed hooks generate warnings but don't stop the specification process | ||
- Non-existent or non-executable hook files are safely ignored |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This bullet implies all hooks receive all four arguments, but earlier sections specify that pre-specify and prepare-feature-num receive only the feature description. Clarify that only post-checkout and post-specify receive (desc, feature number, branch, spec path); pre-specify and prepare-feature-num receive just the description.
Copilot uses AI. Check for mistakes.
Add Git-Style Hook System for Specify Command
Implements a comprehensive hook system for the
/specify
command that allows users to customize the specification creation workflow with Git-style naming conventions.🎯 Addresses Issue
Closes #343 - Add feature number override functionality with hook system
#83
#344
✨ Features Added
Git-Style Hook System
pre-specify
- Pre-workflow validation and setupprepare-feature-num
- Custom feature numbering (replaces auto-increment)post-checkout
- Post-branch creation taskspost-specify
- Post-specification completion tasksCross-Platform Support
.sample
) for Unix/Linux/macOS.ps1.sample
) for WindowsKey Capabilities
BRANCH_NAME
,SPEC_FILE
,FEATURE_NUM
📁 File Structure
hooks/
├── README.md # Comprehensive documentation
├── pre-specify.sample # Bash pre-workflow hook
├── pre-specify.ps1.sample # PowerShell pre-workflow hook├── prepare-feature-num.sample # Bash feature numbering hook
├── prepare-feature-num.ps1.sample # PowerShell feature numbering hook
├── post-checkout.sample # Bash post-branch hook
├── post-checkout.ps1.sample # PowerShell post-branch hook
├── post-specify.sample # Bash post-completion hook
└── post-specify.ps1.sample # PowerShell post-completion hook
🔧 Technical Implementation
Script Enhancements
--feature-num NUMBER
parameter to bash script-FeatureNum NUMBER
parameter to PowerShell scriptCLI Integration
ensure_executable_scripts()
to handle hooksRelease Process
create-release-packages.sh
to include hooks directory.specify/hooks/
structureTemplate Updates
templates/commands/specify.md
with hook execution steps🎨 Hook Execution Flow
pre-specify
→ Validationprepare-feature-num
→ Custom numbering (optional)post-checkout
→ Post-branch setuppost-specify
→ Completion tasks💡 Usage Examples
Custom Feature Numbering
🧪 Testing
📖 Documentation
🔄 Backward Compatibility
🚀 Future Enhancements
This establishes the foundation for extending hooks to /plan and /tasks commands in future PRs.