Skip to content

bugfix-offline-playback - Sync offline progress once a server session is ready, scoped to that server - #1621

Merged
RadicalMuffinMan merged 3 commits into
Moonfin-Client:mainfrom
codyjohnsontx:fix/1603-offline-progress-sync
Sep 27, 2026
Merged

RadicalMuffinMan merged 3 commits into
Moonfin-Client:mainfrom
codyjohnsontx:fix/1603-offline-progress-sync

Conversation

@codyjohnsontx

@codyjohnsontx codyjohnsontx commented Sep 23, 2026 •

Copy link
Copy Markdown
Contributor

Summary

Watch progress recorded offline was lost when the app was reopened with the network already up (#1603). The sync that pushes offline progress only ran when the server went from unreachable to reachable. On a cold start that one boot-time attempt ran before any server client existed, found nothing to sync with, and was never retried. The app then showed the server's older position, and playing from it overwrote the further one.

This PR runs the existing sync (pending ratings, then playback progress, then the metadata refresh) as soon as a server client is signed in, scopes it to that server, and keeps it to one run at a time. The furthest-progress-wins rules from #560 are unchanged; they now actually run on this path.

Related Issues

Type of Change

  • Bug fix

What was wrong

ConnectivityService.initialize() runs before any server client exists. _checkInitialState probes, finds no client, keeps serverReachable at its default true, and calls _triggerSync, which returns silently because no MediaServerClient is registered yet. After that, every other trigger needs a reachable edge that never comes on a live network: the reconnect handler only syncs when !wasReachable, and onAppResumed only replays a sync parked while backgrounded. The startup screen's recheckNow() probes but never syncs.

A debug-print build of current main, cold-started on an iPhone simulator with offline progress on disk, showed it every time:

initial check start
checkConnectivity done [ConnectivityResult.wifi] client=false
initial probe done reachable=true
triggerSync lifecycle=AppLifecycleState.resumed client=false sync=true
client registered            <- about 1.4 s later, nothing retries

It works when the app stays open while the connection comes back, because that path has the unreachable-to-reachable edge. That is the path #623 was tested on.

Two more things turned up while fixing it:

  • syncPlaybackProgress and refreshMetadata read every download row whatever its server. Jellyfin 12.1 answers 204 to a stop report for an item it does not have, so a sync while signed into server B pushed server A's offline progress to B and marked it synced, and A never received it. A trigger at sign-in would hit this on every server switch, so the scoping has to come with it.
  • Only the progress step guarded against running twice. The sign-in, the boot-time check and a network change can all ask within a second, so two runs repeated the ratings push and the whole metadata refresh.

Changes Made

Three commits, each builds and passes its tests on its own:

  1. Scoped the offline progress sync and metadata refresh to the active server. Both take the active serverId, the way syncPendingRatings already does, and only touch that server's downloads. Rows store either the app's server id or the server's base URL (some download paths write _client.baseUrl), so a URL-shaped id is matched the way MediaServerClientFactory.getClientIfExists resolves one. With no active server nothing is pushed.
  2. Synced offline progress once a server client is signed in, fixes bug-offline-playback - Offline play seems overwritten by the server #1603. New ConnectivityService.onServerClientReady(serverId), called at the end of setActiveServerClient with serverIdOf(rawClient). It probes the server, then runs the existing sync chain. Every sign-in path goes through that function: session restore, the login screen, Quick Connect, account and server switches, and deep links. The login screen, Quick Connect, the server screen and deep links never reach the startup screen's recheckNow(). The server id comes from the caller, not from SessionRepository.activeServerId read after an await. The call is skipped for the background auto-download worker (background: true, and ConnectivityService isn't registered there anyway). The existing foreground deferral still applies, so a headless Android engine parks the sync until the app is opened. On Apple TV the hook calls the probe directly rather than recheckNow(), so it doesn't add a connectivity_plus call there.
  3. Kept the offline progress sync to one run at a time. An _inFlightSync guard modelled on the existing _inFlightProbe. A request made while a run is going waits for it; a client that signed in since that run started (a quick server or account switch) gets its own run once it finishes. The early return when no client exists yet now writes a ServerLog.network line instead of dropping the sync silently, and a failed run is logged.

No new strings, nothing under lib/l10n/, and no new dependencies.

Platform

  • All / Shared code

The change is in shared Dart code, but it was tested on iOS only (the iPhone simulator, below). Android is out of scope for this PR. Android's different startup path, where a headless engine parks the sync until the app is opened, is covered by the unit test a headless Android engine parks the sync until the app is opened.

Testing

  • Tested on emulator / simulator (iPhone 17 simulator, iOS 26.5, against Jellyfin 12.1.0)
  • Tested on physical device
  • Manual testing completed

Unit tests. New test/offline/offline_progress_sync_test.dart, 14 tests, the first on this path. They use the real SyncService, OfflineRepository on an in-memory database and ConnectivityService, with a fake server that answers the way Jellyfin does:

  • the furthest-progress-wins rules from feat: implement offline and online watch progress synchronization (#376) #560: local ahead is pushed; server ahead is adopted; played on the server wins; finished offline marks played; newer server progress comes down when nothing was watched offline; a failed push keeps the local position;
  • the cold start: offline progress reaches the server, with one probe shared with the startup screen's recheck;
  • the network coming back while the app is open;
  • the cross-server cases: server A's progress waits while B is active, including a row stored under the server address; with no active server nothing is pushed;
  • two sign-ins at once run one chain; a server switch mid-sync syncs each server with its own rows;
  • a headless Android engine parks the sync until the app is opened.

With each part of the fix removed on purpose, the matching tests fail. The full app suite passes except clearImageDiskCache live-clears ... in test/util/game_artwork_scope_sweep_test.dart, which fails the same way on main (sqflite isn't set up in that test).

Static checks. Full flutter analyze on the root and every package CI analyzes: main and this branch both have 352 infos, 0 warnings, 0 errors, and the lists are identical, so there are no new issues of any severity.

Real app, recorded. The app was driven by XCUITest on the simulator. At every step the positions were read straight from the server's REST API and from the app's own offline.db, not from the screen. main means 2ac20ce19 built the same way.

Test Steps

The reporter's flow (Test 1):

  1. Sign in, download a 10-minute movie, play it online for about a minute, leave the player.
  2. Stop the server. Play the download further, leave the player.
  3. Close the app completely. Start the server.
  4. Open the app.
After step main: server / local this PR: server / local
2 unreachable / 3:23 unsynced unreachable / 4:12 unsynced
3 1:00 / 3:23 unsynced 1:37 / 4:12 unsynced
4, +10 s 1:00 / 3:23 unsynced 4:12 / 4:12 synced (2 s after launch)
4, +60 s 1:00 / 3:23 unsynced 4:12 / 4:12 synced
Detail button "Resume from 1m" "Resume from 4m"
Press Resume, play about 15 s 1:08 / 1:08, offline progress gone 4:22 / 4:22

Repeated cold starts with offline progress on disk: this PR pushed it 6 of 6 times; main 0 of 6.

Two servers (Test 2, this PR): offline progress on server A stayed unsynced while signed into B (B untouched), reached A 2 s after switching back, and a cold start on B pushed B's offline progress without touching A. Server B's own log never mentions A's item.

Before/after recordings of the reporter's flow are in the issue: #1603 (comment)

Regression checks, same steps on both builds:

Check main this PR
App kept open while the server comes back synced after 29 s synced after 27 s
Finish a movie offline, cold start with the server back server stays unplayed server marked played
Sign out, sign back in with a password not pushed pushed within 1 s
Sign out, sign back in with Quick Connect not pushed pushed within 6 s
Sign in with no downloads: every request to the server 62 63; the only extra is GET /System/Ping
Normal online watch: server position, played state, resume button 0:45, then played identical

One visible difference, which is the metadata refresh from #560 now running at sign-in: after an item's played state changes on the server, the local download row takes the server's state on the next sign-in or restart. On main it kept the stale local value (for example still 0:45 and unplayed after marking it watched online), which is what offline mode would show later.

Screenshots (if applicable)

The item detail screen after the cold start in Test 1. Recordings of the flow are linked in the issue.

Before (main) After (this PR)
main-coldstart-detail-resume-from-1m fix-coldstart-detail-resume-from-4m

Not in this PR (questions for the maintainers)

  • Play before the sync lands. In the second or two before the sync finishes, the detail screen still shows the server's older position, and pressing Resume in that window would start from it. Worth waiting for the pending sync of that item, or starting from the further of the two?
  • Servers that accept a stop report but keep their position. After a push, refreshMetadata trusts what the server returns, so a server that answers 2xx without moving its position would bring the older value back down. Jellyfin 12.1 did move it in testing; is this worth hardening for Emby or Jellyfin's resume thresholds?
  • Download rows have no user id. Two users on the same server share the downloads, so one user's offline progress can be pushed under the other's token. Fixing that needs a schema change.
  • Downloads are keyed by item id alone. upsertItem replaces any row with the same item id, and the position helpers update by item id, so the same item id on two servers (two servers sharing one library path, for example) can only be downloaded from one of them at a time. This PR keeps that model; making the repository server-aware would be a separate change.
  • Batching refreshMetadata. It fetches each completed download one by one. The same batched getItems(ids: ...) the progress step uses would make it one request. Now that it runs at every sign-in, is that worth doing?
  • [UI] All Platforms: Emby connectivity check incorrectly reports server as unreachable after restart #1434 (Emby reported unreachable after a Windows restart) is a separate problem with the probe's verdict. This PR adds no new probe on that path, since the startup screen already probes at the same moment.

Checklist

  • Code builds successfully
  • Code follows project style and conventions
  • No unnecessary commented-out code
  • No new warnings introduced

…erver

syncPlaybackProgress and refreshMetadata read every download row whatever
its server. Jellyfin answers a stop report for an item it does not have
with success, so syncing while signed into another server marked the first
server's offline progress synced without that server ever getting it.

Both now take the active server id, the way syncPendingRatings already
does, and only touch that server's downloads. A row stored under the
server's address instead of its id still matches. With no active server
nothing is pushed.

Adds the first tests for the furthest-progress-wins rules from Moonfin-Client#560 and
for the cross-server case.
…fin-Client#1603

The offline progress sync only ran when the server went from unreachable
to reachable. On a cold start with the network already up, the boot-time
attempt ran before any server client existed and was dropped, and nothing
retried it. The app then showed the server's older position, and playing
from it overwrote the further one watched offline.

setActiveServerClient now tells ConnectivityService that a client is
ready, with its server id, and the service probes the server and runs the
ratings, progress and metadata sync. Every sign-in path goes through it:
session restore, the login screen, Quick Connect, account and server
switches, and deep links. The background auto-download worker is skipped,
and a backgrounded engine still waits for the app to come to the front.
The sign-in hook, the boot-time check and a network change can all ask
for the sync within a second of each other, and only the progress step
guarded itself, so two runs repeated the ratings push and the whole
metadata refresh. A request made while a run is going now waits for it,
and a client that signed in since that run started gets its own run once
it finishes, so a quick server switch still syncs the new server.

The early return when no server client exists yet now leaves a network
log line instead of dropping the sync silently, and a failed run is
logged.
@github-actions github-actions Bot added the Missing Template Issue opened without one of the issue forms label Sep 23, 2026
@codyjohnsontx
codyjohnsontx marked this pull request as draft September 23, 2026 17:33
@codyjohnsontx codyjohnsontx changed the title fix(data): sync offline watch progress after sign-in on a cold start bugfix-offline-playback - Sync offline progress once a server session is ready, scoped to that server Sep 23, 2026
@github-actions github-actions Bot added All Bug Something isn't working and removed Missing Template Issue opened without one of the issue forms labels Sep 23, 2026
@codyjohnsontx
codyjohnsontx marked this pull request as ready for review September 24, 2026 04:46
@RadicalMuffinMan
RadicalMuffinMan merged commit 16de6a7 into Moonfin-Client:main Sep 27, 2026
8 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

All Bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

bug-offline-playback - Offline play seems overwritten by the server

2 participants