Skip to content

feat: Add Claude Code hooks, settings, and new commands - #14

Merged
evansenter merged 4 commits into
mainfrom
feat/claude-hooks-and-commands
Dec 28, 2025
Merged

feat: Add Claude Code hooks, settings, and new commands#14
evansenter merged 4 commits into
mainfrom
feat/claude-hooks-and-commands

Conversation

@evansenter

Copy link
Copy Markdown
Owner

Summary

  • Add stop-hook providing git/PR context when Claude Code stops
  • Add settings.json with plugin config and hook registration
  • Add /status-report command for repo status summaries with recommendations
  • Add /pr-feedback command for processing PR review feedback
  • Rename commands for consistency (audit-* prefix, clearer names)
  • Update bootstrap.sh to symlink hooks/ and commands/ directories
  • Update global CLAUDE.md with workflow documentation

Commands

Command Description
/status-report Repo status with recent work, open items, recommendations
/pr-feedback Process PR review feedback with categorization
/pr-followups Find unaddressed/deferred PR comments (renamed)
/audit-codebase Check for AI anti-patterns
/audit-tests Find redundant/stale tests
/audit-issues Categorize open issues (renamed)
/improve-workflow Suggest Claude Code config improvements (renamed)

Test plan

  • Run ./bootstrap.sh -f and verify hooks/ and commands/ symlinked
  • Verify idempotency (re-run bootstrap, no re-linking)
  • Test /status-report output manually

🤖 Generated with Claude Code

- Add stop-hook.sh providing git/PR context on session stop
- Add settings.json with plugin config and hook registration
- Add /status-report command for repo status summaries
- Add /pr-feedback command for processing PR review feedback
- Rename commands for consistency:
  - discover-features → improve-workflow
  - review-prs → pr-followups
  - triage-issues → audit-issues
- Update bootstrap.sh to symlink hooks/ and commands/ directories
- Update CLAUDE.md with autonomous decisions, PR feedback workflow,
  CI handling, and custom commands documentation

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
@claude

claude Bot commented Dec 28, 2025

Copy link
Copy Markdown

PR Review: Claude Code Hooks, Settings, and Commands

This is a substantial enhancement to the Claude Code workflow integration. Overall, the implementation is solid with good attention to detail. Below are my observations organized by category.


Code Quality: ✅ Excellent

Strengths:

  • Clean shell script patterns in stop-hook.sh with proper error handling (set -e)
  • Good use of git subcommands with fallbacks (|| echo "")
  • Consistent markdown formatting across command files
  • Idempotent design in symlink_claude_dir() matching existing symlink_dotfiles() pattern
  • Proper directory existence checks before operations

Minor observations:

  • bootstrap.sh:40 - The exclusion pattern works but creates a very long find command. Consider extracting to a variable for readability

Potential Bugs: ⚠️ One issue found

Critical:

  • stop-hook.sh:28 - Uses {owner}/{repo} placeholders in gh api path, which relies on gh CLI auto-resolution. While this usually works, it's fragile if the git remote is non-standard. Recommend using the more robust approach with 'gh repo view --json nameWithOwner -q .nameWithOwner' first.

Minor:

  • stop-hook.sh:16-18 - check_ci_status() only checks the first check run (.[0].state). If there are multiple checks, this could miss failures. Consider checking all states.

Performance: ✅ Good

  • Lazy evaluation in symlink_claude_dir() with early returns
  • Efficient use of gh api with --jq for filtering server-side
  • No unnecessary subprocess spawning
  • The find command with exclusions is still O(n) on files, which is fine for dotfiles repos

Security: ✅ No concerns

  • Proper quoting of variables throughout
  • Safe use of rm -rf with explicit path checks before removal (bootstrap.sh:62)
  • No command injection vulnerabilities
  • JSON output in stop-hook is properly structured for parsing

Test Coverage: ⚠️ Manual testing only

The PR description mentions manual testing, which is reasonable for infrastructure code. However, consider:

Suggestions for future:

  • Add a test/bootstrap-test.sh script that runs bootstrap in a temporary directory and validates symlinks
  • Add shellcheck to CI for the shell scripts
  • Document expected behavior in edge cases (e.g., what happens if .claude/ already exists as a file instead of directory)

Documentation: ✅ Excellent

  • CLAUDE.md updates clearly explain the new workflow with concrete examples
  • Command files have clear usage instructions and step-by-step guides
  • Good use of tables for categorization in pr-feedback.md
  • Bootstrap changes are well-commented

Suggestions:

  • CLAUDE.md:34-46 - The PR feedback workflow is well-documented, but consider adding an example showing what "Critical" vs "Important" vs "Suggestion" looks like in practice
  • status-report.md:44 - The output format shows placeholder table structures. Consider adding a complete example with realistic data

