feat: [SDK-5170] register push before FCM credentials upload - #2735
Open
fadi-george wants to merge 4 commits into
Open
feat: [SDK-5170] register push before FCM credentials upload#2735fadi-george wants to merge 4 commits into
fadi-george wants to merge 4 commits into
Conversation
Contributor
📊 Diff Coverage ReportDiff Coverage Report (Changed Lines Only)Gate: aggregate coverage on changed executable lines must be ≥ 80% (JaCoCo line data for lines touched in the diff). Changed Files Coverage
Overall (aggregate gate)28/29 touched executable lines covered (96.6% — requires ≥ 80%) Per-file detail (informational; gate is aggregate above):
|
There was a problem hiding this comment.
Multi-model review
Models: Claude Opus 5, GPT 5.6 Sol, Cursor Grok 4.6.
Act on
- Legacy
.tokenregistration now throws on a host vs dashboard sender mismatch, even though the token is minted on the isolatedONESIGNAL_SDK_FCM_APP_NAMEapp (3/3). That is a regression for mixed-project apps, and it also blocks the host-fallback → dashboard-sender upgrade after credentials are uploaded. KeepvalidateSenderIdon the FID path only.
Consider
initFirebaseAppcaches by nullness, not sender (3/3). Once the mismatch throw is removed, an in-process sender change (start/host-fallback thenHYDRATE, or appId switch) can keep serving the old project's token. Recreate the named app when sender/FCM params change.- Empty/
"null"android_sender_idskips the host fallback (Opus);safeStringonly yieldsnullwhen the key is absent. - Host-fallback still pairs the host sender with OneSignal shared FCM credentials on the named app (Grok). The FID path is consistent; the legacy first-session token may not be the identifier that later receives traffic.
- Residual from the FID work: English substring detect of Firebase's "API disabled…register()" message, and no
onRegistered/FID-rotation bridge (Opus/GPT).
Noted / dismissed
- Consumer ProGuard
register()keep,defaultSenderIdderivation, and the Gradlerequirerange look correct. snd=census vsFirebaseOptionssender, and the "FID registration is enabled (…=not set)" error copy, are nits.
Sent by Cursor Automation: PR Reviews
abdulraqeeb33
requested changes
Sep 4, 2026
abdulraqeeb33
left a comment
Contributor
There was a problem hiding this comment.
Answer to the review focus: do not reject a host vs dashboard sender mismatch on the legacy path. That path still mints from ONESIGNAL_SDK_FCM_APP_NAME. Mixed-project apps are supposed to keep working there. 2734 left that alone.
- Drop
validateSenderIdfromgetLegacyToken. Keep it on the FID path only. Invert the mixed-project legacy test. - Same change: rebuild the named app when
senderIdchanges. Host fallback plus HYDRATE can change sender in-process. Cache-by-nullness then uploads the old sender's token.
fadi-george
force-pushed
the
fadi/SDK-5170-register-before-service-account
branch
from
September 4, 2026 20:10
a7d45a8 to
d71828d
Compare
Co-authored-by: Cursor <cursoragent@cursor.com>
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
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.


Description
One Line Summary
Register Android push subscriptions from the host Firebase configuration before dashboard FCM credentials are uploaded.
Details
Motivation
A newly configured Android app currently receives no push token or Firebase Installation ID until its FCM service account is uploaded to OneSignal and the app launches again. This change derives the sender ID from the host default Firebase app when dashboard parameters do not provide one, allowing the identifier to be uploaded during the first app session.
Scope
Review focus
Testing
Unit testing
Added coverage for sender-ID derivation, registration before a dashboard sender exists, legacy-token and FID paths, missing default Firebase apps, mismatches, reflective registration, and failures.
Ran:
testDebugUnitTestManual testing
Built the GMS demo with a local
google-services.jsonand confirmed the generated APK packagesgoogle_app_idandgcm_defaultSenderId. Final end-to-end validation of credential upload activating an existing subscription remains pending because the emulator disconnected before the rebuilt APK could be installed.Affected code checklist
Checklist
Overview
Testing
Final pass
Made with Cursor