fix(hub): a dismissed notification stays dismissed across a restart

Clicking a notification navigated to the group and the entry disappeared —
the intended behaviour — and it was back on the next launch.

Neither half was wrong on its own, which is why it survived. `markRead` drops
the entry locally *and* marks it read on the hub, deliberately: "Reading it is
the point of clicking it: it goes, here and in the count, rather than sitting
there greyed out." The hub honoured that and persisted it. But the startup
fetch asked for `/v1/notifications?limit=20` with no filter, and the endpoint
returns read and unread alike, so every dismissed notification came straight
back. It asks for `unread_only=true` now — a parameter the endpoint already
had and already tested.

The feed still carried the fossil of the older intent:
`class="notif-item ${n.read ? '' : 'notif-unread'}"`, styling for a read entry
rendered greyed out, from before clicking meant dismissing. Nothing read
reaches the feed any more, so that branch was dead code describing behaviour
the application had abandoned — and noticing it is what made the two halves'
disagreement visible. Removed.

`unread_count` is computed server-side over the whole table and is unaffected
by the filter, so the bell is unchanged.

test_notification_dismissal.py holds both halves: the API round trip that is
the reported bug (list, read, list again), its mirror showing the unfiltered
endpoint still returns it — so the fix cannot read as a coincidence — and a
static check that the SPA asks for the filter, which is the only one of the
three that catches the defect that actually happened. Verified by dropping the
parameter again: that one fails, the API tests do not.

Read notifications now accumulate unread in the table rather than being
deleted. Purge removes them; the volume is small. Making dismissal a delete
would suit `purge_notifications`' own docstring — "these are signals, not a
record" — but it would leave `/read` a misnomer and `read-all` inconsistent,
so it is a separate decision.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_014UtzVrzM7e2tG9fSpkR9ML

2 files changed, 152 insertions, 2 deletions

