Keep push alive across a deploy, not just across a week #144

New Issue

Closed

opened 2026-08-31 15:20:45 +00:00 by jcoffey-dev · 0 comments

jcoffey-dev commented

2026-08-31 15:20:45 +00:00

Owner

Blocks deploying #143. This is a bug I introduced in that PR, and the first deploy carrying it would have been the thing that triggered it.

#143 added a device-local pushEnabled flag recording that background notifications were switched on in this browser, and made the renewal on app start key off it. It is not in KEEP_ON_SIGN_OUT (storage.ts:26) — and that's the whole bug, because clearSignedInData() runs on two different endings and only one of them is a sign-out.

The other is a session expiring, which is exactly what a deploy does to every signed-in browser at once (session.ts:139). That path deliberately does not remove the push subscription — there's no session left to remove it with — so the subscription stays registered at Stalwart and the browser keeps its own. Losing the flag there left nothing to renew them:

push goes quiet a week after every deploy;
the switch in Settings still reads on, because both ends of the subscription genuinely still exist;
webPushActive() agrees, because it checks exactly those two things.

Which is the "registered, and no notifications" state the whole feature is built to avoid.

The fix

pushEnabled joins KEEP_ON_SIGN_OUT. Signing out for real still forgets it — that happens directly in unsubscribeThisDevice(), next to destroying the subscription, and it happens even when the server cannot be reached, because a browser that goes on believing it has push would have renewal resurrect it on the next sign-in.

Tests

Both halves, because they're one invariant seen from two sides:

storage.test.ts — the flag survives a session expiry, alongside the existing exceptions.
webpush.test.ts — a real sign-out clears it with the JMAP call failing.

418 web tests / 109 server tests pass; typecheck and build clean.

Note on ordering

Merging this before deploying means existing push users keep working straight through. Deploying #143 first would silently start a seven-day timer on everyone who has background notifications on, and re-enabling from Settings would be the only way back.

Merged 2026-08-31 as coffey-labs/ihasmail@35c6060ceb

_{Rebuilt from: git history, session transcript.}

**Blocks deploying #143.** This is a bug I introduced in that PR, and the first deploy carrying it would have been the thing that triggered it. #143 added a device-local `pushEnabled` flag recording that background notifications were switched on in this browser, and made the renewal on app start key off it. It is not in `KEEP_ON_SIGN_OUT` (`storage.ts:26`) — and that's the whole bug, because `clearSignedInData()` runs on **two different endings and only one of them is a sign-out**. The other is a session expiring, which is exactly what a deploy does to every signed-in browser at once (`session.ts:139`). That path deliberately does *not* remove the push subscription — there's no session left to remove it with — so the subscription stays registered at Stalwart and the browser keeps its own. Losing the flag there left nothing to renew them: - push goes quiet a week after every deploy; - the switch in Settings still reads **on**, because both ends of the subscription genuinely still exist; - `webPushActive()` agrees, because it checks exactly those two things. Which is the "registered, and no notifications" state the whole feature is built to avoid. ## The fix `pushEnabled` joins `KEEP_ON_SIGN_OUT`. Signing out for real still forgets it — that happens directly in `unsubscribeThisDevice()`, next to destroying the subscription, and it happens *even when the server cannot be reached*, because a browser that goes on believing it has push would have renewal resurrect it on the next sign-in. ## Tests Both halves, because they're one invariant seen from two sides: - `storage.test.ts` — the flag survives a session expiry, alongside the existing exceptions. - `webpush.test.ts` — a real sign-out clears it with the JMAP call failing. 418 web tests / 109 server tests pass; typecheck and build clean. ## Note on ordering Merging this before deploying means existing push users keep working straight through. Deploying #143 first would silently start a seven-day timer on everyone who has background notifications on, and re-enabling from Settings would be the only way back. **Merged** 2026-08-31 as coffey-labs/ihasmail@35c6060ceb7d <sub>Rebuilt from: git history, session transcript.</sub>

This repo is archived. You cannot comment on issues.

1 Participants

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: coffey-labs/ihasmail-github-archive#144