From a813935e499a4a42f745179bee9ef549f1f2ef37 Mon Sep 17 00:00:00 2001 From: hamzamerzic <10846014+hamzamerzic@users.noreply.github.com> Date: Tue, 28 Jul 2026 02:04:39 +0000 Subject: [PATCH] Fix notification time and history contracts MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Möbius Agent --- backend/app/push.py | 7 ++- backend/app/routes/notifications.py | 29 +++++++++--- backend/app/schemas.py | 17 ++++++- backend/tests/test_notifications_unread.py | 44 +++++++++++++++++++ frontend/package.json | 2 +- .../NotificationsView/NotificationsView.jsx | 13 +++++- .../__tests__/notificationsModel.test.js | 24 ++++++++++ .../__tests__/swNotificationTarget.test.js | 4 ++ frontend/src/sw.js | 2 +- 9 files changed, 128 insertions(+), 14 deletions(-) create mode 100644 frontend/src/components/NotificationsView/__tests__/notificationsModel.test.js diff --git a/backend/app/push.py b/backend/app/push.py index 2a166c30e..08161d888 100644 --- a/backend/app/push.py +++ b/backend/app/push.py @@ -158,10 +158,9 @@ def notify_owner( agents; the /send default), 'app' + source_id=str(app_id) (also lights the drawer activity dot). New subsystems add a short stable slug. - - target: in-scope deep-link only — '/shell/?app=', - '/shell/?chat=', or '/shell/?view=notifications' (legacy - '/app/:id' and '/chat/:id' still parse). Clients treat it as - UNTRUSTED and fail closed on anything else. + - target: in-scope deep-link only — '/shell/?app=' or + '/shell/?chat=' (legacy '/app/:id' and '/chat/:id' still parse). + Clients treat it as UNTRUSTED and fail closed on anything else. """ notification_id = str(uuid.uuid4()) notif = models.Notification( diff --git a/backend/app/routes/notifications.py b/backend/app/routes/notifications.py index 45fbed2a4..f391f5ac5 100644 --- a/backend/app/routes/notifications.py +++ b/backend/app/routes/notifications.py @@ -3,10 +3,10 @@ import logging from datetime import UTC, datetime -from fastapi import APIRouter, Depends, Query, Request +from fastapi import APIRouter, Depends, HTTPException, Query, Request from slowapi import Limiter from slowapi.util import get_remote_address -from sqlalchemy import func +from sqlalchemy import and_, func, or_ from sqlalchemy.orm import Session from app import models @@ -117,12 +117,29 @@ def list_notifications( q = ( db.query(models.Notification) .filter(models.Notification.owner_id == owner.id) - .order_by(models.Notification.sent_at.desc()) + .order_by( + models.Notification.sent_at.desc(), + models.Notification.id.desc(), + ) ) if before: - ref = db.get(models.Notification, before) - if ref: - q = q.filter(models.Notification.sent_at < ref.sent_at) + ref = ( + db.query(models.Notification) + .filter( + models.Notification.owner_id == owner.id, + models.Notification.id == before, + ) + .one_or_none() + ) + if ref is None: + raise HTTPException(status_code=400, detail="Invalid notification cursor.") + q = q.filter(or_( + models.Notification.sent_at < ref.sent_at, + and_( + models.Notification.sent_at == ref.sent_at, + models.Notification.id < ref.id, + ), + )) return [ NotificationOut.model_validate(n) for n in q.limit(limit).all() ] diff --git a/backend/app/schemas.py b/backend/app/schemas.py index 669efa1c5..e0deb3b4d 100644 --- a/backend/app/schemas.py +++ b/backend/app/schemas.py @@ -1,6 +1,6 @@ """Pydantic request and response schemas.""" -from datetime import datetime +from datetime import UTC, datetime from typing import Literal from pydantic import ( @@ -628,4 +628,19 @@ class NotificationOut(BaseModel): # app-scoped tokens. read_at: datetime | None + @field_validator("sent_at", "clicked_at", "read_at", mode="before") + @classmethod + def restore_utc_offset(cls, value: datetime | None) -> datetime | None: + """Make SQLite's naive notification datetimes honest at the API edge. + + Notification timestamps are written in UTC, but SQLite drops timezone + metadata when reading them back. Without restoring it, JSON contains no + offset and browsers reinterpret UTC wall time as local time. + """ + if value is None: + return None + if value.tzinfo is None: + return value.replace(tzinfo=UTC) + return value.astimezone(UTC) + model_config = {"from_attributes": True} diff --git a/backend/tests/test_notifications_unread.py b/backend/tests/test_notifications_unread.py index a1eb96126..5051abecc 100644 --- a/backend/tests/test_notifications_unread.py +++ b/backend/tests/test_notifications_unread.py @@ -1,5 +1,7 @@ """Unread tracking for the notifications page (bell badge, seen-on-open).""" +from datetime import UTC, datetime + from app import models from app.auth import create_app_token from app.broadcast import get_system_broadcast @@ -27,6 +29,7 @@ def test_seen_on_open_lifecycle(client, auth): listed = client.get("/api/notifications", headers=auth).json() row = next(n for n in listed if n["id"] == sent_id) assert row["read_at"] is None + assert datetime.fromisoformat(row["sent_at"]).tzinfo == UTC first = client.post("/api/notifications/read-all", headers=auth) assert first.status_code == 200, first.text @@ -35,6 +38,7 @@ def test_seen_on_open_lifecycle(client, auth): listed = client.get("/api/notifications", headers=auth).json() row = next(n for n in listed if n["id"] == sent_id) assert row["read_at"] is not None + assert datetime.fromisoformat(row["read_at"]).tzinfo == UTC # Idempotent: a repeat call touches nothing and stamps nothing anew. second = client.post("/api/notifications/read-all", headers=auth) @@ -46,6 +50,46 @@ def test_seen_on_open_lifecycle(client, auth): assert _count(client, auth) == 1 +def test_history_cursor_is_stable_when_timestamps_tie(client, auth, db): + """Keyset pagination must neither skip nor repeat same-instant rows.""" + owner = db.query(models.Owner).first() + sent_at = datetime(2026, 7, 27, 2, 0, tzinfo=UTC) + for notification_id in ("n-a", "n-b", "n-c"): + db.add(models.Notification( + id=notification_id, + owner_id=owner.id, + source_type="system", + title=notification_id, + sent_at=sent_at, + )) + db.commit() + + first = client.get( + "/api/notifications", headers=auth, params={"limit": 2}, + ) + assert first.status_code == 200, first.text + first_ids = [row["id"] for row in first.json()] + assert first_ids == ["n-c", "n-b"] + + second = client.get( + "/api/notifications", + headers=auth, + params={"limit": 2, "before": first_ids[-1]}, + ) + assert second.status_code == 200, second.text + assert [row["id"] for row in second.json()] == ["n-a"] + + +def test_history_rejects_an_unknown_cursor(client, auth): + response = client.get( + "/api/notifications", + headers=auth, + params={"before": "not-a-notification"}, + ) + assert response.status_code == 400 + assert response.json()["detail"] == "Invalid notification cursor." + + def test_notification_created_published_on_system_bus(client, auth): """Every notify_owner call nudges the bell badge over the system stream.""" bus = get_system_broadcast() diff --git a/frontend/package.json b/frontend/package.json index 91351fe2a..da044b6c4 100644 --- a/frontend/package.json +++ b/frontend/package.json @@ -8,7 +8,7 @@ "build": "vite build --configLoader runner && node scripts/check-built-globals.mjs dist", "preview": "vite preview", "test": "npm run test:lib && npm run test:hooks", - "test:lib": "node --loader=./src/lib/__tests__/vite-env-loader.mjs --test $(find src/lib src/utils src/components/DiffView src/components/ProviderAuth src/components/ChatView/__tests__ src/components/Shell/__tests__ -name '*.test.js')", + "test:lib": "node --loader=./src/lib/__tests__/vite-env-loader.mjs --test $(find src/lib src/utils src/components/DiffView src/components/ProviderAuth src/components/NotificationsView src/components/ChatView/__tests__ src/components/Shell/__tests__ -name '*.test.js')", "test:hooks": "node --loader=./src/components/ChatView/hooks/__tests__/react-loader.mjs --test $(find src/components/ChatView/hooks -name '*.test.js') $(find src/components/Shell/__tests__ -name '*.hooktest.js')" }, "dependencies": { diff --git a/frontend/src/components/NotificationsView/NotificationsView.jsx b/frontend/src/components/NotificationsView/NotificationsView.jsx index cfe8530ad..56e6f0162 100644 --- a/frontend/src/components/NotificationsView/NotificationsView.jsx +++ b/frontend/src/components/NotificationsView/NotificationsView.jsx @@ -4,6 +4,7 @@ import BotMessageSquare from 'lucide-react/dist/esm/icons/bot-message-square.mjs import MessageSquare from 'lucide-react/dist/esm/icons/message-square.mjs' import Settings2 from 'lucide-react/dist/esm/icons/settings-2.mjs' import X from 'lucide-react/dist/esm/icons/x.mjs' +import { useEffect, useState } from 'react' import { notificationQueries } from '../../hooks/queries.js' import { parseNotificationTarget } from '../../lib/notificationTarget.js' import { formatRelativeTime, iconKindForSource } from './notificationsModel.js' @@ -23,6 +24,16 @@ const ICONS = { export default function NotificationsView({ active = false, onClose, onOpenTarget }) { const { data, isLoading, isError } = notificationQueries.list.useQuery({ enabled: active }) const rows = data ?? [] + const [now, setNow] = useState(() => Date.now()) + + // Relative labels are live information, not a one-time formatting pass. + // Refreshing once a minute keeps an open preview from saying "now" forever. + useEffect(() => { + if (!active) return undefined + setNow(Date.now()) + const timer = window.setInterval(() => setNow(Date.now()), 60_000) + return () => window.clearInterval(timer) + }, [active]) return (
- {formatRelativeTime(n.sent_at)} + {formatRelativeTime(n.sent_at, now)} ) diff --git a/frontend/src/components/NotificationsView/__tests__/notificationsModel.test.js b/frontend/src/components/NotificationsView/__tests__/notificationsModel.test.js new file mode 100644 index 000000000..fc92c43a3 --- /dev/null +++ b/frontend/src/components/NotificationsView/__tests__/notificationsModel.test.js @@ -0,0 +1,24 @@ +import assert from 'node:assert/strict' +import { test } from 'node:test' + +import { formatRelativeTime } from '../notificationsModel.js' + +const NOW = Date.parse('2026-07-27T02:54:00Z') + +test('relative notification time honors an explicit UTC offset', () => { + assert.equal( + formatRelativeTime('2026-07-27T02:53:30+00:00', NOW), + 'now', + ) +}) + +test('relative notification time keeps stable boundary labels', () => { + assert.equal(formatRelativeTime('2026-07-27T02:53:00Z', NOW), '1m ago') + assert.equal(formatRelativeTime('2026-07-27T01:54:00Z', NOW), '1h ago') + assert.equal(formatRelativeTime('2026-07-26T02:54:00Z', NOW), '1d ago') +}) + +test('future clock skew and invalid values degrade safely', () => { + assert.equal(formatRelativeTime('2026-07-27T03:54:00Z', NOW), 'now') + assert.equal(formatRelativeTime('not-a-date', NOW), '') +}) diff --git a/frontend/src/lib/__tests__/swNotificationTarget.test.js b/frontend/src/lib/__tests__/swNotificationTarget.test.js index 610936dc6..7d9789479 100644 --- a/frontend/src/lib/__tests__/swNotificationTarget.test.js +++ b/frontend/src/lib/__tests__/swNotificationTarget.test.js @@ -49,6 +49,10 @@ test('intent rides a same-origin absolute target and a numeric app id', () => { safeTarget('https://mobius.test/shell/?app=88&intent=artifact:deck-01'), '/shell/?app=88&intent=artifact%3Adeck-01', ) + assert.equal( + safeTarget('HTTPS://mobius.test/shell/?chat=abc'), + '/shell/?chat=abc', + ) }) test('a malformed intent is dropped but the app still opens', () => { diff --git a/frontend/src/sw.js b/frontend/src/sw.js index dade81ca6..c5ffda41a 100644 --- a/frontend/src/sw.js +++ b/frontend/src/sw.js @@ -832,7 +832,7 @@ function _safeTarget(raw) { let path = raw let search = '' try { - if (/^https?:\/\//.test(raw)) { + if (/^https?:\/\//i.test(raw)) { const u = new URL(raw) if (u.origin !== self.location.origin) return '/' path = u.pathname