fix: notifications dashboard fails to display some notifications it should#783
Merged
dlvhdr merged 4 commits intodlvhdr:mainfrom Mar 4, 2026
Merged
Conversation
7ea30f5 to
6e288d4
Compare
dlvhdr
reviewed
Feb 27, 2026
Owner
dlvhdr
left a comment
There was a problem hiding this comment.
had a question on your plans for the done store long term
6e288d4 to
3afe37b
Compare
…tivity GitHub notification thread IDs are per-subject, not per-event. When new activity arrives on a thread that was previously marked "done," the API reuses the same thread ID. The old done store only tracked IDs, so these notifications were permanently hidden. Replace the ID-only store with a timestamp-aware DoneStore that records *when* each notification was marked done (using its updated_at). On subsequent fetches, IsDone compares the stored timestamp against the notification's current updated_at — if the notification has been updated since it was marked done, it resurfaces. BREAKING: on first run after upgrade, all previously-done notifications will reappear. The old format stored only IDs without timestamps, so there is no way to tell which notifications have new activity. This is a one-time resurfacing — once dismissed again, proper timestamps are stored and only genuinely new activity will resurface them going forward.
GitHub only retains Done-marked notifications for about 1 week. So, entries older than that in our Done store are dead weight — because the API won’t be return those any longer anyway. So this change causes Dash to prune the Done store on load, with a 14-day cutoff (1 week + 1 week safety margin) — to keep the Done store from growing unbounded. Zero-time (legacy) entries are also pruned; removing those from the Done store has the same effect as keeping them, since IsDone returns false either way.
3afe37b to
d9f687a
Compare
Document the new timestamp-based DoneStore (separate from NotificationIDStore), the IsDone timestamp comparison logic, 14-day pruning on load, sessionMarkedDone tracking, and legacy format backward compatibility.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Bug: The Notifications dashboard had been failing to show particular new notifications that it should be showing: Namely, new notifications in threads which include an older notification that a user marked Done.
Cause: GitHub notification thread IDs are per-subject (per issue/PR), not per-event. When a user marks a notification Done in gh-dash, the thread ID is stored in
done.json. If new activity later arrives on that same thread, GitHub creates a new unread notification with the same thread ID. And because we hadIsDone()only checking whether the ID existed in the store, some new notifications will unexpectedly never get shown — because they have a thread ID we had already marked as Done.Fix: This PR replaces the current ID-only Done store with a timestamp-aware Done store that records when each notification was marked Done (using its
updated_at). On subsequent fetches,IsDonecompares the stored timestamp against the notification's currentupdated_at. If the notification has been updated since it was marked Done, it resurfaces.Additionally, the Done store now prunes stale entries on load: entries older than 14 days and legacy zero-time entries are removed, keeping the file from growing indefinitely. GitHub only retains Done-marked notifications for about 1 week, so entries beyond 14 days are dead weight.
On first run after a dash upgrade with this change included, dash will re-show users all of the notifications they have previously marked as Done. The old format (
done.jsonas a plain JSON array of IDs) stored no timestamps — so there's unfortunately no way to determine which stored notification-thread IDs have new activity vs. which are genuinely stale.But that resurfacing of all Done notifications is a one-time thing. Now, once a user marks any notification as Done, a proper timestamp is stored for it — and then only any genuinely-new notification in its thread will ever be shown.
How it works
updated_attimestampdone.jsonon upgradeIsDonereturns false either way)