Skip to content

fix(desktop): enable notifications for permission prompts and questions#995

Closed
Kitenite wants to merge 1 commit intomainfrom
kiet-ho/debug-notifs-for-needs-attention
Closed

fix(desktop): enable notifications for permission prompts and questions#995
Kitenite wants to merge 1 commit intomainfrom
kiet-ho/debug-notifs-for-needs-attention

Conversation

@Kitenite
Copy link
Copy Markdown
Collaborator

@Kitenite Kitenite commented Jan 27, 2026

Summary

  • Fix notifications not showing for "needs attention" states (permission prompts, questions)
  • Add Notification hook with permission_prompt and elicitation_dialog matchers to Claude Code settings
  • Update notify shell script to handle Notification events and map them to PermissionRequest

Root Cause

We were only using the PermissionRequest hook which is for decision control (auto-approve/deny), not for receiving notifications when dialogs appear. The Notification hook with specific matchers is what fires when Claude Code actually displays dialogs requiring user attention.

Test plan

  • Restart desktop app to regenerate hook scripts
  • Start a Claude agent and trigger a permission request (e.g., run a bash command)
  • Verify "Input Needed" notification appears
  • Test that agent completion ("Agent Complete") notifications still work

Summary by CodeRabbit

  • New Features

    • Improved notification handling for permission prompts and elicitation dialogs, enabling Claude to trigger notification scripts for these event types.
  • Chores

    • Updated notification system version marker.

✏️ Tip: You can customize this high-level summary in your review settings.

Previously, notifications only worked for agent Stop events but not for
"needs attention" states like permission prompts or questions.

The issue was that we only configured the PermissionRequest hook (for
decision control), but not the Notification hook which actually fires
when Claude Code displays dialogs requiring user attention.

Changes:
- Add Notification hook with permission_prompt and elicitation_dialog matchers
- Update notify shell script to handle Notification events and map them
  to PermissionRequest for desktop notification handling
- Bump notify hook marker version to force script regeneration
@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Jan 27, 2026

📝 Walkthrough

Walkthrough

The changes extend Claude's agent notification system to handle new event types. A Notification hook configuration is added to trigger the notify script for permission-related prompts, the notification script marker is updated to v3, and the notify script logic is enhanced to map Notification events to PermissionRequest events for unified processing.

Changes

Cohort / File(s) Summary
Hook Configuration
apps/desktop/src/main/lib/agent-setup/agent-wrappers.ts
Adds new Notification hook block with matcher pattern "permission_prompt|elicitation_dialog" wired to notifyPath command, enabling Claude Code to trigger notification handling for permission prompts and elicitation dialogs
Notification Script
apps/desktop/src/main/lib/agent-setup/notify-hook.ts, apps/desktop/src/main/lib/agent-setup/templates/notify-hook.template.sh
Updates NOTIFY_SCRIPT_MARKER constant to v3; adds event-type branching logic to detect Notification events, extract notification_type, and remap permission_prompt or elicitation_dialog types to PermissionRequest for downstream processing

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Possibly related PRs

Poem

🐰 Hop! A Notification hook joins the throng,
Permission prompts and dialogs belong,
Remapped to requests with graceful care,
V3's marker marks the change fair!
Scripts dance together, events align—
Claude's agents now have clearer design!

🚥 Pre-merge checks | ✅ 2 | ❌ 1
❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly and specifically describes the main change: enabling notifications for permission prompts and elicitation dialogs, which aligns directly with the changeset's core purpose.
Description check ✅ Passed The description includes a clear summary of changes, root cause explanation, and test plan. While it doesn't follow the exact template sections (Related Issues, Type of Change, etc.), it provides comprehensive context about the PR's objectives and testing approach.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing touches
  • 📝 Generate docstrings

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@Kitenite
Copy link
Copy Markdown
Collaborator Author

Closing: stale notification PR from Jan 27. Notification system has evolved since.

@Kitenite Kitenite closed this Mar 13, 2026
@Kitenite
Copy link
Copy Markdown
Collaborator Author

Hey — just a heads up, this was closed as part of an automated stale PR cleanup. If you think this was done in error, feel free to reopen it!

@Kitenite
Copy link
Copy Markdown
Collaborator Author

Hey — this was closed by an automated cleanup of PRs with major merge conflicts that are 3+ weeks old. If you think this was done incorrectly, please feel free to reopen it. Sorry for any inconvenience!

@Kitenite Kitenite deleted the kiet-ho/debug-notifs-for-needs-attention branch March 15, 2026 16:08
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