Signal also seems to create notification content from scratch so it is probably safe: https://github.com/signalapp/Signal-iOS/blob/main/SignalNSE/NSEEnvironment.swift#L164

In general, I think we should opt for >= here. That way, the releaser can simply replace NEXT_CODE_VERSION with the version they are releasing

1. Nit: spacing
2. I think this type is strange. Is it important that we overload aps.alert for this, or is it possible to have a different key for this?

1. Do we ever set encryptedNotification.aps before this line?
2. Is your theory here that it's important to set the aps key, otherwise the notif might not go through? If so, how come some of the other notifs go through?

My theory, as described and discussed in https://linear.app/comm/issue/ENG-4453/notifs-are-broken-in-production#comment-eaa12167, is that encrypted notifs don't trigger NSE since they have pushType: alert and mutable-content: 1 but don't have the alert field. According to apple docs alert field can be either string or a dict, that can contain may different keys. So logically to test my theory we don't have to modify this type and just put some string under alert field. However I came across this doc: https://developer.apple.com/documentation/usernotifications/modifying_content_in_newly_delivered_notifications?language=objc, where Apple considers the case of encrypted notification handled in NSE. In their example they configure notification payload to have a dict in alert field with body set to string. Therefore I intuitively decided to match apple tutorial as close as possible. To do this I had to modify this type.

it possible to have a different key for this

When alert is a dict it can have a lot of different keys. I didn't add them all since this diff is just to make a test release. If it proves to work I will refactor this type to cover all keys that alert field can accept.

However if you think we should do this now, then please re-request changes and I will add those keys to this type.

aps is always set for apn.Notification. Most of setter methods of this class put data in aps under the hood.
https://github.com/node-apn/node-apn/blob/master/lib/notification/apsProperties.js

Yes - this change is safe for public releases however hiding it behind isStaffRelease check would result in the same behaviour.

After landing this stack staff users will receive encrypted badgeonly notifications with ENCRYPTED alert body. Therefore we must create new notification content without body but with badge count. Public users will be still receiving badgeonly notifications without any body so it would be sage not to enter this code and call contentHandler directly on what has arrived with didReceive.. callback.