fix(book-club): review follow-ups — envelope null-data crash, atomic flag, stale-instance gone, slug encoding

app/src/main/java/com/pinakes/app/data/network/BookClubResult.kt

@@ -8,10 +8,16 @@ import java.io.IOException
  * Bridges the Book Club envelope (`{success, data, error}`) into the shared [ApiResult] the
  * whole app already switches on — the analogue of [apiCall] for [BookClubApi].
  *
- * Success mapping:
+ * Success mapping (data-bearing endpoints):
  *  - `error != null`          → [ApiResult.Failure] carrying the plugin's error code + message.
  *  - `data != null`           → [ApiResult.Success].
- *  - no data (no-content ok)  → [ApiResult.Success] of `Unit` (the `Envelope<Unit>` endpoints).
+ *  - success but no data      → [ApiResult.Failure] (a data-bearing endpoint MUST return data;
+ *                               a bodiless success is a server/proxy fault, not a Unit result).
+ *
+ * `Envelope<Unit>` endpoints (health, propose, vote, rsvp, progress) carry no payload and MUST
+ * use [bookClubCallUnit] instead — otherwise a legitimate no-data success here would map to a
+ * spurious failure. Keeping the two paths separate removes the old `Unit as T` unchecked cast,
+ * which crashed a data-bearing caller with ClassCastException on a bodiless success.
  *
  * HTTP failures reuse the core pipeline ([HttpException.toFailure] → [parseErrorBody]): the
  * plugin's error body decodes through the same `Envelope<Unit>` shape (its extra `success`
@@ -24,10 +30,26 @@ suspend fun <T> bookClubCall(block: suspend () -> BookClubEnvelope<T>): ApiResul
         envelope.error != null ->
             ApiResult.Failure(envelope.error!!.code, envelope.error!!.message, httpStatus = 200)
         envelope.data != null -> ApiResult.Success(envelope.data!!)
