feat: opening the notifications inbox marks what it shows as read (#6275)

The web panel and the Android Notifications screen now mark every unread
notice they show as read the moment they appear, so the badge clears on a
look instead of a dismissal. Rows that were unread keep looking new until
the inbox closes. The read goes through read-all with a cutoff of the newest
notice shown plus 1 ms, so one that arrived unseen stays unread. 'Mark all
read' is gone; opening the inbox does its job.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

android/app/src/main/java/com/fabledsword/minstrel/notifications/ui/NotificationsScreen.kt

@@ -16,8 +16,9 @@ import androidx.compose.material3.HorizontalDivider
 import androidx.compose.material3.MaterialTheme
 import androidx.compose.material3.Scaffold
 import androidx.compose.material3.Text
-import androidx.compose.material3.TextButton
 import androidx.compose.runtime.Composable
+import androidx.compose.runtime.DisposableEffect
+import androidx.compose.runtime.LaunchedEffect
 import androidx.compose.runtime.getValue
 import androidx.compose.ui.Alignment
 import androidx.compose.ui.Modifier
@@ -45,8 +46,9 @@ import kotlin.time.Duration.Companion.hours
 import kotlin.time.Duration.Companion.minutes
 
 /**
- * The notifications inbox (M489): one row per notice, its title on one line
- * and how long ago. Tapping a row marks it read and opens what it is about.
+ * The notifications inbox: one row per notice, its title on one line and how
+ * long ago. Showing the screen marks what it shows read; those rows still look
+ * new until the screen goes. Tapping a row opens what it is about.
  */
 @Composable
 fun NotificationsScreen(
@@ -54,7 +56,9 @@ fun NotificationsScreen(
     viewModel: NotificationsViewModel = hiltViewModel(),
 ) {
     val items by viewModel.items.collectAsStateWithLifecycle()
-    val anyUnread = items?.any { it.readAt == null } == true
+    val fresh by viewModel.fresh.collectAsStateWithLifecycle()
+    LaunchedEffect(items) { items?.let(viewModel::seen) }
+    DisposableEffect(Unit) { onDispose(viewModel::left) }
     Scaffold(
         contentWindowInsets = ShellContentWindowInsets,
         modifier = Modifier.fillMaxSize(),
@@ -64,11 +68,6 @@ fun NotificationsScreen(
                 navController = navController,
                 currentRouteName = Notifications::class.qualifiedName,
                 onBack = { navController.popBackStack() },
-                actions = {
-                    if (anyUnread) {
-                        TextButton(onClick = viewModel::markAllRead) { Text("Mark all read") }
-                    }
-                },
             )
         },
     ) { inner ->
@@ -88,6 +87,7 @@ fun NotificationsScreen(
                     items(list, key = { it.id }) { item ->
                         NotificationRow(
                             item = item,
+                            unread = item.readAt == null || item.id in fresh,
                             onClick = {
                                 viewModel.open(item)
                                 routeForLink(item.link)?.let { navController.navigate(it) }
@@ -102,15 +102,14 @@ fun NotificationsScreen(
 }
 
 @Composable
-private fun NotificationRow(item: CachedNotificationEntity, onClick: () -> Unit) {
-    val unread = item.readAt == null
+private fun NotificationRow(item: CachedNotificationEntity, unread: Boolean, onClick: () -> Unit) {
     Row(
         modifier = Modifier
             .fillMaxWidth()
             .clickable(onClick = onClick)
             .padding(horizontal = 16.dp, vertical = 14.dp)
             .semantics(mergeDescendants = true) {
-                if (unread) contentDescription = "Unread: ${item.title}"
+                if (unread) contentDescription = "New: ${item.title}"
             },
         verticalAlignment = Alignment.CenterVertically,
         horizontalArrangement = Arrangement.spacedBy(12.dp),

android/app/src/main/java/com/fabledsword/minstrel/notifications/ui/NotificationsViewModel.kt

@@ -8,16 +8,23 @@ import com.fabledsword.minstrel.connectivity.recoveries
 import com.fabledsword.minstrel.notifications.data.NotificationsRepository
 import dagger.hilt.android.lifecycle.HiltViewModel
 import kotlinx.coroutines.Job
+import kotlinx.coroutines.flow.MutableStateFlow
 import kotlinx.coroutines.flow.SharingStarted
 import kotlinx.coroutines.flow.StateFlow
+import kotlinx.coroutines.flow.asStateFlow
 import kotlinx.coroutines.flow.map
 import kotlinx.coroutines.flow.stateIn
+import kotlinx.coroutines.flow.update
 import kotlinx.coroutines.launch
 import javax.inject.Inject
 
 private const val SHARE_STOP_TIMEOUT_MS = 5_000L
 
-/** `items` is null until the cache has been read once, so the screen can tell "loading" from "empty". */
+/**
+ * `items` is null until the cache has been read once, so the screen can tell "loading" from "empty".
+ * Showing the inbox is reading it: what the screen shows is marked read at once, and stays in
+ * [fresh], so it still looks new while the screen is up.
+ */
 @HiltViewModel
 class NotificationsViewModel @Inject constructor(
     private val repository: NotificationsRepository,
@@ -27,6 +34,11 @@ class NotificationsViewModel @Inject constructor(
         .map<List<CachedNotificationEntity>, List<CachedNotificationEntity>?> { it }
         .stateIn(viewModelScope, SharingStarted.WhileSubscribed(SHARE_STOP_TIMEOUT_MS), null)
 
+    private val freshIds = MutableStateFlow<Set<String>>(emptySet())
+
+    /** Notices that were unread when the screen showed them. */
+    val fresh: StateFlow<Set<String>> = freshIds.asStateFlow()
+
     init {
         refresh()
         // Back online: the cached page may be stale (#1245's recovery idiom).
@@ -43,7 +55,16 @@ class NotificationsViewModel @Inject constructor(
         viewModelScope.launch { repository.markRead(item.id) }
     }
 
-    fun markAllRead() {
+    /** The screen is showing [shown]: any unread among them are now read. */
+    fun seen(shown: List<CachedNotificationEntity>) {
+        val unread = shown.filter { it.readAt == null && it.id !in freshIds.value }
+        if (unread.isEmpty()) return
+        freshIds.update { it + unread.map(CachedNotificationEntity::id) }
         viewModelScope.launch { repository.markAllRead() }
     }
+
+    /** The screen has gone: next time, only what arrived since looks new. */
+    fun left() {
+        freshIds.value = emptySet()
+    }
 }

android/app/src/test/java/com/fabledsword/minstrel/notifications/ui/NotificationsViewModelTest.kt

@@ -0,0 +1,80 @@
+package com.fabledsword.minstrel.notifications.ui
+
+import com.fabledsword.minstrel.cache.db.entities.CachedNotificationEntity
+import com.fabledsword.minstrel.connectivity.NetworkStatusController
+import com.fabledsword.minstrel.connectivity.ServerHealth
+import com.fabledsword.minstrel.notifications.data.NotificationsRepository
+import com.fabledsword.minstrel.testutil.MainDispatcherExtension
+import io.mockk.coVerify
+import io.mockk.every
+import io.mockk.mockk
+import kotlinx.coroutines.flow.MutableStateFlow
+import kotlinx.coroutines.flow.emptyFlow
+import kotlinx.datetime.Instant
+import org.junit.jupiter.api.Test
+import org.junit.jupiter.api.extension.ExtendWith
+import kotlin.test.assertEquals
+
+/** Showing the inbox reads it: unread notices are marked read once, and keep looking new until the screen goes. */
+@ExtendWith(MainDispatcherExtension::class)
+class NotificationsViewModelTest {
+    private val repository: NotificationsRepository = mockk(relaxed = true) {
+        every { notifications } returns emptyFlow()
+    }
+    private val network: NetworkStatusController = mockk {
+        every { state } returns MutableStateFlow(ServerHealth.Healthy)
+    }
+    private val viewModel = NotificationsViewModel(repository, network)
+
+    private fun row(id: String, read: Boolean) = CachedNotificationEntity(
+        id = id,
+        kind = "request_completed",
+        title = "t",
+        body = "b",
+        link = "/requests",
+        createdAt = Instant.parse("2026-10-23T12:00:00Z"),
+        readAt = if (read) Instant.parse("2026-10-23T12:05:00Z") else null,
+    )
+
+    @Test
+    fun `showing unread notices marks them read and keeps them looking new`() {
+        viewModel.seen(listOf(row("a", read = false), row("b", read = true)))
+
+        coVerify(exactly = 1) { repository.markAllRead() }
+        assertEquals(setOf("a"), viewModel.fresh.value)
+    }
+
+    @Test
+    fun `the same notices shown again are not marked twice`() {
+        viewModel.seen(listOf(row("a", read = false)))
+        // The local mark lands and the cache re-emits; a re-emit before it lands repeats the row unread.
+        viewModel.seen(listOf(row("a", read = false)))
+        viewModel.seen(listOf(row("a", read = true)))
+
+        coVerify(exactly = 1) { repository.markAllRead() }
+    }
+
+    @Test
+    fun `a notice arriving while the screen is up is marked too`() {
+        viewModel.seen(listOf(row("a", read = false)))
+        viewModel.seen(listOf(row("new", read = false), row("a", read = true)))
+
+        coVerify(exactly = 2) { repository.markAllRead() }
+        assertEquals(setOf("a", "new"), viewModel.fresh.value)
+    }
+
+    @Test
+    fun `nothing unread marks nothing`() {
+        viewModel.seen(listOf(row("a", read = true)))
+
+        coVerify(exactly = 0) { repository.markAllRead() }
+    }
+
+    @Test
+    fun `leaving the screen stops them looking new`() {
+        viewModel.seen(listOf(row("a", read = false)))
+        viewModel.left()
+
+        assertEquals(emptySet(), viewModel.fresh.value)
+    }
+}

web/src/lib/api/notifications.ts

@@ -44,8 +44,19 @@ export async function markNotificationRead(id: string): Promise<void> {
   await api.post(`/api/me/notifications/${encodeURIComponent(id)}/read`, {});
 }
 
-export async function markAllNotificationsRead(): Promise<void> {
-  await api.post('/api/me/notifications/read-all', {});
+/** upTo limits it to notices created at or before then, so one the inbox never showed stays unread. */
+export async function markAllNotificationsRead(upTo?: string): Promise<void> {
+  await api.post('/api/me/notifications/read-all', upTo ? { up_to: upTo } : {});
+}
+
+/**
+ * The read-all cutoff for what the inbox shows: the newest notice, plus a
+ * millisecond. The server keeps microseconds and Date only milliseconds, so
+ * without the rounding up the newest notice would stay unread.
+ */
+export function readAllCutoff(items: NotificationItem[]): string | undefined {
+  const newest = Math.max(...items.map((i) => Date.parse(i.created_at)).filter((t) => !Number.isNaN(t)));
+  return Number.isFinite(newest) ? new Date(newest + 1).toISOString() : undefined;
 }
 
 export function createNotificationsQuery() {

web/src/lib/components/NotificationBell.svelte

@@ -1,4 +1,6 @@
 <script lang="ts">
+  import { untrack } from 'svelte';
+  import { SvelteSet } from 'svelte/reactivity';
   import { goto } from '$app/navigation';
   import { useQueryClient } from '@tanstack/svelte-query';
   import { Bell } from 'lucide-svelte';
@@ -8,13 +10,14 @@
     createUnreadCountQuery,
     markAllNotificationsRead,
     markNotificationRead,
+    readAllCutoff,
     type NotificationItem
   } from '#lib/api/notifications.js';
   import { relativeTime } from '#lib/utils/relativeTime.js';
-  import { pushToast } from '#lib/stores/toast.svelte.js';
 
-  // The notifications inbox (M489): a bell in the header with an unread badge,
-  // opening a panel of what happened while you were away.
+  // The notifications inbox: a bell in the header with an unread badge,
+  // opening a panel of what happened while you were away. Opening the panel
+  // is reading it: what it shows is marked read at once.
 
   const client = useQueryClient();
   const unreadStore = createUnreadCountQuery();
@@ -41,6 +44,34 @@
     client.invalidateQueries({ queryKey: ['notifications'] });
   }
 
+  // Notices that were unread when the panel showed them. They keep looking
+  // new until it closes, so marking them read doesn't hide what is new.
+  const fresh = new SvelteSet<string>();
+
+  $effect(() => {
+    if (!open) return;
+    const unseen = items.filter((i) => !i.read_at && !fresh.has(i.id));
+    if (unseen.length === 0) return;
+    untrack(() => markSeen(unseen));
+  });
+
+  $effect(() => {
+    if (!open) untrack(() => fresh.clear());
+  });
+
+  async function markSeen(unseen: NotificationItem[]) {
+    for (const i of unseen) fresh.add(i.id);
+    try {
+      await markAllNotificationsRead(readAllCutoff(unseen));
+    } catch {
+      // The badge stays until the next open marks them, which is the honest state.
+      return;
+    }
+    refresh();
+  }
+
+  const looksNew = (item: NotificationItem) => !item.read_at || fresh.has(item.id);
+
   async function openItem(item: NotificationItem) {
     open = false;
     if (!item.read_at) {
@@ -54,15 +85,6 @@
     }
     if (item.link) await goto(item.link);
   }
-
-  async function markAll() {
-    try {
-      await markAllNotificationsRead();
-      refresh();
-    } catch {
-      pushToast("Couldn't mark them read. Try again in a moment.", 'error');
-    }
-  }
 </script>
 
 <svelte:window onclick={handleWindowClick} onkeydown={(e) => e.key === 'Escape' && (open = false)} />
@@ -96,17 +118,8 @@
       role="dialog"
       aria-label="Notifications"
     >
-      <div class="flex items-center justify-between border-b border-border px-3 py-2">
+      <div class="border-b border-border px-3 py-2">
         <span class="text-sm font-medium text-text-primary">Notifications</span>
-        {#if unread > 0}
-          <button
-            type="button"
-            class="rounded px-2 py-1 text-sm text-text-secondary hover:bg-surface-hover hover:text-text-primary focus-visible:ring-2 focus-visible:ring-accent"
-            onclick={markAll}
-          >
-            Mark all read
-          </button>
-        {/if}
       </div>
 
       {#if items.length === 0}
@@ -118,20 +131,20 @@
               <button
                 type="button"
                 data-testid="notification-item"
-                data-unread={item.read_at ? undefined : 'true'}
+                data-unread={looksNew(item) ? 'true' : undefined}
                 class="flex w-full gap-2 px-3 py-2 text-left hover:bg-surface-hover focus-visible:ring-2 focus-visible:ring-accent"
                 onclick={() => openItem(item)}
               >
                 <span
-                  class="mt-1.5 h-2 w-2 shrink-0 rounded-full {item.read_at ? '' : 'bg-text-primary'}"
+                  class="mt-1.5 h-2 w-2 shrink-0 rounded-full {looksNew(item) ? 'bg-text-primary' : ''}"
                   aria-hidden="true"
                 ></span>
                 <span class="min-w-0 flex-1">
                   <span
-                    class="block text-sm {item.read_at ? 'text-text-secondary' : 'font-medium text-text-primary'}"
+                    class="block text-sm {looksNew(item) ? 'font-medium text-text-primary' : 'text-text-secondary'}"
                   >
                     {item.title}
-                    {#if !item.read_at}<span class="sr-only"> (unread)</span>{/if}
+                    {#if looksNew(item)}<span class="sr-only"> (new)</span>{/if}
                   </span>
                   {#if item.body}
                     <span class="block text-sm text-text-secondary">{item.body}</span>

web/src/lib/components/NotificationBell.test.ts

@@ -1,5 +1,7 @@
 import { beforeEach, describe, expect, test, vi } from 'vitest';
 import { render, screen, fireEvent, waitFor } from '@testing-library/svelte';
+import { tick } from 'svelte';
+import { writable } from 'svelte/store';
 import { mockQuery } from '../../test-utils/query';
 
 const invalidateMock = vi.fn();
@@ -12,6 +14,7 @@ vi.mock('#lib/api/notifications.js', async (orig) => {
   const actual = (await orig()) as Record<string, unknown>;
   return {
     badgeLabel: actual.badgeLabel,
+    readAllCutoff: actual.readAllCutoff,
     createUnreadCountQuery: vi.fn(),
     createNotificationsQuery: vi.fn(),
     markNotificationRead: vi.fn().mockResolvedValue(undefined),
@@ -19,8 +22,6 @@ vi.mock('#lib/api/notifications.js', async (orig) => {
   };
 });
 
-vi.mock('#lib/stores/toast.svelte.js', () => ({ pushToast: vi.fn() }));
-
 import NotificationBell from './NotificationBell.svelte';
 import { goto } from '$app/navigation';
 import {
@@ -29,6 +30,7 @@ import {
   createUnreadCountQuery,
   markAllNotificationsRead,
   markNotificationRead,
+  readAllCutoff,
   type NotificationItem
 } from '#lib/api/notifications.js';
 
@@ -137,21 +139,74 @@ describe('NotificationBell', () => {
     expect(markNotificationRead).not.toHaveBeenCalled();
   });
 
-  test('mark all read clears everything and refreshes', async () => {
-    setInbox([item(), item({ id: 'n2' })]);
+  test('opening the panel marks what it shows read, up to the newest notice', async () => {
+    setInbox([
+      item({ id: 'new', created_at: '2026-10-23T12:00:00.123456Z' }),
+      item({ id: 'older', created_at: '2026-10-23T11:00:00Z' })
+    ]);
     render(NotificationBell);
-    await openPanel();
-    await fireEvent.click(screen.getByRole('button', { name: 'Mark all read' }));
+    expect(markAllNotificationsRead).not.toHaveBeenCalled();
 
-    await waitFor(() => expect(markAllNotificationsRead).toHaveBeenCalled());
+    await openPanel();
+
+    await waitFor(() =>
+      expect(markAllNotificationsRead).toHaveBeenCalledWith('2026-10-23T12:00:00.124Z')
+    );
+    expect(markAllNotificationsRead).toHaveBeenCalledTimes(1);
     expect(invalidateMock).toHaveBeenCalledWith({ queryKey: ['notifications'] });
   });
 
-  test('an empty inbox says so and offers no mark all', async () => {
+  test('a notice read on opening still looks new until the panel closes', async () => {
+    const page = (items: NotificationItem[]) => ({
+      data: { items, unread_count: 0 },
+      isPending: false,
+      isError: false,
+      refetch: () => {}
+    });
+    const list = writable(page([item({ id: 'n1' })]));
+    (createUnreadCountQuery as ReturnType<typeof vi.fn>).mockReturnValue(mockQuery({ data: 1 }));
+    (createNotificationsQuery as ReturnType<typeof vi.fn>).mockReturnValue(list);
+    render(NotificationBell);
+    await openPanel();
+    await waitFor(() => expect(markAllNotificationsRead).toHaveBeenCalled());
+
+    // The refetch after marking brings the row back read.
+    list.set(page([item({ id: 'n1', read_at: new Date().toISOString() })]));
+    await tick();
+    expect(screen.getByTestId('notification-item')).toHaveAttribute('data-unread', 'true');
+
+    await openPanel(); // closes
+    await openPanel(); // opens again
+    expect(screen.getByTestId('notification-item')).not.toHaveAttribute('data-unread');
+  });
+
+  test('opening an inbox with nothing unread marks nothing', async () => {
+    setInbox([item({ read_at: new Date().toISOString() })]);
+    render(NotificationBell);
+    await openPanel();
+    expect(markAllNotificationsRead).not.toHaveBeenCalled();
+  });
+
+  test('an empty inbox says so', async () => {
     setInbox([]);
     render(NotificationBell);
     await openPanel();
     expect(screen.getByText('Nothing waiting for you.')).toBeInTheDocument();
-    expect(screen.queryByRole('button', { name: 'Mark all read' })).toBeNull();
+    expect(markAllNotificationsRead).not.toHaveBeenCalled();
+  });
+});
+
+describe('readAllCutoff', () => {
+  test('is a millisecond past the newest notice, so its microseconds are covered', () => {
+    expect(
+      readAllCutoff([
+        item({ created_at: '2026-10-23T11:00:00Z' }),
+        item({ created_at: '2026-10-23T12:00:00.123456Z' })
+      ])
+    ).toBe('2026-10-23T12:00:00.124Z');
+  });
+
+  test('is undefined with nothing to mark', () => {
+    expect(readAllCutoff([])).toBeUndefined();
   });
 });

web/src/lib/components/Shell.test.ts

@@ -33,6 +33,7 @@ vi.mock('#lib/api/notifications.js', async () => {
     createNotificationsQuery: () => mockQuery({ data: { items: [], unread_count: 0 } }),
     markNotificationRead: vi.fn(),
     markAllNotificationsRead: vi.fn(),
+    readAllCutoff: () => undefined,
     badgeLabel: () => ''
   };
 });