From 688bfdacb4a3113aa78042f18fda8beb39e14749 Mon Sep 17 00:00:00 2001 From: edde746 <86283021+edde746@users.noreply.github.com> Date: Fri, 14 Aug 2026 12:19:58 +0200 Subject: [PATCH] fix(profiles): keep offline downloads visible when a profile's servers are unreachable MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Switching to a profile whose only server is offline verified the PIN, failed the bind with zero reachable servers, and rolled back to the previous profile — whose scope owns none of the downloads. The Downloads UI then showed nothing while the files and pinned metadata sat intact on disk. Startup offline mode only covered the cold-start bind, so such a profile could never be entered while its server was away. The binder now classifies a settled bind failure as connectivity-only when the profile expected servers, reached none, and none were auth-rejected (snapshotting auth markers before the visibility sweep, which clears them via removeServer). On such a failure, switchProfileFromUi keeps the profile active when it owns downloads instead of rolling back; OfflineModeProvider drives the offline UI from the empty visible-server set. Auth failures, PIN cancels, and downloads-free profiles keep the existing rollback and error snackbar. close #1927 --- lib/profiles/active_profile_binder.dart | 24 +++ lib/profiles/profile_activation.dart | 21 +++ test/profiles/active_profile_binder_test.dart | 145 ++++++++++++++++++ test/profiles/profile_activation_test.dart | 62 ++++++++ 4 files changed, 252 insertions(+) diff --git a/lib/profiles/active_profile_binder.dart b/lib/profiles/active_profile_binder.dart index e80640b08..e492240a1 100644 --- a/lib/profiles/active_profile_binder.dart +++ b/lib/profiles/active_profile_binder.dart @@ -91,6 +91,14 @@ class ActiveProfileBinder { /// PIN dialog with no user action. Explicit paths ([rebindActive], a /// user-initiated activation, a pre-verified switch) clear the marker. String? _lastFailedProfileId; + + /// True when the most recently completed rebind pass failed purely for + /// connectivity: the profile expected at least one server, reached none, + /// and no expected server was auth-rejected. Superseded/cancelled passes + /// and thrown binds leave this false. Read by `switchProfileFromUi` to + /// keep a downloads-owning profile active in offline mode instead of + /// rolling back to a profile whose scope does not own those downloads. + bool _lastBindFailureConnectivityOnly = false; bool _pendingRebind = false; // Set when something asks for a rebind of the *currently-active* profile // while a rebind is already in flight. The normal `_pendingRebind` path @@ -116,6 +124,8 @@ class ActiveProfileBinder { @visibleForTesting String? get debugLastBoundProfileId => _lastBoundProfileId; + bool get lastBindFailureConnectivityOnly => _lastBindFailureConnectivityOnly; + void markPlexHomePreVerified(String profileId) { _plexHomePreVerified.add(profileId); if (_lastFailedProfileId == profileId) _lastFailedProfileId = null; @@ -255,6 +265,7 @@ class ActiveProfileBinder { final generation = ++_bindGeneration; final stopwatch = Stopwatch()..start(); var success = false; + _lastBindFailureConnectivityOnly = false; String? attemptedProfileId; try { final profile = activeProfile.active; @@ -334,6 +345,12 @@ class ActiveProfileBinder { expectedServerIds.addAll(result.expectedServerIds); } + // Snapshot before the visibility sweep below: removeServer() clears a + // swept server's auth-error marker, and an auth-rejected server is by + // definition not visible — reading authErrorServerIds after the sweep + // would misclassify a revoked token as a connectivity failure. + final authErrorServerIds = serverManager.authErrorServerIds; + // Remove servers the profile no longer has access to. Always set the // filter to the bound set (even when empty) so a profile with no // connections shows nothing — falling back to "all visible" on empty @@ -346,6 +363,13 @@ class ActiveProfileBinder { multiServerProvider.setExpectedVisibleServerIds(expectedServerIds); multiServerProvider.setVisibleServerIds(visibleServerIds); success = (profile.isLocal && !localProfileHasJoinRows) || visibleServerIds.isNotEmpty; + if (!success) { + // A failed pass that expected servers, reached none (`!success` + // implies `visibleServerIds.isEmpty` here), and saw no auth + // rejection is offline, not misconfigured or revoked. + _lastBindFailureConnectivityOnly = + expectedServerIds.isNotEmpty && expectedServerIds.every((id) => !authErrorServerIds.contains(id)); + } // Once we've bound a profile with real servers in this session, // we've crossed the cold-start boundary — every subsequent rebind // is a user-initiated switch and must re-prompt for PIN where diff --git a/lib/profiles/profile_activation.dart b/lib/profiles/profile_activation.dart index c444c9bba..8de4247c6 100644 --- a/lib/profiles/profile_activation.dart +++ b/lib/profiles/profile_activation.dart @@ -3,6 +3,8 @@ import 'package:provider/provider.dart'; import '../connection/connection.dart'; import '../connection/connection_registry.dart'; +import '../database/app_database.dart'; +import '../database/download_operations.dart'; import '../i18n/strings.g.dart'; import '../screens/profile/pin_entry_dialog.dart'; import '../utils/snackbar_helper.dart'; @@ -171,6 +173,25 @@ Future switchProfileFromUi(BuildContext context, Profile profile) async { if (!isCurrentActivation(profile.id)) return false; if (bound) return true; + if (binder.lastBindFailureConnectivityOnly && context.mounted) { + // The bind failed purely for connectivity — identity was already verified + // before activation (local PIN hash or the Plex /switch round-trip). When + // the profile owns downloads, keep it active instead of rolling back: the + // rollback profile's scope does not own them, so they would vanish from + // the Downloads UI while the files sit on disk. OfflineModeProvider drives + // the offline UI from the empty visible-server set. Auth-classified bind + // failures never set the flag and keep today's rollback + snackbar. + final ownedDownloadKeys = await context.read().getDownloadOwnerKeysForProfile(profile.id); + if (!isCurrentActivation(profile.id)) return false; + if (ownedDownloadKeys.isNotEmpty) { + appLogger.i( + 'Profile switch: keeping ${profile.displayName} active with no reachable servers ' + '(${ownedDownloadKeys.length} offline downloads owned)', + ); + return true; + } + } + if (previousProfile != null && previousProfile.id != profile.id && isCurrentActivation(profile.id)) { final rollbackRequestGeneration = activeProvider.beginIdentityMutationRequest(); try { diff --git a/test/profiles/active_profile_binder_test.dart b/test/profiles/active_profile_binder_test.dart index b37044a93..815137ac9 100644 --- a/test/profiles/active_profile_binder_test.dart +++ b/test/profiles/active_profile_binder_test.dart @@ -242,6 +242,111 @@ void main() { expect(multiServerProvider.expectedServerIds, ['srv-1']); }); + group('connectivity-only bind failure classification', () { + /// Rebuild the binder around [newManager] and seed one local profile with + /// a Plex join row whose account has a single cached server (`srv-1`). + Future setUpPlexProfileWithManager(MultiServerManager newManager) async { + binder.dispose(); + multiServerProvider.dispose(); + manager = newManager; + multiServerProvider = testMultiServerProvider(manager); + binder = ActiveProfileBinder( + activeProfile: activeProfile, + connections: connections, + profileConnections: profileConnections, + serverManager: manager, + multiServerProvider: multiServerProvider, + pinPrompt: (_, {String? errorMessage}) async => null, + shouldDeferInitialBind: (_) async => false, + plexAuth: PlexAuthService.forTesting( + http: MediaServerHttpClient( + client: MockClient( + (_) async => + http.Response(jsonEncode([_serverJson()]), 200, headers: {'content-type': 'application/json'}), + ), + ), + ), + ); + + final profile = await createActiveLocalProfile('local-classification'); + final account = PlexAccountConnection( + id: 'plex.account', + accountToken: 'account-token', + clientIdentifier: 'client-id', + accountLabel: 'Owner', + servers: [_server(accessToken: 'account-server-token')], + createdAt: DateTime(2026, 1, 1), + ); + await connections.upsert(account); + await profileConnections.upsert( + ProfileConnection( + profileId: profile.id, + connectionId: account.id, + userToken: 'home-user-token', + userIdentifier: 'home-user-uuid', + tokenAcquiredAt: DateTime(2026, 1, 1), + ), + ); + } + + test('unreachable servers with no auth rejection classify as connectivity-only', () async { + await setUpPlexProfileWithManager(_FailingPlexMultiServerManager()); + + await binder.rebindActive(); + + expect(activeProfile.lastBindingSucceeded, isFalse); + expect(binder.lastBindFailureConnectivityOnly, isTrue); + }); + + test('auth-rejected server disqualifies connectivity-only classification', () async { + await setUpPlexProfileWithManager(_AuthRejectingPlexManager()); + + await binder.rebindActive(); + + expect(activeProfile.lastBindingSucceeded, isFalse); + expect(binder.lastBindFailureConnectivityOnly, isFalse); + }); + + test('auth marker cleared by the visibility sweep still disqualifies', () async { + await setUpPlexProfileWithManager(_SweptAuthRejectingPlexManager()); + + await binder.rebindActive(); + + expect(activeProfile.lastBindingSucceeded, isFalse); + // The sweep removed the auth-errored server (and its marker) from the + // manager before success was computed; classification must have read + // the pre-sweep snapshot to see the rejection. + expect(manager.authErrorServerIds, isEmpty); + expect(binder.lastBindFailureConnectivityOnly, isFalse); + }); + + test('plex home profile missing parent metadata is not connectivity-only', () async { + final profile = Profile.plexHome(id: 'plex-home-broken', displayName: 'Broken', createdAt: DateTime(2026, 1, 1)); + await profiles.upsert(profile); + await activeProfile.initialize(); + await activeProfile.activate(profile); + + await binder.rebindActive(); + + expect(activeProfile.lastBindingSucceeded, isFalse); + expect(binder.lastBindFailureConnectivityOnly, isFalse); + }); + + test('a later successful cycle resets the classification', () async { + await setUpPlexProfileWithManager(_FailingPlexMultiServerManager()); + await binder.rebindActive(); + expect(binder.lastBindFailureConnectivityOnly, isTrue); + + final fallback = Profile.local(id: 'local-fallback', displayName: 'Fallback', createdAt: DateTime(2026, 1, 2)); + await profiles.upsert(fallback); + await activeProfile.activate(fallback); + await binder.rebindActive(); + + expect(activeProfile.lastBindingSucceeded, isTrue); + expect(binder.lastBindFailureConnectivityOnly, isFalse); + }); + }); + test('binds Plex and Jellyfin join rows in parallel', () async { binder.dispose(); multiServerProvider.dispose(); @@ -1114,6 +1219,46 @@ class _CapturingMultiServerManager extends MultiServerManager { } } +/// Simulates per-server connect 401s in `refreshTokensForProfile`: the server +/// is marked auth-rejected and nothing binds. +class _AuthRejectingPlexManager extends MultiServerManager { + @override + Future> refreshTokensForProfile( + PlexAccountConnection connection, { + required String profileId, + Duration timeout = MediaServerTimeouts.perServerConnect, + }) async { + for (final server in connection.servers) { + debugMarkAuthErrorForTesting(ServerId(server.clientIdentifier)); + } + return const {}; + } +} + +/// Like [_AuthRejectingPlexManager], but also surfaces the rejected server to +/// the binder's visibility sweep via [serverIds] — mirroring a pre-registered +/// client whose token was rejected mid-refresh (it stays in `serverIds` until +/// swept, and `removeServer` then clears its auth marker). +class _SweptAuthRejectingPlexManager extends MultiServerManager { + final Set _rejected = {}; + + @override + List get serverIds => {...super.serverIds, ..._rejected}.toList(); + + @override + Future> refreshTokensForProfile( + PlexAccountConnection connection, { + required String profileId, + Duration timeout = MediaServerTimeouts.perServerConnect, + }) async { + for (final server in connection.servers) { + _rejected.add(server.clientIdentifier); + debugMarkAuthErrorForTesting(ServerId(server.clientIdentifier)); + } + return const {}; + } +} + class _FailingPlexMultiServerManager extends MultiServerManager { int refreshCalls = 0; diff --git a/test/profiles/profile_activation_test.dart b/test/profiles/profile_activation_test.dart index 73656cd74..73ffa32c3 100644 --- a/test/profiles/profile_activation_test.dart +++ b/test/profiles/profile_activation_test.dart @@ -6,6 +6,7 @@ import 'package:flutter/services.dart'; import 'package:flutter_test/flutter_test.dart'; import 'package:plezy/connection/connection_registry.dart'; import 'package:plezy/database/app_database.dart'; +import 'package:plezy/database/download_operations.dart'; import 'package:plezy/profiles/active_profile_binder.dart'; import 'package:plezy/profiles/active_profile_provider.dart'; import 'package:plezy/profiles/plex_home_service.dart'; @@ -37,6 +38,9 @@ class _Binder implements ActiveProfileBinder { _Binder(this.events); final List events; + @override + bool lastBindFailureConnectivityOnly = false; + @override void markUserInitiatedActivation(String profileId) => events.add('mark:$profileId'); @@ -56,6 +60,8 @@ class _RollbackBinder implements ActiveProfileBinder { final Completer targetBindingStarted = Completer(); final Completer? targetBindingRelease; String? boundClientProfileId; + @override + bool lastBindFailureConnectivityOnly = false; bool _targetFailed = false; final Set _successfullyBoundProfileIds = {}; @@ -250,6 +256,61 @@ void main() { expect(SystemShelfService().debugActiveOwner, 'owner'); }); + testWidgets('connectivity-only bind failure keeps a downloads-owning profile active offline', (tester) async { + final harness = await _pumpHarness(tester, channel, simulateBindingRollback: true); + addTearDown(harness.dispose); + final binder = harness.rollbackBinder!; + binder.lastBindFailureConnectivityOnly = true; + await harness.database.addDownloadOwner(profileId: 'target', globalKey: 'srv-1/episode-1'); + + final activated = await switchProfileFromUi(harness.context, harness.target); + + expect(activated, isTrue); + expect(harness.active.activeId, 'target'); + expect(SystemShelfService().debugActiveOwner, 'target'); + expect((await StorageService.getInstance()).getActiveProfileId(), 'target'); + expect(binder.rebindStarted.isCompleted, isFalse); + expect(harness.events, isNot(contains('rebind:owner'))); + await tester.pump(); + expect(find.byType(SnackBar), findsNothing); + }); + + testWidgets('connectivity-only bind failure without owned downloads still rolls back', (tester) async { + final harness = await _pumpHarness(tester, channel, simulateBindingRollback: true); + addTearDown(harness.dispose); + final binder = harness.rollbackBinder!; + binder.lastBindFailureConnectivityOnly = true; + + final switchFuture = switchProfileFromUi(harness.context, harness.target); + await binder.rebindStarted.future; + binder.allowRebind.complete(); + + expect(await switchFuture, isFalse); + expect(harness.active.activeId, 'owner'); + expect(binder.boundClientProfileId, 'owner'); + expect(SystemShelfService().debugActiveOwner, 'owner'); + await tester.pump(); + expect(find.byType(SnackBar), findsOneWidget); + }); + + testWidgets('non-connectivity bind failure rolls back even when the profile owns downloads', (tester) async { + final harness = await _pumpHarness(tester, channel, simulateBindingRollback: true); + addTearDown(harness.dispose); + final binder = harness.rollbackBinder!; + await harness.database.addDownloadOwner(profileId: 'target', globalKey: 'srv-1/episode-1'); + + final switchFuture = switchProfileFromUi(harness.context, harness.target); + await binder.rebindStarted.future; + binder.allowRebind.complete(); + + expect(await switchFuture, isFalse); + expect(harness.active.activeId, 'owner'); + expect(binder.boundClientProfileId, 'owner'); + expect(SystemShelfService().debugActiveOwner, 'owner'); + await tester.pump(); + expect(find.byType(SnackBar), findsOneWidget); + }); + testWidgets('protected switch rolls back to profile active when its activation is admitted', (tester) async { final harness = await _pumpHarness(tester, channel, simulateBindingRollback: true, gateTargetBindingFailure: true); addTearDown(harness.dispose); @@ -660,6 +721,7 @@ Future<_Harness> _pumpHarness( providers: [ ChangeNotifierProvider.value(value: active), Provider.value(value: binder), + Provider.value(value: database), ], child: MaterialApp( home: Scaffold(