51 clippy warnings -> 0, with 8 justified #[allow]s (IPC arity, specta wire
types, and the 9 test-only await-holding-lock sites). 27 raw lock calls moved to
the poison-tolerant helpers - all of them test code; production was already
clean.
Caught a non-neutral clippy --fix: removing the redundant 'use hostname;' in
credentials.rs orphaned its #[cfg(target_os = "linux")] onto SERVICE_NAME,
which would have cfg'd the constant out of every non-Linux build. Compiles clean
on Linux, so only Windows/macOS CI would have caught it.
`cargo clippy --all-targets` went from 51 warnings (23 in the lib) to zero.
Most were mechanical — needless borrows, `assert_eq!` against a bool literal,
`vec!` where an array does, `or_insert_with(Vec::new)`, a loop index used only
to index — and were applied with `clippy --fix`, then reviewed line by line.
That review caught one auto-fix that was *not* semantically neutral: dropping
the redundant `use hostname;` left its `#[cfg(target_os = "linux")]` orphaned
directly above `SERVICE_NAME`, which would have silently cfg'd the constant out
of every non-Linux build. Removed the stray attribute with the import.
Where a lint asked for a risky change rather than a better one, it is suppressed
with a comment saying why:
- `too_many_arguments` on five `#[tauri::command]` handlers and
`ThumbnailCache::save_thumbnail` — most of the arity is `State<'_, _>`
injection, and a parameter struct would change the IPC contract and the
generated TypeScript for no readability gain.
- `large_enum_variant` on `PlayerStatusEvent` and `AutoplayDecision` — both are
serde + specta wire types emitted a handful of times a second, never bulk
allocated; boxing would have to stay invisible to the generated bindings while
every match arm gained a deref.
- `await_holding_lock` on the `hybrid`/`offline` test modules — the guard is a
test-only serialisation lock for the process-global `INCLUDE_CATALOG_BROWSE`
flag, and the await it spans *is* the critical section. Each `#[tokio::test]`
gets its own single-threaded runtime, so this is not the production deadlock
class the lint targets; restructuring would reintroduce the flag race.
Real fixes elsewhere: `JellyfinItem::to_media_item` takes `self` by value, so it
is now `into_media_item`; the five-tuple episode row in the download commands
has a named `EpisodeRow` alias; the mpv `PropertyChange` arm matches
`name: "pause"` instead of guarding on it.
Also converted the last 27 raw `.lock().unwrap()` call sites to `lock_safe()`,
completing the `MutexSafe`/`RwLockSafe` convention. All of them turned out to be
in test modules — production code was already clean — so this is consistency
rather than a fix. The two raw locks in `utils/lock.rs` stay raw on purpose:
those tests deliberately poison a mutex to prove the helpers recover from it.
Pure refactoring: all 698 tests still pass.
The app's data dir was eligible for Google cloud backup: the manifest set
neither allowBackup nor any extraction rules, so the SQLite catalogue
(library metadata, watch history) and the jellytau_secure_prefs
credential blob were shipped to the user's Google account. Restoring that
is worse than not having it — SecureStorage encrypts under an Android
Keystore key, and Keystore keys are never backed up, so a restored
install gets ciphertext with nothing to open it and fails auth silently
while looking signed in.
Backup and device-to-device transfer are both turned off. allowBackup
="false" covers API 24-30 outright and kills cloud backup on 31+; it does
NOT stop D2D there, so @xml/data_extraction_rules excludes every domain
from both channels. Nothing is lost: the catalogue is a rebuildable
mirror of the Jellyfin server, and watch state lives on the server.
The credential-load path degrades instead of erroring, because a device
can still arrive at undecryptable ciphertext (an older install's backup,
a Keystore key invalidated by a lockscreen change). Both backends now
distinguish "nothing stored" from "stored but unreadable" and answer the
second as the first: CredentialStore::load_credentials_file logs and
returns an empty map rather than CredentialError::Encryption — which
storage_get_access_token was turning into a hard Err and
storage_get_active_session into a warning — and SecureStorage.getCredential
discards the dead blob so it cannot fail every subsequent read. The
result is a login screen rather than a broken session, and the next
successful sign-in rewrites the store.
Also removes the half-declared Android TV support: the manifest offered
LEANBACK_LAUNCHER and the leanback uses-feature with no D-pad focus
model, no TV layouts, and neither of the two declarations Play's TV
validation also requires (touchscreen required="false", android:banner).
That fails review while advertising the app to TV launchers. All four go
back together when a focus pass is actually done.
And raises jvmTarget from 1.8 to 17 under compileSdk 36, with matching
compileOptions — AGP 8.11 already requires a JDK 17 toolchain, so 1.8 was
only capping emitted bytecode. Nothing else in the build assumed 1.8.
TRACES: UR-012 | IR-014
Hand video playback off to a native audio-only stream when the app is
backgrounded or locked, with no on-device video decode (UR-040). Adds
player_enter/exit_background_audio commands, an audio-only stream URL
for video items across the repository layer, and the frontend handoff
state machine wired into VideoPlayer. Includes accompanying
repository/offline/player refactoring and regenerates the traceability
matrix.
dtourolle