fix: ensure marked-as-Done notifications don’t unexpectedly resurface#790
Merged
dlvhdr merged 1 commit intodlvhdr:mainfrom Mar 5, 2026
Merged
Conversation
b388e47 to
65f89b5
Compare
Bug: Notifications resurface immediately after being marked as Done. Cause: The markAsDone closure captured the notification pointer returned by GetCurrNotification(), which points into the Notifications slice. When multiple mark-as-Done operations run concurrently, the TaskFinishedMsg handler removes notifications from the slice via append — shifting elements, and causing the pointer to reference a _different_ notification's data. That resulted in wrong UpdatedAt timestamps being stored in the Done store. Fix: Capture UpdatedAt as a time.Time value (copied by value) _before_ the closure — matching how notificationId is already captured. Also increase the DoneStore pruning cutoff from 14 days to 90 days. GitHub can retain Done-marked notifications for much longer than the previously assumed ~1 week, so entries were being pruned on startup and then resurfacing.
65f89b5 to
75930f1
Compare
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.
Bug: Notifications resurface immediately after being marked as Done.
Cause: The
markAsDoneclosure captured the notification pointer returned byGetCurrNotification(), which points into theNotificationsslice. When multiple mark-as-Done operations run concurrently, theTaskFinishedMsghandler removes notifications from the slice viaappend— shifting elements, and causing the pointer to reference a different notification's data. That resulted in wrongUpdatedAttimestamps being stored in the Done store.Fix: Capture
UpdatedAtas atime.Time value(copied by value) before the closure — matching hownotificationIdis already captured.Also increases the DoneStore pruning cutoff from 14 days to 90 days. GitHub can retain Done-marked notifications for much longer than the previously assumed ~1 week, so entries were being pruned on startup and then resurfacing.