Architecture & Design: ✅ Well thought out

Strengths:

  • Separation of concerns: hooks vs commands vs settings
  • The stop-hook provides context without making decisions (decision logic delegated to prompt hooks)
  • Command naming convention (audit-, pr-) creates logical grouping
  • Settings file enables plugin management in a declarative way

Observations:

  • settings.json - Enables many plugins by default. This assumes users have these tools installed (rust-analyzer, pyright, clangd, swift-lsp). Consider documenting prerequisites or making this more dynamic.
  • The hook-based auto-continue pattern is clever but could surprise users. Ensure this behavior is documented somewhere visible (README or CLAUDE.md).

Recommendations

Before merge:

  1. Fix the {owner}/{repo} placeholder issue in stop-hook.sh
  2. Consider improving CI status check to handle multiple check runs

Nice to have (can defer):
3. Add shellcheck validation to CI
4. Add example output to command documentation files
5. Document which LSP plugins require which tools in settings.json


Summary

This is high-quality work that significantly enhances the Claude Code workflow. The code follows existing patterns well, is properly documented, and solves real workflow pain points. The issues identified are minor and mostly in the "robustness" category rather than "broken functionality."

Recommendation: Approve with minor fixes for the {owner}/{repo} placeholder issue.

Great job on the comprehensive approach to workflow automation!

- Add prompt hook in settings.json that interprets stop-hook context
- Prompt hook references CLAUDE.md workflow guidance for decisions
- Fix check_ci_status() to use gh API instead of unsupported --json flag
- Check all CI runs and aggregate: failure > pending > success

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
@claude

claude Bot commented Dec 28, 2025

Copy link
Copy Markdown

PR Review: Claude Code Hooks, Settings, and Commands

This PR adds sophisticated automation for Claude Code workflows. Overall, the implementation is well-structured and addresses real workflow needs. I've identified several issues ranging from critical bugs to suggestions for improvement.


Critical Issues

1. Shell Injection Vulnerability in stop-hook.sh

Location: home/.claude/hooks/stop-hook.sh:54-58

Issue: The JSON output template uses unquoted bash variables, which can cause JSON parsing errors or injection if the values contain special characters (quotes, backslashes, newlines).

Recommendation: Use jq to properly escape values.

2. Missing Error Handling in stop-hook.sh

Location: home/.claude/hooks/stop-hook.sh:7

Issue: set -e causes the script to exit on any error (missing gh, not in git repo, API rate limit), preventing Claude from getting any context instead of degrading gracefully.

Recommendation: Remove set -e and handle errors explicitly.


Important Issues

3. Incomplete Feedback Detection

Location: home/.claude/hooks/stop-hook.sh:35

Issue: The feedback check only counts PR line comments, missing review comments, issue comments, and requested changes.

4. Race Condition in CI Status Check

Location: home/.claude/hooks/stop-hook.sh:18-26

Issue: Checks HEAD_SHA from local git instead of the PR's actual head, which can be wrong if remote moved ahead.

5. Hardcoded Rust Commands in pr-feedback.md

Location: home/.claude/commands/pr-feedback.md:94

Issue: Quality gates assume Rust project (cargo fmt && cargo clippy && cargo test), but this is a shell/dotfiles repo.

6. Stop Hook Logic May Be Too Aggressive

Location: home/.claude/settings.json:22

Issue: Auto-continues for ANY comment including LGTM or Thanks acknowledgments. Should only count unresolved review threads or change requests.


Suggestions

  • Add comment in bootstrap.sh explaining why hooks/commands are excluded from file-level symlinking
  • Document what happens in status-report.md when there are 0 items
  • Clarify command vs skill terminology
  • Add uncommitted_changes and untracked_files to stop hook context
  • Add chmod +x after symlinking hooks or document executable requirement
  • Consider adding bats tests for bash scripts

Summary

Strengths:

  • Excellent automation concept addressing real workflow pain points
  • Well-documented commands with clear instructions
  • Thoughtful integration with existing dotfiles architecture

Concerns:

  • Critical JSON injection vulnerability
  • Missing error handling could break Claude sessions
  • Some logic assumptions need refinement

