From aec10ce787e387818d9c528fe369a6935a81a5f8 Mon Sep 17 00:00:00 2001 From: Bryan Van Deusen Date: Tue, 2 Jun 2026 09:16:36 -0400 Subject: [PATCH 1/5] fix(android): scope BaseUrlInterceptor to placeholder host only MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Discover suggestion artist images failed to load on Android while loading fine in the web client. Root cause: BaseUrlInterceptor unconditionally rewrote every outgoing request's scheme/host/port to AuthStore.baseUrl. That's correct for Minstrel-bound requests built with the http://placeholder.invalid sentinel (Retrofit's frozen baseUrl, every cover-URL builder, ServerImage's resolveServerUrl). But Lidarr surfaces artist artwork as absolute URLs to external hosts (artwork.musicbrainz.org, coverartarchive.org); rewriting those to the Minstrel host produced 404s that Coil silently fell back from to the User icon. Web works because the browser fetches the URL as authored. Coil on Android shares the OkHttp client (and so the interceptor chain) with Retrofit, which is why the bug surfaced here only. Add a PLACEHOLDER_HOST companion constant and short-circuit the rewrite for non-placeholder hosts. Test coverage: - placeholder host → rewritten to live baseUrl - absolute external URL → host/scheme/path preserved - unparseable baseUrl → falls through (no throw) AuthCookieInterceptor still attaches the Minstrel session cookie to external requests; external hosts ignore unrecognized cookies so that's not breaking anything, but it's worth a follow-up DRY pass to scope auth attachment the same way. --- .../minstrel/api/BaseUrlInterceptor.kt | 24 +++- .../minstrel/api/BaseUrlInterceptorTest.kt | 130 ++++++++++++++++++ 2 files changed, 150 insertions(+), 4 deletions(-) create mode 100644 android/app/src/test/java/com/fabledsword/minstrel/api/BaseUrlInterceptorTest.kt diff --git a/android/app/src/main/java/com/fabledsword/minstrel/api/BaseUrlInterceptor.kt b/android/app/src/main/java/com/fabledsword/minstrel/api/BaseUrlInterceptor.kt index 79f838d4..06e59bc2 100644 --- a/android/app/src/main/java/com/fabledsword/minstrel/api/BaseUrlInterceptor.kt +++ b/android/app/src/main/java/com/fabledsword/minstrel/api/BaseUrlInterceptor.kt @@ -9,16 +9,23 @@ import javax.inject.Inject import javax.inject.Singleton /** - * Rewrites every outgoing request's scheme/host/port to match the + * Rewrites Minstrel-server requests' scheme/host/port to match the * current [AuthStore.baseUrl]. Retrofit's `.baseUrl(...)` is read * once at Retrofit creation, but AuthStore.baseUrl loads from Room * asynchronously — so at injection time the Retrofit instance is * frozen pointing at the [AuthStore.DEFAULT_BASE_URL] placeholder. * * This interceptor closes the gap: Retrofit can stay built with the - * placeholder forever, and every actual request gets retargeted at - * the live AuthStore value. Lets the user change server URL in - * Settings without an app relaunch (Phase 11 wiring). + * placeholder forever, and every actual Minstrel-bound request gets + * retargeted at the live AuthStore value. Lets the user change + * server URL in Settings without an app relaunch (Phase 11 wiring). + * + * Only requests whose host is the [PLACEHOLDER_HOST] sentinel are + * rewritten. Absolute external URLs — Lidarr-surfaced artwork from + * artwork.musicbrainz.org / coverartarchive.org, for example — must + * reach their authored host unchanged; rewriting them to the + * Minstrel host produced 404s the user saw as missing Discover + * suggestion covers. * * No-op when the stored base URL is unparseable (falls through to * whatever Retrofit had) — that case shows up as a transport-level @@ -31,6 +38,7 @@ class BaseUrlInterceptor @Inject constructor( override fun intercept(chain: Interceptor.Chain): Response { val original = chain.request() + if (original.url.host != PLACEHOLDER_HOST) return chain.proceed(original) val baseUrl = authStore.baseUrl.value.toHttpUrlOrNull() ?: return chain.proceed(original) val rewritten: HttpUrl = original.url.newBuilder() @@ -40,4 +48,12 @@ class BaseUrlInterceptor @Inject constructor( .build() return chain.proceed(original.newBuilder().url(rewritten).build()) } + + companion object { + /** Sentinel host used by Retrofit + every code path that builds + * a Minstrel-server URL via the `http://placeholder.invalid/...` + * form (see [com.fabledsword.minstrel.shared.resolveServerUrl], + * CoverUrls.kt, AlbumRef/TrackRef cover getters). */ + const val PLACEHOLDER_HOST = "placeholder.invalid" + } } diff --git a/android/app/src/test/java/com/fabledsword/minstrel/api/BaseUrlInterceptorTest.kt b/android/app/src/test/java/com/fabledsword/minstrel/api/BaseUrlInterceptorTest.kt new file mode 100644 index 00000000..a5771f9a --- /dev/null +++ b/android/app/src/test/java/com/fabledsword/minstrel/api/BaseUrlInterceptorTest.kt @@ -0,0 +1,130 @@ +package com.fabledsword.minstrel.api + +import com.fabledsword.minstrel.auth.AuthStore +import com.fabledsword.minstrel.cache.db.dao.AuthSessionDao +import io.mockk.coEvery +import io.mockk.every +import io.mockk.mockk +import kotlinx.coroutines.ExperimentalCoroutinesApi +import kotlinx.coroutines.flow.flowOf +import kotlinx.coroutines.test.TestScope +import kotlinx.coroutines.test.UnconfinedTestDispatcher +import okhttp3.HttpUrl +import okhttp3.OkHttpClient +import okhttp3.Protocol +import okhttp3.Request +import okhttp3.Response +import okhttp3.ResponseBody.Companion.toResponseBody +import okhttp3.mockwebserver.MockResponse +import okhttp3.mockwebserver.MockWebServer +import org.junit.jupiter.api.AfterEach +import org.junit.jupiter.api.BeforeEach +import org.junit.jupiter.api.Test +import kotlin.test.assertEquals + +@OptIn(ExperimentalCoroutinesApi::class) +class BaseUrlInterceptorTest { + private lateinit var server: MockWebServer + private lateinit var authStore: AuthStore + + @BeforeEach + fun setup() { + server = MockWebServer().apply { start() } + val dao = + mockk { + every { observe() } returns flowOf(null) + coEvery { get() } returns null + coEvery { upsert(any()) } returns Unit + coEvery { setSessionCookie(any()) } returns Unit + coEvery { setBaseUrl(any()) } returns Unit + } + authStore = AuthStore(dao, TestScope(UnconfinedTestDispatcher())) + } + + @AfterEach + fun teardown() { + server.shutdown() + } + + @Test + fun `rewrites placeholder host to live baseUrl host port and scheme`() { + // baseUrl points at the mock server; outbound request targets the + // placeholder sentinel — interceptor should rewrite to the mock. + authStore.setBaseUrl(server.url("/").toString()) + server.enqueue(MockResponse().setResponseCode(200)) + val client = OkHttpClient.Builder().addInterceptor(BaseUrlInterceptor(authStore)).build() + + client.newCall( + Request.Builder().url("http://placeholder.invalid/api/test").build(), + ).execute() + + val recorded = server.takeRequest() + assertEquals("/api/test", recorded.path) + // MockWebServer's recorded Host header is the rewritten host:port + assertEquals("${server.hostName}:${server.port}", recorded.getHeader("Host")) + } + + @Test + fun `passes external absolute URL through unchanged`() { + // Lidarr-surfaced artwork hosts (artwork.musicbrainz.org, + // coverartarchive.org) are absolute external URLs. The + // interceptor must NOT rewrite them to the Minstrel base — + // doing so produces 404s the user saw as missing Discover + // suggestion covers. + authStore.setBaseUrl(server.url("/").toString()) + var seen: HttpUrl? = null + val client = + OkHttpClient.Builder() + .addInterceptor(BaseUrlInterceptor(authStore)) + .addInterceptor { chain -> + seen = chain.request().url + Response.Builder() + .request(chain.request()) + .protocol(Protocol.HTTP_1_1) + .code(200) + .message("OK") + .body("".toResponseBody()) + .build() + } + .build() + + client.newCall( + Request.Builder() + .url("https://artwork.musicbrainz.org/release-group/abc/front-250.jpg") + .build(), + ).execute() + + assertEquals("artwork.musicbrainz.org", seen?.host) + assertEquals("https", seen?.scheme) + assertEquals("/release-group/abc/front-250.jpg", seen?.encodedPath) + } + + @Test + fun `falls through when stored baseUrl is unparseable`() { + // Empty / malformed baseUrl can happen between cold start and + // first AuthStore load; the interceptor should leave the + // request alone rather than throw. + authStore.setBaseUrl("not a url") + var seen: HttpUrl? = null + val client = + OkHttpClient.Builder() + .addInterceptor(BaseUrlInterceptor(authStore)) + .addInterceptor { chain -> + seen = chain.request().url + Response.Builder() + .request(chain.request()) + .protocol(Protocol.HTTP_1_1) + .code(200) + .message("OK") + .body("".toResponseBody()) + .build() + } + .build() + + client.newCall( + Request.Builder().url("http://placeholder.invalid/api/test").build(), + ).execute() + + assertEquals("placeholder.invalid", seen?.host) + } +} From 22dc343b39fe0cc386b169cfbd89d7f1a9269e90 Mon Sep 17 00:00:00 2001 From: Bryan Van Deusen Date: Tue, 2 Jun 2026 09:35:58 -0400 Subject: [PATCH 2/5] feat(android): swap top-nav Library icon to Lucide.SquareLibrary MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit LibraryBig (stacked book spines) didn't read as "Library" without the label — easy to misread as a generic stack/columns icon. SquareLibrary frames the same books-on-shelf glyph inside a rounded square, matching the visual weight of the neighbouring House and Search icons better and reading more clearly as a distinct tappable destination. Untouched: the RequestsScreen per-row "album"-kind avatar still uses LibraryBig (parity with Flutter's lib/requests/requests_screen.dart). --- .../fabledsword/minstrel/shared/widgets/MainAppBarActions.kt | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/android/app/src/main/java/com/fabledsword/minstrel/shared/widgets/MainAppBarActions.kt b/android/app/src/main/java/com/fabledsword/minstrel/shared/widgets/MainAppBarActions.kt index a1707c59..995423eb 100644 --- a/android/app/src/main/java/com/fabledsword/minstrel/shared/widgets/MainAppBarActions.kt +++ b/android/app/src/main/java/com/fabledsword/minstrel/shared/widgets/MainAppBarActions.kt @@ -15,8 +15,8 @@ import androidx.hilt.navigation.compose.hiltViewModel import androidx.lifecycle.compose.collectAsStateWithLifecycle import androidx.navigation.NavHostController import com.composables.icons.lucide.House -import com.composables.icons.lucide.LibraryBig import com.composables.icons.lucide.Lucide +import com.composables.icons.lucide.SquareLibrary import com.composables.icons.lucide.EllipsisVertical import com.composables.icons.lucide.Search as SearchIcon import com.fabledsword.minstrel.nav.Admin @@ -58,7 +58,7 @@ fun MainAppBarActions( } if (currentRouteName != Library::class.qualifiedName) { IconButton(onClick = { navController.navigate(Library) }) { - Icon(Lucide.LibraryBig, contentDescription = "Library") + Icon(Lucide.SquareLibrary, contentDescription = "Library") } } if (currentRouteName != SearchRoute::class.qualifiedName) { From faf2cac0c977fa9f1e8f1b51a17cc0181dc62a1a Mon Sep 17 00:00:00 2001 From: Bryan Van Deusen Date: Tue, 2 Jun 2026 09:40:49 -0400 Subject: [PATCH 3/5] feat(android): use Material Outlined LibraryMusic for top-nav Library MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Lucide has no music-library glyph — Library / LibraryBig / SquareLibrary all read as a generic books-on-shelf icon without a label. Operator picked Material's LibraryMusic (the canonical "books + music note" symbol used by every major music app) as the recognizable alternative. Use the Outlined variant: filled icons would clash with the neighbouring stroked Lucide icons (House, Search, EllipsisVertical), but Outlined's stroke style matches Lucide closely enough that the mix is subtle. Adds the compose-material-icons-extended dependency (version pinned by compose-bom). R8 strips unused icons in release builds so the APK cost is just the ones we actually reference. --- android/app/build.gradle.kts | 1 + .../minstrel/shared/widgets/MainAppBarActions.kt | 12 ++++++++++-- android/gradle/libs.versions.toml | 5 +++++ 3 files changed, 16 insertions(+), 2 deletions(-) diff --git a/android/app/build.gradle.kts b/android/app/build.gradle.kts index 2535edf2..5b81d850 100644 --- a/android/app/build.gradle.kts +++ b/android/app/build.gradle.kts @@ -139,6 +139,7 @@ dependencies { implementation(libs.compose.ui) implementation(libs.compose.ui.graphics) implementation(libs.compose.material3) + implementation(libs.compose.material.icons.extended) implementation(libs.compose.ui.text.google.fonts) debugImplementation(libs.compose.ui.tooling) implementation(libs.compose.ui.tooling.preview) diff --git a/android/app/src/main/java/com/fabledsword/minstrel/shared/widgets/MainAppBarActions.kt b/android/app/src/main/java/com/fabledsword/minstrel/shared/widgets/MainAppBarActions.kt index 995423eb..d6b32c7e 100644 --- a/android/app/src/main/java/com/fabledsword/minstrel/shared/widgets/MainAppBarActions.kt +++ b/android/app/src/main/java/com/fabledsword/minstrel/shared/widgets/MainAppBarActions.kt @@ -11,12 +11,13 @@ import androidx.compose.runtime.getValue import androidx.compose.runtime.mutableStateOf import androidx.compose.runtime.remember import androidx.compose.runtime.setValue +import androidx.compose.material.icons.Icons +import androidx.compose.material.icons.outlined.LibraryMusic import androidx.hilt.navigation.compose.hiltViewModel import androidx.lifecycle.compose.collectAsStateWithLifecycle import androidx.navigation.NavHostController import com.composables.icons.lucide.House import com.composables.icons.lucide.Lucide -import com.composables.icons.lucide.SquareLibrary import com.composables.icons.lucide.EllipsisVertical import com.composables.icons.lucide.Search as SearchIcon import com.fabledsword.minstrel.nav.Admin @@ -58,7 +59,14 @@ fun MainAppBarActions( } if (currentRouteName != Library::class.qualifiedName) { IconButton(onClick = { navController.navigate(Library) }) { - Icon(Lucide.SquareLibrary, contentDescription = "Library") + // Intentional cross-family icon — Lucide has no + // equivalent "library + music" glyph and the literal + // library icons (Library / LibraryBig / SquareLibrary) + // don't read as "music collection" without a label. + // Material's Outlined LibraryMusic is the canonical + // music-library symbol; the outlined variant matches + // Lucide's stroke style closely enough to blend. + Icon(Icons.Outlined.LibraryMusic, contentDescription = "Library") } } if (currentRouteName != SearchRoute::class.qualifiedName) { diff --git a/android/gradle/libs.versions.toml b/android/gradle/libs.versions.toml index 8a2be34f..5fc19f10 100644 --- a/android/gradle/libs.versions.toml +++ b/android/gradle/libs.versions.toml @@ -51,6 +51,11 @@ compose-ui-graphics = { module = "androidx.compose.ui:ui-graphics" } compose-ui-tooling = { module = "androidx.compose.ui:ui-tooling" } compose-ui-tooling-preview = { module = "androidx.compose.ui:ui-tooling-preview" } compose-material3 = { module = "androidx.compose.material3:material3" } +# material-icons-extended supplies the full Material Icons catalog +# (Outlined / Filled / Rounded / Sharp / TwoTone). We use the Outlined +# variant for LibraryMusic — its stroke style blends with Lucide. +# Version pinned by compose-bom. +compose-material-icons-extended = { module = "androidx.compose.material:material-icons-extended" } compose-ui-text-google-fonts = { module = "androidx.compose.ui:ui-text-google-fonts" } hilt-android = { module = "com.google.dagger:hilt-android", version.ref = "hilt" } hilt-compiler = { module = "com.google.dagger:hilt-compiler", version.ref = "hilt" } From 438e81a117f768a9c9269347e15c32b4fd561e52 Mon Sep 17 00:00:00 2001 From: Bryan Van Deusen Date: Tue, 2 Jun 2026 09:43:45 -0400 Subject: [PATCH 4/5] feat(android): media-notification tap opens NowPlaying MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Tapping the system media notification previously landed on whatever shell route MainActivity last rendered (Home / Library / Search) because MinstrelPlayerService never configured the session-activity PendingIntent, so Media3 defaulted to the launcher activity entry point. Operator request: tap should go straight to the full player. MinstrelPlayerService.onCreate now builds a PendingIntent targeting MainActivity with an EXTRA_OPEN_NOW_PLAYING flag and passes it to MediaSession.Builder.setSessionActivity. The flag also covers the lock-screen card and the Pixel Watch tile — both use the same session-activity PendingIntent. MainActivity reads the extra in onCreate AND onNewIntent (so a warm app gets the navigation too, not just cold launches), flips a pendingOpenNowPlaying StateFlow, then strips the extra so a config-change recreation doesn't re-trigger. The App composable observes the flag and runs a LaunchedEffect to navigate once the NavHost is mounted — handles both cold start (BootSplash → resolved → navigate) and warm start. launchSingleTop avoids stacking copies if NowPlaying is already on top, and the onOpenedNowPlaying callback clears the flag post-navigation so later recompositions don't re-fire. Divergence from Flutter (intentional): audio_service's default notification tap behavior just opens the launcher activity at whatever screen it was on — exactly the behavior the operator asked to improve. --- .../com/fabledsword/minstrel/MainActivity.kt | 59 ++++++++++++++++++- .../minstrel/player/MinstrelPlayerService.kt | 40 ++++++++++++- 2 files changed, 97 insertions(+), 2 deletions(-) diff --git a/android/app/src/main/java/com/fabledsword/minstrel/MainActivity.kt b/android/app/src/main/java/com/fabledsword/minstrel/MainActivity.kt index cda6d5dc..f1203b98 100644 --- a/android/app/src/main/java/com/fabledsword/minstrel/MainActivity.kt +++ b/android/app/src/main/java/com/fabledsword/minstrel/MainActivity.kt @@ -1,5 +1,6 @@ package com.fabledsword.minstrel +import android.content.Intent import android.os.Bundle import androidx.activity.ComponentActivity import androidx.activity.compose.setContent @@ -11,6 +12,7 @@ import androidx.compose.material3.MaterialTheme import androidx.compose.material3.Surface import androidx.compose.runtime.Composable import androidx.compose.runtime.CompositionLocalProvider +import androidx.compose.runtime.LaunchedEffect import androidx.compose.runtime.getValue import androidx.compose.ui.Alignment import androidx.compose.ui.Modifier @@ -22,10 +24,14 @@ import com.fabledsword.minstrel.cache.CachedTrackIds import com.fabledsword.minstrel.nav.DetailSeedCache import com.fabledsword.minstrel.nav.LocalDetailSeedCache import com.fabledsword.minstrel.nav.MinstrelNavGraph +import com.fabledsword.minstrel.nav.NowPlaying import com.fabledsword.minstrel.shared.widgets.LocalCachedTrackIds import com.fabledsword.minstrel.theme.MinstrelTheme import com.fabledsword.minstrel.theme.ThemePreferenceViewModel import dagger.hilt.android.AndroidEntryPoint +import kotlinx.coroutines.flow.MutableStateFlow +import kotlinx.coroutines.flow.StateFlow +import kotlinx.coroutines.flow.asStateFlow import javax.inject.Inject @AndroidEntryPoint @@ -33,10 +39,46 @@ class MainActivity : ComponentActivity() { @Inject lateinit var seedCache: DetailSeedCache @Inject lateinit var cachedTrackIds: CachedTrackIds + // Flipped to true when the user taps the media notification (or + // any other entry point that asks for the full player). The App + // composable observes this, navigates to NowPlaying once the + // NavHost is ready, then calls back to reset the flag so the + // navigation doesn't re-fire on the next recomposition. + private val pendingOpenNowPlaying = MutableStateFlow(false) + override fun onCreate(savedInstanceState: Bundle?) { super.onCreate(savedInstanceState) enableEdgeToEdge() - setContent { App(seedCache = seedCache, cachedTrackIds = cachedTrackIds) } + consumeOpenNowPlayingIntent(intent) + setContent { + App( + seedCache = seedCache, + cachedTrackIds = cachedTrackIds, + pendingOpenNowPlaying = pendingOpenNowPlaying.asStateFlow(), + onOpenedNowPlaying = { pendingOpenNowPlaying.value = false }, + ) + } + } + + override fun onNewIntent(intent: Intent) { + super.onNewIntent(intent) + consumeOpenNowPlayingIntent(intent) + } + + private fun consumeOpenNowPlayingIntent(intent: Intent?) { + if (intent?.getBooleanExtra(EXTRA_OPEN_NOW_PLAYING, false) == true) { + pendingOpenNowPlaying.value = true + // Strip the extra so a subsequent config-change recreation + // doesn't re-trigger the navigation. + intent.removeExtra(EXTRA_OPEN_NOW_PLAYING) + } + } + + companion object { + /** PendingIntent extra set by [com.fabledsword.minstrel.player.MinstrelPlayerService] + * so a media-notification tap lands on the full NowPlaying screen + * instead of whatever shell route MainActivity last rendered. */ + const val EXTRA_OPEN_NOW_PLAYING = "com.fabledsword.minstrel.action.OPEN_NOW_PLAYING" } } @@ -44,11 +86,14 @@ class MainActivity : ComponentActivity() { private fun App( seedCache: DetailSeedCache, cachedTrackIds: CachedTrackIds, + pendingOpenNowPlaying: StateFlow, + onOpenedNowPlaying: () -> Unit, themeVm: ThemePreferenceViewModel = hiltViewModel(), gate: AuthGateViewModel = hiltViewModel(), ) { val theme by themeVm.themeMode.collectAsStateWithLifecycle() val cached by cachedTrackIds.ids.collectAsStateWithLifecycle() + val pending by pendingOpenNowPlaying.collectAsStateWithLifecycle() MinstrelTheme(darkOverride = theme.toDarkOverride()) { CompositionLocalProvider( LocalDetailSeedCache provides seedCache, @@ -65,6 +110,18 @@ private fun App( // `ShellScaffold` wrap; full-screen routes (NowPlaying / // Queue / unauthenticated) bypass the shell entirely. val navController = rememberNavController() + // Honour a pending notification-tap once the NavHost is + // mounted. launchSingleTop avoids stacking copies of + // NowPlaying if the user taps the notification while + // already on it; the callback clears the flag so a later + // recomposition (config change, theme switch) doesn't + // re-navigate. + LaunchedEffect(pending, navController) { + if (pending) { + navController.navigate(NowPlaying) { launchSingleTop = true } + onOpenedNowPlaying() + } + } MinstrelNavGraph( navController = navController, startDestination = resolved, diff --git a/android/app/src/main/java/com/fabledsword/minstrel/player/MinstrelPlayerService.kt b/android/app/src/main/java/com/fabledsword/minstrel/player/MinstrelPlayerService.kt index fba943f6..bc9cbba0 100644 --- a/android/app/src/main/java/com/fabledsword/minstrel/player/MinstrelPlayerService.kt +++ b/android/app/src/main/java/com/fabledsword/minstrel/player/MinstrelPlayerService.kt @@ -1,9 +1,11 @@ package com.fabledsword.minstrel.player +import android.app.PendingIntent import android.content.Intent import androidx.media3.common.Player import androidx.media3.session.MediaSession import androidx.media3.session.MediaSessionService +import com.fabledsword.minstrel.MainActivity import dagger.hilt.android.AndroidEntryPoint import javax.inject.Inject @@ -38,7 +40,36 @@ class MinstrelPlayerService : MediaSessionService() { override fun onCreate() { super.onCreate() val player = playerFactory.build() - mediaSession = MediaSession.Builder(this, player).build() + mediaSession = MediaSession.Builder(this, player) + .setSessionActivity(buildNowPlayingPendingIntent()) + .build() + } + + /** + * Intent that the media notification / lock-screen card / Pixel + * Watch tile launches when tapped. Carries an extra MainActivity + * consumes to navigate the NavController straight to the full + * NowPlaying screen — without it, Media3 defaults to the launcher + * activity entry point, which lands on whatever shell route + * MainActivity last rendered. + * + * FLAG_ACTIVITY_SINGLE_TOP keeps the existing MainActivity instance + * alive when one is already foregrounded; onNewIntent fires and + * picks up the fresh extra. FLAG_IMMUTABLE is required on + * Android 12+; FLAG_UPDATE_CURRENT keeps the extra in sync if the + * Builder rebuilds the PendingIntent. + */ + private fun buildNowPlayingPendingIntent(): PendingIntent { + val intent = Intent(this, MainActivity::class.java).apply { + flags = Intent.FLAG_ACTIVITY_SINGLE_TOP + putExtra(MainActivity.EXTRA_OPEN_NOW_PLAYING, true) + } + return PendingIntent.getActivity( + this, + REQUEST_OPEN_NOW_PLAYING, + intent, + PendingIntent.FLAG_IMMUTABLE or PendingIntent.FLAG_UPDATE_CURRENT, + ) } override fun onGetSession(controllerInfo: MediaSession.ControllerInfo): MediaSession? = @@ -61,4 +92,11 @@ class MinstrelPlayerService : MediaSessionService() { mediaSession = null super.onDestroy() } + + private companion object { + // Stable request code for the session-activity PendingIntent. + // Arbitrary but distinct from any other PendingIntent we ever + // build against MainActivity. + const val REQUEST_OPEN_NOW_PLAYING = 0x100 + } } From 4cd38aa62fe94c1cdf6e5a7ac8d104cda42f50be Mon Sep 17 00:00:00 2001 From: Bryan Van Deusen Date: Tue, 2 Jun 2026 09:50:05 -0400 Subject: [PATCH 5/5] fix(android): keep BaseUrlInterceptor.intercept under detekt ReturnCount MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit aec10ce7 added a third early return (the placeholder-host bail) on top of the existing unparseable-baseUrl elvis return, tripping detekt's ReturnCount ceiling of 2 on the dev test workflow. Refactor: keep the placeholder-host early bail (it preserves the no-op cost for external URLs — no AuthStore read, no URL parse), fold the unparseable-baseUrl case into a `?:` that falls back to the original URL. Result is two returns and identical observable behavior — placeholder hosts get rewritten when baseUrl parses, fall through unchanged when it doesn't. Existing unit tests cover all three paths and continue to assert the same outputs. --- .../minstrel/api/BaseUrlInterceptor.kt | 18 +++++++++++------- 1 file changed, 11 insertions(+), 7 deletions(-) diff --git a/android/app/src/main/java/com/fabledsword/minstrel/api/BaseUrlInterceptor.kt b/android/app/src/main/java/com/fabledsword/minstrel/api/BaseUrlInterceptor.kt index 06e59bc2..8bc5fee1 100644 --- a/android/app/src/main/java/com/fabledsword/minstrel/api/BaseUrlInterceptor.kt +++ b/android/app/src/main/java/com/fabledsword/minstrel/api/BaseUrlInterceptor.kt @@ -38,14 +38,18 @@ class BaseUrlInterceptor @Inject constructor( override fun intercept(chain: Interceptor.Chain): Response { val original = chain.request() + // Early-bail keeps the no-op path for external URLs cheap + // (no AuthStore read, no URL parse). if (original.url.host != PLACEHOLDER_HOST) return chain.proceed(original) - val baseUrl = authStore.baseUrl.value.toHttpUrlOrNull() - ?: return chain.proceed(original) - val rewritten: HttpUrl = original.url.newBuilder() - .scheme(baseUrl.scheme) - .host(baseUrl.host) - .port(baseUrl.port) - .build() + // Unparseable baseUrl folds into the same proceed path — keeps + // detekt's ReturnCount happy by avoiding a second early return. + val rewritten: HttpUrl = authStore.baseUrl.value.toHttpUrlOrNull()?.let { baseUrl -> + original.url.newBuilder() + .scheme(baseUrl.scheme) + .host(baseUrl.host) + .port(baseUrl.port) + .build() + } ?: original.url return chain.proceed(original.newBuilder().url(rewritten).build()) }