Guarding playback position syncing with playedUpToModified check by bjtitus · Pull Request #4085 · Automattic/pocket-casts-ios

bjtitus · 2026-03-20T03:22:54Z

📘 Part of: Watch + Up Next Sync

Fixes PCIOS-572

Summary

Fixes playback position jumping backward when syncing between iPhone and Apple Watch (possibly other devices)
SyncTask.importEpisode unconditionally overwrites the local playedUpTo with whatever the server returns with no recency check. When multiple sync responses arrive with different positions for the same episode, a stale response can overwrite a newer one, causing the player to seek backward.
The protobuf already includes a playedUpToModified timestamp field, but importEpisode never checks it.

Log evidence

User reported skip-back on episode 27fe8dee. Phone logs show four sync-driven seeks in 5 seconds including a backward jump:

07:54:26 seek to 105.035675763 startPlaybackAfterSeek false
07:54:28 seek to 146.500060847 startPlaybackAfterSeek false
07:54:30 seek to 132.255547613 startPlaybackAfterSeek false  <- backward
07:54:31 seek to 173.199683495 startPlaybackAfterSeek false

All startPlaybackAfterSeek false — the signature of seekToFromSync, the code path this PR guards.

Changes

All behind the syncPlayedUpToTimestampCheck feature flag:

New guarded DB write — EpisodeDataManager.saveIfNotModified(playedUpTo:remoteModified:) atomically sets both playedUpTo and playedUpToModified in one SQL UPDATE with WHERE playedUpToModified < ?. A stale remote write is rejected if a newer one was already accepted.
importEpisode uses the guard — SyncTask+ServerChanges.swift now checks the remote playedUpToModified timestamp before writing. The seek is gated on the write succeeding.
markAllSynced preserves the timestamp — Previously, markAllSynced zeroed playedUpToModified before imports, destroying the cross-cycle guard. With the flag on, it skips zeroing playedUpToModified. Trade-off: the episode stays in unsyncedEpisodes and its position is re-sent on subsequent syncs (idempotent, harmless).

Things to look for in logs

Once the flag is enabled in a release, look for these in user logs to verify the fix:

importEpisode: rejected stale playedUpTo X (remoteModified Y not newer than local) - the guard blocked a stale write. If this appears alongside a user report that skip-back stopped, it confirms the root cause and that the fix is working.
seek to X startPlaybackAfterSeek false for episode [name] - sync triggered a seek, and the episode name lets us match phone and watch activity for the same episode across devices.
remoteModified in a rejection log is always 0 - the server is not populating playedUpToModified, so the client is blocking all remote position updates rather than only stale ones. That points to a server-side issue.
No rejected stale playedUpTo logs, but multiple seek to X startPlaybackAfterSeek false for episode [same name] lines still appear in quick succession - the backward seeks are likely coming from somewhere other than importEpisode (for example RetrieveCustomFilesTask or changePlaybackPositionCommand).
Matching watch and phone seek to X startPlaybackAfterSeek false for episode [name] lines at similar timestamps - both devices are seeking the same episode, which was previously hard to verify because watch logs did not include episode names.

Test plan

Playback

Listen on iPhone, pause
Listen on watch to advance position
Sync
✅ Phone should not jump backward

Seek

Seek on iPhone and Watch
Ensure values are updated properly on each

Checklist

I have considered if this change warrants user-facing release notes and have added them to CHANGELOG.md if necessary.
I have considered adding unit tests for my changes.
I have updated (or requested that someone edit) the spreadsheet to reflect any new or changed analytics.

…sition overwrites SyncTask.importEpisode unconditionally overwrites the local playedUpTo with whatever the server returns, without checking recency. This can cause playback position to jump backward when multiple sync responses arrive with different positions for the same episode. The fix, behind the syncPlayedUpToTimestampCheck feature flag: - EpisodeDataManager: Add saveIfNotModified(playedUpTo:remoteModified:) that atomically sets both playedUpTo AND playedUpToModified in a single SQL UPDATE with a WHERE playedUpToModified < ? guard. This ensures a stale remote write is rejected if a newer one was already accepted. - SyncTask+ServerChanges: When the flag is enabled, use the new guarded write instead of the unconditional saveEpisode. The seek is gated on the write succeeding, so both the DB and player are protected together. - EpisodeDataManager.markAllSynced: When the flag is enabled, preserve playedUpToModified instead of zeroing it. This allows the timestamp guard to work across sync cycles. The trade-off is that the episode remains in unsyncedEpisodes queries and its position is re-sent on subsequent syncs (idempotent and harmless). - FeatureFlag: Add syncPlayedUpToTimestampCheck, defaulting to false for controlled rollout via remote config. - Tests: Add SyncTaskTests+PositionRace covering both flag-off (documenting the current unguarded behavior) and flag-on (verifying stale writes are rejected within and across sync cycles).

