Skip to content

Commit 5b93421

Browse files
devartifexCopilot
andauthored
fix: prevent replayed push notifications and improve re-subscription
* Fix duplicate iOS notifications: mark replayed messages to suppress client notifications Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Add replayed flag to TurnEnd/Done messages and include Co-authored-by trailer * Skip client notification for replayed messages; Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * fix: restore types/index.ts truncated by editing error, re-apply replayed flag Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * fix: re-register push subscription on every WS connect to survive redeploy EmptyDir volumes in Azure Container Apps are wiped on replica replacement (redeployments). Re-POST the browser's existing PushSubscription to /api/push/subscribe on every WebSocket onopen so the server always has the subscription without requiring user action. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * fix: pre-select model on new chat; no session restart on model change - effectiveModel derived: chatStore.currentModel || settings.selectedModel || 'gpt-4.1' so TopBar and ModelSheet show the correct model immediately on load, before session_created arrives from the server. - handleSetReasoning no longer calls clearMessages()+requestNewSession(). Changing the reasoning effort (or switching to a reasoning model) now only persists the preference in settings; it takes effect on the next explicit new session rather than silently wiping the current chat. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * test: add push-notifications and sw-register coverage Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --------- Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
1 parent ff0ee40 commit 5b93421

7 files changed

Lines changed: 405 additions & 11 deletions

File tree

src/lib/server/ws/handler.ts

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -104,12 +104,14 @@ export function setupWebSocket(
104104
}
105105
entry.ws = ws;
106106

107-
// Replay only messages the client hasn't seen (based on sequence numbers)
107+
// Replay only messages the client hasn't seen (based on sequence numbers).
108+
// Mark each replayed message so the client can suppress duplicate notifications
109+
// (the server already sent a push notification while the client was unreachable).
108110
const buffer = entry.messageBuffer.splice(0);
109111
for (const msg of buffer) {
110112
const msgSeq = typeof msg.seq === 'number' ? msg.seq : -1;
111113
if (msgSeq > lastSeq && ws.readyState === WebSocket.OPEN) {
112-
ws.send(JSON.stringify(msg));
114+
ws.send(JSON.stringify({ ...msg, replayed: true }));
113115
}
114116
}
115117

