Cleanup feature flags

CLAUDE.md

@@ -235,6 +235,16 @@ backstack.removeAllOf<InboxKey>()
 
 Never hold a long-lived reference to `entries` snapshot contents; mutate through the controller/extensions so changes are observed and persisted.
 
+**Critical navigation rule — `navigateUp` is reserved for the top app bar back button:**
+
+`backstack.navigateUp()` may **only** be wired to the back arrow in a screen's top app bar. In every other case — "done"/"close"/"continue" buttons, success screens, dismissing a flow, programmatic pops after an action — call `backstack.popBackstack()` instead.
+
+**Why:** `navigateUp` carries deep-link/synthetic-stack semantics (the `:app` `BackstackController` overrides it to rebuild a parent stack when the user arrived via a lone deep link). That behavior is correct for the top app bar's "up" affordance, but wrong for an in-content button, where the user expects a plain temporal pop of the current entry. Mixing them makes a button behave differently depending on how the screen was reached, and can diverge from predictive (system) back.
+
+**How to apply:** When arranging the backstack for a flow, do it *at navigation time* (when navigating to a screen), so that a later plain `popBackstack()` and the system back gesture always land in the same place — never special-case the pop inside a button handler.
+
+### Dependency Injection
+
 **Registering destinations.** Each feature exposes a `fun EntryProviderScope<HedvigNavKey>.featureEntries(...)` that calls `entry<Key> { }` for each of its screens. `:app` calls all of them from `hedvigEntryProvider`. Cross-feature navigation is done by `:app` passing `navigateToX` lambdas into each feature's entries function — features never import each other's keys.
 
 ```kotlin
@@ -256,6 +266,14 @@ fun EntryProviderScope<HedvigNavKey>.insuranceEntries(backstack: Backstack, /* n
 - `SessionReconciler.kt` — auth↔back-stack reconciliation; gates the splash via `isReady`; forced logout.
 - `HedvigEntryProvider.kt` — all destination registration and cross-feature lambda wiring.
 
+**Critical Metro KMP rule — never put a platform-overridable `@ContributesBinding` default in `commonMain`:**
+
+If an interface needs a different implementation per platform, bind it **per-platform** with explicit `@Provides`/`@ContributesBinding` in each platform source set (`androidMain`, `iosMain`/`nativeMain`, `jvmMain`) — the way `:featureflags:feature-flags` binds `FeatureManager` (`UnleashFeatureFlagProvider` on Android, provided via `FeatureFlagsAndroidMetroProviders`). Do **not** annotate a `commonMain` default impl with `@ContributesBinding`.
+
+**Why:** a `commonMain` `@ContributesBinding` contributes that binding to **every** target. A platform-specific impl (e.g. an `androidMain` class) that forgets its own contribution annotation is then **silently shadowed** by the common default at runtime — no compile error, just wrong behavior. This actually happened: `NoopPermissionManager` (commonMain, `isPermissionGranted` always `false`) shadowed the real `ActivityCompatPermissionManager` on Android, so every notification sender behaved as if `POST_NOTIFICATIONS` was never granted. With per-platform binding instead, a missing binding is a **compile-time** error (loud), not a silent fallback.
+
+**How to apply:** When you see a `commonMain` interface with platform-specific impls, bind per-platform and keep `commonMain` free of the default binding. If you must keep a `commonMain` default (Metro 1.1.1 has no `rank`), the platform override **must** carry `@ContributesBinding(AppScope::class, replaces = [TheCommonDefault::class])` — but prefer the per-platform pattern, since `replaces` only protects the impls that exist today and silently re-breaks if a future platform impl forgets to contribute.
+
 ### Deep Links
 
 Each feature builds `DeepLinkMatcher`s from its `HedvigDeepLinkContainer` patterns and contributes a `DeepLinkMatcherProvider` (`@ContributesIntoSet`). `:app` aggregates them into one `HedvigDeepLinkMatcher`. `MainActivity` forwards `ACTION_VIEW` intents as raw URI strings down a `deepLinkChannel`; `HedvigApp` matches each to a key and routes it through the controller once logged in. A `DeepLinkAncestry` key entered while logged out is held as `pendingDeepLink` and landed after login.
@@ -530,6 +548,28 @@ dependencies {
 3. Build generates type-safe Kotlin code.
 4. Use the generated query in an internal repository/use case impl; return a project-owned type.
 
+### Working with Feature Flags
+
+Feature flags are backed by Unleash. Before adding or changing a flag, read
+`app/featureflags/feature-flags/FEATURE_FLAG_DEFAULTS.md` — it explains why we never use
+the SDK's `defaultValue` parameter (Unleash Android SDK issue #141), how a flag's value is
+resolved when Unleash has never been fetched, and when bootstrap is required.
+
+To add a new flag:
+1. Add the enum value to `Feature` (commonMain), named to mirror its Unleash key polarity
+   (`ENABLE_X` for `enable_x`, `DISABLE_X` for `disable_x`), with a short explanation.
+2. Map it to its raw Unleash key in `Feature.unleashKey` (androidMain).
+3. `UnleashFeatureFlagProvider` needs no change — it returns the raw `isEnabled(key)` for
+   every flag. At the read site, use the value directly for a positive flag, or invert it
+   (`if (!disableX)`) for a kill switch.
+
+**IMPORTANT — always reconsider bootstrap when adding a feature:** Decide what the flag
+should resolve to when it has *never been fetched* (offline first launch / fresh install
+before the first poll returns). If the natural polarity default is acceptable, do nothing.
+If a rollout needs the opposite default, add a `Toggle(...)` to the bootstrap list in
+`HedvigUnleashClient.start(...)`. Never bootstrap an app-gating flag (e.g.
+`UPDATE_NECESSARY`) into its blocking state — that can brick the app for offline users.
+
 ### Working with Translations
 
 ```bash

