fix(codebase) : Security fixes, data layer changes, file decomposition by lostf1sh · Pull Request #2055 · PixelPlayerHQ/PixelPlayer

lostf1sh · 2026-05-19T21:16:14Z

Builds on top of #2048. Applies ~115 surgical fixes and several architectural first-slices from CODEBASE_REVIEW.md, with all builds and tests green.

Summary

Security: Gemini API key → header, ZipShareHelper .. sanitization, ArtworkTransportSanitizer sourceBytesLimit enforced, WearCommandReceiver scheme guard, MusicService dead-code removed, onTaskRemoved super() ordering, AudioDeviceCallback Main handler
Concurrency: signatureMimeCache / codecInfoCache → ConcurrentHashMap; DualPlayerEngine listener lists → CopyOnWriteArrayList; ThemeStateHolder LRU lock; ArtistImageRepository concurrent sets
Data layer: SyncWorker FNV-1a 64-bit hash (was String.hashCode collision-prone), exponential backoff + retry; M3uManager UTF-8 + BOM strip + 1M-line cap; DailyMix LocalDate compare; MIGRATION_16_17 duplicate-column differentiation
Media stack: HTTP server latch await 10m → 2m; pauseAtEndOfMediaItems on both players; resolvedUriCache 15-min TTL
DataStore split: New playbackStore + 11 keys end-to-end migrated with consumers reading from the new store and dual-write on setters
Singleton lifecycle: 8 of 9 @Singleton StateHolders inject @AppScope
PlayerViewModel decomposition (first slices): hasGeminiApiKey, hasActiveAiProviderApiKey, AiUiSnapshot flows lifted to AiStateHolder; observeSong cached
LibraryScreen extraction: 8 files lifted into presentation/screens/library/. LibraryScreen.kt: 3,730 → 2,831 lines (-24%)
Compose stability: Several composables migrated from List<T> → ImmutableList<T>; MarqueeText / Theme.kt / EditSongSheet / HomeScreen key fixes; SmartImage hardware default
Build / deps / CI: lint re-enabled (non-blocking), x86_64 ABI split, unused deps purged
Testing: Robolectric infra + 10 new test files (~100 cases), including the first MusicService-adjacent Robolectric tests (CrashHandler, framework constants)

REFACTOR_NOTES.md tracks the remaining multi-PR architectural items (4 more PlayerViewModel controllers, ~59 more DataStore keys, LibraryFoldersTab extraction, full-flow MusicService Robolectric tests).

Test plan

./gradlew :app:compileDebugKotlin — green
./gradlew :app:testDebugUnitTest — green
./gradlew :wear:compileDebugKotlin — green
Manual smoke on a device: playback, queue restore, Cast session, sleep timer, theme follow

🤖 Generated with Claude Code

@androidentrypoint

