Skip to content

fix: remove persistent notifications from retry lock#587

Open
tykeal wants to merge 1 commit intoFutureTense:mainfrom
tykeal:fix-retry-lock-notifications
Open

fix: remove persistent notifications from retry lock#587
tykeal wants to merge 1 commit intoFutureTense:mainfrom
tykeal:fix-retry-lock-notifications

Conversation

@tykeal
Copy link
Collaborator

@tykeal tykeal commented Mar 19, 2026

Summary

Remove persistent notifications from the autolock retry lock feature.

Problem

When the retry lock option fires and the door is open, a persistent notification is created informing the user the lock couldn't lock. When the door closes and retry fires, the old notification is dismissed and a new one is created. These notifications are a holdover from the old automation-based architecture and are no longer needed — lock state is already tracked by entities. Users (see #384) have reported these notifications as an annoyance since they must be manually cleared.

Changes

  • coordinator.py: Removed persistent notification creation in _timer_triggered() (door open notification) and _door_closed() (dismiss + door closed notification). Removed now-unused imports of send_persistent_notification, dismiss_persistent_notification, and slugify.

This is a pure deletion — no new code, no behavioral changes beyond the notification removal. The retry lock logic itself (setting pending_retry_lock, calling _lock_lock() on door close) remains unchanged.

Closes #384

The autolock retry lock feature created persistent notifications when
the door was open (unable to lock) and when the door closed (retrying).
These were a holdover from the old automation-based architecture and
are no longer needed since lock state is tracked by entities.

Removes the send/dismiss of persistent notifications in
_timer_triggered() and _door_closed(), along with the now-unused
imports of send_persistent_notification, dismiss_persistent_notification,
and slugify.

Closes FutureTense#384

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Signed-off-by: Andrew Grimberg <tykeal@bardicgrove.org>
@github-actions github-actions bot added the bugfix Fixes a bug label Mar 19, 2026
@tykeal tykeal requested a review from Copilot March 19, 2026 23:49
@codecov-commenter
Copy link

⚠️ Please install the 'codecov app svg image' to ensure uploads and comments are reliably processed by Codecov.

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 88.80%. Comparing base (cdb4922) to head (a2587d6).
⚠️ Report is 68 commits behind head on main.
❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #587      +/-   ##
==========================================
+ Coverage   84.14%   88.80%   +4.66%     
==========================================
  Files          10       27      +17     
  Lines         801     3207    +2406     
==========================================
+ Hits          674     2848    +2174     
- Misses        127      359     +232     
Flag Coverage Δ
python 88.80% <ø> (?)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Copy link
Contributor

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

Removes persistent notification creation/dismissal from the autolock “retry lock” flow in the Keymaster coordinator, addressing reported user annoyance (#384) now that lock state is tracked via entities.

Changes:

  • Removed send_persistent_notification calls from _timer_triggered() when a retry is queued due to an open door.
  • Removed dismiss_persistent_notification and follow-up “door closed” persistent notification from _door_closed().
  • Cleaned up now-unused imports (slugify, persistent notification helpers).

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

@tykeal tykeal requested review from firstof9 and raman325 March 19, 2026 23:52
Copy link
Collaborator

@firstof9 firstof9 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks good to me.

Is there another notice that get's generated by chance however, similar to when the lock code is used?

I would figure the door not auto locking would be something people would want a notification for.

@tykeal
Copy link
Collaborator Author

tykeal commented Mar 21, 2026

This is all the UI notifications we have left in the system at this point. I'm waffling on completely removing the notification like this does or having it autoclear instead of lingering when there isn't a reason for it to.

@firstof9
Copy link
Collaborator

Keymaster used to have a notification that would auto-clear after the yaml generation was completed.

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

Labels

bugfix Fixes a bug

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Feature Request] Allow disabling persistent_notification creation

4 participants