Skip to content

Commit 8a0f4be

Browse files
committed
fix: filter basic save to avoid restoring user-deleted notifications
When checkNotifications' basic save ran after a markAsRead had already removed a notification, it would unconditionally write the GitHub API response back to storage, restoring the deleted entry. Re-read current storage before the basic save and apply safeBasic filtering: new notifications are kept unconditionally, but pre-existing ones are only kept if still present in storage (not user-deleted). Update desktop notifications to use the same filtered list.
1 parent 90f85fc commit 8a0f4be

1 file changed

Lines changed: 21 additions & 6 deletions

File tree

src/background/service-worker.js

Lines changed: 21 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -326,7 +326,6 @@ async function checkNotifications() {
326326

327327
if (result) {
328328
const { items: notifications, hasMore } = result;
329-
hasMoreNotifications = hasMore;
330329

331330
// Check if a newer fetch has already started
332331
if (currentFetchVersion < notificationFetchVersion) {
@@ -374,9 +373,25 @@ async function checkNotifications() {
374373
return;
375374
}
376375

377-
// Save basic data immediately - popup can display now
378-
await storage.setNotifications(basicProcessed);
379-
await updateBadge(basicProcessed.length, hasMore);
376+
// Re-read current storage to catch any user actions (markAsRead etc.) that
377+
// completed while the fetch was in-flight. Keep new notifications unconditionally,
378+
// but only keep pre-existing ones if they're still in storage (not user-deleted).
379+
const currentStored = await storage.getNotifications();
380+
// Re-check version after async read: a user mark-as-read may have bumped
381+
// notificationFetchVersion while we were reading, making our snapshot stale.
382+
if (currentFetchVersion < notificationFetchVersion) {
383+
console.log(`Fetch #${currentFetchVersion} superseded during safeBasic re-read, aborting`);
384+
return;
385+
}
386+
const currentStoredIds = new Set(currentStored.map((n) => n.id));
387+
const safeBasic = basicProcessed.filter((n) => !existingIds.has(n.id) || currentStoredIds.has(n.id));
388+
389+
// Save basic data immediately - popup can display now.
390+
// Update hasMoreNotifications here (inside all version checks) so it only
391+
// reflects fetches that actually commit to storage.
392+
hasMoreNotifications = hasMore;
393+
await storage.setNotifications(safeBasic);
394+
await updateBadge(safeBasic.length, hasMore);
380395

381396
// Second pass: Fetch details asynchronously for new/updated notifications
382397
// Create a deep copy to avoid race conditions with concurrent updates
@@ -486,8 +501,8 @@ async function checkNotifications() {
486501
});
487502
}
488503

489-
// Show desktop notifications for new items (using basic data)
490-
await showDesktopNotificationsForNew(basicProcessed);
504+
// Show desktop notifications for new items (using safe-filtered list)
505+
await showDesktopNotificationsForNew(safeBasic);
491506
}
492507
} catch (error) {
493508
console.error(`Failed to check notifications (fetch #${currentFetchVersion}):`, error);

0 commit comments

Comments
 (0)