Surgical and architectural fixes against the codebase review: Security - Gemini API key moved from URL query to x-goog-api-key header - ZipShareHelper.sanitizeFileName rejects ".." and leading dots - ArtworkTransportSanitizer enforces sourceBytesLimit before decode - WearCommandReceiver.openSongFile scheme guard (no File() for cloud URIs) - MusicService.shouldRejectWearController dead-return removed - MusicService.onTaskRemoved always calls super first - MusicService AudioDeviceCallback uses Main-looper Handler Concurrency - signatureMimeCache, codecInfoCache → ConcurrentHashMap - DualPlayerEngine listener lists → CopyOnWriteArrayList - ThemeStateHolder individualAlbumColorSchemes LRU guarded by lock - ArtistImageRepository pendingFetches/failedFetches → concurrent sets Data layer - SyncWorker: 64-bit FNV-1a hash for synthetic Telegram IDs - SyncWorker: exponential backoff + Result.retry for transient failures - M3uManager: UTF-8 charset, BOM strip, 1M-line cap - DailyMixStateHolder: LocalDate compare (DAY_OF_YEAR boundary bug) - MIGRATION_16_17: differentiate duplicate-column from real failures Media stack - MediaFileHttpServerService: latch.await tightened 10min → 2min - DualPlayerEngine: pauseAtEndOfMediaItems applied to both players - DualPlayerEngine: resolvedUriCache gets 15-min TTL Architecture — DataStore split - New playbackStore (separate Preferences DataStore) - 11 playback keys end-to-end migrated with consumer reads + dual-write on setters: persistent_shuffle_enabled, is_shuffle_on, repeat_mode, is_crossfade_enabled, crossfade_duration, hi_fi_mode_enabled, global_transition_settings, playback_queue_snapshot, keep_playing_in_background, replaygain_enabled, replaygain_use_album_gain, disable_cast_autoplay Architecture — Singleton lifecycle - 8 of 9 Singleton StateHolders inject @AppScope: AiStateHolder, SearchStateHolder, LibraryStateHolder, LyricsStateHolder, CastStateHolder, CastTransferStateHolder, SleepTimerStateHolder, ConnectivityStateHolder, MusicRepositoryImpl Architecture — PlayerViewModel decomposition (first slices) - hasGeminiApiKey, hasActiveAiProviderApiKey, AiUiSnapshot flows extracted to AiStateHolder - observeSong cached per-songId Architecture — LibraryScreen extraction - 8 files extracted to presentation/screens/library/: WatchTransferProgressDialog, LibrarySyncIndicators, FolderItems, FolderSortHelpers, LibraryTabGridItem, ArtistListItem, AlbumListItem, AlbumGridItemRedesigned - LibraryScreen.kt: 3,730 → 2,831 lines (-24%) Compose stability - PlaylistArtCollage / PlaylistCover / SearchResultPlaylistItem: List<Song> → ImmutableList<Song> - LibraryScreen.previewSongs → toPersistentList - MarqueeText LaunchedEffect keyed on text - SmartImage allowHardware=true default - Theme.kt SideEffect → LaunchedEffect gated on icon-mode - EditSongSheet derivedStateOf keyed on density - HomeScreen rotationIndex hoisted out of conditional Build / deps / CI - lint.checkReleaseBuilds=true with abortOnError=false - ABI splits include x86_64 - libs.versions.toml: removed unused deps (pytorch, tensorflow-lite, spleeter, compose-dnd, duktape, google-genai); consolidated duplicate version keys (accompanist, junitJupiter, mediarouter) Testing - Robolectric infra: robolectric:4.14 + isIncludeAndroidResources=true - New test files (~100 cases): ZipShareHelperSanitizationTest, ArtworkTransportSanitizerTest, SyncWorkerHashTest, FileDeletionUtilsTest, FolderSortHelpersTest, AudioSignatureDetectionTest, CastSessionSecurityTest expansions, CastHttpRouteAuthTest, WearPlaybackCommandFuzzTest, CrashHandlerRobolectricTest, MusicServiceConstantsRobolectricTest - REFACTOR_NOTES.md documents remaining architectural work Wear OS - backup_rules.xml + data_extraction_rules.xml added - @androidentrypoint guards

lostf1sh · 2026-05-19T21:20:37Z

@copilot review huh

`_selectedSongs.update {}` / `_selectedPlaylists.update {}` only made the list flow atomic; the sibling writes to ids / count / mode happened after the CAS, so a concurrent toggle landing in that gap could leave the four flows out of sync (list shows [X, Y] while ids shows just {Y}). Wrap the whole read-modify-write under a single `mutationLock` so all four `.value =` assignments land together.

Copilot

Pull request overview

This PR applies a broad set of “CODEBASE_REVIEW.md” follow-up fixes across the app and wear modules, focusing on security hardening, concurrency/lifecycle correctness, incremental architectural extraction, and expanded JVM/Robolectric test coverage.

Changes:

Add security and robustness hardening across sharing, artwork transport, Cast HTTP auth, crash log handling, and preferences storage/backup exclusions.
Improve concurrency safety and singleton lifecycle behavior (AppScope usage, thread-safe caches/collections, bounded caches/TTLs, reduced main-thread work).
Decompose large UI modules (LibraryScreen slices), introduce Robolectric + Ktor route tests, and clean up dependency/version catalog.

Reviewed changes

Copilot reviewed 81 out of 82 changed files in this pull request and generated 7 comments.

Show a summary per file