-        else -> {
-            @Suppress("UNCHECKED_CAST")
-            ApiResult.Success(Unit as T)
-        }
+        else -> ApiResult.Failure(ErrorCodes.UNKNOWN, "Empty response body", 200)
+    }
+} catch (e: HttpException) {
+    e.toFailure()
+} catch (e: IOException) {
+    ApiResult.Failure(ErrorCodes.NETWORK, e.message ?: "Network error", 0)
+} catch (e: Throwable) {
+    ApiResult.Failure(ErrorCodes.UNKNOWN, e.message ?: "Unexpected error", 0)
+}
+
+/**
+ * Envelope<Unit> variant: the endpoint carries no payload, so any non-error success maps to
+ * [ApiResult.Success] of `Unit`. Same HTTP/exception mapping as [bookClubCall].
+ */
+suspend fun bookClubCallUnit(block: suspend () -> BookClubEnvelope<Unit>): ApiResult<Unit> = try {
+    val envelope = block()
+    if (envelope.error != null) {
+        ApiResult.Failure(envelope.error!!.code, envelope.error!!.message, httpStatus = 200)
+    } else {
+        ApiResult.Success(Unit)
     }
 } catch (e: HttpException) {
     e.toFailure()

app/src/main/java/com/pinakes/app/data/repository/BookClubRepository.kt

@@ -12,6 +12,7 @@ import com.pinakes.app.data.network.ApiResult
 import com.pinakes.app.data.network.ErrorCodes
 import com.pinakes.app.data.network.NetworkModule
 import com.pinakes.app.data.network.bookClubCall
+import com.pinakes.app.data.network.bookClubCallUnit
 import com.pinakes.app.data.store.FeatureStore
 import com.pinakes.app.data.store.SessionStore
 
@@ -36,7 +37,7 @@ class BookClubRepository(
      * section on a network blip.
      */
     suspend fun probeAvailability(): Boolean? =
-        when (val res = bookClubCall { network.bookClubApi().health() }) {
+        when (val res = bookClubCallUnit { network.bookClubApi().health() }) {
             is ApiResult.Success -> true
             is ApiResult.Failure ->
                 if (res.httpStatus == 404 || res.code == ErrorCodes.NOT_FOUND) false else null
@@ -47,10 +48,11 @@ class BookClubRepository(
      * previous instance (the user tapped "change library" mid-flight) must not resurrect
      * or clobber the flag of the instance now configured. Null keeps the last-known value.
      */
-    fun applyAvailability(available: Boolean?, probedInstanceUrl: String?) {
-        if (available == null) return
-        if (probedInstanceUrl == null || session.instanceUrl != probedInstanceUrl) return
+    fun applyAvailability(available: Boolean?, probedInstanceUrl: String?): Boolean {
+        if (available == null) return false
+        if (probedInstanceUrl == null || session.instanceUrl != probedInstanceUrl) return false
         features.setBookClubAvailable(available)
+        return true
     }
 
     suspend fun clubs(): ApiResult<BookClubClubs> =
@@ -66,16 +68,16 @@ class BookClubRepository(
         bookClubCall { network.bookClubApi().join(slug) }
 
     suspend fun propose(slug: String, libroId: Int, motivation: String?): ApiResult<Unit> =
-        bookClubCall { network.bookClubApi().propose(slug, ClubProposalRequest(libroId, motivation)) }
+        bookClubCallUnit { network.bookClubApi().propose(slug, ClubProposalRequest(libroId, motivation)) }
 
     suspend fun vote(slug: String, pollId: Int, optionIds: List<Int>): ApiResult<Unit> =
-        bookClubCall { network.bookClubApi().vote(slug, pollId, ClubVoteRequest(optionIds)) }
+        bookClubCallUnit { network.bookClubApi().vote(slug, pollId, ClubVoteRequest(optionIds)) }
 
     suspend fun rsvp(slug: String, meetingId: Int, response: String): ApiResult<Unit> =
-        bookClubCall { network.bookClubApi().rsvp(slug, meetingId, ClubRsvpRequest(response)) }
+        bookClubCallUnit { network.bookClubApi().rsvp(slug, meetingId, ClubRsvpRequest(response)) }
 
     suspend fun progress(slug: String, clubBookId: Int, percent: Int, finished: Boolean?): ApiResult<Unit> =
-        bookClubCall { network.bookClubApi().progress(slug, clubBookId, ClubProgressRequest(percent, finished)) }
+        bookClubCallUnit { network.bookClubApi().progress(slug, clubBookId, ClubProgressRequest(percent, finished)) }
 
     /**
      * A bookclub endpoint answered 404: re-probe the plugin health and, when the
@@ -86,8 +88,11 @@ class BookClubRepository(
     suspend fun confirmGone(): Boolean {
         val instance = session.instanceUrl
         val available = probeAvailability()
-        applyAvailability(available, instance)
-        return available == false
+        // Only treat the plugin as gone when the probe actually applied to the
+        // still-current instance — a stale 404 from a since-switched instance
+        // must not drive pluginGone on the now-current screen.
+        val applied = applyAvailability(available, instance)
+        return applied && available == false
     }
 
     /** Web URL of a poll page — the deep-link target for ballots the app can't render. */

app/src/main/java/com/pinakes/app/data/store/FeatureStore.kt

@@ -8,6 +8,7 @@ import com.pinakes.app.data.model.HealthPayload
 import kotlinx.coroutines.flow.MutableStateFlow
 import kotlinx.coroutines.flow.StateFlow
 import kotlinx.coroutines.flow.asStateFlow
+import kotlinx.coroutines.flow.update
 
 /**
  * Instance feature flags that gate the UI for CATALOGUE-ONLY MODE.
@@ -113,7 +114,7 @@ class FeatureStore(context: Context) {
     /** Persist + publish the Book Club plugin availability (from its own health probe). */
     fun setBookClubAvailable(available: Boolean) {
         prefs.edit().putBoolean(KEY_BOOK_CLUB, available).apply()
-        _features.value = _features.value.copy(bookClubAvailable = available)
+        _features.update { it.copy(bookClubAvailable = available) }
     }
 
     /** Reset to all-enabled (e.g. when forgetting the instance). */

app/src/main/java/com/pinakes/app/ui/navigation/Routes.kt

@@ -1,5 +1,7 @@
 package com.pinakes.app.ui.navigation
 
+import android.net.Uri
+
 /** Centralized navigation route keys. Nested routes carry typed args via path segments. */
 object Routes {
     // Top-level auth graph
@@ -26,7 +28,7 @@ object Routes {
     // Book Club (optional plugin)
     const val BOOK_CLUB = "book-club"
     const val CLUB_DETAIL = "book-club/{slug}"
-    fun clubDetail(slug: String): String = "book-club/$slug"
+    fun clubDetail(slug: String): String = "book-club/${Uri.encode(slug)}"
     const val ARG_CLUB_SLUG = "slug"
 
     /** Graph hosting the bottom-nav + nested authed screens. */