Address Copilot review feedback on push provider abstraction

- Remove suspend from WebsocketManager.restart() (no suspending calls)
- Remove unused context parameter from WebSocketPushProvider
- Clear server-side push config when switching to WebSocket
- Rethrow CancellationException in PushProviderManager and FcmPushProvider
- Derive initial push provider from available providers instead of hardcoded default
- Stabilize getAllProviders() ordering with sortedBy name
- Fix PushProvider KDoc to reflect user-selection model
- Wire handlePushProviderChange to actually call selectAndRegister
- Remove dead getString/putString cases for notification_push_provider
- Move push provider label resolution to Fragment using string resources

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

Author: 2 people

app/src/full/kotlin/io/homeassistant/companion/android/push/FcmPushProvider.kt

@@ -5,6 +5,7 @@ import io.homeassistant.companion.android.common.push.PushRegistrationResult
 import io.homeassistant.companion.android.common.util.MessagingTokenProvider
 import javax.inject.Inject
 import javax.inject.Singleton
+import kotlin.coroutines.cancellation.CancellationException
 import timber.log.Timber
 
 /**
@@ -24,6 +25,7 @@ class FcmPushProvider @Inject constructor(
             val token = messagingTokenProvider()
             !token.isBlank()
         } catch (e: Exception) {
+            if (e is CancellationException) throw e
             Timber.e(e, "FCM is not available")
             false
         }
@@ -34,6 +36,7 @@ class FcmPushProvider @Inject constructor(
             val token = messagingTokenProvider()
             !token.isBlank()
         } catch (e: Exception) {
+            if (e is CancellationException) throw e
             false
         }
     }
@@ -52,6 +55,7 @@ class FcmPushProvider @Inject constructor(
                 )
             }
         } catch (e: Exception) {
+            if (e is CancellationException) throw e
             Timber.e(e, "Failed to register FCM")
             null
         }

app/src/main/kotlin/io/homeassistant/companion/android/push/WebSocketPushProvider.kt

@@ -1,7 +1,5 @@
 package io.homeassistant.companion.android.push
 
-import android.content.Context
-import dagger.hilt.android.qualifiers.ApplicationContext
 import io.homeassistant.companion.android.common.data.servers.ServerManager
 import io.homeassistant.companion.android.common.push.PushProvider
 import io.homeassistant.companion.android.common.push.PushRegistrationResult
@@ -19,7 +17,6 @@ import timber.log.Timber
  */
 @Singleton
 class WebSocketPushProvider @Inject constructor(
-    @ApplicationContext private val context: Context,
     private val serverManager: ServerManager,
     private val settingsDao: SettingsDao,
 ) : PushProvider {

app/src/main/kotlin/io/homeassistant/companion/android/settings/SettingsFragment.kt

@@ -555,22 +555,21 @@ class SettingsFragment(
                 }
                 if (value == "WebSocket") {
                     Toast.makeText(requireContext(), commonR.string.push_provider_websocket_enabled, Toast.LENGTH_SHORT).show()
-                    lifecycleScope.launch {
-                        WebsocketManager.restart(requireContext())
-                    }
+                    WebsocketManager.restart(requireContext())
                 }
                 true
             }
 
             lifecycleScope.launch(Dispatchers.IO) {
-                val entries = mutableListOf<String>()
-                val values = mutableListOf<String>()
-
-                val providers = presenter.getAvailablePushProviders()
-                for (provider in providers) {
-                    entries.add(provider.second)
-                    values.add(provider.first)
-                }
+                val providerNames = presenter.getAvailablePushProviders()
+                val values = providerNames.toMutableList()
+                val entries = providerNames.map { name ->
+                    when (name) {
+                        "FCM" -> getString(commonR.string.push_provider_fcm)
+                        "WebSocket" -> getString(commonR.string.push_provider_websocket)
+                        else -> name
+                    }
+                }.toMutableList()
 
                 val activeValue = presenter.getActivePushProviderValue()
 

app/src/main/kotlin/io/homeassistant/companion/android/settings/SettingsPresenter.kt

@@ -24,7 +24,7 @@ interface SettingsPresenter {
     suspend fun showChangeLog(context: Context)
     suspend fun isChangeLogPopupEnabled(): Boolean
     suspend fun setChangeLogPopupEnabled(enabled: Boolean)
-    fun getAvailablePushProviders(): List<Pair<String, String>>
+    fun getAvailablePushProviders(): List<String>
     fun getActivePushProviderValue(): String
     fun handlePushProviderChange(value: String?)
 }

app/src/main/kotlin/io/homeassistant/companion/android/settings/SettingsPresenterImpl.kt

@@ -116,7 +116,6 @@ class SettingsPresenterImpl @Inject constructor(
             "languages" -> langsManager.getCurrentLang()
             "page_zoom" -> prefsRepository.getPageZoomLevel().toString()
             "screen_orientation" -> prefsRepository.getScreenOrientation()
-            "notification_push_provider" -> selectedPushProvider
             else -> throw IllegalArgumentException("No string found by this key: $key")
         }
     }