FEATURE_FLAG_CLEANUP_NOTES.md

@@ -0,0 +1,80 @@
+# Feature flag cleanup — working notes
+
+Scratch doc to resume the feature-flag audit/cleanup later. Not meant to be committed
+long-term. Last updated 2026-05-30.
+
+## Context
+
+We added the `PUPPY_GUIDE` kill switch (`disable_puppy_guide`) and, while doing so,
+reworked how flag defaults/bootstrap work. The reasoning is documented permanently in
+`app/featureflags/feature-flags/FEATURE_FLAG_DEFAULTS.md` and pointed to
+from the repo `CLAUDE.md` ("Working with Feature Flags"). This notes file is about the
+*next* step: retiring stale flags to reduce clutter.
+
+## Important caveat
+
+Code only reveals each flag's **age** and **read sites** — NOT the actual flip history or
+current rollout %. That lives in the Unleash dashboard. Every "make permanent"
+recommendation below is conditional on confirming in Unleash that the flag is at 100% in
+production before removing it.
+
+Key distinction:
+- **Rollout flags** (gradually turn a new feature on) are meant to die once at 100%. These
+  are the clutter.
+- **Operational kill switches** (`UPDATE_NECESSARY`, `DISABLE_CHAT`) are meant to live
+  forever so we can react to incidents without a release. Age is irrelevant for these.
+
+## Audit of all 12 flags
+
+| Flag | Added | Read sites (excl. provider/tests) | Type | Recommendation |
+|---|---|---|---|---|
+| DISABLE_REDEEM_CAMPAIGN | 2025-03 | **none** | kill switch | **Delete — dead code** |
+| MOVING_FLOW | 2022-05 | insurances, help-center | rollout | Make permanent (likely ON) |
+| PAYMENT_SCREEN | 2023-02 | profile, insurances, help-center | rollout | Make permanent (likely ON) |
+| EDIT_COINSURED | 2023-12 | insurances, help-center, reminders | rollout | Make permanent (likely ON) |
+| HELP_CENTER | 2023-12 | home, profile | kill switch | Make permanent (now core — puppy guide lives in it) |
+| TRAVEL_ADDON | 2024-12 | movingflow, addon-purchase, changetier, addons | rollout | Keep — still new, wide surface |
+| ENABLE_VIDEO_PLAYER_IN_CHAT_MESSAGES | 2025-03 | chat | rollout | Keep — recent |
+| ENABLE_CLAIM_HISTORY | 2026-04 | home, profile, delete-account | rollout | Keep — brand new |
+| PUPPY_GUIDE | 2026-05 | help-center | kill switch | Keep — just shipped |
+| UPDATE_NECESSARY | 2022-05 | HedvigAppState | operational kill switch | **Keep forever** — app-version gate |
+| DISABLE_CHAT | 2023-09 | home | operational kill switch | **Keep forever** — disable chat during incidents |
+| TERMINATION_FLOW | 2023-02 | insurances, data-termination | kill switch | Keep / confirm — may be deliberate legal/ops switch |
+
+## Action plan
+
+### 1. Safe, unambiguous win — delete `DISABLE_REDEEM_CAMPAIGN` (dead code) — DONE 2026-05-30
+Was defined in the enum, `unleashKey`, and the provider, but read **nowhere** in the app.
+Removed from all three files.
+
+### 2. Make permanent — after confirming 100% rollout in Unleash
+`MOVING_FLOW`, `PAYMENT_SCREEN`, `EDIT_COINSURED`, `HELP_CENTER`. For each:
+- Delete the enum value, `unleashKey` arm, and provider arm.
+- At each read site, delete the flag branch and collapse to the enabled path (remove the
+  `combine` / `flatMapLatest` arm that gated on the flag).
+- Delete the related test cases / `FakeFeatureManager` entries.
+
+Read sites to touch (from the audit grep):
+- MOVING_FLOW: `GetInsuranceContractsUseCase`, `GetMemberActionsUseCase`.
+- PAYMENT_SCREEN: `ProfileViewModel`, `GetMemberActionsUseCase`, (+ insurances test data).
+- EDIT_COINSURED: `GetInsuranceContractsUseCase`, `GetInsuranceForEditCoInsuredUseCase`,
+  `GetMemberActionsUseCase`, `GetNeedsCoInsuredInfoRemindersUseCase`.
+- HELP_CENTER: `GetHomeDataUseCase`, `ProfileViewModel` (+ profile/home tests).
+
+### 3. Keep — no action
+Operational kill switches: `UPDATE_NECESSARY`, `DISABLE_CHAT`.
+Too new / still rolling out: `TRAVEL_ADDON`, `ENABLE_VIDEO_PLAYER_IN_CHAT_MESSAGES`,
+`ENABLE_CLAIM_HISTORY`, `PUPPY_GUIDE`.
+
+### 4. Confirm intent — `TERMINATION_FLOW`
+Old enough to retire, but `disable_termination_flow` may be a deliberate legal/operational
+lever. Check before treating as stale.
+
+## Where things live (for when resuming)
+- Enum: `app/featureflags/feature-flags/src/commonMain/.../flags/Feature.kt`
+- Key map: `app/featureflags/feature-flags/src/androidMain/.../flags/FeatureUnleashKey.kt`
+- Resolution + negation: `app/featureflags/feature-flags/src/androidMain/.../flags/UnleashFeatureFlagProvider.kt`
+- Bootstrap: `app/featureflags/feature-flags/src/androidMain/.../HedvigUnleashClient.kt`
+- Verify a build with: `./gradlew :feature-flags:ktlintFormat :feature-flags:compileAndroidMain`
+- iOS equivalent flags live in the separate repo at `../ugglan` (different flag names; e.g.
+  `help_center` positive there vs `disable_help_center` here).

