Skip to content

fix: don't delete temp icon file before notification is enqueued#5

Merged
send merged 3 commits intomainfrom
fix/icon-attachment-cleanup
Mar 1, 2026
Merged

fix: don't delete temp icon file before notification is enqueued#5
send merged 3 commits intomainfrom
fix/icon-attachment-cleanup

Conversation

@send
Copy link
Copy Markdown
Owner

@send send commented Mar 1, 2026

Summary

  • Remove premature removeItem(at: tmpDir) call in createIconAttachment
  • UNNotificationAttachment init only records the file path; the actual file move into the data store happens during center.add(). Deleting the temp directory immediately after init caused "Failed to move attachment file into data store", which made the entire notification fail silently and left no process to handle clicks.

Test plan

  • macnotifier -m "test" --icon ~/.claude/assets/claude-icon.png --sound Glass -a com.github.wez.wezterm shows icon, plays sound, and activates WezTerm on click
  • Original icon file is preserved after notification
  • ./scripts/test.sh passes (15/15)

🤖 Generated with Claude Code

UNNotificationAttachment init only records the file path; the actual
file move happens during center.add(). Removing the temp directory
immediately after init deleted the file before it could be moved,
causing "Failed to move attachment file into data store".

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Copy link
Copy Markdown

Copilot AI left a 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 fixes a macOS notification failure caused by deleting a temporary icon file/directory before the notification request is actually enqueued and the system has moved the attachment into its data store.

Changes:

  • Remove the immediate temp-directory cleanup after UNNotificationAttachment initialization in createIconAttachment.
  • Keep cleanup only on the error path to avoid breaking attachment moves during UNUserNotificationCenter.add().

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Use a single reusable directory (sh.send.macnotifier) instead of
per-notification UUID directories to avoid accumulation in /tmp.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Copy link
Copy Markdown

Copilot AI left a 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 1 out of 1 changed files in this pull request and generated 2 comments.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Avoid race conditions between concurrent processes by using unique
filenames, while cleaning up previous copies best-effort before
creating new ones.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Copy link
Copy Markdown

Copilot AI left a 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 1 out of 1 changed files in this pull request and generated 1 comment.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@send send merged commit a10e4de into main Mar 1, 2026
7 checks passed
@send send deleted the fix/icon-attachment-cleanup branch March 1, 2026 11:09
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.

2 participants