File	Description
wear/src/main/res/xml/wear_data_extraction_rules.xml	Excludes credential/sharedpref + datastore from device/cloud extraction on Wear.
wear/src/main/res/xml/wear_backup_rules.xml	Excludes Wear-side credential/sharedpref + datastore from backup.
wear/src/main/AndroidManifest.xml	Wires Wear backup/data-extraction rules into the app manifest.
REFACTOR_NOTES.md	Tracks multi-PR architectural refactor plan and what has already landed.
gradle/libs.versions.toml	Consolidates versions, removes unused entries, adds test deps, tweaks catalog structure.
app/build.gradle.kts	Enables Robolectric resources, re-enables lint (non-blocking), adds x86_64 ABI split, adds test deps.
app/src/main/java/com/theveloper/pixelplay/utils/ZipShareHelper.kt	Strengthens filename sanitization and exposes internal helper for tests.
app/src/test/java/com/theveloper/pixelplay/utils/ZipShareHelperSanitizationTest.kt	Adds adversarial tests for share filename sanitization.
app/src/main/java/com/theveloper/pixelplay/utils/ArtworkTransportSanitizer.kt	Enforces source byte-size cap before bitmap decode.
app/src/test/java/com/theveloper/pixelplay/utils/ArtworkTransportSanitizerTest.kt	Adds JVM tests for sanitizer caps/null handling.
app/src/test/java/com/theveloper/pixelplay/utils/FileDeletionUtilsTest.kt	Adds JVM tests for context-free deletion helpers.
app/src/test/java/com/theveloper/pixelplay/utils/CrashHandlerRobolectricTest.kt	Adds Robolectric coverage for crash capture/redaction persistence.
app/src/main/java/com/theveloper/pixelplay/ui/theme/Theme.kt	Reduces window updates by keying side-effects (LaunchedEffect).
app/src/main/java/com/theveloper/pixelplay/ui/theme/ColorRoles.kt	Makes extracted color cache effectively inert/bounded.
app/src/main/java/com/theveloper/pixelplay/ui/glancewidget/WidgetUtils.kt	Replaces O(n) contentHashCode with prefix hash for widget art cache key.
app/src/main/java/com/theveloper/pixelplay/presentation/viewmodel/ThemeStateHolder.kt	Adds locking around LRU map access to avoid concurrent mutation crashes.
app/src/main/java/com/theveloper/pixelplay/presentation/viewmodel/SleepTimerStateHolder.kt	Injects AppScope; lazies system services; keeps jobs across VM teardown.
app/src/main/java/com/theveloper/pixelplay/presentation/viewmodel/SearchStateHolder.kt	Injects AppScope; simplifies search job cancellation + request handling.
app/src/main/java/com/theveloper/pixelplay/presentation/viewmodel/LyricsStateHolder.kt	Injects AppScope; ensures observer job lifecycle is handled safely.
app/src/test/java/com/theveloper/pixelplay/presentation/viewmodel/LyricsStateHolderTest.kt	Updates holder construction for new AppScope dependency.
app/src/main/java/com/theveloper/pixelplay/presentation/viewmodel/LibraryStateHolder.kt	Injects AppScope; parallelizes initial preference loads; removes nullable scope.
app/src/main/java/com/theveloper/pixelplay/presentation/viewmodel/DailyMixStateHolder.kt	Fixes date comparison correctness via LocalDate.
app/src/main/java/com/theveloper/pixelplay/presentation/viewmodel/ConnectivityStateHolder.kt	Injects AppScope; lazies system-service lookups.
app/src/main/java/com/theveloper/pixelplay/presentation/viewmodel/CastTransferStateHolder.kt	Injects AppScope; avoids cancellation mid transfer; keeps per-VM callbacks reset.
app/src/main/java/com/theveloper/pixelplay/presentation/viewmodel/CastStateHolder.kt	Injects AppScope; lazies MediaRouter; makes refreshRoutes resilient to teardown.
app/src/main/java/com/theveloper/pixelplay/presentation/viewmodel/AiStateHolder.kt	Moves API-key presence flows into AI holder; injects AppScope.
app/src/main/java/com/theveloper/pixelplay/presentation/viewmodel/PlayerViewModel.kt	Decomposes AI flow wiring; adds observeSong() flow caching; parallelizes album song fetch.
app/src/main/java/com/theveloper/pixelplay/presentation/viewmodel/MainViewModel.kt	Avoids eager collectors; replaces “load all songs to check empty” with count flow.
app/src/main/java/com/theveloper/pixelplay/presentation/viewmodel/PlayerUiState.kt	Removes unused field from UI state.
app/src/main/java/com/theveloper/pixelplay/presentation/viewmodel/MultiSelectionStateHolder.kt	Attempts to make selection toggles atomic via StateFlow.update().
app/src/main/java/com/theveloper/pixelplay/presentation/viewmodel/PlaylistSelectionStateHolder.kt	Attempts to make playlist selection toggles atomic via StateFlow.update().
app/src/main/java/com/theveloper/pixelplay/presentation/viewmodel/ArtistDetailViewModel.kt	Ensures image URL resolution runs on IO dispatcher.
app/src/main/java/com/theveloper/pixelplay/presentation/screens/SearchScreen.kt	Migrates playlist song collections to immutable lists for Compose stability.
app/src/main/java/com/theveloper/pixelplay/presentation/screens/LibrarySongsTab.kt	Removes redundant remember wrapper around callback per item.
app/src/main/java/com/theveloper/pixelplay/presentation/screens/LibraryMediaTabs.kt	Updates imports to extracted library UI components.
app/src/main/java/com/theveloper/pixelplay/presentation/screens/library/WatchTransferProgressDialog.kt	Extracts Wear transfer dialog from LibraryScreen.
app/src/main/java/com/theveloper/pixelplay/presentation/screens/library/LibraryTabGridItem.kt	Extracts library tab grid item + helpers.
app/src/main/java/com/theveloper/pixelplay/presentation/screens/library/LibrarySyncIndicators.kt	Extracts sync/loading UI and localizes SyncProgress collection.
app/src/main/java/com/theveloper/pixelplay/presentation/screens/library/FolderSortHelpers.kt	Extracts pure sort/flatten helpers from LibraryScreen.
app/src/test/java/com/theveloper/pixelplay/presentation/screens/library/FolderSortHelpersTest.kt	Adds JVM tests for extracted folder/song sort helpers.
app/src/main/java/com/theveloper/pixelplay/presentation/screens/library/FolderItems.kt	Extracts folder item composables and switches to immutable lists.
app/src/main/java/com/theveloper/pixelplay/presentation/screens/library/ArtistListItem.kt	Extracts artist list item composable.
app/src/main/java/com/theveloper/pixelplay/presentation/screens/library/AlbumListItem.kt	Extracts album list item composable and palette-driven styling.
app/src/main/java/com/theveloper/pixelplay/presentation/screens/library/AlbumGridItemRedesigned.kt	Extracts album grid item composable and palette-driven styling.
app/src/main/java/com/theveloper/pixelplay/presentation/screens/HomeScreen.kt	Fixes rememberSaveable placement to preserve rotation state across toggles.
app/src/main/java/com/theveloper/pixelplay/presentation/components/SmartImage.kt	Defaults to hardware bitmaps for memory efficiency.
app/src/main/java/com/theveloper/pixelplay/presentation/components/MarqueeText.kt	Keys scrolling timer on text changes.
app/src/main/java/com/theveloper/pixelplay/presentation/components/EditSongSheet.kt	Keys derived IME visibility on Density.
app/src/main/java/com/theveloper/pixelplay/presentation/components/PlaylistCover.kt	Switches playlist songs to ImmutableList.
app/src/main/java/com/theveloper/pixelplay/presentation/components/PlaylistContainer.kt	Converts nullable playlistSongs to persistent list before rendering cover.
app/src/main/java/com/theveloper/pixelplay/presentation/components/PlaylistArtCollage.kt	Switches songs param to ImmutableList.
app/src/main/java/com/theveloper/pixelplay/di/Qualifiers.kt	Adds qualifiers for playback and default DataStore injections.
app/src/main/java/com/theveloper/pixelplay/di/AppModule.kt	Provides playback DataStore; adds OkHttp cache; threads AppScope into repos.
app/src/main/java/com/theveloper/pixelplay/data/preferences/UserPreferencesRepository.kt	Introduces dedicated playback DataStore + migration + dual-write for migrated keys.
app/src/test/java/com/theveloper/pixelplay/data/preferences/UserPreferencesRepositoryTest.kt	Updates tests to pass playbackStore and migrationScope.
app/src/main/java/com/theveloper/pixelplay/data/worker/SyncWorker.kt	Adds retry/backoff logic, 64-bit synthetic hashing, and reduces redundant DB reads.
app/src/test/java/com/theveloper/pixelplay/data/worker/SyncWorkerHashTest.kt	Adds determinism/collision tests for new synthetic-id hashing.
app/src/main/java/com/theveloper/pixelplay/data/repository/MusicRepositoryImpl.kt	Uses AppScope-backed repository scope; caps LIKE query length.
app/src/test/java/com/theveloper/pixelplay/data/repository/MusicRepositoryImplTest.kt	Updates construction for new AppScope dependency.
app/src/main/java/com/theveloper/pixelplay/data/repository/LyricsRepositoryImpl.kt	Adds IO size caps for cache JSON and temp audio copying.
app/src/main/java/com/theveloper/pixelplay/data/repository/ArtistImageRepository.kt	Uses concurrent sets for pending/failed fetch tracking.
app/src/main/java/com/theveloper/pixelplay/data/playlist/M3uManager.kt	Pins UTF-8 decoding, strips BOM, and caps line count to prevent OOM.
app/src/main/java/com/theveloper/pixelplay/data/paging/MediaStorePagingSource.kt	Uses parameterized placeholders for IN clause selection.
app/src/main/java/com/theveloper/pixelplay/data/gdrive/GDriveConstants.kt	Documents least-privilege Drive scope and deprecates broad readonly scope.
app/src/main/java/com/theveloper/pixelplay/data/database/PixelPlayDatabase.kt	Makes MIGRATION_16_17 duplicate-column handling precise (don’t swallow real errors).
app/src/main/java/com/theveloper/pixelplay/data/database/MusicDao.kt	Caps FTS token length and adapts SQLite variable limits by API level.
app/src/main/java/com/theveloper/pixelplay/data/telegram/TelegramClientManager.kt	Sources TDLib creds from BuildConfig (overridable), avoiding hardcoded constants.
app/src/main/java/com/theveloper/pixelplay/data/service/wear/WearCommandReceiver.kt	Guards direct file access by path scheme; falls back to ContentResolver.
app/src/main/java/com/theveloper/pixelplay/data/service/player/DualPlayerEngine.kt	Makes listeners COW; adds TTL to resolved URI cache; applies pauseAtEnd to both players.
app/src/main/java/com/theveloper/pixelplay/data/service/player/CastPlayer.kt	Moves android.util.Log usage to Timber and improves structured logging.
app/src/main/java/com/theveloper/pixelplay/data/service/MusicService.kt	Fixes wear-controller rejection logic, onTaskRemoved ordering, and audio callback threading.
app/src/main/java/com/theveloper/pixelplay/data/service/http/MediaFileHttpServerService.kt	Makes caches concurrent + negative-caching; extracts signature detection; shortens latch waits.
app/src/main/java/com/theveloper/pixelplay/data/service/http/AudioSignatureDetection.kt	Extracts pure signature detection helpers for unit testing.
app/src/test/java/com/theveloper/pixelplay/data/service/http/AudioSignatureDetectionTest.kt	Adds unit tests for extracted signature detection.
app/src/test/java/com/theveloper/pixelplay/data/service/http/CastSessionSecurityTest.kt	Expands Cast access-policy/auth-token/loopback/redaction test coverage.
app/src/test/java/com/theveloper/pixelplay/data/service/http/CastHttpRouteAuthTest.kt	Adds Ktor route-level auth enforcement tests for Cast server endpoints.
app/src/main/java/com/theveloper/pixelplay/data/service/cast/CastOptionsProvider.kt	Disables Cast SDK MediaSession to avoid session duplication with Media3.
app/src/main/java/com/theveloper/pixelplay/data/service/auto/AutoMediaBrowseTree.kt	Improves search result mixing (round-robin) to reduce song-only bias.
app/src/test/java/com/theveloper/pixelplay/data/service/MusicServiceConstantsRobolectricTest.kt	Adds Robolectric smoke test validating infra/resources availability.
app/src/main/java/com/theveloper/pixelplay/data/ai/provider/GeminiAiClient.kt	Sends API key via header (not query param) when listing models.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