This can help debug seek issues between devices. Right now we can't tell _which_ episode was seeking.

Copilot

Pull request overview

This PR addresses playback position “skip-back” during sync (notably between iPhone and Apple Watch) by preventing stale server playedUpTo values from overwriting newer local progress, using the server-provided playedUpToModified timestamp as a recency guard (behind a feature flag).

Changes:

Add syncPlayedUpToTimestampCheck feature flag (default off) and gate the new behavior behind it.
Update SyncTask.importEpisode to conditionally write playedUpTo only when the remote playedUpToModified is newer, and only seek when the guarded write succeeds.
Add DB support for an atomic guarded update (playedUpTo + playedUpToModified) and add a regression test suite documenting the race and verifying the guarded behavior.

Reviewed changes

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

Show a summary per file

File	Description
podcasts/PlaybackManager.swift	Adds episode title context to sync-seek logging to aid diagnosis.
Modules/Utils/Sources/PocketCastsUtils/Feature Flags/FeatureFlag.swift	Introduces the `syncPlayedUpToTimestampCheck` flag (default off).
Modules/Server/Tests/PocketCastsServerTests/SyncTaskTests+PositionRace.swift	Adds tests covering the stale-position overwrite race and the flagged guarded behavior.
Modules/Server/Sources/PocketCastsServer/Public/Sync/SyncTask+ServerChanges.swift	Uses the guarded `playedUpTo` write (and gates seek) when the flag is enabled.
Modules/DataModel/Sources/PocketCastsDataModel/Public/DataManager.swift	Exposes a public `saveIfNotModified(playedUpTo:remoteModified:episodeUuid:)` entry point.
Modules/DataModel/Sources/PocketCastsDataModel/Private/Managers/EpisodeDataManager.swift	Implements the atomic guarded SQL UPDATE and changes `markAllSynced` behavior under the flag.

Copilot · 2026-03-20T03:28:11Z

+    /// SQL fragment for columns to zero when marking episodes as synced.
+    /// When syncPlayedUpToTimestampCheck is enabled, playedUpToModified is
+    /// preserved so that the timestamp guard in importEpisode can reject stale
+    /// remote positions across sync cycles. The trade-off is that the episode
+    /// remains in unsyncedEpisodes queries and its playedUpTo will be re-sent
+    /// in subsequent sync requests (idempotent and harmless).
+    private var markSyncedSetClause: String {
+        if FeatureFlag.syncPlayedUpToTimestampCheck.enabled {
+            return "playingStatusModified = 0, durationModified = 0, keepEpisodeModified = 0, archivedModified = 0"
+        } else {
+            return "playingStatusModified = 0, playedUpToModified = 0, durationModified = 0, keepEpisodeModified = 0, archivedModified = 0"
+        }


With syncPlayedUpToTimestampCheck enabled, markAllSynced intentionally preserves playedUpToModified, but unsyncedEpisodes(...) treats playedUpToModified > 0 as “needs upload” and SyncTask.changedEpisodes will include playedUpTo/playedUpToModified in outgoing sync payloads. This means any episode that received a remote position update will be re-sent on every subsequent sync until something else clears playedUpToModified, increasing sync traffic and risking ping‑pong between devices. Consider storing the remote recency marker separately from the “local unsynced” modified column (or otherwise preventing preserved remote timestamps from being treated as outbound changes).

@SergioEstevao I believe this comment is accurate and is something we need to fix before merging this PR.

EpisodeDataManager.markSyncedSetClause (lines 880‑886) skips playedUpToModified = 0 when
syncPlayedUpToTimestampCheck is on, by design (to keep the guard in saveIfNotModified working
across syncs).

EpisodeDataManager.unsyncedEpisodes (line 207) uses playedUpToModified > 0 as the "dirty"
predicate.

SyncTask+LocalChanges.changedEpisodes (lines 63‑66) gates playedUpTo/playedUpToModified on that same column.

So any episode that ever received a remote position update stays in the unsynced set forever and gets re-uploaded on every sync. It's not a true ping-pong (the server's own timestamp guard should reject the echo since playedUpToModified matches), but outbound payloads grow unboundedly over time.

