NavidromeApp/CHANGELOG.md

# CHANGELOG — Race Condition & Crash Audit

## Build 13 (1.0.0) — 2026-04-30

### 🟢 ARCHITECTURE — Remove HealthKit, use standard audio background mode for speaker

**Files:** `watchOS/Audio/WatchAudioPlayer.swift`, `watchOS/Resources/Info.plist`, `watchOS/Resources/NavidromeWatch.entitlements`
**What changed:**
1. Removed `import HealthKit`, `HKHealthStore`, `HKWorkoutSession`, `HKLiveWorkoutBuilder`, and both delegate extensions
2. Removed `startWorkoutSession()` / `stopWorkoutSession()` and all call sites (play, resume, stop, reconfigure)
3. Info.plist: `WKBackgroundModes` changed from `workout-processing` to `audio`; removed `NSHealthShareUsageDescription` and `NSHealthUpdateUsageDescription`
4. Entitlements: removed `com.apple.developer.healthkit` and `com.apple.developer.healthkit.access`
5. Speaker mode now uses `setActive(true)` in `configureAudioSession` instead of deferring to workout session
**Why:** The HKWorkoutSession workaround was a watchOS 3 technique that Apple closed in watchOS 4. The standard `audio` background mode (available since watchOS 5/WWDC 2018) keeps the app alive while audio is actively playing through AVAudioSession — no HealthKit needed. The workout session was activating the heart rate sensor (green LEDs), showing the green workout indicator, contributing to Activity Rings, and draining 40%+ extra battery. Speaker routing is controlled entirely by AVAudioSession policy: `.default` = speaker, `.longFormAudio` = Bluetooth.

---

### 🔴 NEW — `SmartDJCache.bulkImport` concurrent Dictionary crash

**File:** `iOS/Views/Companion/CompanionAPIService.swift`
**Function:** `SmartDJCache` — `get()`, `store()`, `bulkImport()`, `loadBulkCache()`, `clearAll()`, `cachedCount`
**What changed:** Added `NSLock` to serialize all `memoryCache` reads and writes.
**Why:** Two crash logs (April 27 `EXC_BAD_ACCESS` + April 28 `SIGABRT doesNotRecognizeSelector`) both crash inside `SmartDJCache.bulkImport()`. Root cause: `memoryCache` is a plain `[String: SmartDJProfile]` dictionary mutated from the main thread (`loadBulkCache` at app launch) and a background `Task.detached` (`bulkImport` from server fetch) simultaneously. Swift Dictionary is not thread-safe — concurrent mutation corrupts the hash table, causing either a segfault on the corrupted bucket chain or a garbage pointer that fails `objc_msgSend`.
**Crash logs:** `NavidromePlayer-2026-04-27-180336.ips`, `NavidromePlayer-2026-04-28-211506.ips`

---

### 🔴 NEW — `stopAVPlayer` crashes on `removeTimeObserver` after crossfade player swap

**File:** `Shared/Audio/AudioPlayer.swift`
**Functions:** `stopAVPlayer()`, `play(song:)` crossfade path, `prepareNextForCrossfade()` needsNextTrack callback
**What changed:** 
1. Both player swap sites (`player = crossfade.activePlayer`) now remove `timeObserver` from the OLD player first
2. `stopAVPlayer()` reordered: observer removal now happens BEFORE `replaceCurrentItem(with: nil)`
**Why:** Crash log shows `SIGABRT` in `-[AVPlayer removeTimeObserver:]` called from `stopAVPlayer()` → `stopAll()` → `play(song:)` → `previous()`. Root cause: SmartCrossfadeManager's `finalizeCrossfade()` swaps `self.player` to a different AVPlayer instance, but `timeObserver` was registered on the OLD player. When `stopAVPlayer()` later tries `player?.removeTimeObserver(observer)`, it removes from the wrong AVPlayer → NSException. The fix ensures the observer is always removed from the correct player before the swap.
**Crash log:** `NavidromePlayer-2026-04-30-141820.ips`

---

### 🔴 Finding 2 — KVO in `radioSeekBack` fires on non-main thread

**File:** `Shared/Audio/AudioPlayer.swift`
**Function:** `radioSeekBack(to:)`
**What changed:** Wrapped the entire `item.observe(\.status)` KVO callback body in `DispatchQueue.main.async { }`.
**Why:** `NSKeyValueObservation` fires on whatever thread changes the observed property. `AVPlayerItem.status` is changed on AVFoundation's internal media processing thread, NOT the main thread. The callback was accessing `self.player?.seek()`, `self.snapshotStatusObservation?.invalidate()`, `self.radioGoLive()`, and other main-thread-only state from a background thread — a data race that could corrupt AudioPlayer state or crash under the right timing (rapid seek-back → go-live during radio timeshift).

---

### 🟡 Finding 1 — Stale `analysisTask` overwrites visualizer buffer after track skip

**File:** `Shared/Audio/AudioPlayer.swift`
**Function:** `loadOfflineVisualizer(songId:url:)`
**What changed:** Added `guard self.currentSong?.id == songId else { return }` to all three `MainActor.run` blocks (cache hit, server fetch, local analysis) and to the progress callback.
**Why:** When the user skips tracks quickly, `stopAll()` cancels `analysisTask` and clears `offlineVisBuffer`. But if the previous Task's async work had already completed, its `MainActor.run` block was already queued and would fire AFTER `stopAll()`, overwriting the new song's cleared buffer with the old song's stale vis data. This caused the visualizer to either show the wrong song's waveform or fail to start entirely for the new song. The songId guard ensures stale completions are silently discarded.

---

### Findings NOT fixed (documented, acceptable risk)

| # | Sev | Finding | Rationale |
|---|---|---|---|
| 5 | 🟡 | `shazamHandler` heap alloc on render thread | Pre-existing pattern, 15s max duration, no crash risk |
| 6 | 🟡 | `debugWriteSamples` I/O on render thread | Diagnostic tool only, 5s capture window, acceptable |
| 3 | 🟢 | `processFFT` dead code data race | `playLocalWithEngine` is never called — unreachable |
| 4 | 🟢 | `timeUpdate` closure fragile without `@MainActor` | All callers use `.main` queue today — future risk only |
| 7 | 🟢 | `sourceFormat` data race | ARM64 naturally-atomic pointer writes — safe in practice |
| 8 | 🟢 | Stale status observer memory spike | Guard `self.playerItem === itemToObserve` handles it |