@@ -128,7 +127,6 @@ class SettingsPresenterImpl @Inject constructor(
                 "languages" -> langsManager.saveLang(value)
                 "page_zoom" -> prefsRepository.setPageZoomLevel(value?.toIntOrNull())
                 "screen_orientation" -> prefsRepository.saveScreenOrientation(value)
-                "notification_push_provider" -> handlePushProviderChange(value)
                 else -> throw IllegalArgumentException("No string found by this key: $key")
             }
         }
@@ -223,25 +221,21 @@ class SettingsPresenterImpl @Inject constructor(
         }
     }
 
-    override fun getAvailablePushProviders(): List<Pair<String, String>> {
-        return pushProviderManager.getAllProviders().map { provider ->
-            val label = when (provider.name) {
-                "FCM" -> "Firebase Cloud Messaging"
-                "WebSocket" -> "WebSocket"
-                else -> provider.name
-            }
-            provider.name to label
-        }
+    override fun getAvailablePushProviders(): List<String> {
+        return pushProviderManager.getAllProviders().map { it.name }
     }
 
     override fun getActivePushProviderValue(): String {
         selectedPushProvider?.let { return it }
-        return "WebSocket"
+        return pushProviderManager.getAllProviders().firstOrNull()?.name ?: "WebSocket"
     }
 
     override fun handlePushProviderChange(value: String?) {
         if (value == null) return
         selectedPushProvider = value
+        mainScope.launch {
+            pushProviderManager.selectAndRegister(value)
+        }
     }
 
     private fun enableLauncherMode(enable: Boolean) {

app/src/main/kotlin/io/homeassistant/companion/android/websocket/WebsocketManager.kt

@@ -62,7 +62,7 @@ class WebsocketManager(appContext: Context, workerParams: WorkerParameters) :
             WebsocketSetting.ALWAYS
         }
 
-        suspend fun restart(context: Context) {
+        fun restart(context: Context) {
             val websocketNotifications =
                 PeriodicWorkRequestBuilder<WebsocketManager>(15, TimeUnit.MINUTES)
                     .build()

common/src/main/kotlin/io/homeassistant/companion/android/common/data/integration/impl/IntegrationRepositoryImpl.kt

@@ -690,6 +690,11 @@ class IntegrationRepositoryImpl @AssistedInject constructor(
             appData["push_url"] = pushUrl
             appData["push_token"] = ""
             appData["push_encrypt"] = false
+        } else {
+            // No token and no URL: explicitly clear server-side push config (e.g. when switching to WebSocket)
+            appData["push_url"] = ""
+            appData["push_token"] = ""
+            appData["push_encrypt"] = false
         }
 
         return RegisterDeviceRequest(

common/src/main/kotlin/io/homeassistant/companion/android/common/push/PushProvider.kt

@@ -9,7 +9,8 @@ package io.homeassistant.companion.android.common.push
  * - Indicating whether it requires a persistent connection (e.g. WebSocket)
  *
  * Implementations should be registered via Dagger multibinding so that
- * [PushProviderManager] can select the best available provider at runtime.
+ * [PushProviderManager] can discover all available providers and expose them
+ * for user selection.
  */
 interface PushProvider {
 

common/src/main/kotlin/io/homeassistant/companion/android/common/push/PushProviderManager.kt

@@ -5,6 +5,7 @@ import io.homeassistant.companion.android.common.data.servers.ServerManager
 import io.homeassistant.companion.android.common.util.MessagingToken
 import javax.inject.Inject
 import javax.inject.Singleton
+import kotlin.coroutines.cancellation.CancellationException
 import timber.log.Timber
 
 /**
@@ -27,7 +28,7 @@ class PushProviderManager @Inject constructor(
     /**
      * Get all registered providers.
      */
-    fun getAllProviders(): List<PushProvider> = providers.toList()
+    fun getAllProviders(): List<PushProvider> = providers.sortedBy { it.name }
 
     /**
      * Get a provider by name.
@@ -95,6 +96,7 @@ class PushProviderManager @Inject constructor(
                     allowReregistration = false,
                 )
             } catch (e: Exception) {
+                if (e is CancellationException) throw e
                 Timber.e(e, "Failed to update push registration for server ${server.id}")
             }
         }