Support UnifiedPush as alternative push provider#12
Conversation
|
Warning Rate limit exceeded
You’ve run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (4)
📝 WalkthroughWalkthroughThis PR introduces UnifiedPush as an alternative push notification provider independent of Google Play Services, while simultaneously removing Google Play Integrity, SafetyNet, and reCAPTCHA authentication mechanisms. Build infrastructure is updated to align with new project configuration and Firebase settings. ChangesUnifiedPush Feature Integration
Remove Google Play Services
Build Infrastructure Update
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (6)
patches/misc/remove-play.patch (2)
292-297:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winReplace the stale SafetyNet-specific error copy.
This dialog still shows
R.string.SafetyNetErrorOccurred, but the SafetyNet flow is gone in this patch. Users will get a misleading error for a provider the app no longer supports.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@patches/misc/remove-play.patch` around lines 292 - 297, The dialog currently calls setMessage(getString(R.string.SafetyNetErrorOccurred)), which is stale because SafetyNet support was removed; update this call to use a generic/appropriate string resource (e.g., R.string.RestorePasswordError or R.string.AuthenticationError) and add that resource in strings.xml, and remove any remaining SafetyNet-specific references (like forceDisableSafetyNet usage) so the message no longer mentions SafetyNet; locate the dialog creation code that calls setTitle/setMessage (and references VIEW_PHONE_INPUT) to make the replacement.
247-255:⚠️ Potential issue | 🟠 Major | 🏗️ Heavy liftUnconditionally failing backend challenges breaks gated auth flows.
onIntegrityCheckClassic()andonCaptchaCheck()now always send failure sentinels back to native without any alternate supported path. Any login/signup flow that hits either backend challenge will hard-fail instead of degrading gracefully.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@patches/misc/remove-play.patch` around lines 247 - 255, The changes unconditionally return failure sentinels from onIntegrityCheckClassic (native_receivedIntegrityCheckClassic with "PLAYINTEGRITY_DISABLED") and onCaptchaCheck (native_receivedCaptchaResult "RECAPTCHA_FAILED_TOKEN_NULL"), breaking gated auth flows; revert to the original behavior by only sending those failure callbacks when the integrity/captcha feature is actually disabled or when a real error occurs, and otherwise invoke the proper handling paths (e.g., call the original integrity flow instead of always native_receivedIntegrityCheckClassic and restore CaptchaController.request in onCaptchaCheck), ensuring success/error results are forwarded to native only after real outcome resolution.patches/misc/build-support.patch (4)
909-915:⚠️ Potential issue | 🟠 Major | ⚡ Quick win
TMessagesProj_AppTestsis no longer part of the build.Removing the include here means the
TMessagesProj_AppTests/build.gradlechanges in this same patch are dead code, and any CI task that still targets that module will stop resolving. If the test project is still intended to exist, keep it insettings.gradle; otherwise drop the module changes from this patch.Suggested fix
include ':TMessagesProj' include ':TMessagesProj_App' +include ':TMessagesProj_AppTests' include ':InuCore'🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@patches/misc/build-support.patch` around lines 909 - 915, The settings.gradle change removed the ':TMessagesProj_AppTests' include but the patch still modifies TMessagesProj_AppTests files; either re-add the include line for ':TMessagesProj_AppTests' in settings (so the test module is built) or remove all changes to the TMessagesProj_AppTests module from this patch; locate the include list (lines adding/removing includes, e.g., the include block that now contains ':InuCore') and either restore "include ':TMessagesProj_AppTests'" or delete the test-module edits elsewhere so CI won't reference a missing module.
577-582:⚠️ Potential issue | 🟠 Major | ⚡ Quick winDon't sign debug builds with the release key.
This widens exposure of the production signing credentials and makes everyday debug builds depend on the release keystore. Keep
signingConfigs.releasefor release only and letdebuguse the default debug keystore instead.Suggested fix
debug { debuggable true jniDebuggable true - signingConfig signingConfigs.release applicationIdSuffix ".beta" minifyEnabled false shrinkResources false🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@patches/misc/build-support.patch` around lines 577 - 582, The debug build type is incorrectly set to use signingConfigs.release; revert the debug block to use the debug keystore by removing or replacing signingConfig signingConfigs.release with the default debug signing (i.e., remove the line or set signingConfig signingConfigs.debug) inside the debug buildType definition so debug builds do not use signingConfigs.release and continue to use the debug keystore.
483-484:⚠️ Potential issue | 🟠 Major | ⚡ Quick winDon't call mangled Stripe internal symbols from Java.
validateExpYear$stripe_release(Calendar.getInstance())is not part of the documented publicCardAPI in Stripe Android SDK 11.x. The public validation methods arevalidateExpiryDate(),validateExpMonth(), andvalidateNumber(). The$stripe_releasesuffix indicates a Kotlin name-mangled internal method that will break if Stripe refactors its internal APIs. Use the stable public methodvalidateExpiryDate()instead, which validates the entire expiry date as documented.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@patches/misc/build-support.patch` around lines 483 - 484, The code is calling a mangled internal Stripe symbol validateExpYear$stripe_release(Calendar.getInstance()) which is unstable; replace that check by relying on the public Card API—remove the mangled call and use the documented validateExpiryDate() (together with validateExpMonth() if still desired) so the condition becomes e.g. !card.validateExpMonth() || !card.validateExpiryDate() instead of calling validateExpYear$stripe_release(...).
874-879:⚠️ Potential issue | 🟠 Major | ⚡ Quick winAvoid spawning
shduring Gradle configuration.The
ccachePathassignment executes a shell process at configuration time using['sh', '-c', 'command -v ccache'].execute(). This will fail on Windows and other environments without POSIXsh, causing the entire build to fail during Gradle initialization before any tasks run. SinceccachePathis already conditionally used (checked withif (rootProject.ext.ccachePath)), add an OS check before spawning the process, or use Gradle'sExecOperationsto handle this portably.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@patches/misc/build-support.patch` around lines 874 - 879, The ccachePath currently spawns a POSIX shell (['sh','-c','command -v ccache'].execute()) during Gradle configuration which will fail on Windows; change ccachePath to only attempt the external command on non-Windows platforms and wrap it in a safe try/catch (or use Gradle's ExecOperations when available). Concretely, replace the direct execute call in the ext block that defines ccachePath with a guarded branch using System.getProperty("os.name") (e.g. if osName contains "windows" then null else try { ['sh','-c','command -v ccache'].execute().text.trim() ?: null } catch(Exception e){ null }), keeping the other ext properties (verName, verCode, stockVerCode, buildDate) untouched. Ensure the result remains null when the command cannot be run so existing checks (if (rootProject.ext.ccachePath)) continue to work.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@patches/feature/unified-push.patch`:
- Line 33: The current check short-circuits push startup when
UnifiedPushHelper.isEnabled() is true; change the condition so we only return
when the feature is disabled (i.e., invert the check) so startPushService() runs
when UnifiedPush is enabled and lets the existing distributor/Google-fallback
logic execute; update the conditional guarding startPushService() that
references desu.inugram.helpers.UnifiedPushHelper.isEnabled() accordingly.
In `@src/kotlin/helpers/UnifiedPushHelper.kt`:
- Around line 97-115: The globalQueue task currently calls latch.await() which
can block forever; change the wait to a timed wait on the CountDownLatch (e.g.,
latch.await(timeout, timeUnit)) inside Utilities.globalQueue.postRunnable and
handle the boolean result (timed out vs counted down) so the queue isn't blocked
indefinitely; reference the existing latch created with
java.util.concurrent.CountDownLatch(1), the Utilities.stageQueue.postRunnable
that calls latch.countDown(), and the Utilities.globalQueue.postRunnable where
latch.await() is called — add a reasonable timeout value and optional
logging/error handling when await returns false.
- Around line 31-37: Wrap calls to UnifiedPush.getDistributors(context) in
try/catch similar to getActiveDistributor(): in hasDistributors(), catch any
Throwable from UnifiedPush.getDistributors(context) and return false on error;
in getDistributors(), catch any Throwable and return an empty List<String> on
error; optionally log the exception using the same logging mechanism used by
getActiveDistributor() so failures are visible but won't crash provider
selection or settings rendering.
In `@src/kotlin/ui/InuBehaviorSettingsActivity.kt`:
- Line 189: Replace the hardcoded fallback "default" used with
InuConfig.UNIFIED_PUSH_GATEWAY.value with a localized string via
LocaleController.getString(...); specifically change the fallback expression to
use LocaleController.getString(R.string.InuDefault) (following the Access
strings via LocaleController.getString(R.string.InuXxx) pattern) so the UI is
translatable; update the string resource name if your project uses a different
Inu* key.
- Around line 365-367: Replace the direct UnifiedPush.register(ctx, "default",
"Telegram Simple Push") call with the helper API UnifiedPushHelper.register(ctx)
so registration runs off the UI thread and uses the helper's error handling and
distributor selection; keep UnifiedPush.saveDistributor(ctx, chosen) and the
listView.adapter.update(true) calls, but call UnifiedPushHelper.register(ctx)
(or await its completion if it returns a Future/Job) instead of
UnifiedPush.register to ensure background execution and proper tracking.
---
Outside diff comments:
In `@patches/misc/build-support.patch`:
- Around line 909-915: The settings.gradle change removed the
':TMessagesProj_AppTests' include but the patch still modifies
TMessagesProj_AppTests files; either re-add the include line for
':TMessagesProj_AppTests' in settings (so the test module is built) or remove
all changes to the TMessagesProj_AppTests module from this patch; locate the
include list (lines adding/removing includes, e.g., the include block that now
contains ':InuCore') and either restore "include ':TMessagesProj_AppTests'" or
delete the test-module edits elsewhere so CI won't reference a missing module.
- Around line 577-582: The debug build type is incorrectly set to use
signingConfigs.release; revert the debug block to use the debug keystore by
removing or replacing signingConfig signingConfigs.release with the default
debug signing (i.e., remove the line or set signingConfig signingConfigs.debug)
inside the debug buildType definition so debug builds do not use
signingConfigs.release and continue to use the debug keystore.
- Around line 483-484: The code is calling a mangled internal Stripe symbol
validateExpYear$stripe_release(Calendar.getInstance()) which is unstable;
replace that check by relying on the public Card API—remove the mangled call and
use the documented validateExpiryDate() (together with validateExpMonth() if
still desired) so the condition becomes e.g. !card.validateExpMonth() ||
!card.validateExpiryDate() instead of calling
validateExpYear$stripe_release(...).
- Around line 874-879: The ccachePath currently spawns a POSIX shell
(['sh','-c','command -v ccache'].execute()) during Gradle configuration which
will fail on Windows; change ccachePath to only attempt the external command on
non-Windows platforms and wrap it in a safe try/catch (or use Gradle's
ExecOperations when available). Concretely, replace the direct execute call in
the ext block that defines ccachePath with a guarded branch using
System.getProperty("os.name") (e.g. if osName contains "windows" then null else
try { ['sh','-c','command -v ccache'].execute().text.trim() ?: null }
catch(Exception e){ null }), keeping the other ext properties (verName, verCode,
stockVerCode, buildDate) untouched. Ensure the result remains null when the
command cannot be run so existing checks (if (rootProject.ext.ccachePath))
continue to work.
In `@patches/misc/remove-play.patch`:
- Around line 292-297: The dialog currently calls
setMessage(getString(R.string.SafetyNetErrorOccurred)), which is stale because
SafetyNet support was removed; update this call to use a generic/appropriate
string resource (e.g., R.string.RestorePasswordError or
R.string.AuthenticationError) and add that resource in strings.xml, and remove
any remaining SafetyNet-specific references (like forceDisableSafetyNet usage)
so the message no longer mentions SafetyNet; locate the dialog creation code
that calls setTitle/setMessage (and references VIEW_PHONE_INPUT) to make the
replacement.
- Around line 247-255: The changes unconditionally return failure sentinels from
onIntegrityCheckClassic (native_receivedIntegrityCheckClassic with
"PLAYINTEGRITY_DISABLED") and onCaptchaCheck (native_receivedCaptchaResult
"RECAPTCHA_FAILED_TOKEN_NULL"), breaking gated auth flows; revert to the
original behavior by only sending those failure callbacks when the
integrity/captcha feature is actually disabled or when a real error occurs, and
otherwise invoke the proper handling paths (e.g., call the original integrity
flow instead of always native_receivedIntegrityCheckClassic and restore
CaptchaController.request in onCaptchaCheck), ensuring success/error results are
forwarded to native only after real outcome resolution.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 27bbbfde-ad97-4dea-9675-a222fd0fff32
📒 Files selected for processing (11)
FEATURES.mdpatches/feature/unified-push.patchpatches/misc/build-support.patchpatches/misc/remove-play.patchseriessrc/kotlin-app/ApplicationLoaderImpl.ktsrc/kotlin/InuConfig.ktsrc/kotlin/UnifiedPushService.ktsrc/kotlin/helpers/UnifiedPushHelper.ktsrc/kotlin/ui/InuBehaviorSettingsActivity.ktsrc/res/values/strings_inu.xml
| UItem.asButton( | ||
| BUTTON_UNIFIED_PUSH_GATEWAY, | ||
| LocaleController.getString(R.string.InuUnifiedPushGateway), | ||
| InuConfig.UNIFIED_PUSH_GATEWAY.value.ifBlank { "default" }, |
There was a problem hiding this comment.
Localize the fallback label instead of hardcoding "default".
Line 189 should use a string resource via LocaleController.getString(...) to keep this UI translatable.
As per coding guidelines, Access strings via LocaleController.getString(R.string.InuXxx).
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/kotlin/ui/InuBehaviorSettingsActivity.kt` at line 189, Replace the
hardcoded fallback "default" used with InuConfig.UNIFIED_PUSH_GATEWAY.value with
a localized string via LocaleController.getString(...); specifically change the
fallback expression to use LocaleController.getString(R.string.InuDefault)
(following the Access strings via LocaleController.getString(R.string.InuXxx)
pattern) so the UI is translatable; update the string resource name if your
project uses a different Inu* key.
| - ], | ||
| - "services": { | ||
| - "appinvite_service": { | ||
| + "current_key": "AIzaSyBUTDbLj-lMgQkor-hY57AIpB2s033xaB0" |
There was a problem hiding this comment.
why did you replace our fcm key?
| @JvmStatic | ||
| fun getGateway(): String { | ||
| val gw = InuConfig.UNIFIED_PUSH_GATEWAY.value | ||
| return if (gw.isBlank()) "https://p2p.belloworld.it/" else gw |
There was a problem hiding this comment.
whose instance this is? why not push.services.mozilla.com or something?
(i might be wrong and mozilla's wont work, i never used unified push so idk)
| @JvmStatic | ||
| fun hasDistributors(context: Context): Boolean { | ||
| return try { | ||
| UnifiedPush.getDistributors(context).isNotEmpty() | ||
| } catch (_: Throwable) { | ||
| false | ||
| } | ||
| } | ||
|
|
||
| @JvmStatic | ||
| fun getDistributors(context: Context): List<String> { | ||
| return try { | ||
| UnifiedPush.getDistributors(context) | ||
| } catch (_: Throwable) { | ||
| emptyList() | ||
| } | ||
| } | ||
|
|
||
| @JvmStatic | ||
| fun getActiveDistributor(context: Context): String? { | ||
| return try { | ||
| UnifiedPush.getAckDistributor(context) | ||
| } catch (_: Throwable) { | ||
| null | ||
| } | ||
| } |
There was a problem hiding this comment.
literally whats the point of these
| for (a in 0 until UserConfig.MAX_ACCOUNT_COUNT) { | ||
| if (UserConfig.getInstance(a).isClientActivated) { | ||
| ConnectionsManager.onInternalPushReceived(a) | ||
| ConnectionsManager.getInstance(a).resumeNetworkMaybe() | ||
| } | ||
| } |
There was a problem hiding this comment.
im pretty sure this can be done per-account by providing some sort of sentinel to differentiate between accounts. waking up every account to check for updates sounds kinda expensive, is there really no other way?
| input.setText(InuConfig.UNIFIED_PUSH_GATEWAY.value) | ||
| input.setTextColor(Theme.getColor(Theme.key_windowBackgroundWhiteBlackText)) | ||
| input.setHintTextColor(Theme.getColor(Theme.key_windowBackgroundWhiteGrayText)) | ||
| input.hint = "https://p2p.belloworld.it/" |
There was a problem hiding this comment.
at least lets make it a shared const in the helper
| @JvmField | ||
| val UNIFIED_PUSH_GATEWAY = StringItem("unified_push_gateway", "https://p2p.belloworld.it/") |
| - manifest.srcFile '../TMessagesProj/config/release/AndroidManifest.xml' | ||
| - } | ||
| - | ||
| + manifest.srcFile '../TMessagesProj/config/release/AndroidManifest_SDK23.xml' | ||
| } | ||
|
|
||
| - flavorDimensions "minApi" | ||
| - | ||
| - productFlavors { | ||
| - bundleAfat { | ||
| - ndk { | ||
| - abiFilters "armeabi-v7a", "arm64-v8a", "x86", "x86_64" | ||
| - } | ||
| - ext { | ||
| - abiVersionCode = 1 | ||
| - } | ||
| - buildConfigField "boolean", "BUNDLE", "true" | ||
| - } | ||
| - bundleAfat_SDK23 { | ||
| - ndk { | ||
| - abiFilters "armeabi-v7a", "arm64-v8a", "x86", "x86_64" | ||
| - } | ||
| - sourceSets.debug { | ||
| - manifest.srcFile '../TMessagesProj/config/debug/AndroidManifest_SDK23.xml' | ||
| - } | ||
| - sourceSets.release { | ||
| - manifest.srcFile '../TMessagesProj/config/release/AndroidManifest_SDK23.xml' | ||
| - } | ||
| - minSdkVersion 23 | ||
| - ext { | ||
| - abiVersionCode = 2 | ||
| - } | ||
| - buildConfigField "boolean", "BUNDLE", "true" | ||
| - } | ||
| - afat { | ||
| - ndk { | ||
| - abiFilters "armeabi-v7a", "arm64-v8a", "x86", "x86_64" | ||
| - } | ||
| - sourceSets.debug { | ||
| - manifest.srcFile '../TMessagesProj/config/debug/AndroidManifest_SDK23.xml' | ||
| - } | ||
| - sourceSets.release { | ||
| - manifest.srcFile '../TMessagesProj/config/release/AndroidManifest_SDK23.xml' | ||
| - } | ||
| - sourceSets.standalone { | ||
| - manifest.srcFile '../TMessagesProj/config/release/AndroidManifest_standalone.xml' | ||
| - } | ||
| - ext { | ||
| - abiVersionCode = 9 | ||
| - } | ||
| - buildConfigField "boolean", "BUNDLE", "false" | ||
| - } | ||
| + manifest.srcFile '../TMessagesProj/config/release/AndroidManifest_SDK23.xml' | ||
| } | ||
|
|
||
| - } |
There was a problem hiding this comment.
also not sure what's happening here..
|
overall honestly unifiedpush is such a niche feature, do we actually really need an external library for it? like genuinely, what's the use-case for unified push? most de-googled users still have microg for fcm pushes to work, and those who don't – can just use the background service, which really isn't that much more battery-hungry than the unifiedpush, considering that unifiedpush means waking up the account and fetching all the updates and processing them locally, instead of just pushing the message contents directly via the push. |
|
Background service works like ass tbh, UnifiedPish would be more reliable. |
fcm would be even more reliable, just saying :p |
Well, duh??? But not everyone uses microG/Gservices though, why not add? |
|
But that "remove play" patch is not necessary though. |
nah its not like im against adding it, i just genuinely can't find any reason for a sane person to not be using microg even if they decided to de-google. like even if some apps do have alternative push delivery mechanisms, most other apps just don't lol |
I kinda agree. honestly I don't think that people who are that fucked up about privacy would use telegram at all. |
|
Seems like this PR is ded |
3 participants
Summary
Changes
feature__unified-pushpatch:PUSH_TYPE_SIMPLEconstant, tag strings,startPushService()guard, connector dependency, manifest servicesrc/kotlin:UnifiedPushHelper,UnifiedPushService, config items, settings UIMade with Claude Code
Screenshots
Summary by CodeRabbit
New Features
Chores