Recommendation: Address the critical issues (#1, #2) before merging. The important issues (#3-6) would improve reliability but aren't blockers. Suggestions are nice-to-haves for future iteration.

Overall, this is a sophisticated addition that will significantly improve Claude Code workflows once the critical bugs are fixed.

stop-hook.sh:
- Use jq for JSON output to prevent shell injection from special chars
- Remove set -e, handle errors gracefully with fallbacks
- Improve feedback detection: count line comments, issue comments, and reviews

pr-feedback.md:
- Remove hardcoded Rust commands, reference project CLAUDE.md instead

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
@claude

claude Bot commented Dec 28, 2025

Copy link
Copy Markdown

PR Review: Claude Code Hooks, Settings, and Commands

This PR adds sophisticated automation for Claude Code workflows with hooks, custom commands, and settings. The implementation is well-structured and follows good practices. Here's my detailed review:


Code Quality & Best Practices ✅

Strengths:

  1. Excellent error handling in stop-hook.sh - Graceful degradation with 2>/dev/null || echo "" fallback pattern throughout
  2. Security-conscious JSON generation - Using jq -n with --arg flags prevents injection vulnerabilities from git output (home/.claude/hooks/stop-hook.sh:65-79)
  3. Idempotent installation - Bootstrap properly checks existing symlinks before creating new ones (bootstrap.sh:55-56)
  4. Clear separation of concerns - Hooks, commands, and settings are properly organized
  5. Good documentation - Commands have clear usage instructions and workflow guidance

Minor Issues:

  1. Regex escaping in stop-hook.sh:32 - The grep pattern ^in_progress\|^queued should use extended regex with -E or escape the pipe:

    # Current (works but unclear)
    elif echo "$STATUSES" | grep -q "^in_progress\|^queued"; then
    
    # Better options:
    elif echo "$STATUSES" | grep -Eq "^(in_progress|queued)"; then
    # OR
    elif echo "$STATUSES" | grep -q "^in_progress" || echo "$STATUSES" | grep -q "^queued"; then
  2. Missing executable check - stop-hook.sh:65 uses jq but doesn't verify it's installed. Consider adding:

    if ! command -v jq >/dev/null 2>&1; then
        echo '{"context": {"error": "jq not installed"}}' >&2
        exit 1
    fi

Potential Bugs 🐛

  1. Race condition in CI status detection (stop-hook.sh:21-28)

    • Gets HEAD_SHA but doesn't verify it matches the PR's head commit
    • If a push occurs between getting PR_NUMBER and HEAD_SHA, you might check the wrong commit's status
    • Fix: Get head SHA from PR data: gh pr view --json headRefOid -q .headRefOid
  2. Integer arithmetic assumes valid numbers (stop-hook.sh:55)

    • If API calls fail and return "0" (string), the arithmetic still works by luck
    • More robust: TOTAL=$((${LINE_COMMENTS:-0} + ${ISSUE_COMMENTS:-0} + ${REVIEWS:-0}))
  3. Directory symlink removal is aggressive (bootstrap.sh:60-61)

    • rm -rf "$dest_dir" removes entire directory trees without backup
    • If user has local modifications in ~/.claude/hooks/, they're lost
    • Recommendation: Add a check or backup step, or document this behavior clearly

Architecture & Design 🏗️

Excellent decisions:

  1. Directory-level symlinks for hooks/commands - Simpler than per-file symlinking, easier to maintain
  2. Stop hook context approach - Providing context for Claude to decide vs hardcoded auto-continue logic
  3. Command-based workflow helpers - Reusable across projects, well-scoped responsibilities

Considerations:

  1. settings.json plugin list - Hardcoded plugin enables for specific LSPs (rust, python, C, swift). Consider if this should be in global vs project-specific config
  2. Stop hook prompt injection - The prompt in settings.json:22 references CLAUDE.md workflow guidance. Ensure CLAUDE.md is always present or hook will have unclear context

Performance Considerations ⚡

  1. Multiple API calls in stop-hook.sh - Makes 4+ GitHub API requests on every stop

    • For repos with many PRs/issues, this could be slow
    • Consider caching or rate limiting if this becomes a bottleneck
    • Currently acceptable for personal dotfiles repo
  2. Feedback count includes all historical comments (stop-hook.sh:50-52)

    • Counts ALL comments, not just unaddressed ones
    • Could lead to false positives if PR has many resolved threads
    • Enhancement: Filter to comments created after last push: gh api "repos/{owner}/{repo}/pulls/$PR_NUMBER/comments" --jq '[.[] | select(.created_at > "LAST_PUSH_TIME")] | length'

Security Concerns 🔒

Good security practices:

  1. ✅ Proper JSON escaping with jq --arg prevents command injection
  2. ✅ No hardcoded credentials or tokens
  3. ✅ Read-only GitHub API operations in hooks

Observations:

  1. Hook execution trust - Stop hook runs arbitrary shell commands. Since it's version-controlled in your dotfiles, this is acceptable, but users should understand hooks execute with full shell access
  2. API credential inheritance - Relies on gh CLI auth, which is good (better than embedding tokens)

Test Coverage 📊

Manual testing completed (per PR body):

  • ✅ Bootstrap symlinking
  • ✅ Idempotency verification
  • /status-report command

Missing automated tests:

  1. No unit tests for stop-hook.sh logic (CI status aggregation, feedback counting)
  2. No integration tests for bootstrap.sh symlink behavior
  3. Commands are markdown specs, no executable validation

Recommendation: For a personal dotfiles repo, manual testing is reasonable. If this becomes a shared tool, consider:

  • ShellSpec or BATS tests for hook scripts
  • Test fixtures for various PR states
  • CI job that runs bootstrap.sh in clean environment

Documentation 📚

Excellent:

  1. Comprehensive CLAUDE.md updates explaining workflow
  2. Clear command usage documentation
  3. Inline comments in stop-hook.sh explain logic

Suggestions:

  1. Add troubleshooting section to CLAUDE.md - What to do if hooks fail, how to debug
  2. Document dependencies - Explicitly list: gh, jq, git required for hooks
  3. Example hook output - Show what the JSON context looks like in practice

Recommended Changes

Critical:

  • None - no blocking issues

Important:

  1. Fix potential race condition in CI status check (use PR's headRefOid)
  2. Add jq availability check in stop-hook.sh
  3. Document or safeguard against directory removal in symlink_claude_dir

Suggestions:

  1. Use grep -E for clearer regex in line 32
  2. Filter feedback count to recent comments (after last push)
  3. Add dependency list to CLAUDE.md
  4. Consider per-project vs global settings.json placement

Summary

This is a high-quality PR that adds valuable automation to your Claude Code workflow. The implementation demonstrates good shell scripting practices, security awareness, and thoughtful design. The identified issues are minor and non-blocking.

Recommendation: Approve with optional follow-up for the improvements noted above.

Great work on the command architecture and hook design! The /pr-feedback command workflow is particularly well thought out.

- Use extended regex (-E) for clearer grep pattern
- Add jq availability check with graceful fallback
- Get head SHA from PR API to avoid race condition with local git

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
@evansenter
evansenter merged commit 9d56a65 into main Dec 28, 2025
1 check passed
@evansenter
evansenter deleted the feat/claude-hooks-and-commands branch December 28, 2025 19:03
@claude

claude Bot commented Dec 28, 2025

Copy link
Copy Markdown

PR Review: Claude Code Hooks and Commands

This is a well-structured PR that adds sophisticated workflow automation for Claude Code. The implementation is solid with good attention to detail.

Strengths

1. Excellent Error Handling

  • stop-hook.sh:11-13 - Graceful degradation when jq is missing
  • stop-hook.sh:17-19 - All git/gh commands have proper fallbacks
  • stop-hook.sh:73-87 - Using jq -n with --arg prevents JSON injection

2. Smart Architecture

  • bootstrap.sh:43-70 - Separate symlink_claude_dir() function is cleaner
  • bootstrap.sh:40 - Exclusion pattern prevents duplicate symlinks
  • stop-hook.sh:28-29 - Using PR headRefOid prevents race conditions

3. Well-Documented

  • pr-feedback.md - Clear categorization with actionable rules
  • stop-hook.sh:2-8 - Header explains purpose and graceful degradation
  • CLAUDE.md:10-27 - Helpful distinction between autonomous vs. discussion-required decisions

Issues and Recommendations

Critical: None identified.

Important:

  1. Potential Data Loss (bootstrap.sh:61-63) - Using rm -rf on directories could delete user data if ~/.claude/hooks/ or ~/.claude/commands/ contains non-symlinked files. Add safety check to verify directory is a symlink before removing.

  2. Redundant Command (stop-hook.sh:17) - head -1 is unnecessary since git log -1 already limits output.

  3. Hook Decision Logic Ambiguity (settings.json:22) - The semantics of block vs approve could be confusing. Consider adding clarifying comment.

Suggestions:

  1. CI Detection (stop-hook.sh:40) - Consider using grep -q "in_progress|queued" instead of regex for robustness

  2. Feedback Count (stop-hook.sh:58-60) - Counts ALL comments, not just unaddressed ones. Consider documenting this limitation.

  3. Bootstrap Feedback (bootstrap.sh:56-57) - Add output when symlink already correct for clarity

  4. Terminology (pr-feedback.md:15) - Uses "skill" but these are commands. Verify if correct term.

Security

No security concerns. Proper escaping and jq usage throughout.

Test Coverage

Missing edge case tests:

  • Bootstrap when ~/.claude/hooks/ is non-symlinked directory
  • stop-hook when not in git repo
  • stop-hook when gh not authenticated

Performance

No concerns. 4-5 API calls on stop is reasonable.

Summary

High-quality work with thoughtful error handling. Main concern is potential data loss in symlink_claude_dir() - recommend addressing before merge.

Assessment: Approve with minor changes recommended

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant