fix: Halt playback for the data warning and respect it on stream retry

[joashrajin]

Description

With Warn before using data ON and on a metered connection, two playback paths stream undownloaded episodes with no warning:

1. Autoplay / Up Next auto-advance — when the playing episode ends and the queue advances to a not-downloaded episode, it streams over mobile immediately.
2. Missing-download fallback — when a "downloaded" episode's file is gone (ENOENT), the player clears the download status and retries via stream with forceStream = true, bypassing the warning entirely.

Root cause of path 1: #5017 removed the return at the end of the data-warning branch in PlaybackManager.loadCurrentEpisode() (the deletion sits inside the episodeObservable if/else → when refactor hunk; the notification, its "Yes, keep playing" resume action, and the EMPTY state push all survived, so the halt was clearly still intended). The branch still posts the "This episode is not downloaded, do you want to stream it?" notification, but execution now falls through and play() starts anyway — since 8.10 the warning has been decorative for any non-forced play. This became much more visible after the setting became ON by default (PCDROID-534).

Changes (all in PlaybackManager.kt):

• Restore the return in the data-warning branch of loadCurrentEpisode(), so playback halts until the user taps Yes, keep playing on the notification (which resumes via the existing playNow(forceStream = true) receiver path) or Play next downloaded.
• The missing-download stream retry in onPlayerError() no longer passes forceStream = true, so the retry goes through the same warning check. Behavior on unmetered networks, or with the warning off, is unchanged.
• Skip the warning while Android Auto is connected (Util.isAndroidAutoConnectedFlow), because the confirmation notification is only visible on the phone — restoring the return without this would bring back the "playback silently stops in the car" behavior reported in PCDROID-397. The setting's description already says the warning doesn't apply to Android Auto.

Fixes PCDROID-647
Addresses PCDROID-397 (the halt-in-car symptom; keeping it open pending on-device Android Auto verification)

Testing Instructions

Preconditions: metered/mobile connection, Settings → Storage & data use → Warn before using data = ON, Autoplay ON.

• Play a downloaded episode with a not-downloaded episode next in Up Next, and let it finish
• Verify playback stops when the queue advances and a "do you want to stream it?" notification appears (no mobile-data streaming starts)
• Tap Yes, keep playing on the notification
• Verify streaming starts
• Manually tap play on a not-downloaded episode
• Verify the streaming-warning dialog still appears and confirming plays as before
• For the missing-file path: download an episode, delete the audio file from Android/data/au.com.shiftyjelly.pocketcasts/files/PocketCasts/podcasts/ (adb or file manager), then tap play on it while on mobile data
• Verify the log shows "Downloaded file missing … retrying via stream" and the data warning is shown instead of silent streaming
• On WiFi, repeat auto-advance and the missing-file retry
• Verify both stream immediately with no warning (unchanged behavior)
• Connect to Android Auto (head unit or DHU) on mobile data with the same queue setup
• Verify auto-advance keeps playing in the car with no halt (PCDROID-397)

Screenshots or Screencast

n/a

Checklist

• If this is a user-facing change, I have added an entry in CHANGELOG.md (added after PR creation, references the PR number)
• Ensure the linter passes (./gradlew spotlessApply to automatically apply formatting/linting) (ran spotlessCheck — passed)
• I have considered whether it makes sense to add tests for my changes (no unit test added: loadCurrentEpisode's warning branch has no existing test harness and depends on player/network/notification state; behavior is covered by the manual steps above)
• All strings that need to be localized are in modules/services/localization/src/main/res/values/strings.xml — n/a, no new strings
• Any jetpack compose components I added or changed are covered by compose previews — n/a, no UI components changed
• I have updated (or requested that someone edit) the Event Horizon schema to reflect any new or changed analytics. — n/a, no analytics changes

I have tested any UI changes...

n/a — no UI changes (existing notification/dialog behavior only)

[claude]

Claude finished @joashrajin's task in 3m 44s —— View job

---

Review: PR #5533 — Halt playback for the data warning and respect it on stream retry

• Read the diff against origin/main
• Analyze loadCurrentEpisode() data-warning branch
• Analyze onPlayerError() missing-download retry
• Check isAndroidAutoConnectedFlow usage & coroutine safety
• Post review feedback

---

Nice, well-scoped fix with a clear write-up. The core logic is correct: restoring the return at PlaybackManager.kt:2020 properly halts playback until the user confirms via the notification, and dropping forceStream = true on the missing-file retry (:1338) routes it back through the same warning gate. Both match the intended behavior described in the PR. I have one substantive concern and a couple of minor notes.

🟠 Condition ordering makes the Android Auto flow check run on the hot path (:1998)

This is the main thing worth addressing (also left as an inline comment). Because && short-circuits left-to-right and !isCarUiMode && !isWearOs are already true on a phone, the new suspending call

!Util.isAndroidAutoConnectedFlow(application).first()

is evaluated on every not-downloaded episode load, before the cheap gates that would normally rule out a warning. That includes:

• WiFi playback (!Network.isUnmeteredConnection would be false),
• warning turned off (warnOnMeteredNetwork.value false),
• forceStream = true retries,
• and play = false preloads — note several callers pass play = false (:726, :748, :814, :853, :1243, :1285, :2637, :2650).

In all of those, no warning can ever be shown, yet each now pays a CarConnection LiveData round-trip. Since every operand is joined by &&, moving the check to the end (just after play) is semantically identical but only collects the flow in the rare metered + warning-on + playing case:

if (!Util.isCarUiMode(application) &&
    !Util.isWearOs(application) &&
    // The watch handles these warnings before this is called
    settings.warnOnMeteredNetwork.value &&
    episode.uuid != lastWarnedPlayedEpisodeUuid &&
    !Network.isUnmeteredConnection(application) &&
    !forceStream &&
    play &&
    // Don't block playback while driving as the warning is only shown on the phone
    !Util.isAndroidAutoConnectedFlow(application).first()
) {

Fix this →

🟡 .first() robustness on a playback-critical path (:1998)

Util.isAndroidAutoConnectedFlow is CarConnection(context).type (LiveData) → .asFlow(). .first():

• suspends until the first emission — the CarConnection LiveData queries the car host asynchronously, so if it is slow (or never emits), loadCurrentEpisode() blocks and playback silently never starts;
• throws NoSuchElementException if the flow ever completes without emitting.

Reordering (above) shrinks the exposure to just the metered-warning case, but a hang there would still stall playback. Consider firstOrNull() == true, ideally wrapped in a short withTimeoutOrNull { … }, so an unavailable connection state degrades to "show the warning" rather than "hang". Minor, but this is the playback start path.

🟢 Minor / non-blocking

• The restored return at :2020 also skips the lastPlayedEpisodeUuid = episode.uuid update at :2064. That matches the pre-Replace downloads code #5017 behavior (playback is halted anyway), so this looks intentional and fine — flagging only for the record.
• Agree with the PR's note that a unit test around this branch is awkward given the player/network/notification coupling. If you wanted a cheap regression guard, the reordered boolean could be extracted into a small pure shouldWarnBeforeStreaming(...) helper that's unit-testable without the flow, but that's optional.
• The missing-file retry correctness still depends on downloadQueue.cancel(...) / clearPlaybackErrorBlocking(...) leaving the episode as not downloaded in the DB so the re-fetched copy enters this branch — that's pre-existing behavior unchanged by this PR, just worth keeping in mind during the manual on-device verification you flagged as TODO.

Overall this is a solid correctness fix. The ordering change is the one I'd recommend before merge; the rest is optional hardening.
• Branch: pcdroid-647-android-autoplay-streams-undownloaded-phantom-deleted-up

[claude]

Claude finished @joashrajin's task in 2m 1s —— View job

---

Reviewing PR #5533

• Read the diff against origin/main
• Analyze loadCurrentEpisode() data-warning branch
• Analyze onPlayerError() missing-download retry
• Check isAndroidAutoConnectedFlow usage & thread/coroutine safety
• Post review feedback

---

Summary

The fix is correct and well-scoped. Restoring the return in the data-warning branch, dropping forceStream = true from the missing-file retry, and exempting Android Auto all match the intent described in the PR. I verified the retry path actually re-triggers the warning: onPlayerError → playNow → playNowSuspend (sets forcePlayerSwitch = true) → playNowSync → loadCurrentEpisode(play = true, forceStream = false), so the warning branch is reachable with play == true. 👍

I do have one performance / ordering concern worth addressing before merge, plus a couple of minor notes.

🟠 Reorder the Android Auto check to the end of the && chain

PlaybackManager.kt:1994-2004

if (!Util.isCarUiMode(application) &&
    !Util.isWearOs(application) &&
    // Don't block playback while driving as the warning is only shown on the phone
    !Util.isAndroidAutoConnectedFlow(application).first() &&   // ← runs on every phone stream
    settings.warnOnMeteredNetwork.value &&
    episode.uuid != lastWarnedPlayedEpisodeUuid &&
    !Network.isUnmeteredConnection(application) &&
    !forceStream &&
    play
) {

On a phone isCarUiMode and isWearOs are both false, so && never short-circuits before the new check. That means every load of a not-downloaded episode now evaluates isAndroidAutoConnectedFlow(application).first() — even on WiFi, or when Warn before using data is off, i.e. exactly the "stream immediately, no warning" paths the PR says are unchanged.

That call is not cheap: Util.isAndroidAutoConnectedFlow constructs a fresh CarConnection(context) and its type is a LiveData backed by an async ContentProvider query (CarConnection(context).type.map { … }.asFlow()). .first() suspends until that query posts its first value, so this adds a ContentProvider round-trip and a suspension point to the common streaming-start path.

The cheap synchronous checks (settings.warnOnMeteredNetwork.value, !Network.isUnmeteredConnection(application), !forceStream, play) should gate the expensive async one. Moving it last means the CarConnection query only runs when a warning would otherwise actually fire:

if (!Util.isCarUiMode(application) &&
    !Util.isWearOs(application) &&
    settings.warnOnMeteredNetwork.value &&
    episode.uuid != lastWarnedPlayedEpisodeUuid &&
    !Network.isUnmeteredConnection(application) &&
    !forceStream &&
    play &&
    // Don't block playback while driving as the warning is only shown on the phone
    !Util.isAndroidAutoConnectedFlow(application).first()
) {

Fix this →

🟡 Minor: exception safety of .first()

Unlike the existing call site in Media3SessionCallback.kt:330 (wrapped in try/catch on Dispatchers.Main), this one sits bare inside a boolean expression. If the CarConnection flow ever emits an error, it will propagate out of loadCurrentEpisode. In practice androidx's CarConnection posts CONNECTION_TYPE_NOT_CONNECTED on query failure rather than throwing, so .first() should reliably terminate — but given the fix's whole purpose is to not silently break playback, it may be worth guarding this so a CarConnection hiccup can't abort the load. Low priority.

🟢 Minor: notes, no change needed

• The early return skips lastPlayedEpisodeUuid = episode.uuid and the auto-subscribe block below. This matches the pre-#5017 halt behavior being restored, and in the auto-advance case showedStreamWarning is false so the lastWarnedPlayedEpisodeUuid update was already a no-op. Correct.
• Not testing the ordering/behavior is understandable given the notification/network/player dependencies, but a small unit test asserting "warning setting on + metered + not-downloaded ⇒ play() is not called" would guard against a future refactor silently dropping the return again (this is the second time this regressed — the first was Replace downloads code #5017). Consider it a follow-up.

Everything else looks good — the CHANGELOG entry is correct and the diff is minimal and on-target.

[claude]

Claude finished @joashrajin's task in 1m 6s —— View job

---

Re-review of PR #5533 (after latest push)

• Read the updated diff against origin/main
• Verify the two prior review points were addressed
• Check the withTimeoutOrNull + Android Auto ordering fix
• Post feedback

---

The latest commit (15d1d80) cleanly resolves both concerns from the earlier reviews. This is good to go from my side.

✅ Ordering fixed (PlaybackManager.kt:1996-2006)

The Util.isAndroidAutoConnectedFlow(application).first() operand is now the last term of the && chain, after the cheap synchronous gates (warnOnMeteredNetwork.value, !Network.isUnmeteredConnection, !forceStream, play). So the CarConnection LiveData/ContentProvider round-trip only runs in the narrow metered + warning-on + playing + not-forced case where a warning would actually fire — WiFi playback, warning-off, forceStream, and play = false preloads all short-circuit before it. Exactly the hot-path fix that was recommended.

✅ Robustness fixed (:2005)

withTimeoutOrNull(CAR_CONNECTION_TIMEOUT) { Util.isAndroidAutoConnectedFlow(application).first() } != true

The withTimeoutOrNull(1000) wrapper plus != true comparison degrades safely in every branch:

CarConnection state	withTimeoutOrNull { … }	!= true	Result
Connected (PROJECTION)	true	false	warning skipped — car keeps playing ✅
Not connected	false	true	warning shown ✅
Slow / never emits	null (timeout)	true	warning shown ✅ (no hang)

This removes the "playback silently never starts if the LiveData is slow" risk from .first() and picks the correct fail-open default (show the warning) rather than silently streaming.

🟢 Minor / non-blocking (unchanged from before)

• withTimeoutOrNull guards against a hang, but not against the flow throwing — .first() on an empty flow throws NoSuchElementException, which propagates past the timeout wrapper. In practice androidx CarConnection posts CONNECTION_TYPE_NOT_CONNECTED on query failure rather than completing empty, so this stays theoretical. Not worth changing.
• The early return at :2022 still (intentionally) skips the lastPlayedEpisodeUuid update below — matches the pre-Replace downloads code #5017 halt behavior. Correct.
• Still no unit test on this branch. Understandable given the player/network/notification coupling, but since this is the second time the return regressed (first was Replace downloads code #5017), a small pure shouldWarnBeforeStreaming(...) helper + test would be a cheap regression guard for a follow-up. Optional.

The CHANGELOG.md entry and the missing-file retry change (:1339, dropping forceStream = true) are both correct. The manual Android Auto / missing-file on-device verification you flagged as TODO is the only thing left to confirm before merge.

Nice iteration. 👍
• Branch: pcdroid-647-android-autoplay-streams-undownloaded-phantom-deleted-up

[Copilot]

Pull request overview

This PR fixes playback continuing to stream over mobile data without honoring the “Warn before using data” setting, particularly for Up Next auto-advance and “missing downloaded file” fallback streaming. It updates the central playback flow in PlaybackManager and documents the user-facing fix in the changelog.

Changes:

• Restore the early return in the metered-data warning path so playback halts until the user confirms streaming.
• Ensure “missing downloaded file” retry no longer forces streaming, so it re-enters the same warning logic.
• Bypass the warning halt when Android Auto is connected (since the confirmation notification is on the phone).

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File	Description
modules/services/repositories/src/main/java/au/com/shiftyjelly/pocketcasts/repositories/playback/PlaybackManager.kt	Re-introduces the halt/return in the metered-data warning path, removes forced-stream on missing-file retry, and skips the halt when Android Auto is connected.
CHANGELOG.md	Adds an 8.17 bug-fix entry describing the restored “Warn before using data” behavior for auto-advance and missing-file retry.

Comments suppressed due to low confidence (1)

modules/services/repositories/src/main/java/au/com/shiftyjelly/pocketcasts/repositories/playback/PlaybackManager.kt:1999

• Util.isAndroidAutoConnectedFlow(application).first() is currently evaluated before cheap checks like warnOnMeteredNetwork, isUnmeteredConnection, and even play. Because first() suspends and allocates a new CarConnection, this can add unnecessary latency/overhead on every not-downloaded episode load (including Wi‑Fi / warning-off cases where the branch will never run). Reorder the conditions so the Android Auto Flow is only queried when all other warning prerequisites are already satisfied.

        episodeSubscription?.dispose()
        if (!episode.isDownloaded) {
            if (!Util.isCarUiMode(application) &&
                !Util.isWearOs(application) &&
                // The watch handles these warnings before this is called
                settings.warnOnMeteredNetwork.value &&

[claude]

Claude finished @joashrajin's task in 1m 42s —— View job

---

Re-review of PR #5533 (after latest push)

• Read the updated diff against origin/main
• Review the new MainActivity.kt notification-permission change
• Confirm the earlier PlaybackManager concerns remain resolved
• Post feedback

---

The latest push adds a MainActivity.kt change on top of the already-solid PlaybackManager fix. This new piece is a necessary companion to restoring the return, and it closes two real gaps — nice catch.

✅ Why the MainActivity change matters

sendDataWarningNotification() posts the "do you want to stream it?" notification through notificationPermissionChecker.checkNotificationPermission { manager.notify(...) } (PlaybackManager.kt:2220-2224). Now that loadCurrentEpisode() halts playback and returns, that notification is the only way the user can resume the auto-advanced episode — so the callback must fire, otherwise playback silently stops with no way back. The previous checkForNotificationPermission had two holes that this push fixes:

1. Pre-Android 13 (< TIRAMISU) — the old code had no else branch, so onPermissionGranted was never invoked and the notification never posted. The new else (MainActivity.kt:469-472) invokes it directly, which is correct since POST_NOTIFICATIONS is not a runtime permission before 13.
2. Permission-request path — the launcher callback was {} (registerForActivityResult(...) {}), so even when the user tapped Allow the pending notification was dropped. Storing onNotificationPermissionGranted and invoking it on granted (:421-430, :463) fixes that.

The onNotificationPermissionGranted = {} reset after invocation correctly prevents a stale callback from double-firing on a later unrelated grant. 👍

🟡 Minor: the rationale branch still swallows the callback

In checkForNotificationPermission, the shouldShowRequestPermissionRationale branch (MainActivity.kt:442-460) neither stores nor invokes onPermissionGranted — it only shows the "notifications blocked" snackbar and returns. So a user who has previously denied notifications, on metered data with the warning on, will now hit the new halt with no notification to resume from. It's a genuine (narrow) edge case introduced by restoring the return: pre-PR, playback just continued; post-PR it stops and the only recovery is manually tapping play again (which routes through the separate in-app streaming dialog, so they aren't fully stuck).

Not necessarily blocking — arguably acceptable since the user actively declined notifications — but worth a deliberate decision rather than leaving it implicit. At minimum consider a code comment noting the rationale path intentionally drops the callback, or fall back to the in-app dialog in that case.

✅ PlaybackManager (unchanged since last review, still correct)

• Android Auto check is last in the && chain and wrapped in withTimeoutOrNull(CAR_CONNECTION_TIMEOUT) { … } != true — the CarConnection/ContentProvider round-trip only runs in the metered + warning-on + playing + not-forced case, and fails open to "show the warning" on timeout. Correct hot-path + robustness handling.
• The missing-file retry drops forceStream = true (:1339) so it re-enters the same warning gate. Verified reachable with play == true via onPlayerError → playNow → playNowSync → loadCurrentEpisode(forceStream = false).
• The restored return (:2022) intentionally skips the lastPlayedEpisodeUuid update — matches pre-Replace downloads code #5017 halt behavior.

🟢 Non-blocking (carried over)

• .first() inside withTimeoutOrNull still throws NoSuchElementException if the flow ever completes empty; androidx CarConnection posts CONNECTION_TYPE_NOT_CONNECTED rather than completing, so it stays theoretical.
• Still no regression test. Since the return has now regressed twice (Replace downloads code #5017, and this fix), extracting a pure shouldWarnBeforeStreaming(...) helper for a cheap unit test would be a worthwhile follow-up.

Overall the PR is in good shape. The rationale-branch gap is the one thing I'd suggest making an explicit decision on before merge; everything else is optional hardening. The Android Auto / missing-file on-device verification you flagged as TODO is still the remaining thing to confirm.
• Branch: pcdroid-647-android-autoplay-streams-undownloaded-phantom-deleted-up

[claude]

Claude finished @joashrajin's task in 1m 8s —— View job

---

Re-review of PR #5533 (after latest push)

• Read the updated diff against origin/main
• Review the two new commits (e233ef7, 95a71cb) on MainActivity.kt
• Confirm the earlier PlaybackManager concerns remain resolved
• Post feedback

---

The two new commits close the notification-permission gaps flagged in the previous review. Both the pre-Android-13 path and the "callback intentionally dropped" documentation are now handled. This looks good to merge from my side.

✅ Pre-Android-13 path now fires the callback (MainActivity.kt:472-475)

} else {
    // No runtime permission is needed to post notifications on older versions
    onPermissionGranted()
}

On < TIRAMISU the callback is now invoked directly. Since POST_NOTIFICATIONS isn't a runtime permission before Android 13, this is correct — the data-warning notification will post (and the halted episode can be resumed) on older devices, which the previous no-else version silently prevented.

✅ Rationale branch is now a documented, deliberate drop (:442-444)

// The user denied notifications, so onPermissionGranted is intentionally dropped.
// Callers relying on a notification, such as the data warning, stay halted and the
// snackbar points the user at the blocked notification setting.
shouldShowRequestPermissionRationale(...) -> { ... }

This resolves the "implicit gap" I raised last time — a user who previously denied notifications now hits a documented, intentional halt with the snackbar guiding them to re-enable notifications (and manual play still routes through the separate in-app streaming dialog). Making it an explicit decision with a comment is exactly what I was hoping for. 👍

The onNotificationPermissionGranted = {} reset in the launcher callback (:429) also correctly prevents a stale callback from double-firing on a later unrelated grant.

🟢 Minor / non-blocking

• Callback lost across activity recreation. notificationPermissionLauncher (an ActivityResultLauncher) survives config changes and re-delivers its result, but the onNotificationPermissionGranted field is re-initialized to {} when MainActivity is recreated (e.g. rotation while the system permission dialog is up). In that narrow case the granted result would fire with an empty callback, so the pending data-warning notification wouldn't post. Pre-existing pattern limitation, very narrow, and the user can recover by tapping play again — noting only for the record.
• The if (settings.isNotificationsDisabledMessageShown()) return early-out in the rationale branch means the snackbar isn't re-shown on repeat denials, which is consistent with the documented intent (callback stays dropped). Correct.

✅ PlaybackManager (unchanged since last review, still correct)

• Android Auto check is last in the && chain, wrapped in withTimeoutOrNull(CAR_CONNECTION_TIMEOUT) { … } != true — the CarConnection round-trip only runs in the metered + warning-on + playing + not-forced case, and fails open to "show the warning" on timeout.
• Missing-file retry drops forceStream = true (:1339) so it re-enters the same warning gate.
• The restored return (:2022) intentionally skips the lastPlayedEpisodeUuid update — matches pre-Replace downloads code #5017 halt behavior.

Still no regression test (understandable given the coupling); a pure shouldWarnBeforeStreaming(...) helper would be a cheap follow-up guard since the return has now regressed twice. The Android Auto / missing-file on-device verification you flagged as TODO remains the last thing to confirm before merge.

Nice iteration. 👍

• Branch: pcdroid-647-android-autoplay-streams-undownloaded-phantom-deleted-up

[claude]

Claude finished @joashrajin's task in 51s —— View job

---

Re-review of PR #5533 (after latest push)

• Read the updated diff against origin/main
• Re-verify the PlaybackManager data-warning + missing-file changes
• Re-verify the MainActivity notification-permission changes
• Post review

---

The latest push is the merge of origin/main (c2ef9da) into the branch; the substantive diff is unchanged from the last approved iteration. I re-verified the three touched files against origin/main...HEAD and everything still lines up. No new issues introduced by the merge — this remains in good shape from my side.

✅ PlaybackManager.kt — data-warning halt (:1996-2022)

• The return at :2022 is restored, so a not-downloaded auto-advance/queue load halts and posts the "do you want to stream it?" notification instead of silently streaming. Correct.
• The Android Auto check is the last operand of the && chain and wrapped:

  withTimeoutOrNull(CAR_CONNECTION_TIMEOUT) { Util.isAndroidAutoConnectedFlow(application).first() } != true

  So the CarConnection/ContentProvider round-trip only runs in the narrow metered + warning-on + not-forced + play case — WiFi, warning-off, forceStream, and play = false preloads all short-circuit before it. The withTimeoutOrNull(1000) + != true fails open (shows the warning) on a slow/absent connection rather than hanging. Both prior concerns stay resolved.

✅ PlaybackManager.kt — missing-file retry (:1339)

Dropping forceStream = true routes the ENOENT retry back through the same warning gate. Reachable with play == true via onPlayerError → playNow → playNowSync → loadCurrentEpisode(forceStream = false). Correct.

✅ MainActivity.kt — notification-permission callback (:421-475)

Since the halted episode can now only be resumed from the notification, the callback must fire whenever the notification can post:

• Pre-Android-13 else branch (:472-475) invokes onPermissionGranted() directly — correct, POST_NOTIFICATIONS isn't a runtime permission there.
• Permission-request path stores onNotificationPermissionGranted and invokes it on granted, resetting to {} afterwards to avoid a stale double-fire (:421-430, :466).
• The rationale (previously-denied) branch's dropped callback is now documented as a deliberate halt with the snackbar guiding the user to re-enable (:442-444).

🟢 Minor / non-blocking (carried over, no change needed)

• .first() inside withTimeoutOrNull would still throw NoSuchElementException if the flow ever completed empty; androidx CarConnection posts CONNECTION_TYPE_NOT_CONNECTED on failure rather than completing, so it stays theoretical.
• Callback field is re-initialized to {} on MainActivity recreation (e.g. rotation while the system permission dialog is up), so a grant delivered across recreation wouldn't post the pending notification — very narrow, user recovers by tapping play again.
• Still no regression test. Since the return has now regressed twice (Replace downloads code #5017 and the bug this PR fixes), a small pure shouldWarnBeforeStreaming(...) helper would be a cheap unit-testable guard for a follow-up.

The CHANGELOG.md entry is correct. The Android Auto / missing-file on-device verification flagged as TODO in the description remains the only outstanding item before merge. Nice work. 👍
• Branch: pcdroid-647-android-autoplay-streams-undownloaded-phantom-deleted-up

[claude]

Claude finished @joashrajin's task in 47s —— View job

---

Re-review of PR #5533 (after latest push)

• Read the updated diff against origin/main
• Re-verify the PlaybackManager data-warning + missing-file changes
• Re-verify the MainActivity notification-permission changes
• Post review

---

I re-checked the full diff (git diff origin/main...HEAD) and the three touched files. The substantive changes are unchanged from the last approved iteration — this push looks like the merge of origin/main into the branch, and it introduces no new issues. Still in good shape from my side.

✅ PlaybackManager.kt — data-warning halt (:2048-2075)

• The return at :2075 is present, so a not-downloaded auto-advance/queue load halts and posts the "do you want to stream it?" notification instead of silently streaming. Correct.
• The Android Auto check is the last operand of the && chain and wrapped:

  withTimeoutOrNull(CAR_CONNECTION_TIMEOUT) { Util.isAndroidAutoConnectedFlow(application).first() } != true

  so the CarConnection/ContentProvider round-trip only runs in the narrow metered + warning-on + not-forced + play case. WiFi, warning-off, forceStream, and play = false preloads all short-circuit before it, and withTimeoutOrNull(1000) + != true fails open (shows the warning) on a slow/absent connection rather than hanging. Both earlier concerns stay resolved.

✅ PlaybackManager.kt — missing-file retry (:1371)

Dropping forceStream = true routes the ENOENT retry back through the same warning gate. Reachable with play == true via onPlayerError → playNow → playNowSync → loadCurrentEpisode(forceStream = false). Correct.

✅ MainActivity.kt — notification-permission callback (:421-475)

Since the halted episode can now only be resumed from the notification, the callback must fire whenever the notification can post:

• Pre-Android-13 else branch (:472-475) invokes onPermissionGranted() directly — correct, POST_NOTIFICATIONS isn't a runtime permission there.
• The permission-request path stores onNotificationPermissionGranted and invokes it on granted, resetting to {} afterwards to avoid a stale double-fire (:421-430, :466).
• The rationale (previously-denied) branch's dropped callback is now documented as a deliberate halt with the snackbar guiding the user to re-enable (:442-444).

🟢 Minor / non-blocking (carried over, no change needed)

• .first() inside withTimeoutOrNull would still throw NoSuchElementException if the flow ever completed empty; androidx CarConnection posts CONNECTION_TYPE_NOT_CONNECTED on failure rather than completing, so it stays theoretical.
• The onNotificationPermissionGranted field is re-initialized to {} on MainActivity recreation (e.g. rotation while the system permission dialog is up), so a grant delivered across recreation wouldn't post the pending notification — very narrow, user recovers by tapping play again.
• Still no regression test. Since the return has now regressed twice (Replace downloads code #5017 and the bug this PR fixes), a small pure shouldWarnBeforeStreaming(...) helper would be a cheap unit-testable guard for a follow-up.

The CHANGELOG.md entry is correct. The Android Auto / missing-file on-device verification flagged as TODO in the description remains the only outstanding item before merge. Nice work. 👍
• Branch: pcdroid-647-android-autoplay-streams-undownloaded-phantom-deleted-up

[claude]

Claude finished @geekygecko's task in 1m 27s —— View job

---

Re-review of PR #5533 (after latest push)

• Read the updated diff against origin/main
• Review the new Util.kt Android Auto crash fix
• Re-verify the PlaybackManager .first() → .firstOrNull() change
• Confirm other isAndroidAutoConnectedFlow callers still work
• Post review

---

This push adds the "Fix Android Auto check crash" commit (a405d4f), which reworks Util.isAndroidAutoConnectedFlow and switches the PlaybackManager call from .first() to .firstOrNull(). Both changes are correct and they also close the last theoretical concern I'd carried across previous reviews. Good to go from my side.

✅ Util.kt — CarConnection now constructed on the main thread (:26-32)

// CarConnection's constructor creates an AsyncQueryHandler, which needs a Looper, so build it on the main thread.
fun isAndroidAutoConnectedFlow(context: Context): Flow<Boolean> = flow {
    val connectionType = withContext(Dispatchers.Main) {
        CarConnection(context).type.map { it == CarConnection.CONNECTION_TYPE_PROJECTION }
    }
    emitAll(connectionType.asFlow())
}

This is a real fix. The previous expression-body version built CarConnection(context) lazily on whatever dispatcher collected the flow. Now that PlaybackManager.loadCurrentEpisode() collects it inside withTimeoutOrNull { … } on its own (non-main) coroutine context, the AsyncQueryHandler inside CarConnection's constructor would hit Can't create handler inside thread … that has not called Looper.prepare() and crash. Wrapping the construction + LiveData.map in withContext(Dispatchers.Main) guarantees a Looper. The subsequent emitAll(connectionType.asFlow()) is safe to run off-main because LiveData.asFlow() internally observes on the main dispatcher. Correct and well-commented.

✅ PlaybackManager.kt — .firstOrNull() + timeout (:2058)

withTimeoutOrNull(CAR_CONNECTION_TIMEOUT) { Util.isAndroidAutoConnectedFlow(application).firstOrNull() } != true

.firstOrNull() replaces .first(), which resolves the NoSuchElementException-on-empty-flow risk flagged in earlier reviews — an empty completion now yields null rather than throwing. The fail-open behaviour is preserved across every branch:

CarConnection state	inner result	withTimeoutOrNull	!= true	outcome
Connected (PROJECTION)	true	true	false	warning skipped — car keeps playing ✅
Not connected	false	false	true	warning shown ✅
Slow / never emits	(suspends)	null (timeout)	true	warning shown, no hang ✅
Empty completion	null	null	true	warning shown ✅ (no more throw)

The check is still the last operand of the && chain, so the CarConnection/ContentProvider round-trip only runs in the narrow metered + warning-on + not-forced + play case — WiFi, warning-off, forceStream, and play = false preloads all short-circuit before it.

✅ Other callers unaffected

• MediaSessionManager.kt:1269 (onAddBookmark) collects on Dispatchers.Main already — the extra withContext(Dispatchers.Main) is a harmless no-op there.
• Media3SessionCallback.kt:330 uses .first() inside its own try/catch on Dispatchers.Main — still fine, and now the construction is guaranteed on main regardless of caller.

🟢 Minor / non-blocking (carried over)

• The restored return (:2075) intentionally skips the lastPlayedEpisodeUuid update — matches pre-Replace downloads code #5017 halt behavior. Correct.
• MainActivity.kt notification-permission handling (pre-Android-13 else, stored-then-reset callback, documented rationale-branch drop) is unchanged and still correct.
• Still no regression test. Since the return has now regressed twice (Replace downloads code #5017 and this bug), extracting a pure shouldWarnBeforeStreaming(...) helper for a cheap unit test remains a worthwhile follow-up — the crash fix reinforces this, since the warning path is now exercised more broadly.

The CHANGELOG.md entry is correct. The Android Auto / missing-file on-device verification flagged as TODO in the description remains the only outstanding item before merge. Nice iteration. 👍

• Branch: pcdroid-647-android-autoplay-streams-undownloaded-phantom-deleted-up

[geekygecko]

Thanks for making this fix.

[geekygecko]

The app crashed when auto playing an episode.

java.lang.RuntimeException: Can't create handler inside thread Thread[DefaultDispatcher-worker-14,5,main] that has not called Looper.prepare()
	at android.os.Handler.<init>(Handler.java:253)
	at android.os.Handler.<init>(Handler.java:155)
	at android.content.AsyncQueryHandler.<init>(AsyncQueryHandler.java:125)
	at androidx.car.app.connection.CarConnectionTypeLiveData$CarConnectionQueryHandler.<init>(CarConnectionTypeLiveData.java:92)
	at androidx.car.app.connection.CarConnectionTypeLiveData.<init>(CarConnectionTypeLiveData.java:59)
	at androidx.car.app.connection.CarConnection.<init>(CarConnection.java:94)
	at au.com.shiftyjelly.pocketcasts.utils.Util.isAndroidAutoConnectedFlow(Util.kt:21)
	at au.com.shiftyjelly.pocketcasts.repositories.playback.PlaybackManager$loadCurrentEpisode$2.invokeSuspend(PlaybackManager.kt:2058)

I have merged some minor changes to fix this.

There's also a warning "Legacy Long overload can be converted to Duration" so I have changed CAR_CONNECTION_TIMEOUT to a Duration.

[joashrajin]

Thank you so much Phil 🙇

[claude]

Claude finished @geekygecko's task in 1m 26s —— View job

---

Re-review of PR #5533 (after latest push)

• Read the updated diff against origin/main
• Review the new Util.kt Android Auto crash fix + Duration change
• Verify the .first() → .firstOrNull() switch in PlaybackManager
• Confirm other isAndroidAutoConnectedFlow callers still compile/behave
• Check CI status

---

This push is the "Fix Android Auto check crash" commit (a405d4f) plus a merge of origin/main. The crash fix is correct, and it also closes the last theoretical concern I'd carried across earlier reviews (NoSuchElementException on an empty flow). Good to go from my side.

✅ Util.kt — CarConnection now built on the main thread (:26-32)

// CarConnection's constructor creates an AsyncQueryHandler, which needs a Looper, so build it on the main thread.
fun isAndroidAutoConnectedFlow(context: Context): Flow<Boolean> = flow {
    val connectionType = withContext(Dispatchers.Main) {
        CarConnection(context).type.map { it == CarConnection.CONNECTION_TYPE_PROJECTION }
    }
    emitAll(connectionType.asFlow())
}

This is the right fix for the reported crash:

Can't create handler inside thread Thread[DefaultDispatcher-worker-14,…] that has not called Looper.prepare()
    at androidx.car.app.connection.CarConnection.<init>
    at …Util.isAndroidAutoConnectedFlow(Util.kt:21)
    at …PlaybackManager$loadCurrentEpisode$2

The previous expression-body version constructed CarConnection(context) lazily on whatever dispatcher collected the flow. Since loadCurrentEpisode() collects it inside withTimeoutOrNull { … } on a non-main coroutine context, the AsyncQueryHandler in CarConnection's constructor hit a thread with no Looper and crashed. Wrapping the construction + LiveData.map in withContext(Dispatchers.Main) guarantees a Looper. emitAll(connectionType.asFlow()) is safe to run off-main afterwards because LiveData.asFlow() adds/removes its observer on Dispatchers.Main.immediate internally. Correct and well-commented.

✅ PlaybackManager.kt — .firstOrNull() + Duration timeout (:2058)

withTimeoutOrNull(CAR_CONNECTION_TIMEOUT) { Util.isAndroidAutoConnectedFlow(application).firstOrNull() } != true

.firstOrNull() replaces .first(), removing the NoSuchElementException-on-empty-flow risk flagged in earlier reviews — an empty completion now yields null rather than throwing. Fail-open is preserved in every branch:

CarConnection state	inner result	withTimeoutOrNull	!= true	outcome
Connected (PROJECTION)	true	true	false	warning skipped — car keeps playing ✅
Not connected	false	false	true	warning shown ✅
Slow / never emits	(suspends)	null (timeout)	true	warning shown, no hang ✅
Empty completion	null	null	true	warning shown, no throw ✅

CAR_CONNECTION_TIMEOUT is now 1000.milliseconds (Duration) — resolves the "legacy Long overload can be converted to Duration" warning while keeping the same 1s budget. The check is still the last operand of the && chain, so the CarConnection/ContentProvider round-trip only runs in the narrow metered + warning-on + not-forced + play case; WiFi, warning-off, forceStream, and play = false preloads all short-circuit before it.

✅ Other callers unaffected

• MediaSessionManager.kt:1269 (onAddBookmark) already collects on Dispatchers.Main; the added withContext(Dispatchers.Main) is a harmless no-op there.
• Media3SessionCallback.kt:330 still uses .first() inside its own try/catch on Dispatchers.Main — construction is now guaranteed on main regardless of caller, and the try/catch still guards the empty-flow throw. Unchanged and fine.

🟢 Minor / non-blocking (carried over, no change needed)

• The restored return (:2075) intentionally skips the lastPlayedEpisodeUuid update — matches pre-Replace downloads code #5017 halt behavior.
• MainActivity.kt notification-permission handling (pre-Android-13 else, stored-then-reset callback, documented rationale-branch drop) is unchanged and still correct.
• Still no regression test. Since the return has now regressed twice (Replace downloads code #5017 and this bug), a small pure shouldWarnBeforeStreaming(...) helper for a cheap unit test remains a worthwhile follow-up — the crash fix reinforces this, as the warning path is now exercised more broadly.

CI: Gradle Wrapper validation passed; the Claude Code Review run is still in progress. The CHANGELOG.md entry is correct. The Android Auto / missing-file on-device verification flagged as TODO in the description is the only outstanding item. Nice iteration. 👍
• Branch: pcdroid-647-android-autoplay-streams-undownloaded-phantom-deleted-up

[wpmobilebot]

Version 8.17 has now entered code-freeze, so the milestone of this PR has been updated to 8.18.

[wpmobilebot]

Version 8.18 has now entered code-freeze, so the milestone of this PR has been updated to 8.19.