@leandroalonso do you think we could change playedUpToModified to be an enum so different values will translate differently in terms of sync process?

Copilot

Pull request overview

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

Copilot · 2026-03-20T04:21:18Z

+    /// when the local playedUpToModified is 0 (which it always is during sync,
+    /// because processServerData calls markAllSynced first).


The doc comment for testFlagEnabled_remotePositionAcceptedWhenLocalTimestampIsZero says playedUpToModified is “always … 0 during sync because processServerData calls markAllSynced first”, but this PR explicitly changes markAllSynced to preserve playedUpToModified when syncPlayedUpToTimestampCheck is enabled. Updating this comment to reflect the new behavior (e.g., “when the local timestamp is 0”) will keep the test documentation accurate.

Suggested change

/// when the local playedUpToModified is 0 (which it always is during sync,

/// because processServerData calls markAllSynced first).

/// when the local playedUpToModified is 0 (i.e. when the local timestamp is 0).

///

pocketcasts · 2026-03-30T10:50:33Z

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

# Conflicts: # Modules/Sources/PocketCastsUtils/Feature Flags/FeatureFlag.swift # Modules/Tests/PocketCastsServerTests/SyncTaskTests+PositionRace.swift

pocketcasts · 2026-04-14T08:12:59Z

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

pocketcasts · 2026-04-27T13:43:21Z

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

pocketcasts · 2026-05-11T14:11:09Z

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

pocketcasts · 2026-05-25T16:19:06Z

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

pocketcasts · 2026-06-08T18:04:52Z

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

bjtitus added 3 commits March 19, 2026 18:31

Add unit tests to reproduce this issue

b100e15

Add episode name logging during sync

1074737

This can help debug seek issues between devices. Right now we can't tell _which_ episode was seeking.

Copilot AI review requested due to automatic review settings March 20, 2026 03:22

bjtitus added the [Project] Watch + Up Next Sync Improvements for Watch + Up Next Sync issues label Mar 20, 2026

bjtitus added this to the 8.9 milestone Mar 20, 2026

Copilot started reviewing on behalf of bjtitus March 20, 2026 03:23 View session

Copilot AI reviewed Mar 20, 2026

View reviewed changes

bjtitus added 2 commits March 19, 2026 21:28

Enable feature flag

4f52fe4

Clean up comments

0de8b12

bjtitus marked this pull request as ready for review March 20, 2026 04:16

bjtitus requested a review from a team as a code owner March 20, 2026 04:16

bjtitus requested review from SergioEstevao and Copilot and removed request for a team March 20, 2026 04:16

Copilot started reviewing on behalf of bjtitus March 20, 2026 04:17 View session

Copilot AI reviewed Mar 20, 2026

View reviewed changes

pocketcasts modified the milestones: 8.9, 8.10 Mar 30, 2026

Merge branch 'trunk' into bjtitus/playback-position-syncing

4edb87e

# Conflicts: # Modules/Sources/PocketCastsUtils/Feature Flags/FeatureFlag.swift # Modules/Tests/PocketCastsServerTests/SyncTaskTests+PositionRace.swift

pocketcasts modified the milestones: 8.10, 8.11 Apr 14, 2026

pocketcasts modified the milestones: 8.11, 8.12 Apr 27, 2026

pocketcasts modified the milestones: 8.12, 8.13 May 11, 2026

pocketcasts modified the milestones: 8.13, 8.14 May 25, 2026

pocketcasts modified the milestones: 8.14, 8.15 Jun 8, 2026

kean modified the milestones: 8.15, Future Jun 8, 2026

		/// when the local playedUpToModified is 0 (which it always is during sync,
		/// because processServerData calls markAllSynced first).

Uh oh!

Conversation

bjtitus commented Mar 20, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Log evidence

Changes

Things to look for in logs

Test plan

Playback

Seek

Checklist

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Copilot AI Mar 20, 2026

Choose a reason for hiding this comment

Uh oh!

leandroalonso Apr 20, 2026

Choose a reason for hiding this comment

Uh oh!

SergioEstevao Apr 21, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Copilot AI Mar 20, 2026

Choose a reason for hiding this comment

Uh oh!

pocketcasts commented Mar 30, 2026

Uh oh!

pocketcasts commented Apr 14, 2026

Uh oh!

pocketcasts commented Apr 27, 2026

Uh oh!

pocketcasts commented May 11, 2026

Uh oh!

pocketcasts commented May 25, 2026

Uh oh!

pocketcasts commented Jun 8, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

bjtitus commented Mar 20, 2026 •

edited

Loading