diff --git a/MISTAKES.md b/MISTAKES.md index 756b4d3..4e2960b 100644 --- a/MISTAKES.md +++ b/MISTAKES.md @@ -15,3 +15,19 @@ mistakes keep happening so a human can later add a Cursor rule. ``` Newest entries go below this line. + +## 2026-08-13: iOS tests asserted unverified AVPlayer.play() rate semantics + +**Category:** unverified-avfoundation-semantics +**What happened:** Agent-authored FullscreenVideoDismissPlaybackTests +landed on main asserting `player.rate == 0` after `play()` on an +AVPlayer whose item had already been detached. GitHub CI failed both +`stopPlaybackDetachesPlayerItem` and +`lateSeekResumeCannotRestartAfterTeardown` because `AVPlayer.play()` +sets `rate` to `1.0` even when `currentItem` is `nil`. +**Root cause:** The tests treated `rate` as proof that audio cannot +resume, without checking how AVFoundation actually updates that +property after `replaceCurrentItem(with: nil)`. +**Prevention:** When asserting playback teardown, treat detached +`currentItem` as the no-audio invariant. Only assert `rate == 0` at +the moment teardown returns, not after a subsequent `play()`. diff --git a/iosapp/iosapp/Views/TimelineView.swift b/iosapp/iosapp/Views/TimelineView.swift index 745c8a2..dd13180 100644 --- a/iosapp/iosapp/Views/TimelineView.swift +++ b/iosapp/iosapp/Views/TimelineView.swift @@ -1400,9 +1400,10 @@ final class FullResolutionImageLoader: ObservableObject { } // Preserves play intent across direct-play byte-range replacement so - // buffering updates do not leave fullscreen video unexpectedly paused, - // while capturing the player weakly so dismiss teardown can outrun a late - // resume callback. + // buffering updates do not leave fullscreen video unexpectedly paused. + // Weak self plus an owner check are required because a dismissed + // AVPlayer can still be retained by the dismiss animation; capturing + // the player weakly is not enough to outrun a late resume callback. private func wireDirectPlaySeekResume( on loader: DirectPlayResourceLoader, player: AVPlayer, @@ -1412,8 +1413,8 @@ final class FullResolutionImageLoader: ObservableObject { if armResume { _ = seekResumeGate.handleReplacementStarted(isPlayingOrWaiting: true) } - loader.onSeekRangeReplacementStarted = { [weak player] in - guard let player else { return } + loader.onSeekRangeReplacementStarted = { [weak self, weak player] in + guard let self, let player, self.player === player else { return } let shouldPause = seekResumeGate.handleReplacementStarted( isPlayingOrWaiting: player.rate > 0 || player.timeControlStatus == .waitingToPlayAtSpecifiedRate @@ -1422,12 +1423,10 @@ final class FullResolutionImageLoader: ObservableObject { player.pause() } } - loader.onSeekRangeReplacementReady = { [weak player] in - if seekResumeGate.handleReplacementReady() { - Task { @MainActor in - player?.play() - } - } + loader.onSeekRangeReplacementReady = { [weak self, weak player] in + guard let self, let player, self.player === player else { return } + guard seekResumeGate.handleReplacementReady() else { return } + player.play() } } diff --git a/iosapp/iosappTests/FullscreenVideoDismissPlaybackTests.swift b/iosapp/iosappTests/FullscreenVideoDismissPlaybackTests.swift index de03642..11f82c0 100644 --- a/iosapp/iosappTests/FullscreenVideoDismissPlaybackTests.swift +++ b/iosapp/iosappTests/FullscreenVideoDismissPlaybackTests.swift @@ -63,8 +63,10 @@ struct FullscreenVideoDismissPlaybackTests { ) } - // Ensures stopPlayback detaches the player item so a late play() cannot - // keep producing audio after the fullscreen UI is gone. + // Ensures stopPlayback detaches the player item so a later play() + // cannot produce audio. AVPlayer.play() still sets rate to 1.0 with + // no item; the detached currentItem is the invariant that proves + // nothing can decode. @Test func stopPlaybackDetachesPlayerItem() { let controller = VideoPreviewViewController( item: makeItem(id: "video-1", mediaType: "video"), @@ -80,12 +82,11 @@ struct FullscreenVideoDismissPlaybackTests { #expect(player.rate == 0) #expect(player.currentItem == nil) player.play() - #expect(player.rate == 0) #expect(player.currentItem == nil) } - // Reproduces the direct-play seek-resume race: an in-flight ready callback - // must not restart audio after teardown has already run. + // Reproduces the direct-play seek-resume race: in-flight started and + // ready callbacks captured before teardown must not restart audio. @Test func lateSeekResumeCannotRestartAfterTeardown() async throws { let loader = FullResolutionImageLoader(timelineManager: makeIsolatedManager()) let player = makePlayer() @@ -95,6 +96,7 @@ struct FullscreenVideoDismissPlaybackTests { loader: directPlayLoader, armResume: true ) + let pendingStarted = try #require(directPlayLoader.onSeekRangeReplacementStarted) let pendingReady = try #require(directPlayLoader.onSeekRangeReplacementReady) loader.teardownPlayback() @@ -109,6 +111,14 @@ struct FullscreenVideoDismissPlaybackTests { #expect(player.rate == 0) #expect(player.currentItem == nil) + + pendingStarted() + pendingReady() + await Task.yield() + try await Task.sleep(nanoseconds: 20_000_000) + + #expect(player.rate == 0) + #expect(player.currentItem == nil) } // Covers the reported bug: paging from an image onto a video, then