src/lib/stores/chat.svelte.ts

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -303,10 +303,14 @@ export function createChatStore(wsStore: WsStore): ChatStore {
303303

304304
case 'turn_end':
305305
case 'done':
306-
notify('Response ready', {
307-
body: currentStreamContent.trim().slice(0, 100) || undefined,
308-
tag: 'response-ready',
309-
});
306+
// Skip client-side notification for replayed messages: the server already sent a
307+
// push notification while the client was unreachable (iOS backgrounded / WS closed).
308+
if (!msg.replayed) {
309+
notify('Response ready', {
310+
body: currentStreamContent.trim().slice(0, 100) || undefined,
311+
tag: 'response-ready',
312+
});
313+
}
310314
finalizeStream();
311315
break;
312316

src/lib/stores/ws.svelte.ts

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ import type {
1010
McpServerDefinition,
1111
} from '$lib/types/index.js';
1212
import { notify } from '$lib/utils/notifications.js';
13+
import { getPushSubscription } from '$lib/utils/push-notifications.js';
1314

1415
const INITIAL_RECONNECT_DELAY = 3000;
1516
const MAX_RECONNECT_DELAY = 60_000;
@@ -200,6 +201,23 @@ export function createWsStore(): WsStore {
200201

201202
// ── Connection lifecycle ────────────────────────────────────────────────
202203

204+
/** Fire-and-forget: re-register the browser's push subscription with the server.
205+
* Called on every WS connect so the server recovers subscriptions lost on redeploy
206+
* (EmptyDir volumes are wiped when the container replica is replaced). */
207+
async function reRegisterPushSubscription(): Promise<void> {
208+
try {
209+
const sub = await getPushSubscription();
210+
if (!sub) return;
211+
await fetch('/api/push/subscribe', {
212+
method: 'POST',
213+
headers: { 'Content-Type': 'application/json' },
214+
body: JSON.stringify(sub.toJSON()),
215+
});
216+
} catch {
217+
// Non-fatal — push will self-heal on the next connect
218+
}
219+
}
220+
203221
function connect(): void {
204222
if (typeof window === 'undefined') return;
205223

@@ -230,6 +248,9 @@ export function createWsStore(): WsStore {
230248
hasConnectedOnce = true;
231249
clearReconnectTimer();
232250
startHeartbeat();
251+
// Re-register push subscription after every connect/reconnect so the server
252+
// always has it — EmptyDir storage is wiped on redeploy.
253+
reRegisterPushSubscription();
233254
};
234255

235256
socket.onmessage = (event: MessageEvent) => {

src/lib/types/index.ts

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -173,10 +173,14 @@ export interface DeltaMessage {
173173

174174
export interface TurnEndMessage {
175175
type: 'turn_end';
176+
/** True when this message was replayed from the server buffer after reconnecting. */
177+
replayed?: boolean;
176178
}
177179

178180
export interface DoneMessage {
179181
type: 'done';
182+
/** True when this message was replayed from the server buffer after reconnecting. */
183+
replayed?: boolean;
180184
}
181185

182186
export interface ReasoningDeltaMessage {
Lines changed: 293 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,293 @@
1+
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest';
2+
import {
3+
isPushSupported,
4+
isStandalone,
5+
getPushSubscription,
6+
subscribeToPush,
7+
unsubscribeFromPush,
8+
} from '$lib/utils/push-notifications';
9+
10+
// ---- Helpers ----------------------------------------------------------------
11+
12+
function makePushManager(overrides: {
13+
getSubscription?: ReturnType<typeof vi.fn>;
14+
subscribe?: ReturnType<typeof vi.fn>;
15+
} = {}) {
16+
return {
17+
getSubscription: overrides.getSubscription ?? vi.fn().mockResolvedValue(null),
18+
subscribe: overrides.subscribe ?? vi.fn(),
19+
};
20+
}
21+
22+
function installServiceWorkerMock(pushManager = makePushManager()) {
23+
const swMock = {
24+
ready: Promise.resolve({ pushManager }),
25+
register: vi.fn(),
26+
};
27+
Object.defineProperty(navigator, 'serviceWorker', {
28+
configurable: true,
29+
value: swMock,
30+
});
31+
return swMock;
32+
}
33+
34+
/** Replace globalThis.navigator with a Proxy that hides the serviceWorker key. */
35+
function hideServiceWorker() {
36+
const orig = globalThis.navigator;
37+
const proxy = new Proxy(orig, {
38+
has(target, key) {
39+
return key !== 'serviceWorker' && key in target;
40+
},
41+
get(target, key, receiver) {
42+
if (key === 'serviceWorker') return undefined;
43+
return Reflect.get(target, key, receiver);
44+
},
45+
});
46+
Object.defineProperty(globalThis, 'navigator', { configurable: true, value: proxy });
47+
return () => Object.defineProperty(globalThis, 'navigator', { configurable: true, value: orig });
48+
}
49+
50+
function setPushManagerPresent(present: boolean) {
51+
if (present) {
52+
Object.defineProperty(window, 'PushManager', { configurable: true, value: class PushManager {} });
53+
}
54+
// When false: just ensure PushManager is not defined. Since jsdom does not include
55+
// PushManager natively, no action is needed — but we guard against a previous test
56+
// having set it by deleting it via the configurable descriptor.
57+
else {
58+
try {
59+
Object.defineProperty(window, 'PushManager', { configurable: true, value: undefined });
60+
// eslint-disable-next-line @typescript-eslint/no-explicit-any
61+
delete (window as unknown as any).PushManager;
62+
} catch {
63+
// best-effort — jsdom may not allow deletion
64+
}
65+
}
66+
}
67+
68+
function installNotificationMock(permission: NotificationPermission = 'granted') {
69+
const mock = {
70+
requestPermission: vi.fn().mockResolvedValue(permission),
71+
};
72+
Object.defineProperty(globalThis, 'Notification', { configurable: true, value: mock });
73+
return mock;
74+
}
75+
76+
function makeSubscription(overrides: Partial<{
77+
endpoint: string;
78+
unsubscribe: ReturnType<typeof vi.fn>;
79+
}> = {}): PushSubscription {
80+
return {
81+
endpoint: overrides.endpoint ?? 'https://example.com/push/endpoint',
82+
toJSON: () => ({ endpoint: 'https://example.com/push/endpoint' }),
83+
unsubscribe: overrides.unsubscribe ?? vi.fn().mockResolvedValue(true),
84+
} as unknown as PushSubscription;
85+
}
86+
87+
// A valid URL-safe base64 VAPID key (no padding needed for atob)
88+
const VALID_VAPID_KEY = btoa('a'.repeat(65)).replace(/\+/g, '-').replace(/\//g, '_').replace(/=/g, '');
89+
90+
// ---- Setup / teardown -------------------------------------------------------
91+
92+
beforeEach(() => {
93+
vi.spyOn(console, 'warn').mockImplementation(() => {});
94+
vi.spyOn(console, 'error').mockImplementation(() => {});
95+
vi.spyOn(console, 'log').mockImplementation(() => {});
96+
// Ensure matchMedia exists (jsdom doesn't implement it by default)
97+
Object.defineProperty(window, 'matchMedia', {
98+
configurable: true,
99+
value: vi.fn().mockReturnValue({ matches: false }),
100+
});
101+
});
102+
103+
afterEach(() => {
104+
vi.restoreAllMocks();
105+
});
106+
107+
// ---- isPushSupported --------------------------------------------------------
108+
109+
describe('isPushSupported', () => {
110+
it('returns false when serviceWorker is not in navigator', () => {
111+
const restore = hideServiceWorker();
112+
expect(isPushSupported()).toBe(false);
113+
restore();
114+
});
115+
116+
it('returns false when PushManager is not in window', () => {
117+
installServiceWorkerMock();
118+
setPushManagerPresent(false);
119+
expect(isPushSupported()).toBe(false);
120+
});
121+
122+
it('returns true when serviceWorker and PushManager are both present', () => {
123+
installServiceWorkerMock();
124+
setPushManagerPresent(true);
125+
expect(isPushSupported()).toBe(true);
126+
});
127+
});
128+
129+
// ---- isStandalone -----------------------------------------------------------
130+
131+
describe('isStandalone', () => {
132+
it('returns true when display-mode is standalone', () => {
133+
Object.defineProperty(window, 'matchMedia', {
134+
configurable: true,
135+
value: vi.fn().mockReturnValue({ matches: true }),
136+
});
137+
expect(isStandalone()).toBe(true);
138+
});
139+
140+
it('returns true when navigator.standalone is true (iOS)', () => {
141+
// matchMedia already set to { matches: false } in beforeEach
142+
Object.defineProperty(navigator, 'standalone', { configurable: true, value: true });
143+
expect(isStandalone()).toBe(true);
144+
Object.defineProperty(navigator, 'standalone', { configurable: true, value: undefined });
145+
});
146+
147+
it('returns false when neither condition is met', () => {
148+
// matchMedia already set to { matches: false } in beforeEach
149+
Object.defineProperty(navigator, 'standalone', { configurable: true, value: undefined });
150+
expect(isStandalone()).toBe(false);
151+
});
152+
});
153+
154+
// ---- getPushSubscription ----------------------------------------------------
155+
156+
describe('getPushSubscription', () => {
157+
it('returns null when push is not supported', async () => {
158+
const restore = hideServiceWorker();
159+
const result = await getPushSubscription();
160+
restore();
161+
expect(result).toBeNull();
162+
});
163+
164+
it('returns the subscription from pushManager', async () => {
165+
const sub = makeSubscription();
166+
const pm = makePushManager({ getSubscription: vi.fn().mockResolvedValue(sub) });
167+
installServiceWorkerMock(pm);
168+
setPushManagerPresent(true);
169+
170+
expect(await getPushSubscription()).toBe(sub);
171+
expect(pm.getSubscription).toHaveBeenCalledOnce();
172+
});
173+
174+
it('returns null when there is no active subscription', async () => {
175+
const pm = makePushManager({ getSubscription: vi.fn().mockResolvedValue(null) });
176+
installServiceWorkerMock(pm);
177+
setPushManagerPresent(true);
178+
179+
expect(await getPushSubscription()).toBeNull();
180+
});
181+
});
182+
183+
// ---- subscribeToPush --------------------------------------------------------
184+
185+
describe('subscribeToPush', () => {
186+
it('returns null and warns when push is not supported', async () => {
187+
const restore = hideServiceWorker();
188+
const result = await subscribeToPush();
189+
restore();
190+
expect(result).toBeNull();
191+
expect(console.warn).toHaveBeenCalledWith('[PUSH] Push notifications not supported');
192+
});
193+
194+
it('returns null when notification permission is denied', async () => {
195+
installServiceWorkerMock();
196+
setPushManagerPresent(true);
197+
installNotificationMock('denied');
198+
199+
const result = await subscribeToPush();
200+
expect(result).toBeNull();
201+
expect(console.warn).toHaveBeenCalledWith('[PUSH] Notification permission denied');
202+
});
203+
204+
it('returns null when VAPID key fetch fails', async () => {
205+
installServiceWorkerMock();
206+
setPushManagerPresent(true);
207+
installNotificationMock('granted');
208+
vi.spyOn(globalThis, 'fetch').mockResolvedValueOnce(new Response(null, { status: 500 }));
209+
210+
const result = await subscribeToPush();
211+
expect(result).toBeNull();
212+
expect(console.error).toHaveBeenCalledWith('[PUSH] Failed to fetch VAPID key:', 500);
213+
});
214+
215+
it('returns null when no VAPID public key is configured', async () => {
216+
installServiceWorkerMock();
217+
setPushManagerPresent(true);
218+
installNotificationMock('granted');
219+
vi.spyOn(globalThis, 'fetch').mockResolvedValueOnce(
220+
new Response(JSON.stringify({ publicKey: null }), { status: 200 })
221+
);
222+
223+
const result = await subscribeToPush();
224+
expect(result).toBeNull();
225+
expect(console.warn).toHaveBeenCalledWith('[PUSH] No VAPID public key configured');
226+
});
227+
228+
it('returns null and unsubscribes locally when server registration fails', async () => {
229+
const sub = makeSubscription();
230+
const pm = makePushManager({ subscribe: vi.fn().mockResolvedValue(sub) });
231+
installServiceWorkerMock(pm);
232+
setPushManagerPresent(true);
233+
installNotificationMock('granted');
234+
vi.spyOn(globalThis, 'fetch')
235+
.mockResolvedValueOnce(new Response(JSON.stringify({ publicKey: VALID_VAPID_KEY }), { status: 200 }))
236+
.mockResolvedValueOnce(new Response(null, { status: 500 }));
237+
238+
const result = await subscribeToPush();
239+
expect(result).toBeNull();
240+
expect(sub.unsubscribe).toHaveBeenCalledOnce();
241+
expect(console.error).toHaveBeenCalledWith('[PUSH] Failed to register subscription:', 500);
242+
});
243+
244+
it('returns the subscription on the happy path', async () => {
245+
const sub = makeSubscription();
246+
const pm = makePushManager({ subscribe: vi.fn().mockResolvedValue(sub) });
247+
installServiceWorkerMock(pm);
248+
setPushManagerPresent(true);
249+
installNotificationMock('granted');
250+
vi.spyOn(globalThis, 'fetch')
251+
.mockResolvedValueOnce(new Response(JSON.stringify({ publicKey: VALID_VAPID_KEY }), { status: 200 }))
252+
.mockResolvedValueOnce(new Response(null, { status: 200 }));
253+
254+
const result = await subscribeToPush();
255+
expect(result).toBe(sub);
256+
expect(console.log).toHaveBeenCalledWith('[PUSH] Successfully subscribed');
257+
});
258+
});
259+
260+
// ---- unsubscribeFromPush ----------------------------------------------------
261+
262+
describe('unsubscribeFromPush', () => {
263+
it('returns true immediately when there is no existing subscription', async () => {
264+
installServiceWorkerMock();
265+
setPushManagerPresent(true);
266+
267+
expect(await unsubscribeFromPush()).toBe(true);
268+
});
269+
270+
it('notifies the server and unsubscribes locally', async () => {
271+
const sub = makeSubscription({ endpoint: 'https://push.example.com/sub' });
272+
const pm = makePushManager({ getSubscription: vi.fn().mockResolvedValue(sub) });
273+
installServiceWorkerMock(pm);
274+
setPushManagerPresent(true);
275+
const fetchSpy = vi.spyOn(globalThis, 'fetch').mockResolvedValueOnce(new Response(null, { status: 200 }));
276+
277+
expect(await unsubscribeFromPush()).toBe(true);
278+
expect(fetchSpy).toHaveBeenCalledWith('/api/push/unsubscribe', expect.objectContaining({ method: 'POST' }));
279+
expect(sub.unsubscribe).toHaveBeenCalledOnce();
280+
});
281+
282+
it('still unsubscribes locally when the server request throws', async () => {
283+
const sub = makeSubscription();
284+
const pm = makePushManager({ getSubscription: vi.fn().mockResolvedValue(sub) });
285+
installServiceWorkerMock(pm);
286+
setPushManagerPresent(true);
287+
vi.spyOn(globalThis, 'fetch').mockRejectedValueOnce(new Error('Network error'));
288+
289+
expect(await unsubscribeFromPush()).toBe(true);
290+
expect(console.warn).toHaveBeenCalledWith('[PUSH] Server unsubscribe failed:', expect.any(Error));
291+
expect(sub.unsubscribe).toHaveBeenCalledOnce();
292+
});
293+
});

0 commit comments

Comments
 (0)