+            } else {
+                (currentList + song) to (currentIds + song.id)
+            }
+            updateStateLocked(newList, newIds)
        }


    fun toggleSelection(playlist: Playlist) {
-        val currentList = _selectedPlaylists.value.toMutableList()
-        val currentIds = _selectedPlaylistIds.value.toMutableSet()
-
-        if (currentIds.contains(playlist.id)) {
-            // Remove from selection
-            currentList.removeAll { it.id == playlist.id }
-            currentIds.remove(playlist.id)
-        } else {
-            // Add to selection (preserving order)
-            currentList.add(playlist)
-            currentIds.add(playlist.id)
+        synchronized(mutationLock) {
+            val currentList = _selectedPlaylists.value
+            val currentIds = _selectedPlaylistIds.value
+            val (newList, newIds) = if (playlist.id in currentIds) {


    val persistentShuffleEnabledFlow: Flow<Boolean> =
-            dataStore.data.map { preferences ->
-                preferences[PreferencesKeys.PERSISTENT_SHUFFLE_ENABLED] ?: false
+            playbackStore.data.map { prefs ->
+                prefs[PlaybackPreferencesKeys.PERSISTENT_SHUFFLE_ENABLED] ?: false
            }


+androidx-mediarouter = { group = "androidx.mediarouter", name = "mediarouter", version.ref = "mediaRouter" }
 androidx-navigation-runtime-ktx = { group = "androidx.navigation", name = "navigation-runtime-ktx", version.ref = "navigationRuntimeKtx" }
 androidx-compose-material3 = { group = "androidx.compose.material3", name = "material3", version.ref = "material3" }
 androidx-uiautomator = { group = "androidx.test.uiautomator", name = "uiautomator", version.ref = "uiautomator" }


+        // Run the three searches concurrently and round-robin-merge the
+        // results so an album/artist hit isn't squeezed out by 30+ song
+        // matches. Previous behaviour always biased to songs.
        val songs = musicRepository.searchSongs(trimmedQuery).first()


+         * Stable 64-bit FNV-1a hash. Replaces `String.hashCode()` for synthetic
+         * Telegram/Netease song/album/artist IDs — the JDK's 32-bit hash has
+         * ~50% collision probability around 65k entries, which is reachable
+         * for large Telegram channels. FNV-1a keeps the full 64 bits and the
+         * collision probability stays below 1e-10 well past a million entries.
+         */
+        internal fun stableFnv1aHash64(input: String): Long {
+            var hash = -3750763034362895579L // FNV-1a 64-bit offset basis
+            for (c in input) {
+                hash = hash xor (c.code.toLong() and 0xFFL)



                    Result.success(workDataOf(OUTPUT_TOTAL_SONGS to finalTotalSongs.toLong()))
+                } catch (e: CancellationException) {
+                    Log.w(TAG, "Sync cancelled — returning retry so WorkManager re-runs", e)


- MultiSelectionStateHolder / PlaylistSelectionStateHolder: replace 4 parallel StateFlows + mutex with a single source-of-truth list flow; ids/count/mode are derived via stateIn(@AppScope, Eagerly) and toggles use StateFlow.update {} for atomic CAS. Removes the cross-flow tear Copilot flagged even with the synchronized block. - UserPreferencesRepository: add playbackKeyFlow() helper that falls back to the legacy "settings" DataStore until MIGRATION_DONE is set. Applied to all 12 migrated playback keys, so existing installs don't briefly read defaults during the migration grace window. - libs.versions.toml: clarify that the material3 alpha pin is deliberate (ExperimentalMaterial3ExpressiveApi components used across StatsScreen, LibrarySyncIndicators, Telegram screens) and intentionally overrides the Compose BOM — the BOM-managed comment was the misleading bit, not the pin. - AutoMediaBrowseTree.search: actually run the three LRCLIB searches concurrently via coroutineScope { async {…} }.awaitAll() so the comment matches the behaviour. - SyncWorker.stableFnv1aHash64: hash UTF-8 bytes instead of (Char.code and 0xFF). ASCII inputs are unaffected; CJK/Cyrillic/accented names stop collapsing onto each other. - SyncWorker CancellationException branch: log message no longer claims WorkManager will retry — cancellation is propagated, not retried.

Copilot

Pull request overview

Copilot reviewed 81 out of 82 changed files in this pull request and generated 3 comments.

Comments suppressed due to low confidence (1)

app/src/main/java/com/theveloper/pixelplay/ui/theme/Theme.kt:72

PixelPlayStatusBarStyle() no longer uses the color/navigationColor parameters to set the window’s status/navigation bar colors (it always writes TRANSPARENT). If callers expect the bars to match the provided colors (many call sites compute a fallback color), this is a functional regression. If transparency is intentional, consider removing/renaming the unused parameters and related luminance logic; otherwise write window.statusBarColor = color.toArgb() and window.navigationBarColor = navigationColor.toArgb() (and key the effect on those colors).

Copilot · 2026-05-20T14:05:45Z

-                    // trimmedLine is likely a file path or URI
+
+                    // Strip UTF-8 BOM if it leaked through readLine on line 1.
+                    val payload = if (processed == 1) trimmedLine.removePrefix("") else trimmedLine


Fixed in e6c1d4c — replaced the invisible BOM literal with the explicit "\uFEFF" escape.

Copilot · 2026-05-20T14:05:49Z

     * @return True if the playlist is selected, false otherwise
     */
    fun isSelected(playlistId: String): Boolean {
-        return _selectedPlaylistIds.value.contains(playlistId)
+        return _selectedPlaylists.value.any { it.id == playlistId }
    }


Fixed in e6c1d4c — isSelected() now uses selectedPlaylistIds.value.contains(playlistId) for O(1) set lookup.

Copilot · 2026-05-20T14:05:54Z

     * @param songId The ID of the song to check
     * @return True if the song is selected, false otherwise
     */
    fun isSelected(songId: String): Boolean {
-        return _selectedSongIds.value.contains(songId)
+        return _selectedSongs.value.any { it.id == songId }
    }


Fixed in e6c1d4c — isSelected() now uses selectedSongIds.value.contains(songId) for O(1) set lookup.

Agent-Logs-Url: https://github.com/theovilardo/PixelPlayer/sessions/c690e76a-0876-4396-9ef9-0669670793f7 Co-authored-by: lostf1sh <136324426+lostf1sh@users.noreply.github.com>

- Ensure MediaRouter callback and route access run on the main thread - Avoid an extra post when already on Main

Copilot

Pull request overview

Copilot reviewed 81 out of 82 changed files in this pull request and generated 3 comments.

        val holder = LyricsStateHolder(
            musicRepository = musicRepository,
            userPreferencesRepository = userPreferencesRepository,
-            songMetadataEditor = songMetadataEditor
+            songMetadataEditor = songMetadataEditor,
+            appScope = kotlinx.coroutines.CoroutineScope(
+                kotlinx.coroutines.SupervisorJob() + kotlinx.coroutines.Dispatchers.Unconfined
+            ),
        )


+            folderTreeBuilder = mockk(relaxed = true),
+            appScope = kotlinx.coroutines.CoroutineScope(
+                kotlinx.coroutines.SupervisorJob() + kotlinx.coroutines.Dispatchers.Unconfined
+            ),
        )


+                // Cap the copy at TEMP_AUDIO_COPY_MAX_BYTES so a malicious or
+                // mis-pointed content URI cannot fill the cache directory. The
+                // downstream TagLib reader only needs file headers (~10 MB
+                // covers every realistic embedded-tag layout); abort cleanly
+                // if more is required than the cap allows.
                FileOutputStream(tempFile).use { output ->
-                    inputStream.copyTo(output)
+                    val buffer = ByteArray(64 * 1024)
+                    var totalCopied = 0L
+                    while (true) {
+                        val read = inputStream.read(buffer)
+                        if (read <= 0) break
+                        if (totalCopied + read > TEMP_AUDIO_COPY_MAX_BYTES) {
+                            output.write(buffer, 0, (TEMP_AUDIO_COPY_MAX_BYTES - totalCopied).toInt())
+                            break
+                        }
+                        output.write(buffer, 0, read)
+                        totalCopied += read
+                    }
                }
                tempFile


…imits Agent-Logs-Url: https://github.com/theovilardo/PixelPlayer/sessions/b226c60c-fcc1-4427-ab6e-4d12e6826110 Co-authored-by: lostf1sh <136324426+lostf1sh@users.noreply.github.com>

Copilot · 2026-05-21T21:02:10Z

Just as a heads up, I was blocked by some firewall rules while working on your feedback. Expand below for details.

Warning

Firewall rules blocked me from connecting to one or more addresses (expand for details)

I tried to connect to the following addresses, but was blocked by firewall rules:

api.foojay.io
- Triggering command: /usr/lib/jvm/temurin-17-jdk-amd64/bin/java /usr/lib/jvm/temurin-17-jdk-amd64/bin/java -Xmx64m -Xms64m -Dorg.gradle.appname=gradlew -jar /home/REDACTED/work/PixelPlayer/PixelPlayer/gradle/wrapper/gradle-wrapper.jar --no-daemon :app:testDebugUnitTest --tests com.theveloper.pixelplay.presentation.viewmodel.LyricsStateHolderTest --tests com.theveloper.pixelplay.data.repository.MusicRepositoryImplTest (dns block)
- Triggering command: /usr/lib/jvm/temurin-17-jdk-amd64/bin/java /usr/lib/jvm/temurin-17-jdk-amd64/bin/java -Xmx64m -Xms64m -Dorg.gradle.appname=gradlew -jar /home/REDACTED/work/PixelPlayer/PixelPlayer/gradle/wrapper/gradle-wrapper.jar --no-daemon -Dorg.gradle.java.home=/usr/lib/jvm/temurin-21-jdk-amd64 :app:testDebugUnitTest --tests com.theveloper.pixelplay.presentation.viewmodel.LyricsStateHolderTest --tests com.theveloper.pixelplay.data.repository.MusicRepositoryImplTest (dns block)
dl.google.com
- Triggering command: /usr/lib/jvm/temurin-21-jdk-amd64/bin/java /usr/lib/jvm/temurin-21-jdk-amd64/bin/java --add-opens=java.base/java.lang=ALL-UNNAMED --add-opens=java.base/java.lang.invoke=ALL-UNNAMED --add-opens=java.base/java.util=ALL-UNNAMED --add-opens=java.prefs/java.util.prefs=ALL-UNNAMED --add-exports=jdk.compiler/com.sun.tools.javac.api=ALL-UNNAMED --add-exports=jdk.compiler/com.sun.tools.javac.util=ALL-UNNAMED --add-opens=java.base/java.nio.charset=ALL-UNNAMED --add-opens=java.base/java.net=ALL-UNNAMED --add-opens=java.base/java.util.concurrent=ALL-UNNAMED --add-opens=java.base/java.util.concurrent.atomic=ALL-UNNAMED --add-opens=java.xml/javax.xml.namespace=ALL-UNNAMED --add-opens=java.base/java.time=ALL-UNNAMED -Xmx4096m -Dfile.encoding=UTF-8 -Duser.country -Duser.language=en -Duser.variant -cp /home/REDACTED/.gradle/wrapper/dists/gradle-9.5.1-bin/iq79hdu3mqx29lgffhp8bfmx/gradle-9.5.1/lib/gradle-daemon-main-9.5.1.jar (dns block)

If you need me to access, download, or install something from one of these locations, you can either:

Configure Actions setup steps to set up my environment, which run before the firewall is enabled
Add the appropriate URLs or hosts to the custom allowlist in this repository's Copilot coding agent settings (admins only)

lostf1sh · 2026-05-21T21:03:09Z

js merging ts atp

Copilot started work on behalf of lostf1sh May 19, 2026 21:20 View session

Copilot finished work on behalf of lostf1sh May 19, 2026 21:21

lostf1sh requested a review from Copilot May 19, 2026 21:25

Copilot started reviewing on behalf of lostf1sh May 19, 2026 21:26 View session

Copilot AI reviewed May 19, 2026

View reviewed changes

lostf1sh requested a review from Copilot May 20, 2026 13:54

Copilot started reviewing on behalf of lostf1sh May 20, 2026 13:55 View session

Copilot AI reviewed May 20, 2026

View reviewed changes

Copilot started work on behalf of lostf1sh May 20, 2026 14:03 View session

Fix BOM stripping and isSelected O(1) lookup per review 4329200173

e6c1d4c

Agent-Logs-Url: https://github.com/theovilardo/PixelPlayer/sessions/c690e76a-0876-4396-9ef9-0669670793f7 Co-authored-by: lostf1sh <136324426+lostf1sh@users.noreply.github.com>

Copilot finished work on behalf of lostf1sh May 20, 2026 14:07

Dispatch cast route refresh on Main.immediate

31e80ac

- Ensure MediaRouter callback and route access run on the main thread - Avoid an extra post when already on Main

lostf1sh changed the title ~~review: apply CODEBASE_REVIEW.md fixes across all 5 sections~~ fix(codebase) : Security fixes, data layer changes, file decomposition May 21, 2026

lostf1sh requested a review from Copilot May 21, 2026 20:48

Copilot started reviewing on behalf of lostf1sh May 21, 2026 20:49 View session

Copilot AI reviewed May 21, 2026

View reviewed changes

Copilot started work on behalf of lostf1sh May 21, 2026 20:57 View session

fix: address latest PR review comments on test scopes and temp copy l…

3a712dd

…imits Agent-Logs-Url: https://github.com/theovilardo/PixelPlayer/sessions/b226c60c-fcc1-4427-ab6e-4d12e6826110 Co-authored-by: lostf1sh <136324426+lostf1sh@users.noreply.github.com>

Copilot finished work on behalf of lostf1sh May 21, 2026 21:02

lostf1sh merged commit 0a5eaaf into security/xxe-allowlist-ai-keys-fk May 21, 2026

lostf1sh deleted the review/codebase-review-fixes branch May 21, 2026 21:03

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix(codebase) : Security fixes, data layer changes, file decomposition#2055

fix(codebase) : Security fixes, data layer changes, file decomposition#2055
lostf1sh merged 6 commits into
security/xxe-allowlist-ai-keys-fkfrom
review/codebase-review-fixes

lostf1sh commented May 19, 2026

Uh oh!

lostf1sh commented May 19, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI May 20, 2026

Uh oh!

Copilot AI May 20, 2026

Uh oh!

Copilot AI May 20, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI commented May 21, 2026

I tried to connect to the following addresses, but was blocked by firewall rules:

Uh oh!

lostf1sh commented May 21, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

lostf1sh commented May 19, 2026

Summary

Test plan

Uh oh!

lostf1sh commented May 19, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI May 20, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI May 20, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI May 20, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI commented May 21, 2026

I tried to connect to the following addresses, but was blocked by firewall rules:

Uh oh!

lostf1sh commented May 21, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants