-
Notifications
You must be signed in to change notification settings - Fork 1
feat(claude): refactor Stop hook to use external notification script #22
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
Conversation
- Extract notification logic from settings.json to separate bash script - Add notify.sh script that displays last Claude prompt in macOS notification - Improve maintainability and reusability of notification functionality Co-Authored-By: Claude <noreply@anthropic.com> Co-Authored-By: rogue <49047211+shlifedev@users.noreply.github.com>
Summary of ChangesHello @kimchanhyung98, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request significantly upgrades the user notification experience for the Claude application. By centralizing notification logic into a dedicated shell script, the system can now dynamically fetch and display the last user prompt, offering immediate and relevant context rather than a generic 'Task completed' message. This change streamlines how users receive feedback from Claude, making interactions more informative and efficient. Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
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.
Code Review
This pull request refactors the notification mechanism to use an external script, which is a good improvement for modularity. The new script notify.sh introduces some good features like showing the last prompt. However, I've identified a few issues related to robustness and correctness. Specifically, the script can fail if the prompt contains special characters, it doesn't handle the absence of the history file gracefully, and it has an unstated dependency on jq. Additionally, the path to the script in settings.json is fragile. My review includes suggestions to address these points to make the new notification system more reliable.
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.
2 files reviewed, 2 comments
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 pull request refactors the Stop hook notification system for Claude by extracting the notification logic into a dedicated shell script. The script reads the last prompt from Claude's history file and displays it in a macOS notification, providing more context than the previous static message.
Changes:
- Created a new
notify.shscript that extracts the last prompt from Claude's JSONL history file and displays it in a macOS notification (truncated to 25 characters) - Updated the Stop hook configuration to invoke the new script instead of using an inline osascript command
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 6 comments.
| File | Description |
|---|---|
.claude/settings.json |
Updated Stop hook command to call the new notify.sh script |
.claude/hooks/notify.sh |
New shell script that reads Claude history and sends notifications with the last prompt text |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
This pull request updates the notification mechanism for Claude by introducing a new shell script to handle notifications and updating the related configuration to use this script. The main improvement is that notifications now display the last prompt from Claude's history, providing more context in the notification.
Notification system improvements:
notify.shin.claude/hooks/that reads the last prompt from the Claude history file and sends a macOS notification with this content..claude/settings.jsonto use the newnotify.shscript for notifications instead of a static message.Greptile Summary
Refactored notification system to extract and display the last Claude prompt from history, moving from inline osascript to an external
notify.shscript.notify.shscript that reads~/.claude/history.jsonl, extracts the.displayfield, and shows it in macOS notifications.claude/settings.jsonStop hook to call the new scriptnotify.sh- unescaped variable interpolation in osascript commandConfidence Score: 2/5
notify.shis a critical security issue - if the.displayfield in the history file contains shell metacharacters (quotes, backticks, or command substitution), they will be executed. While this is a local configuration file, it's a security best practice to properly escape user-controlled data before passing to shell commands..claude/hooks/notify.shrequires immediate attention to fix command injection vulnerabilityImportant Files Changed
Sequence Diagram
sequenceDiagram participant Claude participant Stop Hook participant notify.sh participant tail participant jq participant osascript participant macOS Claude->>Stop Hook: Task completed event Stop Hook->>notify.sh: Execute bash .claude/hooks/notify.sh notify.sh->>tail: Read last line of ~/.claude/history.jsonl tail-->>notify.sh: Last JSONL entry notify.sh->>jq: Parse .display field jq-->>notify.sh: Display text (or empty) notify.sh->>notify.sh: Truncate to 25 chars notify.sh->>osascript: display notification with message osascript->>macOS: Show system notification macOS-->>User: Notification with last prompt