docs: Document PostToolUse hook limitation and skill-based workaround#2
docs: Document PostToolUse hook limitation and skill-based workaround#2
Conversation
- Add KNOWN_ISSUES.md with comprehensive PostToolUse analysis - Create GitHub issue template for PostToolUse limitation - Update analyze-commits.sh header to clarify manual-only usage - Enable proactive mode for git-memory-tracker skill - Clarify that skill-based approach is the recommended workaround This documents the discovery that PostToolUse hooks don't fire in Claude Code, and that the git-memory-tracker skill is the working alternative. The skill provides better UX anyway (explicit control, batch tracking, retroactive analysis). Related: Testing in v1.2.0 revealed PostToolUse never triggers 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
|
Caution Review failedThe pull request is closed. WalkthroughDocuments a limitation where PostToolUse hooks with matcher "Bash" do not fire; adds templates and KNOWN_ISSUES, updates Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant Dev as Developer (runs git commit)
participant Shell as Bash Hook (PostToolUse)
participant Skill as git-memory-tracker Skill
participant Script as scripts/analyze-commits.sh
participant Store as Memory Store
rect rgb(255,245,230)
note right of Shell: Expected flow (not firing for Bash)
Dev->>Shell: git commit (PostToolUse)
Shell-->>Dev: (no firing / no input)
end
rect rgb(230,255,245)
note right of Skill: Manual/workaround flow (works)
Dev->>Skill: invoke git-memory-tracker (manual or proactive)
Skill->>Script: run analyze-commits.sh (stdin or args)
Script->>Store: send sanitized JSON (json_escape)
Store-->>Skill: confirm/store entries (MEMORY_COMMITS_COUNT++)
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
Possibly related PRs
Suggested labels
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (4)
Comment |
PR Compliance Guide 🔍Below is a summary of compliance checks for this PR:
Compliance status legend🟢 - Fully Compliant🟡 - Partial Compliant 🔴 - Not Compliant ⚪ - Requires Further Human Verification 🏷️ - Compliance label |
|||||||||||||||||||||||
PR Code Suggestions ✨No code suggestions found for the PR. |
There was a problem hiding this comment.
Important
Looks good to me! 👍
Reviewed everything up to 688ce47 in 1 minute and 37 seconds. Click for details.
- Reviewed
333lines of code in4files - Skipped
0files when reviewing. - Skipped posting
4draft comments. View those below. - Modify your settings and rules to customize what types of comments Ellipsis leaves. And don't forget to react with 👍 or 👎 to teach Ellipsis.
1. .github/ISSUE_TEMPLATE/posttooluse-hooks.md:166
- Draft comment:
Consider adding a reference/link to KNOWN_ISSUES.md for additional context on the PostToolUse limitation. - Reason this comment was not posted:
Comment did not seem useful. Confidence is useful =0%<= threshold50%The comment suggests adding a reference to a documentation file for additional context. This is a purely informative comment and does not provide a specific code suggestion or highlight a potential issue with the code itself. It violates the rule against making purely informative comments.
2. KNOWN_ISSUES.md:90
- Draft comment:
Replace the placeholder '[Create GitHub issue for this]' with an actual URL or reference if available. - Reason this comment was not posted:
Decided after close inspection that this draft comment was likely wrong and/or not actionable: usefulness confidence = 10% vs. threshold = 50% This comment is essentially asking the author to either create a GitHub issue right now or remove the placeholder. However, the placeholder is clearly intentional - it's a documentation file tracking known issues, and the author may want to merge this first before creating the issue, or may want to leave it as a TODO. The comment doesn't identify a bug or problem with the code - it's just suggesting the author do more work. This falls under "Do NOT comment unless there is clearly a code change required" and "Do NOT make comments that are obvious or unimportant." The author clearly knows this is a placeholder and can decide when/if to create the issue. The placeholder might actually be problematic if this documentation is meant to be user-facing, as it looks unprofessional. The comment could be seen as a reasonable suggestion to clean up the documentation before merging. While the placeholder might look unprofessional, this is a KNOWN_ISSUES.md file that appears to be internal documentation. The author clearly left this as a TODO intentionally, and it's their decision whether to create the issue before or after merging. The comment is not identifying a bug or required code change - it's just suggesting additional work that the author may have already planned. This comment should be deleted. It's asking the author to do additional work (create a GitHub issue or remove the placeholder) that isn't a required code change. The placeholder is clearly intentional, and the author can decide when to address it.
3. scripts/analyze-commits.sh:96
- Draft comment:
The call to 'log' is undefined. Consider replacing it with 'echo' or defining a logging function. - Reason this comment was not posted:
Comment was not on a location in the diff, so it can't be submitted as a review comment.
4. skills/git-memory-tracker/SKILL.md:2
- Draft comment:
Good update: proactive mode is now enabled and documentation is comprehensive. - Reason this comment was not posted:
Confidence changes required:0%<= threshold50%None
Workflow ID: wflow_nlniG4uZc1JGN8FY
You can customize by changing your verbosity settings, reacting with 👍 or 👎, replying to comments, or adding code review rules.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
skills/git-memory-tracker/SKILL.md (1)
112-129: Clarify MCP tool invocation syntax consistency.The examples show two different invocation styles: piped JSON (line 112–117) and inline JavaScript object syntax (line 124–129). The actual script in
analyze-commits.sh(line 164) uses the piped format. To avoid confusion, either unify the examples or clearly note that these are pseudo-code patterns. The inline JavaScript syntax at line 124 may mislead readers into thinking it's executable shell code.Consider updating lines 124–129 to match the actual pipe-based invocation shown in
analyze-commits.sh:### Step 3: Create Summary After analyzing all commits, create a summary memory: -```javascript -mcp__memory-store__record({ - "memory": "Feature branch merged: <branch-name> with <N> commits", - "background": "Commits: <list>. Total files changed: <count>. Date range: <start> to <end>", - "importance": "high" -}) -``` +```bash +mcp__memory-store__record '{ + "memory": "Feature branch merged: <branch-name> with <N> commits", + "background": "Commits: <list>. Total files changed: <count>. Date range: <start> to <end>", + "importance": "high" +}' | claude mcp call memory-store record +```
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
.github/ISSUE_TEMPLATE/posttooluse-hooks.md(1 hunks)KNOWN_ISSUES.md(1 hunks)scripts/analyze-commits.sh(2 hunks)skills/git-memory-tracker/SKILL.md(1 hunks)
🔇 Additional comments (10)
skills/git-memory-tracker/SKILL.md (2)
1-9: Proactive mode enabled correctly.The YAML frontmatter and banner clearly signal proactive mode, aligning with the PR objective to encourage manual commit tracking when appropriate.
41-357: Excellent comprehensive skill documentation.The documentation is well-structured, with clear use-case guidance, practical examples, error handling, and testing instructions. The integration section effectively contextualizes the skill within the larger system. This provides users with both quick-start guidance and advanced usage patterns.
KNOWN_ISSUES.md (1)
1-96: Well-structured known issues documentation.The documentation clearly articulates the PostToolUse hook limitation, provides evidence-based root cause analysis, distinguishes between working and non-working hook types, and presents a practical workaround. The Medium severity rating is appropriate given that the workaround (git-memory-tracker skill) provides full manual functionality. The metadata and version range align with the PR context (v1.2.0 testing).
scripts/analyze-commits.sh (4)
2-10: Header comments clearly document manual invocation model.The updated header accurately reflects the shift from automatic hook-based execution to manual invocation via the git-memory-tracker skill. The reference to the skill documentation and explanation of why hooks aren't used provides context.
14-28: JSON escaping utility is robust and cross-platform.The
json_escapefunction uses awk for portability and correctly escapes all essential JSON special characters (backslash, quote, newline, tab, carriage return). This prevents command injection and ensures safe payload construction even with user input or commit messages containing special characters.
30-36: Stdin handling aligns with manual invocation model.The stdin discard logic (lines 33–36) prevents accidental pipeline contamination and is consistent with the documented shift from hook-based to manual invocation. This is a reasonable defensive measure for manual script execution.
148-176: JSON payload construction with escaping is secure.The script correctly applies
json_escapeto both COMMIT_SUMMARY and BACKGROUND_CONTEXT before interpolation into the JSON payload, preventing injection vulnerabilities. The MCP invocation uses piped JSON with appropriate error filtering, and the output JSON includes informational context for the user..github/ISSUE_TEMPLATE/posttooluse-hooks.md (3)
1-80: Comprehensive issue template with clear reproduction steps.The template effectively documents the PostToolUse hook limitation, provides actionable reproduction steps with debug logging, and clearly distinguishes between working (SessionStart, SessionEnd, PreToolUse, PreCompact, Notification) and non-working (PostToolUse with Bash) hooks. The environment section requests appropriate context for investigation.
82-136: Investigation guidance and proposed solutions are well-reasoned.The theories presented (PostToolUse not implemented, marketplace vs local plugin differences, async execution) are plausible and guide future investigation. The three proposed solutions (enable PostToolUse support, alternative hook mechanisms, document skill-based approach) represent realistic resolution paths. The current workaround section effectively communicates benefits of the manual approach.
137-168: Impact assessment and documentation are accurate and well-coordinated.The Medium severity rating aligns with KNOWN_ISSUES.md, and the impact summary correctly identifies what still works (file tracking, session tracking, manual commit tracking). Cross-references to related files (KNOWN_ISSUES.md, analyze-commits.sh, SKILL.md) are consistent with the broader PR changes and help users find additional documentation.
Based on research showing skills activate only 20% of the time with vague descriptions, updated all skill frontmatter with specific trigger words and use cases to improve activation reliability. Changes: - git-memory-tracker: Added triggers 'track this commit', 'analyze commits' - memory-auto-track: Added triggers 'how did we', 'what patterns' - memory-context-retrieval: Added triggers 'who worked on', 'show me previous' - anchor-suggester: Added triggers 'add anchors', 'document this code' - Enabled proactive: true on all skills Research shows specific trigger words in descriptions can improve activation from 20% baseline to 80-84% success rate. References: - https://scottspence.com/posts/how-to-make-claude-code-skills-activate-reliably - Claude Code skills documentation 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
There was a problem hiding this comment.
Important
Looks good to me! 👍
Reviewed 7d6e33c in 48 seconds. Click for details.
- Reviewed
53lines of code in4files - Skipped
0files when reviewing. - Skipped posting
4draft comments. View those below. - Modify your settings and rules to customize what types of comments Ellipsis leaves. And don't forget to react with 👍 or 👎 to teach Ellipsis.
1. skills/anchor-suggester/SKILL.md:1
- Draft comment:
Updated description is more detailed and clarifies use cases. The removal of the 'trigger: conversational' field in favor of 'proactive: true' seems intentional, but ensure this aligns with the intended activation mechanism. - Reason this comment was not posted:
Confidence changes required:0%<= threshold50%None
2. skills/git-memory-tracker/SKILL.md:1
- Draft comment:
The revised description now clearly outlines when and how to use the skill. This makes the usage scenarios more explicit and user-friendly. - Reason this comment was not posted:
Confidence changes required:0%<= threshold50%None
3. skills/memory-auto-track/SKILL.md:1
- Draft comment:
The updated description provides a more comprehensive explanation of when the memory auto-track skill should be invoked, which improves clarity and consistency. - Reason this comment was not posted:
Confidence changes required:0%<= threshold50%None
4. skills/memory-context-retrieval/SKILL.md:1
- Draft comment:
The new description and inclusion of 'proactive: true' simplify the skill definition. Note that the removal of the 'capabilities' field should be verified to not affect any downstream integrations if they rely on that metadata. - Reason this comment was not posted:
Confidence changes required:0%<= threshold50%None
Workflow ID: wflow_oEXUtRvhmC359tJq
You can customize by changing your verbosity settings, reacting with 👍 or 👎, replying to comments, or adding code review rules.
Update: Skill Activation Improvements AddedBased on research showing Claude Code skills activate only 20% of the time with vague descriptions, I've improved all skill descriptions with specific trigger words: Changes in latest commit (7d6e33c):
Expected improvement: 20% → 80-84% activation success rate References:
Related IssueCreated issue #3 to track the PostToolUse limitation separately. Ready for review! This PR now includes:
|
…tion Closes #3 This PR documents the PostToolUse hook limitation discovered during v1.2.0 testing and improves skill activation from 20% to 80-84%. Changes: - Add KNOWN_ISSUES.md with PostToolUse analysis - Create GitHub issue template - Update analyze-commits.sh documentation - Improve all skill descriptions with trigger words - Add CONTRIBUTING.md with branch workflow - Enable proactive mode on all skills 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
User description
Summary
Documents the PostToolUse hook limitation discovered during v1.2.0 testing and establishes the git-memory-tracker skill as the recommended workaround.
Changes
Documentation Added
KNOWN_ISSUES.md - Comprehensive analysis of PostToolUse limitation
GitHub Issue Template -
.github/ISSUE_TEMPLATE/posttooluse-hooks.mdCode Updates
scripts/analyze-commits.sh - Updated header comments
skills/git-memory-tracker/SKILL.md - Enabled proactive mode
proactive: truein frontmatterTesting Discovery
During v1.2.0 testing, we discovered:
What Doesn't Work ❌
matcher: "Bash"never fireWhat Does Work ✅
Workaround: Skill-Based Tracking
The git-memory-tracker skill provides a better solution:
User triggers:
Advantages over automatic hooks:
Impact
Severity: Medium (workaround available)
Users are NOT blocked:
Session counter tracking:
Why Document Instead of Remove?
Related Commits
Testing
Verified:
Checklist
Recommendation: Merge this to establish clear documentation of the limitation and promote the skill-based workaround as the canonical solution.
🤖 Generated with Claude Code
PR Type
Documentation
Description
Documents PostToolUse hook limitation discovered in v1.2.0 testing
Establishes git-memory-tracker skill as recommended workaround
Creates GitHub issue template for community tracking
Enables proactive mode for skill-based commit tracking
Diagram Walkthrough
File Walkthrough
KNOWN_ISSUES.md
Comprehensive PostToolUse limitation documentationKNOWN_ISSUES.md
analysis
posttooluse-hooks.md
GitHub issue template for hook limitation tracking.github/ISSUE_TEMPLATE/posttooluse-hooks.md
results
analyze-commits.sh
Clarify script as manual skill invocation onlyscripts/analyze-commits.sh
execution
Code
SKILL.md
Enable proactive mode for skill-based trackingskills/git-memory-tracker/SKILL.md
proactive: truein frontmatterSummary by CodeRabbit
Documentation
Features
Scripts
✏️ Tip: You can customize this high-level summary in your review settings.
Important
Documents PostToolUse hook limitation and recommends git-memory-tracker skill as a workaround, with updates to documentation and code.
KNOWN_ISSUES.mddetailing PostToolUse hook limitation and workaround usinggit-memory-trackerskill..github/ISSUE_TEMPLATE/posttooluse-hooks.mdfor community issue tracking.scripts/analyze-commits.shto clarify manual invocation and remove misleading hook references.skills/git-memory-tracker/SKILL.mdto suggest commit tracking.This description was created by
for 7d6e33c. You can customize this summary. It will automatically update as commits are pushed.