Emit persistentIds snapshots so consumers can persist correctly - #16
Draft
cassiostp wants to merge 1 commit into
Draft
Emit persistentIds snapshots so consumers can persist correctly#16cassiostp wants to merge 1 commit into
cassiostp wants to merge 1 commit into
Conversation
Consumers (electron-push-receiver) keep a disk copy of persistentIds that only ever grows: the in-memory list clears when the MCS login response acks the ids, but nothing tells the consumer, so the disk copy diverges and accumulates without bound (35MB+ config files in the field). Emit a 'persistentIds' snapshot on every mutation: after each append (including ids recorded for reportable decryption errors, which were previously invisible to consumers) and after the login-response clear. The clear now also checks LoginResponse.error: an errored login did not process receivedPersistentId, so the ids are not acked and must be kept. A 'loginResponse' event is emitted either way so consumers can measure login latency. The snapshot is emitted before ON_NOTIFICATION_RECEIVED so persistence can complete before consumers act on the message, and it is a copy so consumers cannot share the mutable internal array.
cassiostp
force-pushed
the
cs/emit-persistent-ids
branch
from
August 13, 2026 21:28
29d5ec3 to
b42ddac
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The disk copy of persistentIds kept by electron-push-receiver only ever grows: this client clears its in-memory list when the MCS login response acks the ids (client.js, added in #6 back in 2018 as an explicitly partial fix), but nothing tells the consumer, so the two copies diverge and the disk copy accumulates without bound. Field impact: 35MB+ config.json files rewritten synchronously per notification and a 30MB LoginRequest uploaded on every connect (superhuman/desktop CLI-2311).
This emits a
persistentIdssnapshot (a copy, not the live array) on every mutation: both append sites, including ids recorded for reportable decryption errors which consumers previously never saw, and the login clear. The clear now checksLoginResponse.errorfirst: an errored login did not processreceivedPersistentId, so those ids are not acked and must be kept. AloginResponseevent is emitted either way so consumers can measure login latency. The snapshot is emitted beforeON_NOTIFICATION_RECEIVEDso persistence completes before consumers act on the message.Publish as 2.1.8. electron-push-receiver 2.2.0 (superhuman/electron-push-receiver PR to follow) consumes the new event; merge and publish this one first. Note:
test/notification.test.jsfails without a localkeys.js(pre-existing on master, needs the template filled in);client.test.jscovers the new behavior.🤖 Generated with Claude Code