chore: Release 5.4.6#1948
Conversation
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
There was a problem hiding this comment.
LGTM — straightforward release PR with version bumps and Android SDK update from 5.8.1 to 5.9.0.
Extended reasoning...
Overview
This is a release PR for version 5.4.6. The changes are minimal and mechanical: bumping the Android OneSignal SDK from 5.8.1 to 5.9.0 in android/build.gradle, updating the SDK version string from 050405 to 050406 in both Android (RNOneSignal.java) and iOS (RCTOneSignal.mm) native bridges, and bumping the npm package version to 5.4.6 in package.json.
Security risks
No security risks introduced by this PR itself. The bumped Android SDK version 5.9.0 includes Identity Verification (a security feature) and defensive JSON parsing fixes — both improve, rather than degrade, the security posture. The version-string updates are purely metadata.
Level of scrutiny
Low scrutiny appropriate. This is a standard release PR following an established pattern (compare to the prior release commit bb93270 chore: Release 5.4.5). All four files contain only version-string updates with no logic changes. The OkHttp exclusion in build.gradle is preserved unchanged.
Other factors
No bugs were found by the bug hunting system. The version strings are consistent across all three locations (050406 / 5.4.6). No CODEOWNER-sensitive code paths were touched beyond the routine release metadata.
onesignal-deploy
force-pushed
the
rel/5.4.6
branch
from
01df89a to
753f199
Compare
There was a problem hiding this comment.
LGTM — straightforward release bump.
Extended reasoning...
Overview
This is a routine release PR for version 5.4.6 that bumps the Android SDK dependency from 5.8.1 to 5.9.1, updates the SDK version string (050405 → 050406) in both the Android (RNOneSignal.java) and iOS (RCTOneSignal.mm) wrappers, and bumps the npm package version in package.json. No logic changes — purely mechanical version bumps.
Security risks
None. No auth/crypto/permissions code is touched. The bundled Android SDK changes (per the PR description) are upstream bug fixes and a new Identity Verification feature, but those are vetted in the upstream OneSignal-Android-SDK repo, not in this PR.
Level of scrutiny
Low. Release PRs of this shape (dep version bump + version string sync) are highly mechanical and follow an established pattern in this repo (compare the recent 753f199 Release 5.4.6 commit and prior Android SDK bumps). The four edits are all single-line version-string changes that are easy to verify against the PR description.
Other factors
No bugs were flagged by the bug hunting system. No outstanding reviewer comments. The version strings are consistent across all three locations (package.json, Android, iOS).
| // (via opentelemetry-exporter-sender-okhttp) which is binary-incompatible with React Native's | ||
| // networking stack (okhttp3.internal.Util removed in 5.x). React Native already provides OkHttp 4.x. | ||
| api('com.onesignal:OneSignal:5.8.1') { | ||
| api('com.onesignal:OneSignal:5.9.2') { |
There was a problem hiding this comment.
🟡 The PR description heading reads "Update Android SDK from 5.8.1 to 5.9.1", but android/build.gradle line 42 actually bumps the dependency to com.onesignal:OneSignal:5.9.2. Since this PR's description becomes the public CHANGELOG / GitHub release notes for 5.4.6, the version mismatch is user-visible — please update the heading to 5.9.2 and add any release-notes bullets for changes that landed exclusively in 5.9.2.
Extended reasoning...
What's wrong
The PR description in this 'chore: Release 5.4.6' PR contains a stale heading:
🛠️ Native Dependency Updates
- Update Android SDK from 5.8.1 to 5.9.1
But android/build.gradle line 42 actually bumps the dependency to 5.9.2:
api('com.onesignal:OneSignal:5.9.2') {
exclude group: 'com.squareup.okhttp3', module: 'okhttp'
}How it happened (step-by-step proof)
Looking at the recent git log on this branch:
27e71ef— Update Android SDK to 5.9.0bf03cce— Update Android SDK to 5.9.1 ← PR description was written to match this commit078d6db— Update Android SDK to 5.9.2 ← description was never updated
The PR description (and its bullet list of upstream Android-SDK PRs #2637–#2645) was authored when the dependency was still at 5.9.1 and was simply not refreshed when the follow-up bump to 5.9.2 landed. The bullet list covers fixes that went into 5.9.0/5.9.1, but anything that shipped exclusively in 5.9.2 (e.g. any patches landed on top of bf03cce in the Android SDK) is undocumented.
Why this matters
This is the release PR for 5.4.6, and its description is what becomes the public CHANGELOG entry / GitHub release notes once the tag is cut. Consumers reading the release notes will see an announcement that the underlying Android SDK was upgraded to 5.9.1, while the artifact they actually install pulls 5.9.2. This is a small but user-visible inconsistency: dependency-pinning tooling, security scanners, and curious users who diff the lockfile against the release notes will all see the mismatch.
How to fix
Either:
- Update the heading to read 'Update Android SDK from 5.8.1 to 5.9.2' and append the release-notes bullets for any PRs that landed in 5.9.2 on top of the existing 5.9.1 list, or
- If 5.9.2 was a regression and 5.9.1 is what's intended to ship, roll
android/build.gradleback to5.9.1(the description would then match).
The first option is almost certainly what's wanted — the latest commit (078d6db) explicitly bumped the dependency to 5.9.2, so the code is correct and the description is stale. Nothing about runtime behavior is affected, so this is documentation-only (nit).
| { | ||
| "name": "react-native-onesignal", | ||
| "version": "5.4.5", | ||
| "version": "5.4.6", |
There was a problem hiding this comment.
🟣 Pre-existing: examples/demo/ios/Podfile.lock still pins react-native-onesignal (5.4.4) while package.json is being bumped to 5.4.6 — already two releases stale. The release workflow's "Refresh demo Podfile.lock" step (.github/workflows/create-release-pr.yml line 190) is gated behind if: inputs.ios_version != '', so any Android-only or wrapper-only release silently skips the pod-install refresh. Recommend dropping that gate (or running pod install manually for this release) so the lock keeps pace with version bumps.
Extended reasoning...
What the bug is
The demo app's examples/demo/ios/Podfile.lock records react-native-onesignal (5.4.4) at line 1449, but package.json in this PR is being bumped to 5.4.6. The lock file was already stale from the 5.4.5 release and this PR widens the gap to two versions.
Code path that triggers it
In .github/workflows/create-release-pr.yml, the step that refreshes the demo lock file is gated by an iOS-only conditional:
189: - name: Refresh demo Podfile.lock
190: if: inputs.ios_version != ''
191: run: |
...
201: pod installThis PR is a wrapper + Android-SDK-only release (only android/build.gradle bumped to OneSignal 5.9.2, no iOS native pin change), so inputs.ios_version is empty and the entire pod install step is skipped — even though the wrapper itself is being versioned.
Why existing code doesn't prevent it
The gate was likely written assuming that only iOS-native version bumps require a Podfile.lock refresh, but the path-based pod entry (:path: "../node_modules/react-native-onesignal" at line 2230) still records the wrapper's declared version (5.4.4 at line 1449). Whenever package.json changes, that pin gets stale until someone re-runs pod install.
Step-by-step proof
- Open
examples/demo/ios/Podfile.lock, line 1449 →- react-native-onesignal (5.4.4): - Open
package.jsonin this PR's tree, line 3 →"version": "5.4.6" - Trace the release workflow: this PR was produced by
create-release-pr.ymlwithios_versionempty (only Android SDK bumped). On line 190,if: inputs.ios_version != ''evaluates false → the entire refresh step (includingpod installat line 201) is skipped. - Net result: package.json (5.4.6) and Podfile.lock (5.4.4) diverge by 2 versions in this commit.
Impact
Limited but real. The pod entry is path-based, so anyone running pod install locally will re-resolve to whatever is in node_modules/react-native-onesignal (i.e. 5.4.6) — so working copies self-heal. The harm is to repo hygiene and reproducibility: a clean checkout reading the in-tree lock without re-running pod install will see the wrong wrapper version recorded, and git diff after a local pod install will keep showing churn until someone commits the refresh.
How to fix
Either (a) drop the if: inputs.ios_version != '' conditional on the Refresh step so any version bump triggers pod install, or (b) broaden the condition to fire when inputs.wrapper_version (or any other version input) is set, or (c) refresh the lock manually for this 5.4.6 release as a one-off catch-up. Option (a) is simplest — pod install is idempotent and inexpensive — and would have caught both the 5.4.5 miss and this one.
This bug is pre_existing: the workflow gate predates this PR. Flagging because this PR is a concrete instance that further widens the divergence.
1 participant
Channels: Current
🛠️ Native Dependency Updates