Skip to content

Commit a7314d8

Browse files
authored
fix(notifications): stop toast replay across nav and page reload (#134)
## Summary - ToastContainer's dedup state was component-local, so it reset every time the component remounted — and since it only renders inside Header (mounted for /dashboard but not /settings), navigating away and back replayed a toast for every notification still active in the store, most visibly a GitHub-status outage that can sit unchanged for hours - A hard page refresh reproduced the same symptom via a different path: it wipes the notification store itself, so the next poll's unchanged outage looked brand new again - Persist the last-toasted message per source to sessionStorage, checked before showing a toast and pruned once a source's notification clears — sessionStorage survives both a same-tab remount and a refresh, so one mechanism covers both triggers without touching notifyTransitions()'s existing unconditional-push behavior
1 parent aa989f3 commit a7314d8

5 files changed

Lines changed: 511 additions & 24 deletions

File tree

src/app/components/shared/ToastContainer.tsx

Lines changed: 123 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ import {
55
type AppNotification,
66
type NotificationSeverity,
77
} from "../../lib/errors";
8+
import { onAuthCleared } from "../../stores/auth";
89

910
export interface SeverityConfig {
1011
path: string;
@@ -47,14 +48,83 @@ interface ToastItem {
4748
dismissing: boolean;
4849
}
4950

51+
// Persisted (sessionStorage) record of the last message actually toasted per
52+
// source. This component only renders inside Header, which the router mounts
53+
// for /dashboard but not /settings — navigating away and back fully unmounts
54+
// and remounts it, wiping any in-memory-only dedup state. A hard page refresh
55+
// wipes the entire notification store too. Either way, a still-active,
56+
// unchanged notification (most visibly a GitHub-status outage, which can sit
57+
// unchanged in the store for hours) would otherwise look brand new again and
58+
// re-toast. sessionStorage survives both a remount and a refresh, so this is
59+
// the one place dedup needs to persist beyond the component's own lifetime.
60+
const TOASTED_MESSAGES_KEY = "github-tracker:toasted-messages";
61+
62+
function loadToastedMessages(): Map<string, string> {
63+
try {
64+
const raw = sessionStorage.getItem(TOASTED_MESSAGES_KEY);
65+
const parsed: unknown = raw ? JSON.parse(raw) : [];
66+
if (!Array.isArray(parsed)) return new Map();
67+
return new Map(
68+
parsed.filter(
69+
(e): e is [string, string] =>
70+
Array.isArray(e) && e.length === 2 && typeof e[0] === "string" && typeof e[1] === "string"
71+
)
72+
);
73+
} catch {
74+
return new Map();
75+
}
76+
}
77+
78+
function persistToastedMessages(map: Map<string, string>): void {
79+
try {
80+
sessionStorage.setItem(TOASTED_MESSAGES_KEY, JSON.stringify([...map.entries()]));
81+
} catch {
82+
/* best-effort — dedup persistence is low-stakes, no user-facing notification needed */
83+
}
84+
}
85+
86+
// Resets toast dedup state. Called on logout via the onAuthCleared registration
87+
// below, and directly by tests to isolate sessionStorage between cases (mirrors
88+
// resetGitHubStatusState()/resetPollState() etc.).
89+
export function resetToastState(): void {
90+
sessionStorage.removeItem(TOASTED_MESSAGES_KEY);
91+
}
92+
93+
// toastedMessages stores per-source API/search/graphql error text, which is
94+
// user-scoped data (unlike github-status.ts's global GitHub-status feed, which
95+
// intentionally does NOT hook into onAuthCleared — see the note in that file).
96+
// Clear it on logout so a previous user's toast history can't leak into the
97+
// next session on a shared browser tab.
98+
onAuthCleared(resetToastState);
99+
50100
export default function ToastContainer() {
51101
const seenTimestamps = new Map<string, number>();
52-
const lastToastedAt = new Map<string, number>();
102+
const toastedMessages = loadToastedMessages();
53103
const [visibleToasts, setVisibleToasts] = createSignal<Map<string, ToastItem>>(new Map());
54104
const timeouts = new Map<string, ReturnType<typeof setTimeout>>();
55105
const dismissingTimeouts = new Map<string, ReturnType<typeof setTimeout>>();
56106

57-
const COOLDOWN_MS = 60_000;
107+
// lastToastedAt + COALESCE_MS: short, in-memory-only per-source throttle.
108+
// Distinct from toastedMessages above (which persists which exact message
109+
// was last shown, surviving a remount/refresh): lastToastedAt only
110+
// coalesces a rapid burst of textually-DIFFERENT updates from the same
111+
// source (e.g. a fast-ticking rate-limit retry countdown) into a single
112+
// visible toast, so it doesn't need to survive a remount — a genuinely new
113+
// incident more than a few seconds later should always show promptly.
114+
const lastToastedAt = new Map<string, number>();
115+
const COALESCE_MS = 3_000;
116+
117+
// A coalesced (suppressed) update can be the LAST thing that ever happens
118+
// for a source — e.g. a flapping status message settles back to a value
119+
// that's already in toastedMessages, at which point errors.ts's own
120+
// same-message no-op guard means the store never fires another change
121+
// event for it, so this component would never get another chance to
122+
// re-evaluate it. coalesceTimers schedules a one-shot re-check for exactly
123+
// when the coalescing window ends, reading whatever the store holds AT
124+
// THAT TIME (not the coalesced value itself) so a value that was
125+
// suppressed and never superseded still surfaces once the window elapses.
126+
const coalesceTimers = new Map<string, ReturnType<typeof setTimeout>>();
127+
58128
const reducedMotion = window.matchMedia("(prefers-reduced-motion: reduce)").matches;
59129
const animDelay = reducedMotion ? 0 : 300;
60130

@@ -93,6 +163,12 @@ export default function ToastContainer() {
93163
}
94164

95165
function scheduleAutoDismiss(notification: AppNotification) {
166+
// Clear any prior timer so an update to an existing toast gets a fresh
167+
// full dismiss window, rather than inheriting a timer sized for the
168+
// original (now possibly stale) message.
169+
const existing = timeouts.get(notification.id);
170+
if (existing !== undefined) clearTimeout(existing);
171+
96172
const delay = notification.severity === "error" ? 10_000 : 5_000;
97173
const t = setTimeout(() => {
98174
timeouts.delete(notification.id);
@@ -101,6 +177,33 @@ export default function ToastContainer() {
101177
timeouts.set(notification.id, t);
102178
}
103179

180+
function showToast(notif: AppNotification) {
181+
lastToastedAt.set(notif.source, Date.now());
182+
toastedMessages.set(notif.source, notif.message);
183+
persistToastedMessages(toastedMessages);
184+
setVisibleToasts((prev) => {
185+
const next = new Map(prev);
186+
next.set(notif.id, { notification: notif, dismissing: false });
187+
return next;
188+
});
189+
scheduleAutoDismiss(notif);
190+
}
191+
192+
function scheduleCoalesceRecheck(source: string) {
193+
if (coalesceTimers.has(source)) return;
194+
const lastToasted = lastToastedAt.get(source) ?? Date.now();
195+
const remaining = Math.max(0, COALESCE_MS - (Date.now() - lastToasted));
196+
const t = setTimeout(() => {
197+
coalesceTimers.delete(source);
198+
const current = getNotifications().find(n => n.source === source);
199+
if (!current || isMuted(source)) return;
200+
if (toastedMessages.get(source) === current.message) return;
201+
seenTimestamps.set(current.id, current.timestamp);
202+
showToast(current);
203+
}, remaining);
204+
coalesceTimers.set(source, t);
205+
}
206+
104207
createEffect(() => {
105208
const notifs = getNotifications();
106209
for (const notif of notifs) {
@@ -113,28 +216,34 @@ export default function ToastContainer() {
113216
seenTimestamps.set(notif.id, notif.timestamp);
114217

115218
const lastToasted = lastToastedAt.get(notif.source);
116-
const inCooldown = lastToasted !== undefined && Date.now() - lastToasted < COOLDOWN_MS;
219+
const coalescing = lastToasted !== undefined && Date.now() - lastToasted < COALESCE_MS;
117220
const muted = isMuted(notif.source);
221+
const alreadyToasted = toastedMessages.get(notif.source) === notif.message;
118222

119-
if (inCooldown || muted) continue;
223+
if (coalescing || muted || alreadyToasted) {
224+
if (coalescing) scheduleCoalesceRecheck(notif.source);
225+
continue;
226+
}
120227

121-
lastToastedAt.set(notif.source, Date.now());
122-
setVisibleToasts((prev) => {
123-
const next = new Map(prev);
124-
next.set(notif.id, { notification: notif, dismissing: false });
125-
return next;
126-
});
127-
scheduleAutoDismiss(notif);
228+
showToast(notif);
128229
}
129230

130231
const currentIds = new Set(notifs.map(n => n.id));
131232
for (const id of seenTimestamps.keys()) {
132233
if (!currentIds.has(id)) seenTimestamps.delete(id);
133234
}
134235
const currentSources = new Set(notifs.map(n => n.source));
135-
for (const source of lastToastedAt.keys()) {
136-
if (!currentSources.has(source)) lastToastedAt.delete(source);
236+
const staleSources = new Set(
237+
[...lastToastedAt.keys(), ...toastedMessages.keys()].filter(source => !currentSources.has(source))
238+
);
239+
let toastedMessagesChanged = false;
240+
for (const source of staleSources) {
241+
lastToastedAt.delete(source);
242+
if (toastedMessages.delete(source)) toastedMessagesChanged = true;
243+
const ct = coalesceTimers.get(source);
244+
if (ct !== undefined) { clearTimeout(ct); coalesceTimers.delete(source); }
137245
}
246+
if (toastedMessagesChanged) persistToastedMessages(toastedMessages);
138247
for (const id of visibleToasts().keys()) {
139248
if (!currentIds.has(id)) {
140249
const t = timeouts.get(id);
@@ -149,6 +258,7 @@ export default function ToastContainer() {
149258
onCleanup(() => {
150259
for (const t of timeouts.values()) clearTimeout(t);
151260
for (const t of dismissingTimeouts.values()) clearTimeout(t);
261+
for (const t of coalesceTimers.values()) clearTimeout(t);
152262
});
153263

154264
return (

src/app/stores/auth.ts

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -193,6 +193,40 @@ export function setAuth(response: TokenExchangeResponse): void {
193193
}
194194

195195
export function setAuthFromPat(token: string, userData: GitHubUser): void {
196+
const previousLogin = user()?.login;
197+
const isIdentitySwitch =
198+
previousLogin !== undefined && previousLogin.toLowerCase() !== userData.login.toLowerCase();
199+
200+
if (isIdentitySwitch) {
201+
// A different GitHub identity is taking over this browser tab/session
202+
// (Settings > Replace token, used for a user switch rather than rotating
203+
// one's own token) — do a full reset matching clearAuth(): config
204+
// (selectedRepos/selectedOrgs/trackedUsers/etc.) and view state are
205+
// genuinely per-identity data, not just UI preferences, so the incoming
206+
// identity must not inherit the outgoing one's. Reset in-memory stores
207+
// BEFORE clearing localStorage, so the persistence effects re-write
208+
// defaults (not stale user data) — same ordering as clearAuth(). We do
209+
// NOT touch AUTH_STORAGE_KEY or DASHBOARD_STORAGE_KEY here: the former is
210+
// overwritten below with the new token, and the latter is already
211+
// cleared by resetDashboardData() in the callback loop below.
212+
resetConfig();
213+
resetViewState();
214+
localStorage.removeItem(CONFIG_STORAGE_KEY);
215+
localStorage.removeItem(VIEW_STORAGE_KEY);
216+
// Clear IndexedDB cache to prevent data leakage between identities.
217+
clearCache().catch((err) => {
218+
console.warn("[auth] Cache clear failed during identity switch:", err);
219+
Sentry.captureException(err, { tags: { source: "auth-identity-switch-cache-clear" } });
220+
});
221+
// Clear per-user in-memory + cached state (poll data, notifications,
222+
// toast dedup, dashboard cache) the same way a real logout does, BEFORE
223+
// adopting the new identity below, so the incoming identity doesn't
224+
// inherit the outgoing one's data.
225+
for (const cb of _onClearCallbacks) {
226+
try { cb(); } catch (e) { console.warn("[auth] onAuthCleared callback threw during identity switch:", e); }
227+
}
228+
}
229+
196230
setAuth({ access_token: token });
197231
setUser({ login: userData.login, avatar_url: userData.avatar_url, name: userData.name });
198232
updateConfig({ authMethod: "pat" });

tests/components/layout/Header.test.tsx

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@ vi.mock("../../../src/app/stores/auth", () => ({
2424
name: "The Octocat",
2525
}),
2626
clearAuth: vi.fn(),
27+
onAuthCleared: vi.fn(),
2728
}));
2829

2930
// Mock errors module so Header's notification imports work

0 commit comments

Comments
 (0)