diff --git a/packages/meshbay-hub/src/meshbay_hub/static/app.js b/packages/meshbay-hub/src/meshbay_hub/static/app.js
index 06b6c10..7cc7c83 100644
--- a/packages/meshbay-hub/src/meshbay_hub/static/app.js
+++ b/packages/meshbay-hub/src/meshbay_hub/static/app.js
@@ -342,7 +342,7 @@ function NotificationFeed({ notifications, onMarkRead, onPurge }) {
         </button>
       </h3>
       ${notifications.map(n => html`
-        <div key=${n.id} class="notif-item ${n.read ? '' : 'notif-unread'}"
+        <div key=${n.id} class="notif-item notif-unread"
           onClick=${() => {
             // Reading it is the point of clicking it: it goes, here and in the
             // count, rather than sitting there greyed out.
@@ -579,7 +579,13 @@ function App() {
     if (!user || notifDisabled) {
       setNotifications([]); setUnreadCount(0); return;
     }
-    hubFetch('/v1/notifications?limit=20', { token: user.token })
+    // `unread_only`: clicking one is what dismisses it (see markRead), so a
+    // read notification is a dismissed notification and must not come back on
+    // the next launch. Without this the two halves disagreed — the click
+    // removed it here and marked it read on the hub, and the next startup
+    // asked for everything and put it straight back. `unread_count` is
+    // computed server-side and is unaffected by the filter.
+    hubFetch('/v1/notifications?limit=20&unread_only=true', { token: user.token })
       .then(data => {
         setNotifications(data.notifications || []);
         setUnreadCount(data.unread_count || 0);
diff --git a/packages/meshbay-hub/tests/test_notification_dismissal.py b/packages/meshbay-hub/tests/test_notification_dismissal.py
new file mode 100644
index 0000000..7324656
--- /dev/null
+++ b/packages/meshbay-hub/tests/test_notification_dismissal.py
@@ -0,0 +1,144 @@
+"""
+A dismissed notification stays dismissed across a restart.
+
+Reported live: clicking a notification navigated to the group and the entry
+disappeared — the intended behaviour — and it was back on the next launch of
+the application.
+
+Neither half was wrong on its own, which is why it survived. `markRead` in
+app.js drops the entry locally *and* marks it read on the hub, deliberately:
+"Reading it is the point of clicking it: it goes, here and in the count,
+rather than sitting there greyed out." The hub honoured that and persisted it.
+But the startup fetch asked for `/v1/notifications?limit=20` with no filter,
+and the endpoint returns read and unread alike, so every dismissed
+notification came straight back. The feed still carried the fossil of the
+older intent — `class="notif-item ${n.read ? '' : 'notif-unread'}"`, styling
+for a read entry rendered greyed out, from before clicking meant dismissing.
+
+So the contract is between the two halves, and that is what this pins: the
+hub's filter behaves, and the SPA asks for it.
+"""
+
+import re
+from pathlib import Path
+
+import pytest
+from cryptography.hazmat.primitives.asymmetric.ed25519 import Ed25519PrivateKey
+from cryptography.hazmat.primitives.asymmetric.x25519 import X25519PrivateKey
+
+from meshbay_common.crypto import pk_to_b64
+from meshbay_hub.api.deps import set_admin_usernames
+
+STATIC = Path(__file__).resolve().parents[1] / "src" / "meshbay_hub" / "static"
+APP = STATIC / "app.js"
+
+
+def _gen_user_keys():
+    return (pk_to_b64(Ed25519PrivateKey.generate().public_key()),
+            pk_to_b64(X25519PrivateKey.generate().public_key()))
+
+
+async def _register(client, username, email):
+    pk_ed, pk_x = _gen_user_keys()
+    r = await client.post("/v1/users/register", json={
+        "username": username, "email": email, "password": "testpass99",
+        "pk_user_ed25519": pk_ed, "pk_user_x25519": pk_x,
+    })
+    assert r.status_code == 201
+    return r.json()["user_id"]
+
+
+async def _login(client, username):
+    r = await client.post("/v1/users/login",
+                          json={"username": username, "password": "testpass99"})
+    assert r.status_code == 200
+    return r.json()["access_token"]
+
+
+async def _one_notification_for_alice(client):
+    """A role change is the cheapest thing that notifies somebody."""
+    await _register(client, "admin", "admin@x.com")
+    set_admin_usernames(["admin"])
+    admin_token = await _login(client, "admin")
+    uid = await _register(client, "alice", "a@x.com")
+    alice_token = await _login(client, "alice")
+    await client.patch(f"/v1/admin/users/{uid}", json={"role": "moderator"},
+                       headers={"Authorization": f"Bearer {admin_token}"})
+    return alice_token
+
+
+@pytest.mark.asyncio
+async def test_a_read_notification_is_gone_from_what_the_spa_asks_for(client):
+    """The reported bug, as the two requests the application actually makes.
+
+    The second GET is the next launch. Before the fix it answered with the
+    notification the user had just dismissed.
+    """
+    token = await _login_and_notify(client)
+    auth = {"Authorization": f"Bearer {token}"}
+
+    listing = await client.get("/v1/notifications?limit=20&unread_only=true", headers=auth)
+    entries = listing.json()["notifications"]
+    assert len(entries) == 1, "the fixture should have produced exactly one"
+
+    r = await client.post(f"/v1/notifications/{entries[0]['id']}/read", headers=auth)
+    assert r.status_code == 200
+
+    relaunch = await client.get("/v1/notifications?limit=20&unread_only=true", headers=auth)
+    assert relaunch.json()["notifications"] == [], (
+        "a dismissed notification came back on the next launch")
+    assert relaunch.json()["unread_count"] == 0
+
+
+@pytest.mark.asyncio
+async def test_without_the_filter_it_does_come_back(client):
+    """The other half of the same fact, so the fix cannot be read as a
+    coincidence: unfiltered, the endpoint still returns it, and that is the
+    request the SPA used to make."""
+    token = await _login_and_notify(client)
+    auth = {"Authorization": f"Bearer {token}"}
+
+    nid = (await client.get("/v1/notifications", headers=auth)).json()["notifications"][0]["id"]
+    await client.post(f"/v1/notifications/{nid}/read", headers=auth)
+
+    unfiltered = (await client.get("/v1/notifications?limit=20", headers=auth)).json()
+    assert len(unfiltered["notifications"]) == 1
+    assert unfiltered["notifications"][0]["read"] is True
+
+
+# `_one_notification_for_alice` is the setup; this alias keeps the tests reading
+# as what they are about rather than as their fixture.
+_login_and_notify = _one_notification_for_alice
+
+
+@pytest.mark.skipif(not APP.exists(), reason="the SPA sources are not available")
+def test_the_spa_asks_for_unread_only():
+    """The half a browserless API test cannot reach.
+
+    Both assertions above pass against a SPA that has dropped the parameter —
+    the bug was never in the endpoint. Reading the source is worth less than
+    exercising it, and here it is the only thing that catches the defect that
+    actually happened.
+    """
+    src = APP.read_text()
+    fetches = re.findall(r"hubFetch\('(/v1/notifications\?[^']*)'", src)
+    assert fetches, "the notification list fetch is no longer where this reads it"
+    for url in fetches:
+        assert "unread_only=true" in url, (
+            f"the SPA lists notifications with {url!r} — without unread_only a "
+            "dismissed notification comes back on the next launch")
+
+
+@pytest.mark.skipif(not APP.exists(), reason="the SPA sources are not available")
+def test_the_feed_does_not_style_a_state_it_can_no_longer_show():
+    """The fossil that pointed at the bug.
+
+    `n.read ? '' : 'notif-unread'` is from when a read notification stayed on
+    screen greyed out. Nothing read reaches the feed any more, so the ternary
+    was dead code that described behaviour the application had abandoned —
+    and reading it is what made the two halves' disagreement visible.
+    """
+    src = APP.read_text()
+    assert "n.read ?" not in src, (
+        "the feed branches on `read` again; either it is dead code or the "
+        "dismissal contract has changed and this file should say how")