app/data/data-addons/src/main/kotlin/com/hedvig/android/data/addons/data/GetAddonBannerInfoUseCase.kt

@@ -11,14 +11,11 @@ import com.apollographql.apollo.cache.normalized.fetchPolicy
 import com.hedvig.android.apollo.safeFlow
 import com.hedvig.android.core.common.ErrorMessage
 import com.hedvig.android.core.common.di.AppScope
-import com.hedvig.android.featureflags.FeatureManager
-import com.hedvig.android.featureflags.flags.Feature
 import com.hedvig.android.logger.LogPriority
 import com.hedvig.android.logger.logcat
 import dev.zacsweers.metro.Inject
 import dev.zacsweers.metro.SingleIn
 import kotlinx.coroutines.flow.Flow
-import kotlinx.coroutines.flow.combine
 import kotlinx.coroutines.flow.map
 import kotlinx.serialization.Serializable
 import octopus.AddonBannersQuery
@@ -32,7 +29,6 @@ interface GetAddonBannerInfoUseCase {
 @Inject
 internal class GetAddonBannerInfoUseCaseImpl(
   private val apolloClient: ApolloClient,
-  private val featureManager: FeatureManager,
 ) : GetAddonBannerInfoUseCase {
   override fun invoke(source: AddonBannerSource): Flow<Either<ErrorMessage, List<AddonBannerInfo>>> {
     val mappedSource = when (source) {
@@ -49,59 +45,48 @@ internal class GetAddonBannerInfoUseCaseImpl(
 
       AddonBannerSource.CAR_ADDON_DEEPLINK -> listOf(AddonFlow.APP_CAR_PLUS)
     }
-    return combine(
-      featureManager.isFeatureEnabled(Feature.TRAVEL_ADDON),
-      apolloClient
-        .query(AddonBannersQuery(mappedSource))
-        .fetchPolicy(CacheAndNetwork)
-        .safeFlow()
-        .map {
-          it.mapLeft { error ->
+    return apolloClient
+      .query(AddonBannersQuery(mappedSource))
+      .fetchPolicy(CacheAndNetwork)
+      .safeFlow()
+      .map { addonBannersQueryResult ->
+        either {
+          val bannerData = addonBannersQueryResult.mapLeft { error ->
             logcat(LogPriority.WARN, error) {
               "Error from AddonBannersQuery " +
                 "from source: $mappedSource: $error"
             }
             ErrorMessage()
+          }.bind().currentMember.addonBanners
+          if (bannerData.isEmpty()) {
+            logcat(LogPriority.DEBUG) { "Got empty response from AddonBannersQuery" }
+            return@either emptyList()
           }
-        },
-    ) { isAddonFlagOn, addonBannersQueryResult ->
-      either {
-        if (!isAddonFlagOn) {
-          logcat(LogPriority.INFO) {
-            "Tried AddonBannersQuery but travel addon feature flag is off"
-          }
-          return@either emptyList()
-        }
-        val bannerData = addonBannersQueryResult.bind().currentMember.addonBanners
-        if (bannerData.isEmpty()) {
-          logcat(LogPriority.DEBUG) { "Got empty response from AddonBannersQuery" }
-          return@either emptyList()
-        }
-        bannerData.mapNotNull { banner ->
-          val flowType = when (banner.flow) {
-            AddonFlow.APP_TRAVEL_PLUS_SELL_ONLY -> FlowType.APP_TRAVEL_PLUS_SELL_ONLY
-            AddonFlow.APP_TRAVEL_PLUS_SELL_OR_UPGRADE -> FlowType.APP_TRAVEL_PLUS_SELL_OR_UPGRADE
-            AddonFlow.APP_CAR_PLUS -> FlowType.APP_CAR_PLUS
-            AddonFlow.UNKNOWN__ -> null
-          }
-          val eligibleInsurancesIds = banner.contractIds.toNonEmptyListOrNull()
-          if (flowType == null || eligibleInsurancesIds == null) {
-            logcat(LogPriority.DEBUG) {
-              "Got AddonFlow.UNKNOWN or empty contractIds from AddonBannersQuery"
+          bannerData.mapNotNull { banner ->
+            val flowType = when (banner.flow) {
+              AddonFlow.APP_TRAVEL_PLUS_SELL_ONLY -> FlowType.APP_TRAVEL_PLUS_SELL_ONLY
+              AddonFlow.APP_TRAVEL_PLUS_SELL_OR_UPGRADE -> FlowType.APP_TRAVEL_PLUS_SELL_OR_UPGRADE
+              AddonFlow.APP_CAR_PLUS -> FlowType.APP_CAR_PLUS
+              AddonFlow.UNKNOWN__ -> null
+            }
+            val eligibleInsurancesIds = banner.contractIds.toNonEmptyListOrNull()
+            if (flowType == null || eligibleInsurancesIds == null) {
+              logcat(LogPriority.DEBUG) {
+                "Got AddonFlow.UNKNOWN or empty contractIds from AddonBannersQuery"
+              }
+              null
+            } else {
+              AddonBannerInfo(
+                title = banner.displayTitleName,
+                description = banner.descriptionDisplayName,
+                labels = banner.badges,
+                eligibleInsurancesIds = eligibleInsurancesIds,
+                flowType = flowType,
+              )
             }
-            null
-          } else {
-            AddonBannerInfo(
-              title = banner.displayTitleName,
-              description = banner.descriptionDisplayName,
-              labels = banner.badges,
-              eligibleInsurancesIds = eligibleInsurancesIds,
-              flowType = flowType,
-            )
           }
         }
       }
-    }
   }
 }
 

app/data/data-addons/src/test/kotlin/GetTravelAddonBannerInfoUseCaseImplTest.kt

@@ -16,8 +16,6 @@ import com.hedvig.android.data.addons.data.AddonBannerSource.INSURANCES_TAB
 import com.hedvig.android.data.addons.data.AddonBannerSource.TRAVEL_CERTIFICATES
 import com.hedvig.android.data.addons.data.FlowType
 import com.hedvig.android.data.addons.data.GetAddonBannerInfoUseCaseImpl
-import com.hedvig.android.featureflags.flags.Feature.TRAVEL_ADDON
-import com.hedvig.android.featureflags.test.FakeFeatureManager
 import com.hedvig.android.logger.TestLogcatLoggingRule
 import kotlinx.coroutines.flow.first
 import kotlinx.coroutines.test.runTest
@@ -138,31 +136,17 @@ class GetTravelAddonBannerInfoUseCaseImplTest {
       )
     }
 
-  @Test
-  fun `if FF for addons is off return empty list`() = runTest {
-    val featureManager = FakeFeatureManager(fixedMap = mapOf(TRAVEL_ADDON to false))
-    val sut = GetAddonBannerInfoUseCaseImpl(apolloClientWithTwoFlows, featureManager)
-    val resultFromInsurances = sut.invoke(INSURANCES_TAB).first()
-    assertThat(resultFromInsurances)
-      .isEqualTo(emptyList<AddonBannerInfo>().right())
-    val resultFromTravel = sut.invoke(TRAVEL_CERTIFICATES).first()
-    assertThat(resultFromTravel)
-      .isEqualTo(emptyList<AddonBannerInfo>().right())
-  }
-
   @Test
   fun `if get null bannerData from BE return empty list`() = runTest {
-    val featureManager = FakeFeatureManager(fixedMap = mapOf(TRAVEL_ADDON to true))
-    val sut = GetAddonBannerInfoUseCaseImpl(apolloClientWithNullBannerData, featureManager)
+    val sut = GetAddonBannerInfoUseCaseImpl(apolloClientWithNullBannerData)
     val result = sut.invoke(TRAVEL_CERTIFICATES).first()
     assertThat(result)
       .isEqualTo(emptyList<AddonBannerInfo>().right())
   }
 
   @Test
   fun `the source is mapped to the correct flow for the query`() = runTest {
-    val featureManager = FakeFeatureManager(fixedMap = mapOf(TRAVEL_ADDON to true))
-    val sut = GetAddonBannerInfoUseCaseImpl(apolloClientWithTwoFlows, featureManager)
+    val sut = GetAddonBannerInfoUseCaseImpl(apolloClientWithTwoFlows)
     val resultFromTravelCertificates = sut.invoke(TRAVEL_CERTIFICATES).first().getOrNull()
     assertThat(resultFromTravelCertificates).isNotNull()
     val resultFromInsurances = sut.invoke(INSURANCES_TAB).first().getOrNull()
@@ -171,35 +155,31 @@ class GetTravelAddonBannerInfoUseCaseImplTest {
 
   @Test
   fun `if get bannerData from BE is not null but contractIds are empty return empty list`() = runTest {
-    val featureManager = FakeFeatureManager(fixedMap = mapOf(TRAVEL_ADDON to true))
-    val sut = GetAddonBannerInfoUseCaseImpl(apolloClientWithEmptyContracts, featureManager)
+    val sut = GetAddonBannerInfoUseCaseImpl(apolloClientWithEmptyContracts)
     val result = sut.invoke(TRAVEL_CERTIFICATES).first()
     assertThat(result)
       .isEqualTo(emptyList<AddonBannerInfo>().right())
   }
 
   @Test
   fun `if get error from BE return ErrorMessage`() = runTest {
-    val featureManager = FakeFeatureManager(fixedMap = mapOf(TRAVEL_ADDON to true))
-    val sut = GetAddonBannerInfoUseCaseImpl(apolloClientWithError, featureManager)
+    val sut = GetAddonBannerInfoUseCaseImpl(apolloClientWithError)
     val resultFromTravels = sut.invoke(TRAVEL_CERTIFICATES).first().isLeft()
     assertThat(resultFromTravels)
       .isTrue()
   }
 
   @Test
   fun `if get full banner data from BE return TravelAddonBannerInfo`() = runTest {
-    val featureManager = FakeFeatureManager(fixedMap = mapOf(TRAVEL_ADDON to true))
-    val sut = GetAddonBannerInfoUseCaseImpl(apolloClientWithFullBannerData, featureManager)
+    val sut = GetAddonBannerInfoUseCaseImpl(apolloClientWithFullBannerData)
     val resultFromTravel = sut.invoke(TRAVEL_CERTIFICATES).first().getOrNull()
     assertThat(resultFromTravel)
       .isNotNull()
   }
 
   @Test
   fun `the received data is passed correctly and in full`() = runTest {
-    val featureManager = FakeFeatureManager(fixedMap = mapOf(TRAVEL_ADDON to true))
-    val sut = GetAddonBannerInfoUseCaseImpl(apolloClientWithFullBannerData, featureManager)
+    val sut = GetAddonBannerInfoUseCaseImpl(apolloClientWithFullBannerData)
     val resultFromTravel = sut.invoke(TRAVEL_CERTIFICATES).first().getOrNull()
     assertThat(resultFromTravel)
       .isEqualTo(