From d2b414627cfe5a869dd8883ffbf4f46d4e897fa2 Mon Sep 17 00:00:00 2001 From: Tom Brow Date: Mon, 17 Aug 2026 10:59:06 -0700 Subject: [PATCH 01/14] feat(mobile): browse and join open channels Signed-off-by: Tom Brow Co-authored-by: Codex Ai-assisted: true --- mobile/lib/features/channels/channel.dart | 1 + .../lib/features/channels/channels_page.dart | 3 +- .../features/channels/channels_page/body.dart | 4 +- .../channels_page/browse_channels_sheet.dart | 160 ++++++++++++++ .../channels/channels_page/quick_actions.dart | 9 +- .../channels_page/quick_actions_launcher.dart | 9 + .../channels/channels_page/sections.dart | 53 +++-- .../features/channels/channels_provider.dart | 99 ++++++--- .../channels/manage_channel_sheet.dart | 6 +- .../features/channels/channels_page_test.dart | 198 +++++++++++++++++- .../channels/channels_provider_test.dart | 183 +++++++++++++++- 11 files changed, 668 insertions(+), 57 deletions(-) create mode 100644 mobile/lib/features/channels/channels_page/browse_channels_sheet.dart diff --git a/mobile/lib/features/channels/channel.dart b/mobile/lib/features/channels/channel.dart index 30b3f2b48ff..ef1f3f47628 100644 --- a/mobile/lib/features/channels/channel.dart +++ b/mobile/lib/features/channels/channel.dart @@ -82,6 +82,7 @@ class Channel { bool get isForum => channelType == 'forum'; bool get isDm => channelType == 'dm'; bool get isPrivate => visibility == 'private'; + bool get canJoin => visibility == 'open' && !isArchived && !isMember && !isDm; /// Whether [selfRole] may add *another* identity here, mirroring the relay's /// kind:9000 authority (`validate_admin_event` + `add_member`): DMs never, diff --git a/mobile/lib/features/channels/channels_page.dart b/mobile/lib/features/channels/channels_page.dart index c77ef278ec6..4b402139096 100644 --- a/mobile/lib/features/channels/channels_page.dart +++ b/mobile/lib/features/channels/channels_page.dart @@ -53,6 +53,7 @@ import '../../shared/read_state/read_state_time.dart'; import 'unread_badge/observed_unread_event.dart'; part 'channels_page/body.dart'; +part 'channels_page/browse_channels_sheet.dart'; part 'channels_page/sections.dart'; part 'channels_page/channel_tile.dart'; part 'channels_page/sheets.dart'; @@ -62,7 +63,7 @@ part 'channels_page/community.dart'; part 'channels_page/quick_actions.dart'; part 'channels_page/quick_actions_launcher.dart'; -enum _QuickAction { createChannel, newDm } +enum _QuickAction { createChannel, newDm, browseChannels } const double _kChannelSectionInset = Grid.gutter; const double _kChannelLeadingWidth = 22.0; diff --git a/mobile/lib/features/channels/channels_page/body.dart b/mobile/lib/features/channels/channels_page/body.dart index 34faa068eba..68ba6c8e306 100644 --- a/mobile/lib/features/channels/channels_page/body.dart +++ b/mobile/lib/features/channels/channels_page/body.dart @@ -235,7 +235,9 @@ class _SliverChannelsList extends HookConsumerWidget { sliver: SliverList.list( children: [ if (visibleChannels.isEmpty) - const _EmptyState() + _EmptyState( + channels: channels.where((channel) => channel.canJoin).toList(), + ) else ...[ // Starred channels (exclusive — pinned above all sections). if (starredStreamChannels.isNotEmpty) diff --git a/mobile/lib/features/channels/channels_page/browse_channels_sheet.dart b/mobile/lib/features/channels/channels_page/browse_channels_sheet.dart new file mode 100644 index 00000000000..7a6f34b7322 --- /dev/null +++ b/mobile/lib/features/channels/channels_page/browse_channels_sheet.dart @@ -0,0 +1,160 @@ +part of '../channels_page.dart'; + +class _BrowseChannelsSheet extends ConsumerWidget { + const _BrowseChannelsSheet(); + + @override + Widget build(BuildContext context, WidgetRef ref) { + final channelsAsync = ref.watch(channelsProvider); + final channels = channelsAsync.asData?.value + .where((channel) => channel.canJoin) + .toList(); + + return SafeArea( + top: false, + child: Padding( + padding: const EdgeInsets.fromLTRB( + Grid.gutter, + 0, + Grid.gutter, + Grid.xs, + ), + child: ListView( + shrinkWrap: true, + children: [ + Text( + 'Join an open channel to add it to your conversations.', + style: context.textTheme.bodyMedium?.copyWith( + color: context.colors.onSurfaceVariant, + ), + ), + const SizedBox(height: Grid.xs), + if (channelsAsync.isLoading && channels == null) + const Padding( + padding: EdgeInsets.all(Grid.sm), + child: Center(child: BuzzLoadingIndicator()), + ) + else if (channelsAsync.hasError && channels == null) + Padding( + padding: const EdgeInsets.symmetric(vertical: Grid.sm), + child: Text( + 'Could not load open channels.', + textAlign: TextAlign.center, + style: context.textTheme.bodyMedium?.copyWith( + color: context.colors.onSurfaceVariant, + ), + ), + ) + else if (channels == null || channels.isEmpty) + Padding( + padding: const EdgeInsets.symmetric(vertical: Grid.sm), + child: Text( + 'No open channels available to join.', + textAlign: TextAlign.center, + style: context.textTheme.bodyMedium?.copyWith( + color: context.colors.onSurfaceVariant, + ), + ), + ) + else + _JoinableChannelList(channels: channels, closeAfterJoin: true), + ], + ), + ), + ); + } +} + +class _JoinableChannelList extends StatelessWidget { + final List channels; + final bool closeAfterJoin; + + const _JoinableChannelList({ + required this.channels, + this.closeAfterJoin = false, + }); + + @override + Widget build(BuildContext context) { + final sortedChannels = List.of(channels) + ..sort( + (left, right) => + left.name.toLowerCase().compareTo(right.name.toLowerCase()), + ); + return Column( + mainAxisSize: MainAxisSize.min, + children: [ + for (final channel in sortedChannels) + _JoinableChannelTile( + channel: channel, + closeAfterJoin: closeAfterJoin, + ), + ], + ); + } +} + +class _JoinableChannelTile extends HookConsumerWidget { + final Channel channel; + final bool closeAfterJoin; + + const _JoinableChannelTile({ + required this.channel, + required this.closeAfterJoin, + }); + + @override + Widget build(BuildContext context, WidgetRef ref) { + final isJoining = useState(false); + final actionError = useState(null); + + Future join() async { + if (isJoining.value) return; + isJoining.value = true; + actionError.value = null; + try { + await ref.read(channelActionsProvider).joinChannel(channel.id); + if (closeAfterJoin && context.mounted) Navigator.of(context).pop(); + } catch (error) { + actionError.value = error.toString(); + } finally { + isJoining.value = false; + } + } + + return Column( + mainAxisSize: MainAxisSize.min, + children: [ + ListTile( + key: Key('browse-channel-${channel.id}'), + contentPadding: EdgeInsets.zero, + leading: Icon(channelIcon(channel)), + title: Text(channel.name), + subtitle: channel.description.trim().isEmpty + ? null + : Text( + channel.description, + maxLines: 2, + overflow: TextOverflow.ellipsis, + ), + trailing: FilledButton.tonal( + key: Key('browse-channel-join-${channel.id}'), + onPressed: isJoining.value ? null : () => unawaited(join()), + child: Text(isJoining.value ? 'Joining…' : 'Join'), + ), + ), + if (actionError.value case final error?) + Align( + alignment: Alignment.centerLeft, + child: Text( + error, + key: Key('browse-channel-error-${channel.id}'), + style: context.textTheme.bodySmall?.copyWith( + color: context.colors.error, + ), + ), + ), + ], + ); + } +} diff --git a/mobile/lib/features/channels/channels_page/quick_actions.dart b/mobile/lib/features/channels/channels_page/quick_actions.dart index 87548427024..3e0f6982b40 100644 --- a/mobile/lib/features/channels/channels_page/quick_actions.dart +++ b/mobile/lib/features/channels/channels_page/quick_actions.dart @@ -7,7 +7,7 @@ const _kMorphCloseCurve = Cubic(0.22, 1, 0.36, 1); const double _kMorphOpenBounce = 0.14; const double _kMorphCloseBounce = 0.06; const double _kMorphClosedSize = 56; -const double _kMorphOpenHeight = 160; +const double _kMorphOpenHeight = 216; const double _kMorphOpenRadius = 20; const double _kMorphSlide = 40; const double _kMorphScale = 0.97; @@ -274,6 +274,13 @@ class _QuickActionsMenu extends StatelessWidget { key: const Key('quick-action-new-dm-card'), onTap: () => onSelected(_QuickAction.newDm), ), + const SizedBox(height: Grid.xxs), + _QuickActionItem( + icon: LucideIcons.compass, + title: 'Browse channels', + key: const Key('quick-action-browse-channels-card'), + onTap: () => onSelected(_QuickAction.browseChannels), + ), ], ), ); diff --git a/mobile/lib/features/channels/channels_page/quick_actions_launcher.dart b/mobile/lib/features/channels/channels_page/quick_actions_launcher.dart index 8517fa70b9f..b298c192d0a 100644 --- a/mobile/lib/features/channels/channels_page/quick_actions_launcher.dart +++ b/mobile/lib/features/channels/channels_page/quick_actions_launcher.dart @@ -109,6 +109,15 @@ class ChannelQuickActionsLauncher extends HookConsumerWidget { if (opened != null && context.mounted) { await openChannel(opened); } + case _QuickAction.browseChannels: + await showBuzzModalBottomSheet( + context: context, + title: 'Browse channels', + constraints: _quickActionSheetConstraints(context), + isScrollControlled: true, + showDragHandle: true, + builder: (_) => const _BrowseChannelsSheet(), + ); } } diff --git a/mobile/lib/features/channels/channels_page/sections.dart b/mobile/lib/features/channels/channels_page/sections.dart index 6e17845d744..4ac18a448c8 100644 --- a/mobile/lib/features/channels/channels_page/sections.dart +++ b/mobile/lib/features/channels/channels_page/sections.dart @@ -456,29 +456,48 @@ class _ChannelSection extends StatelessWidget { } class _EmptyState extends StatelessWidget { - const _EmptyState(); + final List channels; + + const _EmptyState({required this.channels}); @override Widget build(BuildContext context) { - return SizedBox( - height: MediaQuery.sizeOf(context).height * 0.55, + return ConstrainedBox( + constraints: BoxConstraints( + minHeight: MediaQuery.sizeOf(context).height * 0.55, + ), child: Center( - child: Column( - mainAxisSize: MainAxisSize.min, - children: [ - Icon( - LucideIcons.messagesSquare, - size: Grid.xl, - color: context.colors.onSurfaceVariant, - ), - const SizedBox(height: Grid.xs), - Text( - 'No conversations yet', - style: context.textTheme.bodyLarge?.copyWith( + child: Padding( + padding: const EdgeInsets.symmetric(horizontal: Grid.gutter), + child: Column( + mainAxisSize: MainAxisSize.min, + children: [ + Icon( + LucideIcons.messagesSquare, + size: Grid.xl, color: context.colors.onSurfaceVariant, ), - ), - ], + const SizedBox(height: Grid.xs), + Text( + 'No conversations yet', + style: context.textTheme.bodyLarge?.copyWith( + color: context.colors.onSurfaceVariant, + ), + ), + if (channels.isNotEmpty) ...[ + const SizedBox(height: Grid.xs), + Text( + 'Join an open channel to start a conversation.', + textAlign: TextAlign.center, + style: context.textTheme.bodyMedium?.copyWith( + color: context.colors.onSurfaceVariant, + ), + ), + const SizedBox(height: Grid.xs), + _JoinableChannelList(channels: channels), + ], + ], + ), ), ), ); diff --git a/mobile/lib/features/channels/channels_provider.dart b/mobile/lib/features/channels/channels_provider.dart index 2f8dddf2d92..accc28a6c60 100644 --- a/mobile/lib/features/channels/channels_provider.dart +++ b/mobile/lib/features/channels/channels_provider.dart @@ -20,15 +20,19 @@ import 'unread_badge/should_notify_for_event.dart'; const _channelTypeOrder = {'stream': 0, 'forum': 1, 'dm': 2}; const _unreadCatchUpLimit = 1000; +const _channelDirectoryPageSize = 500; +const _maxChannelDirectoryPages = 100; const _participatedRootIdsPrefix = 'buzz-thread-participation.v1'; const _authoredRootIdsPrefix = 'buzz-thread-authored.v1'; /// Loads the user's channel list from the relay over WebSocket. /// -/// Two-step query: +/// Three-step query: /// 1. Fetch kind:39002 membership events tagged `#p:` to find /// the channel ids I'm a member of. /// 2. Fetch the corresponding kind:39000 channel metadata events. +/// 3. Fetch the paginated kind:39000 directory so open channels that the +/// user has not joined remain discoverable. /// /// Live updates are layered on top via per-channel subscriptions on the /// `#h` tag for any of the visible channel event kinds — incoming events @@ -164,28 +168,66 @@ class ChannelsNotifier extends AsyncNotifier> { until = page.map((e) => e.createdAt).reduce(min) - 1; } } - final channelIds = memberships + final memberChannelIds = memberships .map((e) => e.getTagValue('d')) .whereType() - .toSet() - .toList(); + .toSet(); _cacheMemberSnapshots(memberships, replaceAll: true); - if (channelIds.isEmpty) { - if (subscribeLive) await _subscribeLive(const []); - return const []; - } - // Step 2: pull channel metadata in one batched filter. - final metas = await session.fetchHistory( - NostrFilters.channelMetadata(channelIds), - ); + // Step 2: pull metadata for joined channels. A user with no memberships + // must still continue to directory discovery below. + final memberMetas = memberChannelIds.isEmpty + ? const [] + : await session.fetchHistory( + NostrFilters.channelMetadata(memberChannelIds.toList()), + ); + + // Step 3: fetch the open-channel directory. The relay filters this global + // kind:39000 query by the caller's access, but the client still rejects + // private channels and DMs below so discovery fails closed if that contract + // ever regresses. The composite cursor preserves tied-timestamp rows. + final directoryMetas = []; + final seenDirectoryChannelIds = {}; + int? directoryUntil; + String? directoryBeforeId; + for ( + var pageIndex = 0; + pageIndex < _maxChannelDirectoryPages; + pageIndex++ + ) { + final page = await session.fetchHistory( + NostrFilter( + kinds: const [39000], + limit: _channelDirectoryPageSize, + until: directoryUntil, + extensions: {'before_id': ?directoryBeforeId}, + ), + ); + directoryMetas.addAll(page); - // Dedupe by `d` tag (channel id) — kind:39000 is parameterized-replaceable, - // so logically there's exactly one current event per id, but stale revisions - // from before the relay's d_tag backfill can linger. Keep the highest - // `created_at` per id so the latest channel_type / name wins. + var madeProgress = false; + for (final event in page) { + final channelId = event.getTagValue('d'); + if (channelId != null && seenDirectoryChannelIds.add(channelId)) { + madeProgress = true; + } + } + if (!madeProgress || page.length < _channelDirectoryPageSize) break; + + final last = page.last; + directoryUntil = last.createdAt; + directoryBeforeId = last.id; + if (pageIndex == _maxChannelDirectoryPages - 1) { + throw StateError( + 'Channel directory exceeded $_maxChannelDirectoryPages pages', + ); + } + } + + // Merge and dedupe by `d` tag. Kind:39000 is parameterized-replaceable, + // but stale revisions from before the relay's d_tag backfill can linger. final latestMetaPerId = {}; - for (final event in metas) { + for (final event in [...memberMetas, ...directoryMetas]) { if (event.kind != 39000) continue; final id = event.getTagValue('d'); if (id == null) continue; @@ -232,11 +274,15 @@ class ChannelsNotifier extends AsyncNotifier> { final channels = []; for (final event in dedupedMetas) { + final id = event.getTagValue('d'); + if (id == null) continue; + final isMember = memberChannelIds.contains(id); final channel = _channelFromMeta( event, - isMember: true, + isMember: isMember, displayNames: displayNames, ); + if (!isMember && (channel.isPrivate || channel.isDm)) continue; if (channel.isDm && hiddenDmIds.contains(channel.id)) continue; // Ephemeral (TTL) channels are surfaced in the list with an // `_EphemeralBadge` rendered in `channels_page.dart` — they shouldn't be @@ -246,13 +292,16 @@ class ChannelsNotifier extends AsyncNotifier> { } // Batch-fetch member counts via kind:39002 membership events. - final memberEvents = await session.fetchHistory( - NostrFilter( - kinds: const [39002], - tags: {'#d': channelIds}, - limit: channelIds.length, - ), - ); + final memberCountChannelIds = memberChannelIds.toList(); + final memberEvents = memberCountChannelIds.isEmpty + ? const [] + : await session.fetchHistory( + NostrFilter( + kinds: const [39002], + tags: {'#d': memberCountChannelIds}, + limit: memberCountChannelIds.length, + ), + ); if (memberEvents.isNotEmpty) _cacheMemberSnapshots(memberEvents); final memberCounts = {}; for (final event in memberEvents) { diff --git a/mobile/lib/features/channels/manage_channel_sheet.dart b/mobile/lib/features/channels/manage_channel_sheet.dart index 3f0d6127456..dba5ecdef60 100644 --- a/mobile/lib/features/channels/manage_channel_sheet.dart +++ b/mobile/lib/features/channels/manage_channel_sheet.dart @@ -34,11 +34,7 @@ class ManageChannelSheet extends HookConsumerWidget { final mutesState = ref.watch(channelMutesProvider); final isMuted = mutesState.store.channels[channel.id]?.muted == true; - final canJoin = - channel.visibility == 'open' && - !channel.isArchived && - !channel.isMember && - !channel.isDm; + final canJoin = channel.canJoin; final canLeave = channel.isMember && !channel.isArchived && !channel.isDm; final canEditCanvas = channel.isMember && !channel.isArchived; diff --git a/mobile/test/features/channels/channels_page_test.dart b/mobile/test/features/channels/channels_page_test.dart index b0387b54f84..a940e6b9b2b 100644 --- a/mobile/test/features/channels/channels_page_test.dart +++ b/mobile/test/features/channels/channels_page_test.dart @@ -1241,8 +1241,8 @@ void main() { } await tester.pumpAndSettle(); - expect(largestHeight, greaterThan(160)); - expect(tester.getSize(surface).height, closeTo(160, 0.01)); + expect(largestHeight, greaterThan(216)); + expect(tester.getSize(surface).height, closeTo(216, 0.01)); final screenWidth = MediaQuery.sizeOf(tester.element(surface)).width; final surfaceRect = tester.getRect(surface); expect(surfaceRect.left, closeTo(20, 0.01)); @@ -1255,15 +1255,23 @@ void main() { const Key('quick-action-create-channel-card'), ); final dmCard = find.byKey(const Key('quick-action-new-dm-card')); + final browseCard = find.byKey( + const Key('quick-action-browse-channels-card'), + ); final createRect = tester.getRect(createCard); final dmRect = tester.getRect(dmCard); + final browseRect = tester.getRect(browseCard); expect(createRect.left - menuRect.left, closeTo(8, 0.01)); expect(menuRect.right - createRect.right, closeTo(8, 0.01)); expect(dmRect.left - menuRect.left, closeTo(8, 0.01)); expect(menuRect.right - dmRect.right, closeTo(8, 0.01)); + expect(browseRect.left - menuRect.left, closeTo(8, 0.01)); + expect(menuRect.right - browseRect.right, closeTo(8, 0.01)); expect(dmRect.top - createRect.bottom, closeTo(8, 0.01)); + expect(browseRect.top - dmRect.bottom, closeTo(8, 0.01)); expect(dmRect.width, createRect.width); + expect(browseRect.width, createRect.width); expect(dmRect.width, closeTo(menuRect.width - 16, 0.01)); final cardScheme = Theme.of(tester.element(createCard)).colorScheme; @@ -1277,8 +1285,12 @@ void main() { final dmMaterial = tester.widget( find.descendant(of: dmCard, matching: find.byType(Material)).first, ); + final browseMaterial = tester.widget( + find.descendant(of: browseCard, matching: find.byType(Material)).first, + ); expect(createMaterial.color, expectedCardColor); expect(dmMaterial.color, expectedCardColor); + expect(browseMaterial.color, expectedCardColor); expect( (createMaterial.borderRadius as BorderRadius).topLeft.x, closeTo(12, 0.01), @@ -1297,9 +1309,117 @@ void main() { tester.widget(find.text('New direct message')).style?.fontSize, 16, ); + expect( + tester.widget(find.text('Browse channels')).style?.fontSize, + 16, + ); expect(find.text('Message one or more people'), findsNothing); }); + testWidgets('browse action lists only channels eligible to join', ( + tester, + ) async { + final channels = [ + ...testChannels, + Channel( + id: 'open-to-join', + name: 'announcements', + channelType: 'stream', + visibility: 'open', + description: 'Community announcements', + createdBy: 'abc', + createdAt: DateTime(2025), + memberCount: 8, + ), + Channel( + id: 'private-channel', + name: 'private-planning', + channelType: 'stream', + visibility: 'private', + description: 'Private planning', + createdBy: 'abc', + createdAt: DateTime(2025), + memberCount: 4, + ), + Channel( + id: 'archived-channel', + name: 'old-announcements', + channelType: 'stream', + visibility: 'open', + description: 'Archived announcements', + createdBy: 'abc', + createdAt: DateTime(2025), + memberCount: 3, + archivedAt: DateTime(2025, 1, 2), + ), + Channel( + id: 'unjoined-dm', + name: 'Hidden DM', + channelType: 'dm', + visibility: 'open', + description: 'Direct message', + createdBy: 'abc', + createdAt: DateTime(2025), + memberCount: 2, + ), + ]; + + await tester.pumpWidget( + buildTestable( + disableAnimations: true, + overrides: [ + channelsProvider.overrideWith(() => _FakeNotifier(channels)), + ], + ), + ); + await tester.pumpAndSettle(); + + await tester.tap(find.byTooltip('Create or start conversation')); + await tester.pump(); + await tester.tap( + find.byKey(const Key('quick-action-browse-channels-card')), + ); + await tester.pumpAndSettle(); + + expect( + find.byKey(const Key('browse-channel-open-to-join')), + findsOneWidget, + ); + expect(find.byKey(const Key('browse-channel-1')), findsNothing); + expect( + find.byKey(const Key('browse-channel-private-channel')), + findsNothing, + ); + expect( + find.byKey(const Key('browse-channel-archived-channel')), + findsNothing, + ); + expect(find.byKey(const Key('browse-channel-unjoined-dm')), findsNothing); + }); + + testWidgets('browse action explains when no channels are discoverable', ( + tester, + ) async { + await tester.pumpWidget( + buildTestable( + disableAnimations: true, + overrides: [ + channelsProvider.overrideWith(() => _FakeNotifier(testChannels)), + ], + ), + ); + await tester.pumpAndSettle(); + + await tester.tap(find.byTooltip('Create or start conversation')); + await tester.pump(); + await tester.tap( + find.byKey(const Key('quick-action-browse-channels-card')), + ); + await tester.pumpAndSettle(); + + expect(find.text('No open channels available to join.'), findsOneWidget); + }); + testWidgets('create channel sheet lists type and visibility radio options', ( tester, ) async { @@ -1702,6 +1822,58 @@ void main() { expect(find.text('No conversations yet'), findsOneWidget); }); + testWidgets('empty state lets users join a discovered channel', ( + tester, + ) async { + final discoveredChannel = Channel( + id: 'recovery-channel', + name: 'community-help', + channelType: 'stream', + visibility: 'open', + description: 'Get help from the community', + createdBy: 'abc', + createdAt: DateTime(2025), + memberCount: 7, + ); + final channelsNotifier = _FakeNotifier([discoveredChannel]); + final joinedChannelIds = []; + + await tester.pumpWidget( + buildTestable( + overrides: [ + channelsProvider.overrideWith(() => channelsNotifier), + channelActionsProvider.overrideWith( + (ref) => _FakeChannelActions( + ref, + onJoinChannel: (channelId) async { + joinedChannelIds.add(channelId); + channelsNotifier.setChannels([ + discoveredChannel.copyWith(isMember: true), + ]); + }, + ), + ), + ], + ), + ); + await tester.pumpAndSettle(); + + expect(find.text('No conversations yet'), findsOneWidget); + expect( + find.byKey(const Key('browse-channel-recovery-channel')), + findsOneWidget, + ); + + await tester.tap( + find.byKey(const Key('browse-channel-join-recovery-channel')), + ); + await tester.pumpAndSettle(); + + expect(joinedChannelIds, ['recovery-channel']); + expect(find.text('No conversations yet'), findsNothing); + expect(find.text('community-help'), findsOneWidget); + }); + testWidgets('shows error view with retry button', (tester) async { await tester.pumpWidget( buildTestable( @@ -1972,6 +2144,28 @@ class _FakeNotifier extends ChannelsNotifier { @override Map> get observedUnreadEventsByChannel => _observedEventsByChannel; + + void setChannels(List channels) { + state = AsyncData(channels); + } +} + +class _FakeChannelActions extends ChannelActions { + final Future Function(String channelId) onJoinChannel; + + _FakeChannelActions(Ref ref, {required this.onJoinChannel}) + : super( + ref: ref, + session: ref.read(relaySessionProvider.notifier), + signedEventRelay: SignedEventRelay( + session: ref.read(relaySessionProvider.notifier), + nsec: null, + ), + currentPubkey: 'aabb', + ); + + @override + Future joinChannel(String channelId) => onJoinChannel(channelId); } class _FakeChannelSectionsNotifier extends ChannelSectionsNotifier { diff --git a/mobile/test/features/channels/channels_provider_test.dart b/mobile/test/features/channels/channels_provider_test.dart index ea33fb79444..97c1b65ce86 100644 --- a/mobile/test/features/channels/channels_provider_test.dart +++ b/mobile/test/features/channels/channels_provider_test.dart @@ -9,9 +9,10 @@ import 'package:buzz/shared/relay/relay.dart'; /// Tests for [ChannelsNotifier] in the pure-Nostr world. /// -/// The provider performs a two-step WS query: +/// The provider performs a three-step WS query: /// 1. kind:39002 memberships tagged `#p:` /// 2. kind:39000 metadata for those channel ids +/// 3. paginated kind:39000 metadata for discoverable open channels /// then layers per-channel live subscriptions on the `#h` tag. /// /// Tests stub out the relay session by overriding [relaySessionProvider] with @@ -21,6 +22,150 @@ import 'package:buzz/shared/relay/relay.dart'; void main() { const myPk = 'me'; + test( + 'discovers open channels for a user with zero channel memberships', + () async { + final session = _FakeRelaySession( + memberships: const [], + metadata: [ + _meta(id: _channelA, name: 'general'), + _meta(id: _channelB, name: 'staff', visibility: 'private'), + _meta(id: _channelD, name: 'DM', channelType: 'dm'), + ], + ); + final container = _buildContainer(session: session); + addTearDown(container.dispose); + + final channels = await container.read(channelsProvider.future); + + expect(channels, hasLength(1)); + expect(channels.single.id, _channelA); + expect(channels.single.isMember, isFalse); + expect(session.subscribeFilters, isEmpty); + expect( + session.historyFilters.any( + (filter) => + filter.kinds.length == 1 && + filter.kinds.single == 39000 && + !filter.tags.containsKey('#d'), + ), + isTrue, + ); + }, + ); + + test('paginates channel discovery with a composite cursor', () async { + final firstPage = List.generate( + 500, + (index) => _meta( + id: '${index.toString().padLeft(8, '0')}-0000-4000-8000-000000000000', + name: 'channel-$index', + createdAt: 10, + ), + ); + final finalChannel = _meta( + id: '99999999-9999-4999-8999-999999999999', + name: 'last-page', + createdAt: 9, + ); + final session = _FakeRelaySession( + memberships: const [], + metadataPages: [ + firstPage, + [finalChannel], + ], + ); + final container = _buildContainer(session: session); + addTearDown(container.dispose); + + final channels = await container.read(channelsProvider.future); + + expect(channels, hasLength(501)); + final directoryFilters = session.historyFilters + .where( + (filter) => + filter.kinds.length == 1 && + filter.kinds.single == 39000 && + !filter.tags.containsKey('#d'), + ) + .toList(); + expect(directoryFilters, hasLength(2)); + expect(directoryFilters.first.until, isNull); + expect(directoryFilters.first.extensions, isEmpty); + expect(directoryFilters.last.until, firstPage.last.createdAt); + expect(directoryFilters.last.extensions['before_id'], firstPage.last.id); + }); + + test('stops channel discovery when the relay repeats a full page', () async { + final repeatedPage = List.generate( + 500, + (index) => _meta( + id: 'repeated-channel-$index', + name: 'repeated-$index', + createdAt: 10, + ), + ); + final session = _FakeRelaySession( + memberships: const [], + metadataPages: [repeatedPage], + repeatLastMetadataPage: true, + maxMetadataPageRequests: 2, + ); + final container = _buildContainer(session: session); + addTearDown(container.dispose); + + final channels = await container.read(channelsProvider.future); + + expect(channels, hasLength(500)); + expect(session.metadataPageRequestCount, 2); + }); + + test('fails loudly when channel discovery exceeds its page cap', () async { + final session = _FakeRelaySession( + memberships: const [], + metadataPageBuilder: (pageIndex) => List.generate( + 500, + (eventIndex) => _meta( + id: 'channel-$pageIndex-$eventIndex', + name: 'channel-$pageIndex-$eventIndex', + createdAt: 1000 - pageIndex, + ), + ), + ); + final container = _buildContainer(session: session); + addTearDown(container.dispose); + + await expectLater( + container.read(channelsProvider.future), + throwsA( + isA().having( + (error) => error.message, + 'message', + contains('Channel directory exceeded'), + ), + ), + ); + }); + + test('deduplicates joined channels from directory discovery', () async { + final session = _FakeRelaySession( + memberships: [_membership(_channelA, myPk)], + metadata: [ + _meta(id: _channelA, name: 'general'), + _meta(id: _channelB, name: 'random'), + ], + ); + final container = _buildContainer(session: session); + addTearDown(container.dispose); + + final channels = await container.read(channelsProvider.future); + + expect(channels.map((channel) => channel.id), [_channelA, _channelB]); + expect(channels.first.isMember, isTrue); + expect(channels.last.isMember, isFalse); + expect(session.subscribeFilters, hasLength(1)); + }); + test( 'seeds members from the channel-list snapshot during reconnect', () async { @@ -699,6 +844,7 @@ NostrEvent _meta({ required String id, required String name, String channelType = 'stream', + String visibility = 'open', int createdAt = 1, int? ttlSeconds, bool archived = false, @@ -711,7 +857,7 @@ NostrEvent _meta({ ['d', id], ['name', name], ['t', channelType], - ['public'], + [visibility == 'private' ? 'private' : 'public'], if (ttlSeconds != null) ['ttl', '$ttlSeconds'], if (archived) ['archived', 'true'], ], @@ -743,7 +889,11 @@ Future _waitUntil(bool Function() predicate) async { class _FakeRelaySession extends RelaySessionNotifier { _FakeRelaySession({ required this.memberships, - required this.metadata, + this.metadata = const [], + this.metadataPages, + this.metadataPageBuilder, + this.repeatLastMetadataPage = false, + this.maxMetadataPageRequests, this.hiddenDmEvents = const [], this.recentMessages = const [], this.membershipFailures = 0, @@ -751,9 +901,14 @@ class _FakeRelaySession extends RelaySessionNotifier { List memberships; List metadata; + final List>? metadataPages; + final List Function(int pageIndex)? metadataPageBuilder; + final bool repeatLastMetadataPage; + final int? maxMetadataPageRequests; final List hiddenDmEvents; final List recentMessages; int membershipFailures; + int metadataPageRequestCount = 0; final List historyFilters = []; final List> queryBatches = []; @@ -820,8 +975,26 @@ class _FakeRelaySession extends RelaySessionNotifier { return hiddenDmEvents; } if (filter.kinds.contains(39000)) { - // Metadata query — return all metadata events whose `d` tag matches. - final ids = (filter.tags['#d'] ?? const []).toSet(); + final ids = filter.tags['#d']?.toSet(); + if (ids == null) { + final requestIndex = metadataPageRequestCount++; + final maxRequests = maxMetadataPageRequests; + if (maxRequests != null && requestIndex >= maxRequests) { + throw StateError('Unexpected directory page request'); + } + final pageBuilder = metadataPageBuilder; + if (pageBuilder != null) return List.of(pageBuilder(requestIndex)); + final pages = metadataPages; + if (pages != null) { + if (requestIndex < pages.length) return List.of(pages[requestIndex]); + if (repeatLastMetadataPage && pages.isNotEmpty) { + return List.of(pages.last); + } + return const []; + } + return List.of(metadata); + } + // Member metadata query — return only matching `d` tags. return metadata.where((e) => ids.contains(e.getTagValue('d'))).toList(); } return const []; From ac3464a0faa4ca0df099aaa78f5412fc2bf5196e Mon Sep 17 00:00:00 2001 From: Tom Brow Date: Mon, 17 Aug 2026 11:13:17 -0700 Subject: [PATCH 02/14] fix(mobile): harden channel discovery refresh Signed-off-by: Tom Brow Co-authored-by: Codex Ai-assisted: true --- .../features/channels/channel_directory.dart | 43 +++++++++ .../channels_page/browse_channels_sheet.dart | 84 +++++++++++------- .../channels/channels_page/sections.dart | 2 +- .../features/channels/channels_provider.dart | 57 ++++-------- .../features/channels/channels_page_test.dart | 40 +++++++++ .../channels/channels_provider_test.dart | 88 ++++++++++++++++--- 6 files changed, 230 insertions(+), 84 deletions(-) create mode 100644 mobile/lib/features/channels/channel_directory.dart diff --git a/mobile/lib/features/channels/channel_directory.dart b/mobile/lib/features/channels/channel_directory.dart new file mode 100644 index 00000000000..ae66b1a2009 --- /dev/null +++ b/mobile/lib/features/channels/channel_directory.dart @@ -0,0 +1,43 @@ +part of 'channels_provider.dart'; + +const _channelDirectoryPageSize = 500; +const _maxChannelDirectoryPages = 100; + +Future> _fetchChannelDirectoryMetas( + RelaySessionNotifier session, +) async { + final directoryMetas = []; + final seenDirectoryChannelIds = {}; + int? directoryUntil; + String? directoryBeforeId; + for (var pageIndex = 0; pageIndex < _maxChannelDirectoryPages; pageIndex++) { + final page = await session.fetchHistory( + NostrFilter( + kinds: const [39000], + limit: _channelDirectoryPageSize, + until: directoryUntil, + extensions: {'before_id': ?directoryBeforeId}, + ), + ); + directoryMetas.addAll(page); + + var madeProgress = false; + for (final event in page) { + final channelId = event.getTagValue('d'); + if (channelId != null && seenDirectoryChannelIds.add(channelId)) { + madeProgress = true; + } + } + if (!madeProgress || page.length < _channelDirectoryPageSize) break; + + final last = page.last; + directoryUntil = last.createdAt; + directoryBeforeId = last.id; + if (pageIndex == _maxChannelDirectoryPages - 1) { + throw StateError( + 'Channel directory exceeded $_maxChannelDirectoryPages pages', + ); + } + } + return directoryMetas; +} diff --git a/mobile/lib/features/channels/channels_page/browse_channels_sheet.dart b/mobile/lib/features/channels/channels_page/browse_channels_sheet.dart index 7a6f34b7322..f6ffe71a727 100644 --- a/mobile/lib/features/channels/channels_page/browse_channels_sheet.dart +++ b/mobile/lib/features/channels/channels_page/browse_channels_sheet.dart @@ -9,6 +9,10 @@ class _BrowseChannelsSheet extends ConsumerWidget { final channels = channelsAsync.asData?.value .where((channel) => channel.canJoin) .toList(); + channels?.sort( + (left, right) => + left.name.toLowerCase().compareTo(right.name.toLowerCase()), + ); return SafeArea( top: false, @@ -19,45 +23,64 @@ class _BrowseChannelsSheet extends ConsumerWidget { Grid.gutter, Grid.xs, ), - child: ListView( + child: CustomScrollView( shrinkWrap: true, - children: [ - Text( - 'Join an open channel to add it to your conversations.', - style: context.textTheme.bodyMedium?.copyWith( - color: context.colors.onSurfaceVariant, + slivers: [ + SliverToBoxAdapter( + child: Column( + crossAxisAlignment: CrossAxisAlignment.start, + children: [ + Text( + 'Join an open channel to add it to your conversations.', + style: context.textTheme.bodyMedium?.copyWith( + color: context.colors.onSurfaceVariant, + ), + ), + const SizedBox(height: Grid.xs), + ], ), ), - const SizedBox(height: Grid.xs), if (channelsAsync.isLoading && channels == null) - const Padding( - padding: EdgeInsets.all(Grid.sm), - child: Center(child: BuzzLoadingIndicator()), + const SliverToBoxAdapter( + child: Padding( + padding: EdgeInsets.all(Grid.sm), + child: Center(child: BuzzLoadingIndicator()), + ), ) else if (channelsAsync.hasError && channels == null) - Padding( - padding: const EdgeInsets.symmetric(vertical: Grid.sm), - child: Text( - 'Could not load open channels.', - textAlign: TextAlign.center, - style: context.textTheme.bodyMedium?.copyWith( - color: context.colors.onSurfaceVariant, + SliverToBoxAdapter( + child: Padding( + padding: const EdgeInsets.symmetric(vertical: Grid.sm), + child: Text( + 'Could not load open channels.', + textAlign: TextAlign.center, + style: context.textTheme.bodyMedium?.copyWith( + color: context.colors.onSurfaceVariant, + ), ), ), ) else if (channels == null || channels.isEmpty) - Padding( - padding: const EdgeInsets.symmetric(vertical: Grid.sm), - child: Text( - 'No open channels available to join.', - textAlign: TextAlign.center, - style: context.textTheme.bodyMedium?.copyWith( - color: context.colors.onSurfaceVariant, + SliverToBoxAdapter( + child: Padding( + padding: const EdgeInsets.symmetric(vertical: Grid.sm), + child: Text( + 'No open channels available to join.', + textAlign: TextAlign.center, + style: context.textTheme.bodyMedium?.copyWith( + color: context.colors.onSurfaceVariant, + ), ), ), ) else - _JoinableChannelList(channels: channels, closeAfterJoin: true), + SliverList.builder( + itemCount: channels.length, + itemBuilder: (context, index) => _JoinableChannelTile( + channel: channels[index], + closeAfterJoin: true, + ), + ), ], ), ), @@ -67,12 +90,8 @@ class _BrowseChannelsSheet extends ConsumerWidget { class _JoinableChannelList extends StatelessWidget { final List channels; - final bool closeAfterJoin; - const _JoinableChannelList({ - required this.channels, - this.closeAfterJoin = false, - }); + const _JoinableChannelList({required this.channels}); @override Widget build(BuildContext context) { @@ -85,10 +104,7 @@ class _JoinableChannelList extends StatelessWidget { mainAxisSize: MainAxisSize.min, children: [ for (final channel in sortedChannels) - _JoinableChannelTile( - channel: channel, - closeAfterJoin: closeAfterJoin, - ), + _JoinableChannelTile(channel: channel, closeAfterJoin: false), ], ); } diff --git a/mobile/lib/features/channels/channels_page/sections.dart b/mobile/lib/features/channels/channels_page/sections.dart index 4ac18a448c8..040a1acdb4a 100644 --- a/mobile/lib/features/channels/channels_page/sections.dart +++ b/mobile/lib/features/channels/channels_page/sections.dart @@ -494,7 +494,7 @@ class _EmptyState extends StatelessWidget { ), ), const SizedBox(height: Grid.xs), - _JoinableChannelList(channels: channels), + _JoinableChannelList(channels: channels.take(3).toList()), ], ], ), diff --git a/mobile/lib/features/channels/channels_provider.dart b/mobile/lib/features/channels/channels_provider.dart index accc28a6c60..39fbbdb7ba9 100644 --- a/mobile/lib/features/channels/channels_provider.dart +++ b/mobile/lib/features/channels/channels_provider.dart @@ -18,10 +18,10 @@ import 'unread_badge/is_high_priority_event.dart'; import 'unread_badge/observed_unread_event.dart'; import 'unread_badge/should_notify_for_event.dart'; +part 'channel_directory.dart'; + const _channelTypeOrder = {'stream': 0, 'forum': 1, 'dm': 2}; const _unreadCatchUpLimit = 1000; -const _channelDirectoryPageSize = 500; -const _maxChannelDirectoryPages = 100; const _participatedRootIdsPrefix = 'buzz-thread-participation.v1'; const _authoredRootIdsPrefix = 'buzz-thread-authored.v1'; @@ -57,6 +57,7 @@ class ChannelsNotifier extends AsyncNotifier> { String? _memberSnapshotRelayBaseUrl; String? _memberSnapshotPubkey; Map> _memberSnapshotsByChannelId = const {}; + List _directoryMetas = const []; /// The member snapshot already returned while loading the channel list. /// @@ -84,6 +85,7 @@ class ChannelsNotifier extends AsyncNotifier> { _memberSnapshotRelayBaseUrl = relayBaseUrl; _memberSnapshotPubkey = pubkey; _memberSnapshotsByChannelId = const {}; + _directoryMetas = const []; } final connected = Completer(); final sessionState = ref.read(relaySessionProvider); @@ -128,10 +130,12 @@ class ChannelsNotifier extends AsyncNotifier> { Future> _fetch({ bool subscribeLive = false, bool fetchLastMessage = true, + bool fetchDirectory = true, }) async { final channels = await _fetchChannels( subscribeLive: subscribeLive, fetchLastMessage: fetchLastMessage, + fetchDirectory: fetchDirectory, ); _hasLoaded = true; return channels; @@ -140,6 +144,7 @@ class ChannelsNotifier extends AsyncNotifier> { Future> _fetchChannels({ bool subscribeLive = false, bool fetchLastMessage = true, + bool fetchDirectory = true, }) async { final myPk = ref.read(myPubkeyProvider); if (myPk == null) throw StateError('No signing identity available'); @@ -186,40 +191,13 @@ class ChannelsNotifier extends AsyncNotifier> { // kind:39000 query by the caller's access, but the client still rejects // private channels and DMs below so discovery fails closed if that contract // ever regresses. The composite cursor preserves tied-timestamp rows. - final directoryMetas = []; - final seenDirectoryChannelIds = {}; - int? directoryUntil; - String? directoryBeforeId; - for ( - var pageIndex = 0; - pageIndex < _maxChannelDirectoryPages; - pageIndex++ - ) { - final page = await session.fetchHistory( - NostrFilter( - kinds: const [39000], - limit: _channelDirectoryPageSize, - until: directoryUntil, - extensions: {'before_id': ?directoryBeforeId}, - ), - ); - directoryMetas.addAll(page); - - var madeProgress = false; - for (final event in page) { - final channelId = event.getTagValue('d'); - if (channelId != null && seenDirectoryChannelIds.add(channelId)) { - madeProgress = true; - } - } - if (!madeProgress || page.length < _channelDirectoryPageSize) break; - - final last = page.last; - directoryUntil = last.createdAt; - directoryBeforeId = last.id; - if (pageIndex == _maxChannelDirectoryPages - 1) { - throw StateError( - 'Channel directory exceeded $_maxChannelDirectoryPages pages', + if (fetchDirectory) { + try { + _directoryMetas = await _fetchChannelDirectoryMetas(session); + } catch (error) { + debugPrint( + '[ChannelsNotifier] channel directory refresh failed; retaining ' + 'cached discovery: $error', ); } } @@ -227,7 +205,7 @@ class ChannelsNotifier extends AsyncNotifier> { // Merge and dedupe by `d` tag. Kind:39000 is parameterized-replaceable, // but stale revisions from before the relay's d_tag backfill can linger. final latestMetaPerId = {}; - for (final event in [...memberMetas, ...directoryMetas]) { + for (final event in [...memberMetas, ..._directoryMetas]) { if (event.kind != 39000) continue; final id = event.getTagValue('d'); if (id == null) continue; @@ -576,8 +554,8 @@ class ChannelsNotifier extends AsyncNotifier> { } /// Subscribe per-channel to live events (requires `#h` tag for relay - /// channel-scoped fan-out). Also starts a 60s WS backstop poll to detect - /// newly created channels we don't yet have subscriptions for. + /// channel-scoped fan-out). Also starts a 60s WS backstop poll to reconcile + /// membership changes without repeatedly downloading the global directory. Future _subscribeLive(List channels) { final channelIds = { for (final channel in channels) @@ -919,6 +897,7 @@ class ChannelsNotifier extends AsyncNotifier> { final channels = await _fetch( subscribeLive: sessionState.status == SessionStatus.connected, fetchLastMessage: false, + fetchDirectory: false, ); for (var i = 0; i < channels.length; i++) { final prev = prevLastMessage[channels[i].id]; diff --git a/mobile/test/features/channels/channels_page_test.dart b/mobile/test/features/channels/channels_page_test.dart index a940e6b9b2b..d877e7eb7eb 100644 --- a/mobile/test/features/channels/channels_page_test.dart +++ b/mobile/test/features/channels/channels_page_test.dart @@ -1420,6 +1420,46 @@ void main() { expect(find.text('No open channels available to join.'), findsOneWidget); }); + testWidgets('browse action lazily builds a large channel directory', ( + tester, + ) async { + final channels = List.generate( + 500, + (index) => Channel( + id: 'directory-$index', + name: 'channel-${index.toString().padLeft(3, '0')}', + channelType: 'stream', + visibility: 'open', + description: '', + createdBy: 'abc', + createdAt: DateTime(2025), + memberCount: 0, + ), + ); + await tester.pumpWidget( + buildTestable( + disableAnimations: true, + overrides: [ + channelsProvider.overrideWith(() => _FakeNotifier(channels)), + ], + ), + ); + await tester.pumpAndSettle(); + + await tester.tap(find.byTooltip('Create or start conversation')); + await tester.pump(); + await tester.tap( + find.byKey(const Key('quick-action-browse-channels-card')), + ); + await tester.pumpAndSettle(); + + expect( + find.byKey(const Key('browse-channel-directory-0')), + findsAtLeast(1), + ); + expect(find.byKey(const Key('browse-channel-directory-499')), findsNothing); + }); + testWidgets('create channel sheet lists type and visibility radio options', ( tester, ) async { diff --git a/mobile/test/features/channels/channels_provider_test.dart b/mobile/test/features/channels/channels_provider_test.dart index 97c1b65ce86..a9892c5049c 100644 --- a/mobile/test/features/channels/channels_provider_test.dart +++ b/mobile/test/features/channels/channels_provider_test.dart @@ -120,7 +120,7 @@ void main() { expect(session.metadataPageRequestCount, 2); }); - test('fails loudly when channel discovery exceeds its page cap', () async { + test('directory page-cap failure does not fail channel loading', () async { final session = _FakeRelaySession( memberships: const [], metadataPageBuilder: (pageIndex) => List.generate( @@ -135,16 +135,77 @@ void main() { final container = _buildContainer(session: session); addTearDown(container.dispose); - await expectLater( - container.read(channelsProvider.future), - throwsA( - isA().having( - (error) => error.message, - 'message', - contains('Channel directory exceeded'), - ), - ), + expect(await container.read(channelsProvider.future), isEmpty); + expect(session.metadataPageRequestCount, 100); + }); + + test( + 'directory failure retains discovery while membership refreshes', + () async { + final session = _FakeRelaySession( + memberships: [_membership(_channelA, myPk)], + metadata: [ + _meta(id: _channelA, name: 'general'), + _meta(id: _channelB, name: 'discoverable'), + ], + ); + final container = _buildContainer(session: session); + addTearDown(container.dispose); + + expect( + (await container.read( + channelsProvider.future, + )).map((channel) => channel.id), + unorderedEquals([_channelA, _channelB]), + ); + + session.memberships = [ + _membership(_channelA, myPk), + _membership(_channelD, myPk), + ]; + session.metadata = [ + _meta(id: _channelA, name: 'general'), + _meta(id: _channelB, name: 'discoverable'), + _meta(id: _channelD, name: 'newly joined'), + ]; + session.directoryFailures = 1; + + await container.read(channelsProvider.notifier).refresh(); + + final refreshed = container.read(channelsProvider).requireValue; + expect( + refreshed.map((channel) => channel.id), + unorderedEquals([_channelA, _channelB, _channelD]), + ); + expect( + refreshed.firstWhere((channel) => channel.id == _channelD).isMember, + isTrue, + ); + }, + ); + + test('reconnect backstop does not refetch the channel directory', () async { + final session = _FakeRelaySession( + memberships: [_membership(_channelA, myPk)], + metadata: [_meta(id: _channelA, name: 'general')], ); + final container = _buildContainer(session: session); + addTearDown(container.dispose); + + await container.read(channelsProvider.future); + final initialDirectoryRequests = session.metadataPageRequestCount; + final initialMembershipRequests = session.membershipRequestCount; + + session.setStatus(SessionStatus.reconnecting); + session.setStatus(SessionStatus.connected); + await _waitUntil( + () => session.membershipRequestCount > initialMembershipRequests, + ); + for (var i = 0; i < 10; i++) { + await Future.delayed(Duration.zero); + } + + expect(session.metadataPageRequestCount, initialDirectoryRequests); }); test('deduplicates joined channels from directory discovery', () async { @@ -908,6 +969,8 @@ class _FakeRelaySession extends RelaySessionNotifier { final List hiddenDmEvents; final List recentMessages; int membershipFailures; + int directoryFailures = 0; + int membershipRequestCount = 0; int metadataPageRequestCount = 0; final List historyFilters = []; @@ -958,6 +1021,7 @@ class _FakeRelaySession extends RelaySessionNotifier { }) async { historyFilters.add(filter); if (filter.kinds.contains(39002) && filter.tags['#p'] != null) { + membershipRequestCount++; if (membershipFailures > 0) { membershipFailures--; throw Exception('membership fetch failed'); @@ -977,6 +1041,10 @@ class _FakeRelaySession extends RelaySessionNotifier { if (filter.kinds.contains(39000)) { final ids = filter.tags['#d']?.toSet(); if (ids == null) { + if (directoryFailures > 0) { + directoryFailures--; + throw Exception('directory fetch failed'); + } final requestIndex = metadataPageRequestCount++; final maxRequests = maxMetadataPageRequests; if (maxRequests != null && requestIndex >= maxRequests) { From 502739d29b08f92e6f5121341d7955a98e33577c Mon Sep 17 00:00:00 2001 From: Tom Brow Date: Tue, 18 Aug 2026 09:34:27 -0700 Subject: [PATCH 03/14] fix(mobile): preserve channel directory cursor Signed-off-by: Tom Brow --- .../features/channels/channel_directory.dart | 4 +- .../channels/channels_provider_test.dart | 59 ++++++++++--------- 2 files changed, 32 insertions(+), 31 deletions(-) diff --git a/mobile/lib/features/channels/channel_directory.dart b/mobile/lib/features/channels/channel_directory.dart index ae66b1a2009..328cfbb2683 100644 --- a/mobile/lib/features/channels/channel_directory.dart +++ b/mobile/lib/features/channels/channel_directory.dart @@ -11,14 +11,14 @@ Future> _fetchChannelDirectoryMetas( int? directoryUntil; String? directoryBeforeId; for (var pageIndex = 0; pageIndex < _maxChannelDirectoryPages; pageIndex++) { - final page = await session.fetchHistory( + final page = await session.queryRelay([ NostrFilter( kinds: const [39000], limit: _channelDirectoryPageSize, until: directoryUntil, extensions: {'before_id': ?directoryBeforeId}, ), - ); + ]); directoryMetas.addAll(page); var madeProgress = false; diff --git a/mobile/test/features/channels/channels_provider_test.dart b/mobile/test/features/channels/channels_provider_test.dart index a9892c5049c..a5eb1d751e4 100644 --- a/mobile/test/features/channels/channels_provider_test.dart +++ b/mobile/test/features/channels/channels_provider_test.dart @@ -43,7 +43,7 @@ void main() { expect(channels.single.isMember, isFalse); expect(session.subscribeFilters, isEmpty); expect( - session.historyFilters.any( + session.directoryQueryFilters.any( (filter) => filter.kinds.length == 1 && filter.kinds.single == 39000 && @@ -81,14 +81,7 @@ void main() { final channels = await container.read(channelsProvider.future); expect(channels, hasLength(501)); - final directoryFilters = session.historyFilters - .where( - (filter) => - filter.kinds.length == 1 && - filter.kinds.single == 39000 && - !filter.tags.containsKey('#d'), - ) - .toList(); + final directoryFilters = session.directoryQueryFilters; expect(directoryFilters, hasLength(2)); expect(directoryFilters.first.until, isNull); expect(directoryFilters.first.extensions, isEmpty); @@ -975,6 +968,7 @@ class _FakeRelaySession extends RelaySessionNotifier { final List historyFilters = []; final List> queryBatches = []; + final List directoryQueryFilters = []; final List subscribeFilters = []; final Map _subscriptions = {}; int _nextSubscriptionKey = 0; @@ -1041,26 +1035,7 @@ class _FakeRelaySession extends RelaySessionNotifier { if (filter.kinds.contains(39000)) { final ids = filter.tags['#d']?.toSet(); if (ids == null) { - if (directoryFailures > 0) { - directoryFailures--; - throw Exception('directory fetch failed'); - } - final requestIndex = metadataPageRequestCount++; - final maxRequests = maxMetadataPageRequests; - if (maxRequests != null && requestIndex >= maxRequests) { - throw StateError('Unexpected directory page request'); - } - final pageBuilder = metadataPageBuilder; - if (pageBuilder != null) return List.of(pageBuilder(requestIndex)); - final pages = metadataPages; - if (pages != null) { - if (requestIndex < pages.length) return List.of(pages[requestIndex]); - if (repeatLastMetadataPage && pages.isNotEmpty) { - return List.of(pages.last); - } - return const []; - } - return List.of(metadata); + throw StateError('Directory queries must use the HTTP query bridge'); } // Member metadata query — return only matching `d` tags. return metadata.where((e) => ids.contains(e.getTagValue('d'))).toList(); @@ -1073,6 +1048,32 @@ class _FakeRelaySession extends RelaySessionNotifier { List filters, { Duration timeout = const Duration(seconds: 8), }) async { + if (filters case [final filter] + when filter.kinds.length == 1 && + filter.kinds.single == 39000 && + !filter.tags.containsKey('#d')) { + directoryQueryFilters.add(filter); + if (directoryFailures > 0) { + directoryFailures--; + throw Exception('directory fetch failed'); + } + final requestIndex = metadataPageRequestCount++; + final maxRequests = maxMetadataPageRequests; + if (maxRequests != null && requestIndex >= maxRequests) { + throw StateError('Unexpected directory page request'); + } + final pageBuilder = metadataPageBuilder; + if (pageBuilder != null) return List.of(pageBuilder(requestIndex)); + final pages = metadataPages; + if (pages != null) { + if (requestIndex < pages.length) return List.of(pages[requestIndex]); + if (repeatLastMetadataPage && pages.isNotEmpty) { + return List.of(pages.last); + } + return const []; + } + return List.of(metadata); + } queryBatches.add(filters); return recentMessages.where((event) { return filters.any((filter) { From 7feb172ff4263df93c6d018636f7be2e696d7d67 Mon Sep 17 00:00:00 2001 From: Tom Brow Date: Tue, 18 Aug 2026 09:50:02 -0700 Subject: [PATCH 04/14] refactor(mobile): remove empty channel preview Signed-off-by: Tom Brow Co-authored-by: Codex Ai-assisted: true --- .../features/channels/channels_page/body.dart | 4 +- .../channels_page/browse_channels_sheet.dart | 22 ------ .../channels/channels_page/sections.dart | 53 +++++--------- .../features/channels/channels_page_test.dart | 70 +++---------------- 4 files changed, 27 insertions(+), 122 deletions(-) diff --git a/mobile/lib/features/channels/channels_page/body.dart b/mobile/lib/features/channels/channels_page/body.dart index 68ba6c8e306..34faa068eba 100644 --- a/mobile/lib/features/channels/channels_page/body.dart +++ b/mobile/lib/features/channels/channels_page/body.dart @@ -235,9 +235,7 @@ class _SliverChannelsList extends HookConsumerWidget { sliver: SliverList.list( children: [ if (visibleChannels.isEmpty) - _EmptyState( - channels: channels.where((channel) => channel.canJoin).toList(), - ) + const _EmptyState() else ...[ // Starred channels (exclusive — pinned above all sections). if (starredStreamChannels.isNotEmpty) diff --git a/mobile/lib/features/channels/channels_page/browse_channels_sheet.dart b/mobile/lib/features/channels/channels_page/browse_channels_sheet.dart index f6ffe71a727..24cd1f654ea 100644 --- a/mobile/lib/features/channels/channels_page/browse_channels_sheet.dart +++ b/mobile/lib/features/channels/channels_page/browse_channels_sheet.dart @@ -88,28 +88,6 @@ class _BrowseChannelsSheet extends ConsumerWidget { } } -class _JoinableChannelList extends StatelessWidget { - final List channels; - - const _JoinableChannelList({required this.channels}); - - @override - Widget build(BuildContext context) { - final sortedChannels = List.of(channels) - ..sort( - (left, right) => - left.name.toLowerCase().compareTo(right.name.toLowerCase()), - ); - return Column( - mainAxisSize: MainAxisSize.min, - children: [ - for (final channel in sortedChannels) - _JoinableChannelTile(channel: channel, closeAfterJoin: false), - ], - ); - } -} - class _JoinableChannelTile extends HookConsumerWidget { final Channel channel; final bool closeAfterJoin; diff --git a/mobile/lib/features/channels/channels_page/sections.dart b/mobile/lib/features/channels/channels_page/sections.dart index 040a1acdb4a..6e17845d744 100644 --- a/mobile/lib/features/channels/channels_page/sections.dart +++ b/mobile/lib/features/channels/channels_page/sections.dart @@ -456,48 +456,29 @@ class _ChannelSection extends StatelessWidget { } class _EmptyState extends StatelessWidget { - final List channels; - - const _EmptyState({required this.channels}); + const _EmptyState(); @override Widget build(BuildContext context) { - return ConstrainedBox( - constraints: BoxConstraints( - minHeight: MediaQuery.sizeOf(context).height * 0.55, - ), + return SizedBox( + height: MediaQuery.sizeOf(context).height * 0.55, child: Center( - child: Padding( - padding: const EdgeInsets.symmetric(horizontal: Grid.gutter), - child: Column( - mainAxisSize: MainAxisSize.min, - children: [ - Icon( - LucideIcons.messagesSquare, - size: Grid.xl, + child: Column( + mainAxisSize: MainAxisSize.min, + children: [ + Icon( + LucideIcons.messagesSquare, + size: Grid.xl, + color: context.colors.onSurfaceVariant, + ), + const SizedBox(height: Grid.xs), + Text( + 'No conversations yet', + style: context.textTheme.bodyLarge?.copyWith( color: context.colors.onSurfaceVariant, ), - const SizedBox(height: Grid.xs), - Text( - 'No conversations yet', - style: context.textTheme.bodyLarge?.copyWith( - color: context.colors.onSurfaceVariant, - ), - ), - if (channels.isNotEmpty) ...[ - const SizedBox(height: Grid.xs), - Text( - 'Join an open channel to start a conversation.', - textAlign: TextAlign.center, - style: context.textTheme.bodyMedium?.copyWith( - color: context.colors.onSurfaceVariant, - ), - ), - const SizedBox(height: Grid.xs), - _JoinableChannelList(channels: channels.take(3).toList()), - ], - ], - ), + ), + ], ), ), ); diff --git a/mobile/test/features/channels/channels_page_test.dart b/mobile/test/features/channels/channels_page_test.dart index d877e7eb7eb..cefbb574f7a 100644 --- a/mobile/test/features/channels/channels_page_test.dart +++ b/mobile/test/features/channels/channels_page_test.dart @@ -1851,22 +1851,9 @@ void main() { expect(find.text('archived-stream'), findsNothing); }); - testWidgets('shows empty state when no channels', (tester) async { - await tester.pumpWidget( - buildTestable( - overrides: [channelsProvider.overrideWith(() => _FakeNotifier([]))], - ), - ); - await tester.pumpAndSettle(); - - expect(find.text('No conversations yet'), findsOneWidget); - }); - - testWidgets('empty state lets users join a discovered channel', ( - tester, - ) async { + testWidgets('empty state does not preview unjoined channels', (tester) async { final discoveredChannel = Channel( - id: 'recovery-channel', + id: 'discovered-channel', name: 'community-help', channelType: 'stream', visibility: 'open', @@ -1875,23 +1862,11 @@ void main() { createdAt: DateTime(2025), memberCount: 7, ); - final channelsNotifier = _FakeNotifier([discoveredChannel]); - final joinedChannelIds = []; - await tester.pumpWidget( buildTestable( overrides: [ - channelsProvider.overrideWith(() => channelsNotifier), - channelActionsProvider.overrideWith( - (ref) => _FakeChannelActions( - ref, - onJoinChannel: (channelId) async { - joinedChannelIds.add(channelId); - channelsNotifier.setChannels([ - discoveredChannel.copyWith(isMember: true), - ]); - }, - ), + channelsProvider.overrideWith( + () => _FakeNotifier([discoveredChannel]), ), ], ), @@ -1900,18 +1875,13 @@ void main() { expect(find.text('No conversations yet'), findsOneWidget); expect( - find.byKey(const Key('browse-channel-recovery-channel')), - findsOneWidget, + find.text('Join an open channel to start a conversation.'), + findsNothing, ); - - await tester.tap( - find.byKey(const Key('browse-channel-join-recovery-channel')), + expect( + find.byKey(const Key('browse-channel-discovered-channel')), + findsNothing, ); - await tester.pumpAndSettle(); - - expect(joinedChannelIds, ['recovery-channel']); - expect(find.text('No conversations yet'), findsNothing); - expect(find.text('community-help'), findsOneWidget); }); testWidgets('shows error view with retry button', (tester) async { @@ -2184,28 +2154,6 @@ class _FakeNotifier extends ChannelsNotifier { @override Map> get observedUnreadEventsByChannel => _observedEventsByChannel; - - void setChannels(List channels) { - state = AsyncData(channels); - } -} - -class _FakeChannelActions extends ChannelActions { - final Future Function(String channelId) onJoinChannel; - - _FakeChannelActions(Ref ref, {required this.onJoinChannel}) - : super( - ref: ref, - session: ref.read(relaySessionProvider.notifier), - signedEventRelay: SignedEventRelay( - session: ref.read(relaySessionProvider.notifier), - nsec: null, - ), - currentPubkey: 'aabb', - ); - - @override - Future joinChannel(String channelId) => onJoinChannel(channelId); } class _FakeChannelSectionsNotifier extends ChannelSectionsNotifier { From d4a84e96b496baab3a7c6d56206aaf0c79c71068 Mon Sep 17 00:00:00 2001 From: Tom Brow Date: Tue, 18 Aug 2026 16:25:09 -0700 Subject: [PATCH 05/14] test(mobile): cover scrolling channel directory Signed-off-by: Tom Brow --- .../features/channels/channels_page_test.dart | 56 ++++++++++++++++++- 1 file changed, 55 insertions(+), 1 deletion(-) diff --git a/mobile/test/features/channels/channels_page_test.dart b/mobile/test/features/channels/channels_page_test.dart index cefbb574f7a..f4753eeeac9 100644 --- a/mobile/test/features/channels/channels_page_test.dart +++ b/mobile/test/features/channels/channels_page_test.dart @@ -1420,7 +1420,7 @@ void main() { expect(find.text('No open channels available to join.'), findsOneWidget); }); - testWidgets('browse action lazily builds a large channel directory', ( + testWidgets('browse action scrolls and joins an offscreen channel', ( tester, ) async { final channels = List.generate( @@ -1436,11 +1436,15 @@ void main() { memberCount: 0, ), ); + late _RecordingChannelActions actions; await tester.pumpWidget( buildTestable( disableAnimations: true, overrides: [ channelsProvider.overrideWith(() => _FakeNotifier(channels)), + channelActionsProvider.overrideWith( + (ref) => actions = _RecordingChannelActions(ref), + ), ], ), ); @@ -1458,6 +1462,36 @@ void main() { findsAtLeast(1), ); expect(find.byKey(const Key('browse-channel-directory-499')), findsNothing); + + final sheet = find.byType(BottomSheet).last; + final scrollable = find + .descendant(of: sheet, matching: find.byType(Scrollable)) + .last; + expect( + tester.state(scrollable).position.maxScrollExtent, + greaterThan(0), + ); + await tester.scrollUntilVisible( + find.byKey(const Key('browse-channel-directory-499')), + 500, + scrollable: scrollable, + maxScrolls: 100, + ); + await tester.pumpAndSettle(); + + expect( + find.byKey(const Key('browse-channel-directory-499')), + findsOneWidget, + ); + expect(find.byKey(const Key('browse-channel-directory-0')), findsNothing); + + await tester.tap( + find.byKey(const Key('browse-channel-join-directory-499')), + ); + await tester.pumpAndSettle(); + + expect(actions.joinedChannelIds, ['directory-499']); + expect(find.byType(BottomSheet), findsNothing); }); testWidgets('create channel sheet lists type and visibility radio options', ( @@ -2156,6 +2190,26 @@ class _FakeNotifier extends ChannelsNotifier { get observedUnreadEventsByChannel => _observedEventsByChannel; } +class _RecordingChannelActions extends ChannelActions { + _RecordingChannelActions(Ref ref) + : super( + ref: ref, + session: ref.read(relaySessionProvider.notifier), + signedEventRelay: SignedEventRelay( + session: ref.read(relaySessionProvider.notifier), + nsec: null, + ), + currentPubkey: 'self', + ); + + final List joinedChannelIds = []; + + @override + Future joinChannel(String channelId) async { + joinedChannelIds.add(channelId); + } +} + class _FakeChannelSectionsNotifier extends ChannelSectionsNotifier { _FakeChannelSectionsNotifier(this._store); From 14b8a9e7c5c2daf42f08c89d4f4afe8c3be678f7 Mon Sep 17 00:00:00 2001 From: Tom Brow Date: Wed, 19 Aug 2026 11:59:56 -0700 Subject: [PATCH 06/14] fix(mobile): harden channel directory loading Signed-off-by: Tom Brow --- .../features/channels/channel_directory.dart | 125 +++++++++-- .../channels_page/browse_channels_sheet.dart | 55 ++++- .../features/channels/channels_provider.dart | 79 +++++-- .../features/channels/channels_page_test.dart | 97 ++++++++ .../channels/channels_provider_test.dart | 208 +++++++++++++++--- 5 files changed, 483 insertions(+), 81 deletions(-) diff --git a/mobile/lib/features/channels/channel_directory.dart b/mobile/lib/features/channels/channel_directory.dart index 328cfbb2683..fcb851526bc 100644 --- a/mobile/lib/features/channels/channel_directory.dart +++ b/mobile/lib/features/channels/channel_directory.dart @@ -3,41 +3,128 @@ part of 'channels_provider.dart'; const _channelDirectoryPageSize = 500; const _maxChannelDirectoryPages = 100; +/// Describes whether the open-channel directory is ready to browse. +enum ChannelDirectoryLoadStatus { + /// No directory request has completed for the active identity and relay. + idle, + + /// A directory request is currently in flight. + loading, + + /// The directory request completed, including when it returned no channels. + loaded, + + /// The most recent directory request could not complete. + error, +} + +/// Directory loading state scoped to one relay and signing identity. +class ChannelDirectoryLoadState { + /// Relay-and-identity scope that produced [status]. + final String? scope; + + /// Current loading status for [scope]. + final ChannelDirectoryLoadStatus status; + + /// Creates directory loading state. + const ChannelDirectoryLoadState({required this.scope, required this.status}); + + /// Initial state before any directory request has started. + const ChannelDirectoryLoadState.idle() + : scope = null, + status = ChannelDirectoryLoadStatus.idle; +} + +/// Returns the stable directory scope for a relay and signing identity. +String channelDirectoryScope(String relayBaseUrl, String? pubkey) => + '$relayBaseUrl:${pubkey?.toLowerCase() ?? ''}'; + +/// Owns the independently observable channel-directory loading state. +class ChannelDirectoryLoadNotifier extends Notifier { + @override + ChannelDirectoryLoadState build() => const ChannelDirectoryLoadState.idle(); + + /// Marks the directory as loading. + void markLoading(String scope) => state = ChannelDirectoryLoadState( + scope: scope, + status: ChannelDirectoryLoadStatus.loading, + ); + + /// Marks the directory as successfully loaded. + void markLoaded(String scope) => state = ChannelDirectoryLoadState( + scope: scope, + status: ChannelDirectoryLoadStatus.loaded, + ); + + /// Marks the directory request as unsuccessful. + void markError(String scope) => state = ChannelDirectoryLoadState( + scope: scope, + status: ChannelDirectoryLoadStatus.error, + ); +} + +/// Loading state for open-channel discovery, separate from membership loading. +final channelDirectoryLoadStatusProvider = + NotifierProvider( + ChannelDirectoryLoadNotifier.new, + ); + +Future> _fetchChannelMemberships( + RelaySessionNotifier session, + String pubkey, +) => _fetchPaginatedChannelEvents( + session, + kind: 39002, + tags: { + '#p': [pubkey], + }, + operation: 'Channel memberships', +); + Future> _fetchChannelDirectoryMetas( RelaySessionNotifier session, -) async { - final directoryMetas = []; - final seenDirectoryChannelIds = {}; - int? directoryUntil; - String? directoryBeforeId; +) => _fetchPaginatedChannelEvents( + session, + kind: 39000, + operation: 'Channel directory', +); + +Future> _fetchPaginatedChannelEvents( + RelaySessionNotifier session, { + required int kind, + required String operation, + Map> tags = const {}, +}) async { + final events = []; + final seenEventIds = {}; + int? until; + String? beforeId; for (var pageIndex = 0; pageIndex < _maxChannelDirectoryPages; pageIndex++) { final page = await session.queryRelay([ NostrFilter( - kinds: const [39000], + kinds: [kind], + tags: tags, limit: _channelDirectoryPageSize, - until: directoryUntil, - extensions: {'before_id': ?directoryBeforeId}, + until: until, + extensions: {'before_id': ?beforeId}, ), ]); - directoryMetas.addAll(page); - + if (page.isEmpty) break; var madeProgress = false; for (final event in page) { - final channelId = event.getTagValue('d'); - if (channelId != null && seenDirectoryChannelIds.add(channelId)) { + if (seenEventIds.add(event.id)) { + events.add(event); madeProgress = true; } } - if (!madeProgress || page.length < _channelDirectoryPageSize) break; + if (!madeProgress) break; final last = page.last; - directoryUntil = last.createdAt; - directoryBeforeId = last.id; + until = last.createdAt; + beforeId = last.id; if (pageIndex == _maxChannelDirectoryPages - 1) { - throw StateError( - 'Channel directory exceeded $_maxChannelDirectoryPages pages', - ); + throw StateError('$operation exceeded $_maxChannelDirectoryPages pages'); } } - return directoryMetas; + return events; } diff --git a/mobile/lib/features/channels/channels_page/browse_channels_sheet.dart b/mobile/lib/features/channels/channels_page/browse_channels_sheet.dart index 24cd1f654ea..9d1bb633716 100644 --- a/mobile/lib/features/channels/channels_page/browse_channels_sheet.dart +++ b/mobile/lib/features/channels/channels_page/browse_channels_sheet.dart @@ -1,11 +1,19 @@ part of '../channels_page.dart'; -class _BrowseChannelsSheet extends ConsumerWidget { +class _BrowseChannelsSheet extends HookConsumerWidget { const _BrowseChannelsSheet(); @override Widget build(BuildContext context, WidgetRef ref) { final channelsAsync = ref.watch(channelsProvider); + final directoryState = ref.watch(channelDirectoryLoadStatusProvider); + final activeDirectoryScope = channelDirectoryScope( + ref.watch(relayConfigProvider).baseUrl, + ref.watch(myPubkeyProvider), + ); + final directoryStatus = directoryState.scope == activeDirectoryScope + ? directoryState.status + : ChannelDirectoryLoadStatus.idle; final channels = channelsAsync.asData?.value .where((channel) => channel.canJoin) .toList(); @@ -14,6 +22,22 @@ class _BrowseChannelsSheet extends ConsumerWidget { left.name.toLowerCase().compareTo(right.name.toLowerCase()), ); + useEffect(() { + unawaited( + Future.microtask( + ref.read(channelsProvider.notifier).ensureDirectoryLoaded, + ), + ); + return null; + }, const []); + + final directoryIsLoading = + directoryStatus == ChannelDirectoryLoadStatus.idle || + directoryStatus == ChannelDirectoryLoadStatus.loading; + final directoryHasError = + directoryStatus == ChannelDirectoryLoadStatus.error || + channelsAsync.hasError; + return SafeArea( top: false, child: Padding( @@ -40,23 +64,36 @@ class _BrowseChannelsSheet extends ConsumerWidget { ], ), ), - if (channelsAsync.isLoading && channels == null) + if (directoryIsLoading && (channels == null || channels.isEmpty)) const SliverToBoxAdapter( child: Padding( padding: EdgeInsets.all(Grid.sm), child: Center(child: BuzzLoadingIndicator()), ), ) - else if (channelsAsync.hasError && channels == null) + else if (directoryHasError && + (channels == null || channels.isEmpty)) SliverToBoxAdapter( child: Padding( padding: const EdgeInsets.symmetric(vertical: Grid.sm), - child: Text( - 'Could not load open channels.', - textAlign: TextAlign.center, - style: context.textTheme.bodyMedium?.copyWith( - color: context.colors.onSurfaceVariant, - ), + child: Column( + children: [ + Text( + 'Couldn’t load open channels.', + textAlign: TextAlign.center, + style: context.textTheme.bodyMedium?.copyWith( + color: context.colors.onSurfaceVariant, + ), + ), + const SizedBox(height: Grid.xxs), + TextButton( + key: const Key('browse-channels-retry'), + onPressed: () => unawaited( + ref.read(channelsProvider.notifier).retryDirectory(), + ), + child: const Text('Try again'), + ), + ], ), ), ) diff --git a/mobile/lib/features/channels/channels_provider.dart b/mobile/lib/features/channels/channels_provider.dart index 39fbbdb7ba9..e225974f745 100644 --- a/mobile/lib/features/channels/channels_provider.dart +++ b/mobile/lib/features/channels/channels_provider.dart @@ -153,26 +153,7 @@ class ChannelsNotifier extends AsyncNotifier> { final session = ref.read(relaySessionProvider.notifier); // Step 1: find the channels I'm a member of via kind:39002. - final memberships = []; - { - int? until; - const pageSize = 500; - while (true) { - final page = await session.fetchHistory( - NostrFilter( - kinds: const [39002], - tags: { - '#p': [myPk], - }, - limit: pageSize, - until: until, - ), - ); - memberships.addAll(page); - if (page.length < pageSize) break; - until = page.map((e) => e.createdAt).reduce(min) - 1; - } - } + final memberships = await _fetchChannelMemberships(session, myPk); final memberChannelIds = memberships .map((e) => e.getTagValue('d')) .whereType() @@ -192,12 +173,22 @@ class ChannelsNotifier extends AsyncNotifier> { // private channels and DMs below so discovery fails closed if that contract // ever regresses. The composite cursor preserves tied-timestamp rows. if (fetchDirectory) { + final directoryScope = channelDirectoryScope( + ref.read(relayConfigProvider).baseUrl, + myPk, + ); + final directoryStatus = ref.read( + channelDirectoryLoadStatusProvider.notifier, + ); + directoryStatus.markLoading(directoryScope); try { _directoryMetas = await _fetchChannelDirectoryMetas(session); - } catch (error) { + directoryStatus.markLoaded(directoryScope); + } catch (error, stackTrace) { + directoryStatus.markError(directoryScope); debugPrint( '[ChannelsNotifier] channel directory refresh failed; retaining ' - 'cached discovery: $error', + 'cached discovery: $error\n$stackTrace', ); } } @@ -922,6 +913,50 @@ class ChannelsNotifier extends AsyncNotifier> { state = await AsyncValue.guard(() => _fetch(subscribeLive: true)); } + /// Loads the directory when Browse channels opens after startup or an error. + Future ensureDirectoryLoaded() async { + final directoryState = ref.read(channelDirectoryLoadStatusProvider); + final scope = channelDirectoryScope( + ref.read(relayConfigProvider).baseUrl, + ref.read(myPubkeyProvider), + ); + if (directoryState.scope == scope && + (directoryState.status == ChannelDirectoryLoadStatus.loading || + directoryState.status == ChannelDirectoryLoadStatus.loaded)) { + return; + } + await retryDirectory(); + } + + /// Retries channel discovery while retaining the current channel list. + Future retryDirectory() async { + final previousChannels = state.value; + final directoryStatus = ref.read( + channelDirectoryLoadStatusProvider.notifier, + ); + final scope = channelDirectoryScope( + ref.read(relayConfigProvider).baseUrl, + ref.read(myPubkeyProvider), + ); + final directoryState = ref.read(channelDirectoryLoadStatusProvider); + if (directoryState.scope == scope && + directoryState.status == ChannelDirectoryLoadStatus.loading) { + return; + } + if (ref.read(relaySessionProvider).status != SessionStatus.connected) { + directoryStatus.markError(scope); + return; + } + try { + state = AsyncData(await _fetch(subscribeLive: true)); + } catch (error, stackTrace) { + directoryStatus.markError(scope); + state = previousChannels == null + ? AsyncError(error, stackTrace) + : AsyncData(previousChannels); + } + } + void _clearLiveSubscriptions() { _subscriptionVersion++; _desiredLiveChannels = const []; diff --git a/mobile/test/features/channels/channels_page_test.dart b/mobile/test/features/channels/channels_page_test.dart index 3224fd3d942..914a30e237e 100644 --- a/mobile/test/features/channels/channels_page_test.dart +++ b/mobile/test/features/channels/channels_page_test.dart @@ -1454,6 +1454,57 @@ void main() { expect(find.text('No open channels available to join.'), findsOneWidget); }); + testWidgets('browse action retries an initial directory request problem', ( + tester, + ) async { + final joinable = Channel( + id: 'retry-discovery', + name: 'community-help', + channelType: 'stream', + visibility: 'open', + description: 'Help from the community', + createdBy: 'abc', + createdAt: DateTime(2025), + memberCount: 0, + ); + late _RetryingDirectoryNotifier notifier; + await tester.pumpWidget( + buildTestable( + disableAnimations: true, + overrides: [ + channelsProvider.overrideWith( + () => notifier = _RetryingDirectoryNotifier( + initialChannels: testChannels, + retriedChannels: [...testChannels, joinable], + ), + ), + ], + ), + ); + await tester.pumpAndSettle(); + + await tester.tap(find.byTooltip('Create or start conversation')); + await tester.pump(); + await tester.tap( + find.byKey(const Key('quick-action-browse-channels-card')), + ); + await tester.pumpAndSettle(); + + expect(find.text('Couldn’t load open channels.'), findsOneWidget); + expect(find.text('No open channels available to join.'), findsNothing); + expect(find.byKey(const Key('browse-channels-retry')), findsOneWidget); + + await tester.tap(find.byKey(const Key('browse-channels-retry'))); + await tester.pumpAndSettle(); + + expect(notifier.retryCount, 1); + expect(find.text('Couldn’t load open channels.'), findsNothing); + expect( + find.byKey(const Key('browse-channel-retry-discovery')), + findsOneWidget, + ); + }); + testWidgets('browse action scrolls and joins an offscreen channel', ( tester, ) async { @@ -2210,6 +2261,13 @@ class _FakeNotifier extends ChannelsNotifier { @override Future> build() async => _channels; + @override + Future ensureDirectoryLoaded() async { + ref + .read(channelDirectoryLoadStatusProvider.notifier) + .markLoaded(_activeDirectoryScope(ref)); + } + @override Map get latestObservedByChannel => { for (final entry in _observedEventsByChannel.entries) @@ -2224,6 +2282,45 @@ class _FakeNotifier extends ChannelsNotifier { get observedUnreadEventsByChannel => _observedEventsByChannel; } +class _RetryingDirectoryNotifier extends ChannelsNotifier { + _RetryingDirectoryNotifier({ + required this.initialChannels, + required this.retriedChannels, + }); + + final List initialChannels; + final List retriedChannels; + int retryCount = 0; + + @override + Future> build() async => initialChannels; + + @override + Future ensureDirectoryLoaded() async { + ref + .read(channelDirectoryLoadStatusProvider.notifier) + .markError(_activeDirectoryScope(ref)); + } + + @override + Future retryDirectory() async { + retryCount++; + ref + .read(channelDirectoryLoadStatusProvider.notifier) + .markLoading(_activeDirectoryScope(ref)); + await Future.delayed(Duration.zero); + state = AsyncData(retriedChannels); + ref + .read(channelDirectoryLoadStatusProvider.notifier) + .markLoaded(_activeDirectoryScope(ref)); + } +} + +String _activeDirectoryScope(Ref ref) => channelDirectoryScope( + ref.read(relayConfigProvider).baseUrl, + ref.read(myPubkeyProvider), +); + class _RecordingChannelActions extends ChannelActions { _RecordingChannelActions(Ref ref) : super( diff --git a/mobile/test/features/channels/channels_provider_test.dart b/mobile/test/features/channels/channels_provider_test.dart index a5eb1d751e4..ad604b0db02 100644 --- a/mobile/test/features/channels/channels_provider_test.dart +++ b/mobile/test/features/channels/channels_provider_test.dart @@ -9,8 +9,8 @@ import 'package:buzz/shared/relay/relay.dart'; /// Tests for [ChannelsNotifier] in the pure-Nostr world. /// -/// The provider performs a three-step WS query: -/// 1. kind:39002 memberships tagged `#p:` +/// The provider performs a three-step relay query: +/// 1. paginated kind:39002 memberships tagged `#p:` /// 2. kind:39000 metadata for those channel ids /// 3. paginated kind:39000 metadata for discoverable open channels /// then layers per-channel live subscriptions on the `#h` tag. @@ -82,11 +82,76 @@ void main() { expect(channels, hasLength(501)); final directoryFilters = session.directoryQueryFilters; - expect(directoryFilters, hasLength(2)); + expect(directoryFilters, hasLength(3)); expect(directoryFilters.first.until, isNull); expect(directoryFilters.first.extensions, isEmpty); - expect(directoryFilters.last.until, firstPage.last.createdAt); - expect(directoryFilters.last.extensions['before_id'], firstPage.last.id); + expect(directoryFilters[1].until, firstPage.last.createdAt); + expect(directoryFilters[1].extensions['before_id'], firstPage.last.id); + expect(directoryFilters.last.until, finalChannel.createdAt); + expect(directoryFilters.last.extensions['before_id'], finalChannel.id); + }); + + test( + 'paginates memberships when the relay caps responses below limit', + () async { + final firstPage = List.generate( + 100, + (index) => _membership( + '${index.toString().padLeft(8, '0')}-0000-4000-8000-000000000000', + myPk, + ), + ); + final finalChannelId = '99999999-9999-4999-8999-999999999999'; + final finalMembership = _membership(finalChannelId, myPk); + final session = _FakeRelaySession( + memberships: const [], + membershipPages: [ + firstPage, + [finalMembership], + ], + metadata: [_meta(id: finalChannelId, name: 'last-membership')], + ); + final container = _buildContainer(session: session); + addTearDown(container.dispose); + + final channels = await container.read(channelsProvider.future); + + expect(channels.single.id, finalChannelId); + expect(channels.single.isMember, isTrue); + expect(session.membershipQueryFilters, hasLength(3)); + expect(session.membershipQueryFilters.first.until, isNull); + expect(session.membershipQueryFilters.first.extensions, isEmpty); + expect(session.membershipQueryFilters[1].until, firstPage.last.createdAt); + expect( + session.membershipQueryFilters[1].extensions['before_id'], + firstPage.last.id, + ); + expect( + session.membershipQueryFilters.last.extensions['before_id'], + finalMembership.id, + ); + }, + ); + + test('stops membership pagination when the relay repeats a page', () async { + final repeatedPage = List.generate( + 500, + (index) => _membership( + '${index.toString().padLeft(8, '0')}-0000-4000-8000-000000000000', + myPk, + ), + ); + final session = _FakeRelaySession( + memberships: const [], + membershipPages: [repeatedPage], + repeatLastMembershipPage: true, + maxMembershipPageRequests: 2, + ); + final container = _buildContainer(session: session); + addTearDown(container.dispose); + + expect(await container.read(channelsProvider.future), isEmpty); + expect(session.membershipRequestCount, 2); }); test('stops channel discovery when the relay repeats a full page', () async { @@ -113,24 +178,31 @@ void main() { expect(session.metadataPageRequestCount, 2); }); - test('directory page-cap failure does not fail channel loading', () async { - final session = _FakeRelaySession( - memberships: const [], - metadataPageBuilder: (pageIndex) => List.generate( - 500, - (eventIndex) => _meta( - id: 'channel-$pageIndex-$eventIndex', - name: 'channel-$pageIndex-$eventIndex', - createdAt: 1000 - pageIndex, + test( + 'directory page-cap failure is distinct from an empty directory', + () async { + final session = _FakeRelaySession( + memberships: const [], + metadataPageBuilder: (pageIndex) => List.generate( + 500, + (eventIndex) => _meta( + id: 'channel-$pageIndex-$eventIndex', + name: 'channel-$pageIndex-$eventIndex', + createdAt: 1000 - pageIndex, + ), ), - ), - ); - final container = _buildContainer(session: session); - addTearDown(container.dispose); + ); + final container = _buildContainer(session: session); + addTearDown(container.dispose); - expect(await container.read(channelsProvider.future), isEmpty); - expect(session.metadataPageRequestCount, 100); - }); + expect(await container.read(channelsProvider.future), isEmpty); + expect(session.metadataPageRequestCount, 100); + expect( + container.read(channelDirectoryLoadStatusProvider).status, + ChannelDirectoryLoadStatus.error, + ); + }, + ); test( 'directory failure retains discovery while membership refreshes', @@ -174,9 +246,40 @@ void main() { refreshed.firstWhere((channel) => channel.id == _channelD).isMember, isTrue, ); + expect( + container.read(channelDirectoryLoadStatusProvider).status, + ChannelDirectoryLoadStatus.error, + ); + + await container.read(channelsProvider.notifier).retryDirectory(); + + expect( + container.read(channelDirectoryLoadStatusProvider).status, + ChannelDirectoryLoadStatus.loaded, + ); }, ); + test('directory retry failure retains the current channel list', () async { + final session = _FakeRelaySession( + memberships: [_membership(_channelA, myPk)], + metadata: [_meta(id: _channelA, name: 'general')], + ); + final container = _buildContainer(session: session); + addTearDown(container.dispose); + + final initial = await container.read(channelsProvider.future); + session.membershipFailures = 1; + + await container.read(channelsProvider.notifier).retryDirectory(); + + expect(container.read(channelsProvider).requireValue, initial); + expect( + container.read(channelDirectoryLoadStatusProvider).status, + ChannelDirectoryLoadStatus.error, + ); + }); + test('reconnect backstop does not refetch the channel directory', () async { final session = _FakeRelaySession( memberships: [_membership(_channelA, myPk)], @@ -842,13 +945,17 @@ void main() { await container.read(channelsProvider.future); - // Two history fetches for channel loading, plus one per non-DM channel - // for high-priority event backfill. - expect(session.historyFilters.length, greaterThanOrEqualTo(2)); - expect(session.historyFilters[0].kinds, [39002]); - expect(session.historyFilters[0].tags['#p'], [myPk]); - expect(session.historyFilters[1].kinds, [39000]); - expect(session.historyFilters[1].tags['#d'], [_channelA]); + expect(session.membershipQueryFilters, isNotEmpty); + expect(session.membershipQueryFilters.first.kinds, [39002]); + expect(session.membershipQueryFilters.first.tags['#p'], [myPk]); + expect( + session.historyFilters.any( + (filter) => + filter.kinds.contains(39000) && + filter.tags['#d']?.contains(_channelA) == true, + ), + isTrue, + ); // And one live subscription on the resulting channel. expect(session.subscribeFilters, hasLength(1)); @@ -938,11 +1045,14 @@ Future _waitUntil(bool Function() predicate) async { fail('Timed out waiting for asynchronous provider work'); } -/// Fake [RelaySessionNotifier] that returns canned events from [fetchHistory] -/// and records subscribe calls. +/// Fake [RelaySessionNotifier] that returns canned query results and records +/// subscriptions. class _FakeRelaySession extends RelaySessionNotifier { _FakeRelaySession({ required this.memberships, + this.membershipPages, + this.repeatLastMembershipPage = false, + this.maxMembershipPageRequests, this.metadata = const [], this.metadataPages, this.metadataPageBuilder, @@ -954,6 +1064,9 @@ class _FakeRelaySession extends RelaySessionNotifier { }); List memberships; + final List>? membershipPages; + final bool repeatLastMembershipPage; + final int? maxMembershipPageRequests; List metadata; final List>? metadataPages; final List Function(int pageIndex)? metadataPageBuilder; @@ -969,6 +1082,7 @@ class _FakeRelaySession extends RelaySessionNotifier { final List historyFilters = []; final List> queryBatches = []; final List directoryQueryFilters = []; + final List membershipQueryFilters = []; final List subscribeFilters = []; final Map _subscriptions = {}; int _nextSubscriptionKey = 0; @@ -1048,6 +1162,38 @@ class _FakeRelaySession extends RelaySessionNotifier { List filters, { Duration timeout = const Duration(seconds: 8), }) async { + if (filters case [final filter] + when filter.kinds.length == 1 && + filter.kinds.single == 39002 && + filter.tags['#p'] != null) { + membershipQueryFilters.add(filter); + if (membershipFailures > 0) { + membershipFailures--; + throw Exception('membership fetch failed'); + } + final requestIndex = membershipRequestCount++; + final maxRequests = maxMembershipPageRequests; + if (maxRequests != null && requestIndex >= maxRequests) { + throw StateError('Unexpected membership page request'); + } + final pages = membershipPages; + if (pages != null) { + if (requestIndex < pages.length) return List.of(pages[requestIndex]); + if (repeatLastMembershipPage && pages.isNotEmpty) { + return List.of(pages.last); + } + return const []; + } + if (filter.until != null) return const []; + final myPk = filter.tags['#p']?.single; + return memberships + .where( + (event) => event.tags.any( + (tag) => tag.length >= 2 && tag[0] == 'p' && tag[1] == myPk, + ), + ) + .toList(); + } if (filters case [final filter] when filter.kinds.length == 1 && filter.kinds.single == 39000 && @@ -1072,7 +1218,7 @@ class _FakeRelaySession extends RelaySessionNotifier { } return const []; } - return List.of(metadata); + return filter.until == null ? List.of(metadata) : const []; } queryBatches.add(filters); return recentMessages.where((event) { From fafe4cb74452dd4ad1553c399426b9b6741a5cca Mon Sep 17 00:00:00 2001 From: Tom Brow Date: Wed, 19 Aug 2026 12:24:19 -0700 Subject: [PATCH 07/14] fix(mobile): load channel directory on demand Signed-off-by: Tom Brow --- .../features/channels/channels_provider.dart | 18 ++--- mobile/lib/features/search/search_page.dart | 16 ++-- .../channels/channels_provider_test.dart | 77 +++++++++++++++---- .../features/search/search_page_test.dart | 37 +++++++++ 4 files changed, 114 insertions(+), 34 deletions(-) diff --git a/mobile/lib/features/channels/channels_provider.dart b/mobile/lib/features/channels/channels_provider.dart index e225974f745..1bfabd8d2ac 100644 --- a/mobile/lib/features/channels/channels_provider.dart +++ b/mobile/lib/features/channels/channels_provider.dart @@ -27,13 +27,11 @@ const _authoredRootIdsPrefix = 'buzz-thread-authored.v1'; /// Loads the user's channel list from the relay over WebSocket. /// -/// Three-step query: -/// 1. Fetch kind:39002 membership events tagged `#p:` to find -/// the channel ids I'm a member of. -/// 2. Fetch the corresponding kind:39000 channel metadata events. -/// 3. Fetch the paginated kind:39000 directory so open channels that the -/// user has not joined remain discoverable. +/// Membership loading resolves kind:39002 events tagged `#p:`, +/// then fetches kind:39000 metadata for those channel ids. /// +/// The paginated kind:39000 directory is fetched separately when Browse +/// channels opens, so discovery never delays the main Conversations screen. /// Live updates are layered on top via per-channel subscriptions on the /// `#h` tag for any of the visible channel event kinds — incoming events /// bump `lastMessageAt` for that channel. @@ -130,7 +128,7 @@ class ChannelsNotifier extends AsyncNotifier> { Future> _fetch({ bool subscribeLive = false, bool fetchLastMessage = true, - bool fetchDirectory = true, + bool fetchDirectory = false, }) async { final channels = await _fetchChannels( subscribeLive: subscribeLive, @@ -144,7 +142,7 @@ class ChannelsNotifier extends AsyncNotifier> { Future> _fetchChannels({ bool subscribeLive = false, bool fetchLastMessage = true, - bool fetchDirectory = true, + bool fetchDirectory = false, }) async { final myPk = ref.read(myPubkeyProvider); if (myPk == null) throw StateError('No signing identity available'); @@ -948,7 +946,9 @@ class ChannelsNotifier extends AsyncNotifier> { return; } try { - state = AsyncData(await _fetch(subscribeLive: true)); + state = AsyncData( + await _fetch(subscribeLive: true, fetchDirectory: true), + ); } catch (error, stackTrace) { directoryStatus.markError(scope); state = previousChannels == null diff --git a/mobile/lib/features/search/search_page.dart b/mobile/lib/features/search/search_page.dart index 563a46c9e8b..514629b1f72 100644 --- a/mobile/lib/features/search/search_page.dart +++ b/mobile/lib/features/search/search_page.dart @@ -644,12 +644,10 @@ class _RecentSearches extends StatelessWidget { class _ChannelsSection extends StatelessWidget { final List channels; final VoidCallback onResultSelected; - const _ChannelsSection({ required this.channels, required this.onResultSelected, }); - @override Widget build(BuildContext context) { return Column( @@ -670,12 +668,14 @@ class _ChannelsSection extends StatelessWidget { key: ValueKey('search-channel-title-${channel.id}'), style: contentListTitleTextStyle, ), - subtitle: Text( - '${channel.memberCount} member${channel.memberCount == 1 ? '' : 's'}', - style: contentListBodyTextStyle.copyWith( - color: context.colors.onSurfaceVariant, - ), - ), + subtitle: channel.isMember + ? Text( + '${channel.memberCount} member${channel.memberCount == 1 ? '' : 's'}', + style: contentListBodyTextStyle.copyWith( + color: context.colors.onSurfaceVariant, + ), + ) + : null, trailing: !channel.isMember && !channel.isDm ? Container( padding: const EdgeInsets.symmetric( diff --git a/mobile/test/features/channels/channels_provider_test.dart b/mobile/test/features/channels/channels_provider_test.dart index ad604b0db02..12bcce520d1 100644 --- a/mobile/test/features/channels/channels_provider_test.dart +++ b/mobile/test/features/channels/channels_provider_test.dart @@ -9,11 +9,11 @@ import 'package:buzz/shared/relay/relay.dart'; /// Tests for [ChannelsNotifier] in the pure-Nostr world. /// -/// The provider performs a three-step relay query: +/// The provider loads membership-backed channels first: /// 1. paginated kind:39002 memberships tagged `#p:` /// 2. kind:39000 metadata for those channel ids -/// 3. paginated kind:39000 metadata for discoverable open channels -/// then layers per-channel live subscriptions on the `#h` tag. +/// then layers per-channel live subscriptions on the `#h` tag. Browse channels +/// separately triggers paginated kind:39000 open-channel discovery. /// /// Tests stub out the relay session by overriding [relaySessionProvider] with /// a [_FakeRelaySession] that returns canned events from [fetchHistory] and @@ -36,21 +36,17 @@ void main() { final container = _buildContainer(session: session); addTearDown(container.dispose); - final channels = await container.read(channelsProvider.future); + expect(await container.read(channelsProvider.future), isEmpty); + expect(session.directoryQueryFilters, isEmpty); + + await container.read(channelsProvider.notifier).retryDirectory(); + final channels = container.read(channelsProvider).requireValue; expect(channels, hasLength(1)); expect(channels.single.id, _channelA); expect(channels.single.isMember, isFalse); expect(session.subscribeFilters, isEmpty); - expect( - session.directoryQueryFilters.any( - (filter) => - filter.kinds.length == 1 && - filter.kinds.single == 39000 && - !filter.tags.containsKey('#d'), - ), - isTrue, - ); + expect(session.directoryQueryFilters, isNotEmpty); }, ); @@ -78,7 +74,9 @@ void main() { final container = _buildContainer(session: session); addTearDown(container.dispose); - final channels = await container.read(channelsProvider.future); + expect(await container.read(channelsProvider.future), isEmpty); + await container.read(channelsProvider.notifier).retryDirectory(); + final channels = container.read(channelsProvider).requireValue; expect(channels, hasLength(501)); final directoryFilters = session.directoryQueryFilters; @@ -172,7 +170,9 @@ void main() { final container = _buildContainer(session: session); addTearDown(container.dispose); - final channels = await container.read(channelsProvider.future); + expect(await container.read(channelsProvider.future), isEmpty); + await container.read(channelsProvider.notifier).retryDirectory(); + final channels = container.read(channelsProvider).requireValue; expect(channels, hasLength(500)); expect(session.metadataPageRequestCount, 2); @@ -196,6 +196,7 @@ void main() { addTearDown(container.dispose); expect(await container.read(channelsProvider.future), isEmpty); + await container.read(channelsProvider.notifier).retryDirectory(); expect(session.metadataPageRequestCount, 100); expect( container.read(channelDirectoryLoadStatusProvider).status, @@ -221,6 +222,14 @@ void main() { (await container.read( channelsProvider.future, )).map((channel) => channel.id), + [_channelA], + ); + await container.read(channelsProvider.notifier).retryDirectory(); + expect( + container + .read(channelsProvider) + .requireValue + .map((channel) => channel.id), unorderedEquals([_channelA, _channelB]), ); @@ -235,7 +244,7 @@ void main() { ]; session.directoryFailures = 1; - await container.read(channelsProvider.notifier).refresh(); + await container.read(channelsProvider.notifier).retryDirectory(); final refreshed = container.read(channelsProvider).requireValue; expect( @@ -304,6 +313,33 @@ void main() { expect(session.metadataPageRequestCount, initialDirectoryRequests); }); + test('membership refresh does not refetch a loaded directory', () async { + final session = _FakeRelaySession( + memberships: [_membership(_channelA, myPk)], + metadata: [ + _meta(id: _channelA, name: 'general'), + _meta(id: _channelB, name: 'discoverable'), + ], + ); + final container = _buildContainer(session: session); + addTearDown(container.dispose); + + await container.read(channelsProvider.future); + await container.read(channelsProvider.notifier).retryDirectory(); + final directoryRequestCount = session.metadataPageRequestCount; + + await container.read(channelsProvider.notifier).refresh(); + + expect(session.metadataPageRequestCount, directoryRequestCount); + expect( + container + .read(channelsProvider) + .requireValue + .map((channel) => channel.id), + unorderedEquals([_channelA, _channelB]), + ); + }); + test('deduplicates joined channels from directory discovery', () async { final session = _FakeRelaySession( memberships: [_membership(_channelA, myPk)], @@ -315,7 +351,14 @@ void main() { final container = _buildContainer(session: session); addTearDown(container.dispose); - final channels = await container.read(channelsProvider.future); + expect( + (await container.read( + channelsProvider.future, + )).map((channel) => channel.id), + [_channelA], + ); + await container.read(channelsProvider.notifier).retryDirectory(); + final channels = container.read(channelsProvider).requireValue; expect(channels.map((channel) => channel.id), [_channelA, _channelB]); expect(channels.first.isMember, isTrue); diff --git a/mobile/test/features/search/search_page_test.dart b/mobile/test/features/search/search_page_test.dart index e2951323600..8b4c903b564 100644 --- a/mobile/test/features/search/search_page_test.dart +++ b/mobile/test/features/search/search_page_test.dart @@ -930,6 +930,43 @@ void main() { expect(content.agentMentionPubkeys, contains(agentPubkey)); expect(find.byIcon(LucideIcons.bot), findsOneWidget); }); + + testWidgets('does not label an unjoined channel as having zero members', ( + tester, + ) async { + final state = SearchState( + query: 'community', + channelResults: [ + Channel( + id: 'community-help', + name: 'community-help', + channelType: 'stream', + visibility: 'open', + description: 'Help from the community', + createdBy: 'test', + createdAt: DateTime(2025), + memberCount: 0, + ), + ], + ); + + await tester.pumpWidget( + WidgetHelpers.testable( + overrides: [ + searchProvider.overrideWith(() => _FakeSearchNotifier(state)), + recentSearchesProvider.overrideWith( + () => _FakeRecentSearchesNotifier(const []), + ), + profileProvider.overrideWith(() => _FakeProfileNotifier()), + ], + child: const SearchPage(), + ), + ); + await tester.pumpAndSettle(); + + expect(find.text('Open'), findsOneWidget); + expect(find.text('0 members'), findsNothing); + }); } class _FakeSearchNotifier extends SearchNotifier { From a5fc7c4d836cd0a22b940d92e95079294a9bc160 Mon Sep 17 00:00:00 2001 From: leader <71e9f2c44a6932b6772caaaccda1911d010463c3e2c6c40410b8329956046801@buzz.block.builderlab.xyz> Date: Fri, 21 Aug 2026 11:42:10 -0700 Subject: [PATCH 08/14] fix(mobile): fence channel directory responses to their scope A community or identity switch changes which tenant the channel directory belongs to, but an in-flight directory query kept writing its response into whatever scope was active when it landed. The user switched community, and the previous community's open channels appeared in the new community's list. The load status had the same hole on the failure path: a request that failed after the switch marked the new scope as errored. VISION.md describes tenant isolation as a boundary, not a filter, so a retired response is discarded rather than merged. The fix captures the relay-and-identity scope plus a monotonic generation at request time, then re-checks both after the await and before every write, on the success path and the failure path alike. A retired request throws _StaleDirectoryRequest, which retryDirectory swallows so it writes neither the channel list nor the load status. An in-scope failure still returns null, which keeps the existing "retain the cached discovery" behavior. The loader lives in the sibling part file channel_directory.dart because channels_provider.dart already sits at the 1000-line ceiling that `just file-size-check` enforces, so it may not grow at all. Its inline fetch block moved into the loader and a `forRef` factory keeps the construction to one line, which leaves the provider slightly smaller than before. The split is mechanical: the fence moved out, nothing else changed shape. Tests: four new deterministic arms in channels_provider_test.dart, covering stale success and stale failure for both a community switch and an identity switch. The fake relay session gained a pausable directory query so the switch can be interleaved between request and response, and a request-time snapshot so a paused response reflects the community that issued it. All four failed against the unfixed provider, and the first reproduces the reported symptom exactly. Positive control: mutating the fence's isCurrent() to `=> true` turns all four red again, so the fence and not the harness is what makes them pass. Suite moved from 1560 to 1564 tests, all passing. `just mobile-check` and `just file-size-check` are clean. Co-authored-by: Tom Brow Signed-off-by: Tom Brow --- .../features/channels/channel_directory.dart | 84 +++++++ .../features/channels/channels_provider.dart | 32 ++- .../channels/channels_provider_test.dart | 236 +++++++++++++++++- 3 files changed, 332 insertions(+), 20 deletions(-) diff --git a/mobile/lib/features/channels/channel_directory.dart b/mobile/lib/features/channels/channel_directory.dart index fcb851526bc..81d6a7d5ed1 100644 --- a/mobile/lib/features/channels/channel_directory.dart +++ b/mobile/lib/features/channels/channel_directory.dart @@ -89,6 +89,90 @@ Future> _fetchChannelDirectoryMetas( operation: 'Channel directory', ); +/// Thrown when a directory request is retired before its response lands. +/// +/// Callers must treat this as "write nothing": the scope that issued the +/// request is no longer active, so both its data and its load status belong to +/// a community the user has left. +class _StaleDirectoryRequest implements Exception { + const _StaleDirectoryRequest(); + + @override + String toString() => + 'Channel directory request retired by a community or identity switch'; +} + +/// Loads the open-channel directory fenced to the scope that requested it. +/// +/// A community or identity switch changes the scope, and a newer request bumps +/// the generation. Either one retires an in-flight request, so a delayed +/// response can never populate the current community's state. This is the +/// tenant boundary described in VISION.md: isolation is the boundary, not a +/// filter, so a retired response is discarded rather than merged. +/// +/// Lives in this part file because `channels_provider.dart` sits against the +/// repository-wide 1000-line file ceiling enforced by `just file-size-check`. +class _ChannelDirectoryLoader { + /// Resolves the relay-and-identity scope that is active right now. + final String Function() currentScope; + + /// Owns the externally observable directory load status. + final ChannelDirectoryLoadNotifier Function() loadStatus; + + int _generation = 0; + + _ChannelDirectoryLoader({ + required this.currentScope, + required this.loadStatus, + }); + + /// Binds the fence to a notifier's [Ref] so the provider needs one line. + /// + /// Both closures read rather than watch: the fence asks what the scope is + /// right now, and must not make the notifier depend on it. + factory _ChannelDirectoryLoader.forRef(Ref ref) => _ChannelDirectoryLoader( + currentScope: () => channelDirectoryScope( + ref.read(relayConfigProvider).baseUrl, + ref.read(myPubkeyProvider), + ), + loadStatus: () => ref.read(channelDirectoryLoadStatusProvider.notifier), + ); + + /// Retires any in-flight request without starting a new one. + /// + /// Called when the relay or identity changes so a response already on the + /// wire cannot be written into the new scope. + void retireInFlight() => _generation++; + + /// Fetches the directory, or throws [_StaleDirectoryRequest] if retired. + /// + /// Returns null when the request failed inside the current scope, which means + /// "retain the cached discovery". The fence is re-checked after the await and + /// before every write, on both the success and the failure path. + Future?> load(RelaySessionNotifier session) async { + final scope = currentScope(); + final generation = ++_generation; + bool isCurrent() => generation == _generation && scope == currentScope(); + + loadStatus().markLoading(scope); + final List metas; + try { + metas = await _fetchChannelDirectoryMetas(session); + } catch (error, stackTrace) { + if (!isCurrent()) throw const _StaleDirectoryRequest(); + loadStatus().markError(scope); + debugPrint( + '[ChannelsNotifier] channel directory refresh failed; retaining ' + 'cached discovery: $error\n$stackTrace', + ); + return null; + } + if (!isCurrent()) throw const _StaleDirectoryRequest(); + loadStatus().markLoaded(scope); + return metas; + } +} + Future> _fetchPaginatedChannelEvents( RelaySessionNotifier session, { required int kind, diff --git a/mobile/lib/features/channels/channels_provider.dart b/mobile/lib/features/channels/channels_provider.dart index 1bfabd8d2ac..4840104de6a 100644 --- a/mobile/lib/features/channels/channels_provider.dart +++ b/mobile/lib/features/channels/channels_provider.dart @@ -57,6 +57,10 @@ class ChannelsNotifier extends AsyncNotifier> { Map> _memberSnapshotsByChannelId = const {}; List _directoryMetas = const []; + /// Fences directory responses to the relay and identity that requested them. + late final _ChannelDirectoryLoader _directoryLoader = + _ChannelDirectoryLoader.forRef(ref); + /// The member snapshot already returned while loading the channel list. /// /// Mention autocomplete can use this synchronously while its independent @@ -84,6 +88,9 @@ class ChannelsNotifier extends AsyncNotifier> { _memberSnapshotPubkey = pubkey; _memberSnapshotsByChannelId = const {}; _directoryMetas = const []; + // Retire any in-flight directory request: its response describes the + // previous relay or identity and must not reach this scope's state. + _directoryLoader.retireInFlight(); } final connected = Completer(); final sessionState = ref.read(relaySessionProvider); @@ -171,24 +178,8 @@ class ChannelsNotifier extends AsyncNotifier> { // private channels and DMs below so discovery fails closed if that contract // ever regresses. The composite cursor preserves tied-timestamp rows. if (fetchDirectory) { - final directoryScope = channelDirectoryScope( - ref.read(relayConfigProvider).baseUrl, - myPk, - ); - final directoryStatus = ref.read( - channelDirectoryLoadStatusProvider.notifier, - ); - directoryStatus.markLoading(directoryScope); - try { - _directoryMetas = await _fetchChannelDirectoryMetas(session); - directoryStatus.markLoaded(directoryScope); - } catch (error, stackTrace) { - directoryStatus.markError(directoryScope); - debugPrint( - '[ChannelsNotifier] channel directory refresh failed; retaining ' - 'cached discovery: $error\n$stackTrace', - ); - } + final metas = await _directoryLoader.load(session); + if (metas != null) _directoryMetas = metas; } // Merge and dedupe by `d` tag. Kind:39000 is parameterized-replaceable, @@ -949,6 +940,11 @@ class ChannelsNotifier extends AsyncNotifier> { state = AsyncData( await _fetch(subscribeLive: true, fetchDirectory: true), ); + } on _StaleDirectoryRequest { + // A community or identity switch retired this request. Its response + // describes a scope the user has left, so write neither the channel list + // nor the load status; the new scope owns both now. + return; } catch (error, stackTrace) { directoryStatus.markError(scope); state = previousChannels == null diff --git a/mobile/test/features/channels/channels_provider_test.dart b/mobile/test/features/channels/channels_provider_test.dart index 12bcce520d1..1526f55c7a1 100644 --- a/mobile/test/features/channels/channels_provider_test.dart +++ b/mobile/test/features/channels/channels_provider_test.dart @@ -289,6 +289,177 @@ void main() { ); }); + test( + 'community switch discards a stale directory success from the old relay', + () async { + final session = _FakeRelaySession( + memberships: const [], + metadata: [_meta(id: _channelA, name: 'community-a-open')], + ); + final container = _buildContainer(session: session); + addTearDown(container.dispose); + + expect(await container.read(channelsProvider.future), isEmpty); + + session.pauseNextDirectoryQuery(); + final staleDirectory = container + .read(channelsProvider.notifier) + .retryDirectory(); + await session.nextDirectoryQueryStarted; + + // Switch communities while community A's directory response is paused. + session.memberships = [_membership(_channelB, myPk)]; + session.metadata = [_meta(id: _channelB, name: 'community-b-general')]; + container + .read(relayConfigProvider.notifier) + .update(baseUrl: 'https://community-b.example'); + await container.read(channelsProvider.future); + + session.resumePausedDirectoryQuery(); + await staleDirectory; + await _settle(); + + final ids = container + .read(channelsProvider) + .requireValue + .map((channel) => channel.id); + expect(ids, isNot(contains(_channelA))); + expect(ids, contains(_channelB)); + expect( + container.read(channelDirectoryLoadStatusProvider).scope, + isNot( + channelDirectoryScope( + 'https://community-b.example', + container.read(myPubkeyProvider), + ), + ), + ); + }, + ); + + test( + 'community switch discards a stale directory failure from the old relay', + () async { + final session = _FakeRelaySession( + memberships: const [], + metadata: [_meta(id: _channelA, name: 'community-a-open')], + ); + final container = _buildContainer(session: session); + addTearDown(container.dispose); + + expect(await container.read(channelsProvider.future), isEmpty); + + session.pauseNextDirectoryQuery(); + final staleDirectory = container + .read(channelsProvider.notifier) + .retryDirectory(); + await session.nextDirectoryQueryStarted; + + session.memberships = [_membership(_channelB, myPk)]; + session.metadata = [_meta(id: _channelB, name: 'community-b-general')]; + container + .read(relayConfigProvider.notifier) + .update(baseUrl: 'https://community-b.example'); + await container.read(channelsProvider.future); + + // Community A's directory request fails after the switch. + session.directoryFailures = 1; + session.resumePausedDirectoryQuery(); + await staleDirectory; + await _settle(); + + final ids = container + .read(channelsProvider) + .requireValue + .map((channel) => channel.id); + expect(ids, contains(_channelB)); + expect( + container.read(channelDirectoryLoadStatusProvider).scope, + isNot( + channelDirectoryScope( + 'https://community-b.example', + container.read(myPubkeyProvider), + ), + ), + ); + }, + ); + + test( + 'identity switch discards a stale directory success from the old identity', + () async { + final session = _FakeRelaySession( + memberships: const [], + metadata: [_meta(id: _channelA, name: 'first-identity-open')], + ); + final container = _buildContainer(session: session); + addTearDown(container.dispose); + + expect(await container.read(channelsProvider.future), isEmpty); + + session.pauseNextDirectoryQuery(); + final staleDirectory = container + .read(channelsProvider.notifier) + .retryDirectory(); + await session.nextDirectoryQueryStarted; + + // Switch signing identity while the first identity's response is paused. + session.memberships = [_membership(_channelB, _otherPk)]; + session.metadata = [_meta(id: _channelB, name: 'second-identity-joined')]; + container.read(_testPubkeyProvider.notifier).set(_otherPk); + await container.read(channelsProvider.future); + + session.resumePausedDirectoryQuery(); + await staleDirectory; + await _settle(); + + final ids = container + .read(channelsProvider) + .requireValue + .map((channel) => channel.id); + expect(ids, isNot(contains(_channelA))); + expect(ids, contains(_channelB)); + }, + ); + + test( + 'identity switch discards a stale directory failure from the old identity', + () async { + final session = _FakeRelaySession( + memberships: const [], + metadata: [_meta(id: _channelA, name: 'first-identity-open')], + ); + final container = _buildContainer(session: session); + addTearDown(container.dispose); + + expect(await container.read(channelsProvider.future), isEmpty); + + session.pauseNextDirectoryQuery(); + final staleDirectory = container + .read(channelsProvider.notifier) + .retryDirectory(); + await session.nextDirectoryQueryStarted; + + session.memberships = [_membership(_channelB, _otherPk)]; + session.metadata = [_meta(id: _channelB, name: 'second-identity-joined')]; + container.read(_testPubkeyProvider.notifier).set(_otherPk); + await container.read(channelsProvider.future); + + session.directoryFailures = 1; + session.resumePausedDirectoryQuery(); + await staleDirectory; + await _settle(); + + expect( + container + .read(channelsProvider) + .requireValue + .map((channel) => channel.id), + contains(_channelB), + ); + }, + ); + test('reconnect backstop does not refetch the channel directory', () async { final session = _FakeRelaySession( memberships: [_membership(_channelA, myPk)], @@ -1008,6 +1179,7 @@ void main() { const _channelA = '11111111-1111-4111-8111-111111111111'; const _channelB = '22222222-2222-4222-8222-222222222222'; const _channelD = '44444444-4444-4444-8444-444444444444'; +const _otherPk = 'someone-else'; /// Build a kind:39002 membership event tagged with the channel id and member. NostrEvent _membership( @@ -1075,11 +1247,32 @@ ProviderContainer _buildContainer({required _FakeRelaySession session}) { overrides: [ appLifecycleProvider.overrideWith(() => _FakeAppLifecycleNotifier()), relaySessionProvider.overrideWith(() => session), - myPubkeyProvider.overrideWithValue('me'), + // Route the pubkey through a mutable notifier so tests can switch the + // signing identity mid-flight the way an account change does at runtime. + myPubkeyProvider.overrideWith((ref) => ref.watch(_testPubkeyProvider)), ], ); } +/// Mutable stand-in for the signing identity derived from the active community. +class _TestPubkeyNotifier extends Notifier { + @override + String? build() => 'me'; + + void set(String? pubkey) => state = pubkey; +} + +final _testPubkeyProvider = NotifierProvider<_TestPubkeyNotifier, String?>( + _TestPubkeyNotifier.new, +); + +/// Drains pending microtasks so provider rebuilds and awaited writes land. +Future _settle() async { + for (var i = 0; i < 20; i++) { + await Future.delayed(Duration.zero); + } +} + Future _waitUntil(bool Function() predicate) async { for (var i = 0; i < 100; i++) { if (predicate()) return; @@ -1131,6 +1324,8 @@ class _FakeRelaySession extends RelaySessionNotifier { int _nextSubscriptionKey = 0; Completer? _pausedSubscribe; Completer? _subscribeStarted; + Completer? _pausedDirectory; + Completer? _directoryStarted; int unsubscribeCount = 0; int totalSubscribeCount = 0; @@ -1162,6 +1357,32 @@ class _FakeRelaySession extends RelaySessionNotifier { paused.complete(); } + /// Holds the next directory query open so a community or identity switch can + /// be interleaved between the request and its response. + void pauseNextDirectoryQuery() { + if (_pausedDirectory != null) { + throw StateError('A directory query is already paused'); + } + _pausedDirectory = Completer(); + _directoryStarted = Completer(); + } + + /// Completes once the paused directory query has been requested. + Future get nextDirectoryQueryStarted async { + final started = _directoryStarted; + if (started == null) { + throw StateError('No directory query is pending'); + } + await started.future; + } + + /// Releases the paused directory query so its response lands. + void resumePausedDirectoryQuery() { + final paused = _pausedDirectory; + if (paused == null) throw StateError('No directory query is paused'); + paused.complete(); + } + @override SessionState build() => const SessionState(status: SessionStatus.connected); @@ -1242,6 +1463,17 @@ class _FakeRelaySession extends RelaySessionNotifier { filter.kinds.single == 39000 && !filter.tags.containsKey('#d')) { directoryQueryFilters.add(filter); + // Snapshot the directory contents at request time so a paused response + // reflects the community that issued it, not whichever community is + // active when the response is released. + final directorySnapshot = List.of(metadata); + final paused = _pausedDirectory; + if (paused != null) { + _directoryStarted!.complete(); + await paused.future; + _pausedDirectory = null; + _directoryStarted = null; + } if (directoryFailures > 0) { directoryFailures--; throw Exception('directory fetch failed'); @@ -1261,7 +1493,7 @@ class _FakeRelaySession extends RelaySessionNotifier { } return const []; } - return filter.until == null ? List.of(metadata) : const []; + return filter.until == null ? directorySnapshot : const []; } queryBatches.add(filters); return recentMessages.where((event) { From 035a7b4bf244f95ff624c9d47db0f16a16cc9961 Mon Sep 17 00:00:00 2001 From: leader <71e9f2c44a6932b6772caaaccda1911d010463c3e2c6c40410b8329956046801@buzz.block.builderlab.xyz> Date: Fri, 21 Aug 2026 19:31:45 -0700 Subject: [PATCH 09/14] fix(mobile): carry the directory scope across the whole refresh The stale-directory fence only covered the directory query itself. After that query succeeded, the outer fetch kept crossing awaits for DM profiles, hidden DMs, member counts, latest messages and live subscription sync, and none of them revalidated the scope. A community or identity switch in that later window retired the loader but not the refresh already in flight, so retryDirectory could install an old community's channel list into the current provider state. The refresh now captures one scope-and-generation token before its first await and re-checks it after every later await and immediately before every write to metadata, the member cache, load status, subscriptions and provider state. The error path is fenced too: without that, a retired refresh that failed would look like a failure of the current scope and reinstall the pre-switch list. Two helpers move into the part file so channels_provider.dart stays under the 1000-line ceiling that just file-size-check enforces. Co-authored-by: Tom Brow Signed-off-by: Tom Brow --- .../features/channels/channel_directory.dart | 151 +++++++++- .../features/channels/channels_provider.dart | 112 ++++--- .../channels/channels_provider_test.dart | 279 ++++++++++++++++++ 3 files changed, 474 insertions(+), 68 deletions(-) diff --git a/mobile/lib/features/channels/channel_directory.dart b/mobile/lib/features/channels/channel_directory.dart index 81d6a7d5ed1..1b653bcf139 100644 --- a/mobile/lib/features/channels/channel_directory.dart +++ b/mobile/lib/features/channels/channel_directory.dart @@ -102,6 +102,122 @@ class _StaleDirectoryRequest implements Exception { 'Channel directory request retired by a community or identity switch'; } +/// Carries one directory-triggered refresh's scope across every later await. +/// +/// The loader's own fence only covers the directory query. The refresh that +/// query feeds keeps crossing awaits for DM profiles, hidden DMs, member +/// counts, latest messages and live subscription sync, and each one is another +/// chance for the user to switch community or identity. This token is captured +/// once at the start of the refresh and re-checked after every await, so a +/// response that outlived its scope cannot reach shared state. +class _DirectoryRefreshFence { + /// Relay-and-identity scope that started the refresh. + final String scope; + + final _ChannelDirectoryLoader _loader; + final int _generation; + + _DirectoryRefreshFence(this._loader, this.scope, this._generation); + + /// Whether the refresh still owns the active scope and generation. + bool get isCurrent => + _generation == _loader.generation && scope == _loader.currentScope(); + + /// Throws [_StaleDirectoryRequest] once this refresh has been retired. + /// + /// Call after every await and immediately before every write to metadata, + /// cache, load status, subscriptions or provider state. + void ensureCurrent() { + if (!isCurrent) throw const _StaleDirectoryRequest(); + } +} + +/// Awaits [future], then rejects the result if the refresh was retired. +/// +/// One helper keeps every await site on the fenced path identical, so a new +/// await cannot be added without deciding whether it needs the fence. +/// +/// The error path is fenced too. Without it a retired refresh that fails would +/// surface an ordinary exception, and `retryDirectory` would treat it as a +/// failure of the current scope: it would mark the wrong scope's status and +/// reinstall the channel list it captured before the switch. +Future _fenced(_DirectoryRefreshFence? fence, Future future) async { + final T value; + try { + value = await future; + } catch (_) { + if (fence != null && !fence.isCurrent) throw const _StaleDirectoryRequest(); + rethrow; + } + fence?.ensureCurrent(); + return value; +} + +/// Resolves display labels for the other participants in every DM meta. +/// +/// The relay stores DM channels with the literal name "DM", and the pure-Nostr +/// architecture puts name resolution in the client. So collect the non-self +/// participant pubkeys across all DM metas and batch-fetch their kind:0 +/// profiles in one round-trip. Returns lowercase pubkey to label. +/// +/// Lives in this part file because `channels_provider.dart` sits against the +/// repository-wide 1000-line file ceiling enforced by `just file-size-check`. +Future> _resolveDmDisplayNames( + RelaySessionNotifier session, + _DirectoryRefreshFence? fence, + Iterable dedupedMetas, + String myPk, +) async { + final dmParticipants = {}; + final myPkLower = myPk.toLowerCase(); + for (final event in dedupedMetas) { + final data = ChannelData.fromEvent(event); + if (data.channelType != 'dm') continue; + for (final pk in data.participantPubkeys) { + final lower = pk.toLowerCase(); + if (lower != myPkLower) dmParticipants.add(lower); + } + } + if (dmParticipants.isEmpty) return const {}; + + final profileEvents = await _fenced( + fence, + session.fetchHistory(NostrFilters.profilesBatch(dmParticipants.toList())), + ); + final displayNames = {}; + for (final event in profileEvents) { + if (event.kind != 0) continue; + final profile = ProfileData.fromEvent(event); + final label = profile.displayName?.trim().isNotEmpty == true + ? profile.displayName!.trim() + : profile.nip05?.trim().isNotEmpty == true + ? profile.nip05!.trim() + : shortPubkey(profile.pubkey); + displayNames[profile.pubkey.toLowerCase()] = label; + } + return displayNames; +} + +/// Counts distinct `p`-tagged members per channel from kind:39002 events. +/// +/// Lives in this part file to keep `channels_provider.dart` under the +/// repository-wide 1000-line ceiling enforced by `just file-size-check`. +Map _memberCountsByChannelId(Iterable memberEvents) { + final memberCounts = {}; + for (final event in memberEvents) { + final channelId = event.getTagValue('d'); + if (channelId == null) continue; + final pTags = {}; + for (final tag in event.tags) { + if (tag.isNotEmpty && tag[0] == 'p' && tag.length > 1) { + pTags.add(tag[1].toLowerCase()); + } + } + memberCounts[channelId] = pTags.length; + } + return memberCounts; +} + /// Loads the open-channel directory fenced to the scope that requested it. /// /// A community or identity switch changes the scope, and a newer request bumps @@ -121,6 +237,9 @@ class _ChannelDirectoryLoader { int _generation = 0; + /// Generation of the most recently issued or retired request. + int get generation => _generation; + _ChannelDirectoryLoader({ required this.currentScope, required this.loadStatus, @@ -144,31 +263,41 @@ class _ChannelDirectoryLoader { /// wire cannot be written into the new scope. void retireInFlight() => _generation++; - /// Fetches the directory, or throws [_StaleDirectoryRequest] if retired. + /// Starts a fenced refresh without issuing the directory query. /// - /// Returns null when the request failed inside the current scope, which means - /// "retain the cached discovery". The fence is re-checked after the await and - /// before every write, on both the success and the failure path. - Future?> load(RelaySessionNotifier session) async { + /// Used by callers that must carry the scope across later awaits even when + /// they do not refresh discovery, so a membership-only refresh cannot install + /// an old scope's list either. + _DirectoryRefreshFence beginRefresh() { final scope = currentScope(); final generation = ++_generation; - bool isCurrent() => generation == _generation && scope == currentScope(); + return _DirectoryRefreshFence(this, scope, generation); + } - loadStatus().markLoading(scope); + /// Fetches the directory under [fence], or throws if the fence is retired. + /// + /// Returns null when the request failed inside the current scope, which means + /// "retain the cached discovery". The fence is re-checked after the await and + /// before every write, on both the success and the failure path. + Future?> load( + RelaySessionNotifier session, + _DirectoryRefreshFence fence, + ) async { + loadStatus().markLoading(fence.scope); final List metas; try { metas = await _fetchChannelDirectoryMetas(session); } catch (error, stackTrace) { - if (!isCurrent()) throw const _StaleDirectoryRequest(); - loadStatus().markError(scope); + fence.ensureCurrent(); + loadStatus().markError(fence.scope); debugPrint( '[ChannelsNotifier] channel directory refresh failed; retaining ' 'cached discovery: $error\n$stackTrace', ); return null; } - if (!isCurrent()) throw const _StaleDirectoryRequest(); - loadStatus().markLoaded(scope); + fence.ensureCurrent(); + loadStatus().markLoaded(fence.scope); return metas; } } diff --git a/mobile/lib/features/channels/channels_provider.dart b/mobile/lib/features/channels/channels_provider.dart index 4840104de6a..9a883db55c9 100644 --- a/mobile/lib/features/channels/channels_provider.dart +++ b/mobile/lib/features/channels/channels_provider.dart @@ -157,8 +157,18 @@ class ChannelsNotifier extends AsyncNotifier> { final session = ref.read(relaySessionProvider.notifier); + // A directory-triggered refresh is fenced end to end. The fence is captured + // before the first await and re-checked after every later one, so a + // community or identity switch retires the whole refresh rather than only + // its directory query. Refreshes that do not touch discovery keep the + // previous unfenced behaviour. + final fence = fetchDirectory ? _directoryLoader.beginRefresh() : null; + // Step 1: find the channels I'm a member of via kind:39002. - final memberships = await _fetchChannelMemberships(session, myPk); + final memberships = await _fenced( + fence, + _fetchChannelMemberships(session, myPk), + ); final memberChannelIds = memberships .map((e) => e.getTagValue('d')) .whereType() @@ -169,16 +179,19 @@ class ChannelsNotifier extends AsyncNotifier> { // must still continue to directory discovery below. final memberMetas = memberChannelIds.isEmpty ? const [] - : await session.fetchHistory( - NostrFilters.channelMetadata(memberChannelIds.toList()), + : await _fenced( + fence, + session.fetchHistory( + NostrFilters.channelMetadata(memberChannelIds.toList()), + ), ); // Step 3: fetch the open-channel directory. The relay filters this global // kind:39000 query by the caller's access, but the client still rejects // private channels and DMs below so discovery fails closed if that contract // ever regresses. The composite cursor preserves tied-timestamp rows. - if (fetchDirectory) { - final metas = await _directoryLoader.load(session); + if (fence != null) { + final metas = await _directoryLoader.load(session, fence); if (metas != null) _directoryMetas = metas; } @@ -196,39 +209,17 @@ class ChannelsNotifier extends AsyncNotifier> { } final dedupedMetas = latestMetaPerId.values; - // Resolve DM participant display names. Relay stores DM channels with - // literal name="DM"; pure-Nostr architecture pushes name resolution to - // the client, so collect non-self participant pubkeys across all DM - // metas and batch-fetch their kind:0 profiles in one round-trip. - final dmParticipants = {}; - final myPkLower = myPk.toLowerCase(); - for (final event in dedupedMetas) { - final data = ChannelData.fromEvent(event); - if (data.channelType != 'dm') continue; - for (final pk in data.participantPubkeys) { - final lower = pk.toLowerCase(); - if (lower != myPkLower) dmParticipants.add(lower); - } - } - - final displayNames = {}; - if (dmParticipants.isNotEmpty) { - final profileEvents = await session.fetchHistory( - NostrFilters.profilesBatch(dmParticipants.toList()), - ); - for (final event in profileEvents) { - if (event.kind != 0) continue; - final profile = ProfileData.fromEvent(event); - final label = profile.displayName?.trim().isNotEmpty == true - ? profile.displayName!.trim() - : profile.nip05?.trim().isNotEmpty == true - ? profile.nip05!.trim() - : shortPubkey(profile.pubkey); - displayNames[profile.pubkey.toLowerCase()] = label; - } - } + // Resolve DM participant display names. Extracted into the part file so + // `channels_provider.dart` stays under the 1000-line ceiling enforced by + // `just file-size-check`. + final displayNames = await _resolveDmDisplayNames( + session, + fence, + dedupedMetas, + myPk, + ); - final hiddenDmIds = await _fetchHiddenDmIds(session, myPk); + final hiddenDmIds = await _fenced(fence, _fetchHiddenDmIds(session, myPk)); final channels = []; for (final event in dedupedMetas) { @@ -253,26 +244,18 @@ class ChannelsNotifier extends AsyncNotifier> { final memberCountChannelIds = memberChannelIds.toList(); final memberEvents = memberCountChannelIds.isEmpty ? const [] - : await session.fetchHistory( - NostrFilter( - kinds: const [39002], - tags: {'#d': memberCountChannelIds}, - limit: memberCountChannelIds.length, + : await _fenced( + fence, + session.fetchHistory( + NostrFilter( + kinds: const [39002], + tags: {'#d': memberCountChannelIds}, + limit: memberCountChannelIds.length, + ), ), ); if (memberEvents.isNotEmpty) _cacheMemberSnapshots(memberEvents); - final memberCounts = {}; - for (final event in memberEvents) { - final chId = event.getTagValue('d'); - if (chId == null) continue; - final pTags = {}; - for (final tag in event.tags) { - if (tag.isNotEmpty && tag[0] == 'p' && tag.length > 1) { - pTags.add(tag[1].toLowerCase()); - } - } - memberCounts[chId] = pTags.length; - } + final memberCounts = _memberCountsByChannelId(memberEvents); for (var i = 0; i < channels.length; i++) { final count = memberCounts[channels[i].id]; if (count != null) { @@ -293,7 +276,10 @@ class ChannelsNotifier extends AsyncNotifier> { final channelById = { for (final channel in activeChannels) channel.id: channel, }; - final events = await _fetchLastMessageEvents(session, activeChannels); + final events = await _fenced( + fence, + _fetchLastMessageEvents(session, activeChannels), + ); final lastMessageMap = {}; final mutedChannelIds = _mutedChannelIds(); for (final event in events) { @@ -348,6 +334,12 @@ class ChannelsNotifier extends AsyncNotifier> { // Scoped narrowly to the archived flip — broader metadata staleness // (renames, topic changes, etc.) is a separate, pre-existing concern that // already affects this provider for other reasons. + // Re-check before the first write that other providers can observe. Every + // await above is fenced, but the switch can also land in the synchronous + // gap, so the guard sits immediately before the write rather than only + // after the await. + fence?.ensureCurrent(); + final prevById = { for (final c in state.value ?? const []) c.id: c, }; @@ -359,8 +351,14 @@ class ChannelsNotifier extends AsyncNotifier> { } if (subscribeLive) { - await _subscribeLive(channels); - } + // Subscriptions are shared relay state, so a retired refresh must not + // install them even though its channel list is already built. + fence?.ensureCurrent(); + await _fenced(fence, _subscribeLive(channels)); + } + // Guard the provider-state write in `retryDirectory` and `build`: the + // caller assigns whatever this returns, so the last check belongs here. + fence?.ensureCurrent(); return channels; } diff --git a/mobile/test/features/channels/channels_provider_test.dart b/mobile/test/features/channels/channels_provider_test.dart index 1526f55c7a1..9dd7b914b52 100644 --- a/mobile/test/features/channels/channels_provider_test.dart +++ b/mobile/test/features/channels/channels_provider_test.dart @@ -460,6 +460,199 @@ void main() { }, ); + test( + 'community switch after directory success discards the late refresh', + () async { + final session = _FakeRelaySession( + memberships: const [], + metadata: [_meta(id: _channelA, name: 'community-a-open')], + ); + final container = _buildContainer(session: session); + addTearDown(container.dispose); + + expect(await container.read(channelsProvider.future), isEmpty); + + // Community A now has a joined channel, so the directory-triggered + // refresh reaches the live-subscribe step and parks there. That await is + // AFTER the loader's own directory fence, which is the window this arm + // covers: directory success, then retirement, then settlement. + session.memberships = [_membership(_channelA, myPk)]; + session.pauseNextSubscribe(); + final staleRefresh = container + .read(channelsProvider.notifier) + .retryDirectory(); + await session.nextSubscribeStarted; + + // Record every list emitted from the switch onward. The live-subscription + // queue is serialized, so community B's own subscribe waits behind the + // parked one. That makes the observable defect an emission of community + // A's channel into the current scope, not just a wrong final state. + final emitted = >[]; + container.listen(channelsProvider, (previous, next) { + final value = next.value; + if (value != null) { + emitted.add(value.map((channel) => channel.id).toList()); + } + }); + + session.memberships = [_membership(_channelB, myPk)]; + session.metadata = [_meta(id: _channelB, name: 'community-b-general')]; + container + .read(relayConfigProvider.notifier) + .update(baseUrl: 'https://community-b.example'); + + session.resumePausedSubscribe(); + await staleRefresh; + await _settle(); + + expect( + emitted.where((ids) => ids.contains(_channelA)), + isEmpty, + reason: 'community A channel emitted into community B scope: $emitted', + ); + // The retired refresh must not leave a subscription on the old channel. + expect(session.activeChannels, isNot(contains(_channelA))); + // Nor may it claim the new scope's directory status as its own. + expect( + container.read(channelDirectoryLoadStatusProvider).scope, + isNot( + channelDirectoryScope( + 'https://community-b.example', + container.read(myPubkeyProvider), + ), + ), + ); + }, + ); + + test( + 'community switch after directory success discards a late failure', + () async { + final session = _FakeRelaySession( + memberships: const [], + metadata: [_meta(id: _channelA, name: 'community-a-open')], + ); + final container = _buildContainer(session: session); + addTearDown(container.dispose); + + expect(await container.read(channelsProvider.future), isEmpty); + + // The directory query succeeds, then the refresh parks on the member-count + // query and fails there after the switch. A retired failure must not + // overwrite the new scope's list or push it into an error state. + session.memberships = [_membership(_channelA, myPk)]; + session.pauseNextMemberCountQuery(); + final staleRefresh = container + .read(channelsProvider.notifier) + .retryDirectory(); + await session.nextMemberCountQueryStarted; + + session.memberships = [_membership(_channelB, myPk)]; + session.metadata = [_meta(id: _channelB, name: 'community-b-general')]; + container + .read(relayConfigProvider.notifier) + .update(baseUrl: 'https://community-b.example'); + await container.read(channelsProvider.future); + + session.failClaimedMemberCountQuery = true; + session.resumePausedMemberCountQuery(); + await staleRefresh; + await _settle(); + + final current = container.read(channelsProvider); + expect(current.hasError, isFalse); + final ids = current.requireValue.map((channel) => channel.id); + expect(ids, isNot(contains(_channelA))); + expect(ids, contains(_channelB)); + // A retired failure must not claim the new scope's directory status. + expect( + container.read(channelDirectoryLoadStatusProvider).scope, + isNot( + channelDirectoryScope( + 'https://community-b.example', + container.read(myPubkeyProvider), + ), + ), + ); + }, + ); + + test( + 'identity switch after directory success discards the late refresh', + () async { + final session = _FakeRelaySession( + memberships: const [], + metadata: [_meta(id: _channelA, name: 'first-identity-open')], + ); + final container = _buildContainer(session: session); + addTearDown(container.dispose); + + expect(await container.read(channelsProvider.future), isEmpty); + + // Directory success, then the refresh parks on the hidden-DM query while + // the signing identity changes and the new identity finishes its rebuild. + session.pauseNextHiddenDmQuery(); + final staleRefresh = container + .read(channelsProvider.notifier) + .retryDirectory(); + await session.nextHiddenDmQueryStarted; + + session.memberships = [_membership(_channelB, _otherPk)]; + session.metadata = [_meta(id: _channelB, name: 'second-identity-joined')]; + container.read(_testPubkeyProvider.notifier).set(_otherPk); + await container.read(channelsProvider.future); + + session.resumePausedHiddenDmQuery(); + await staleRefresh; + await _settle(); + + final ids = container + .read(channelsProvider) + .requireValue + .map((channel) => channel.id); + expect(ids, isNot(contains(_channelA))); + expect(ids, contains(_channelB)); + expect(session.activeChannels, isNot(contains(_channelA))); + }, + ); + + test( + 'identity switch after directory success discards a late failure', + () async { + final session = _FakeRelaySession( + memberships: const [], + metadata: [_meta(id: _channelA, name: 'first-identity-open')], + ); + final container = _buildContainer(session: session); + addTearDown(container.dispose); + + expect(await container.read(channelsProvider.future), isEmpty); + + session.memberships = [_membership(_channelA, myPk)]; + session.pauseNextMemberCountQuery(); + final staleRefresh = container + .read(channelsProvider.notifier) + .retryDirectory(); + await session.nextMemberCountQueryStarted; + + session.memberships = [_membership(_channelB, _otherPk)]; + session.metadata = [_meta(id: _channelB, name: 'second-identity-joined')]; + container.read(_testPubkeyProvider.notifier).set(_otherPk); + await container.read(channelsProvider.future); + + session.failClaimedMemberCountQuery = true; + session.resumePausedMemberCountQuery(); + await staleRefresh; + await _settle(); + + final current = container.read(channelsProvider); + expect(current.hasError, isFalse); + final ids = current.requireValue.map((channel) => channel.id); + expect(ids, isNot(contains(_channelA))); + expect(ids, contains(_channelB)); + }, + ); + test('reconnect backstop does not refetch the channel directory', () async { final session = _FakeRelaySession( memberships: [_membership(_channelA, myPk)], @@ -1312,6 +1505,7 @@ class _FakeRelaySession extends RelaySessionNotifier { final List recentMessages; int membershipFailures; int directoryFailures = 0; + bool failClaimedMemberCountQuery = false; int membershipRequestCount = 0; int metadataPageRequestCount = 0; @@ -1326,6 +1520,12 @@ class _FakeRelaySession extends RelaySessionNotifier { Completer? _subscribeStarted; Completer? _pausedDirectory; Completer? _directoryStarted; + Completer? _pausedHiddenDm; + Completer? _hiddenDmStarted; + Completer? _claimedHiddenDm; + Completer? _pausedMemberCount; + Completer? _memberCountStarted; + Completer? _claimedMemberCount; int unsubscribeCount = 0; int totalSubscribeCount = 0; @@ -1383,6 +1583,60 @@ class _FakeRelaySession extends RelaySessionNotifier { paused.complete(); } + /// Holds the next hidden-DM query open, one shot only. + /// + /// One shot matters: the refresh that follows the switch issues its own + /// hidden-DM query, and it must be able to finish while the earlier scope's + /// query is still parked. That is the window Jed's second probe describes. + void pauseNextHiddenDmQuery() { + if (_pausedHiddenDm != null) { + throw StateError('A hidden-DM query is already paused'); + } + _pausedHiddenDm = Completer(); + _hiddenDmStarted = Completer(); + } + + /// Completes once the parked hidden-DM query has been requested. + Future get nextHiddenDmQueryStarted async { + final started = _hiddenDmStarted; + if (started == null) { + throw StateError('No hidden-DM query is pending'); + } + await started.future; + } + + /// Releases the parked hidden-DM query so its response lands. + void resumePausedHiddenDmQuery() { + final paused = _claimedHiddenDm ?? _pausedHiddenDm; + if (paused == null) throw StateError('No hidden-DM query is paused'); + paused.complete(); + } + + /// Holds the next member-count query open, one shot only. + void pauseNextMemberCountQuery() { + if (_pausedMemberCount != null) { + throw StateError('A member-count query is already paused'); + } + _pausedMemberCount = Completer(); + _memberCountStarted = Completer(); + } + + /// Completes once the parked member-count query has been requested. + Future get nextMemberCountQueryStarted async { + final started = _memberCountStarted; + if (started == null) { + throw StateError('No member-count query is pending'); + } + await started.future; + } + + /// Releases the parked member-count query so its response lands. + void resumePausedMemberCountQuery() { + final paused = _claimedMemberCount ?? _pausedMemberCount; + if (paused == null) throw StateError('No member-count query is paused'); + paused.complete(); + } + @override SessionState build() => const SessionState(status: SessionStatus.connected); @@ -1408,8 +1662,33 @@ class _FakeRelaySession extends RelaySessionNotifier { .toList(); } if (filter.kinds.contains(EventKind.dmVisibility)) { + // Claim the parked slot so the switch's own refresh runs unblocked. + final paused = _pausedHiddenDm; + if (paused != null) { + _claimedHiddenDm = paused; + _pausedHiddenDm = null; + _hiddenDmStarted!.complete(); + _hiddenDmStarted = null; + await paused.future; + } return hiddenDmEvents; } + if (filter.kinds.contains(39002) && filter.tags['#d'] != null) { + final paused = _pausedMemberCount; + if (paused != null) { + _claimedMemberCount = paused; + _pausedMemberCount = null; + _memberCountStarted!.complete(); + _memberCountStarted = null; + await paused.future; + if (failClaimedMemberCountQuery) { + throw Exception('member-count fetch failed'); + } + } + // Falls through to the shared empty result so the member-count shape + // stays exactly as it was before this pause hook existed. + return const []; + } if (filter.kinds.contains(39000)) { final ids = filter.tags['#d']?.toSet(); if (ids == null) { From 96f799c98b59ab61144db09a1203a90e8bbf4814 Mon Sep 17 00:00:00 2001 From: leader <71e9f2c44a6932b6772caaaccda1911d010463c3e2c6c40410b8329956046801@buzz.block.builderlab.xyz> Date: Fri, 21 Aug 2026 20:53:34 -0700 Subject: [PATCH 10/14] fix(mobile): fence the detached unread catch-up too The refresh fence stopped short of the unread catch-up. The catch-up is started fire-and-forget at the end of live subscription sync, so it was never handed the refresh's scope token: a community or identity switch could land while its relay batch was in flight, and the unread badges, thread-interest sets and provider state it wrote all belonged to the community the user had just left. The catch-up now takes the same fence and re-checks it after its relay await, before any of those writes. It returns rather than throwing, because nothing awaits it and a thrown stale-request error would only surface unhandled. Round two claimed the whole refresh was fenced. That framing was too broad: it covered every awaited step but not this detached one. Five deterministic regressions cover it: a parked catch-up released after a community switch and after an identity switch, the same two where the parked batch fails, and unread state recorded before a community switch. The test relay gains a one-shot catch-up pause that hands its slot off, so the refresh after the switch runs unblocked while the old batch stays parked. Guards at the catch-up's entry, on its error path, and before its trailing state write were each measured unreachable (the post-await check already returns first), as was clearing the unread maps on scope change (the existing dispose clears already cover it), so none of them are here. Two helpers moved into the existing part file to stay under the 1000-line mobile file ceiling. No behaviour change in the move. Co-authored-by: Tom Brow Signed-off-by: Tom Brow --- .../features/channels/channel_directory.dart | 68 ++++ .../features/channels/channels_provider.dart | 89 ++--- .../channels/channels_provider_test.dart | 335 +++++++++++++++++- 3 files changed, 437 insertions(+), 55 deletions(-) diff --git a/mobile/lib/features/channels/channel_directory.dart b/mobile/lib/features/channels/channel_directory.dart index 1b653bcf139..58fa9ab13f3 100644 --- a/mobile/lib/features/channels/channel_directory.dart +++ b/mobile/lib/features/channels/channel_directory.dart @@ -132,6 +132,14 @@ class _DirectoryRefreshFence { } } +/// Whether [fence] has been retired, so its refresh must write nothing. +/// +/// Used on the detached unread catch-up path, where throwing +/// [_StaleDirectoryRequest] would only surface as an unhandled error: the +/// catch-up runs fire-and-forget, so a retired scope simply stops. +bool _isRetired(_DirectoryRefreshFence? fence) => + fence != null && !fence.isCurrent; + /// Awaits [future], then rejects the result if the refresh was retired. /// /// One helper keeps every await site on the fenced path identical, so a new @@ -341,3 +349,63 @@ Future> _fetchPaginatedChannelEvents( } return events; } + +/// Thread-interest and unread helpers shared by [ChannelsNotifier]. +/// +/// Lives in this part file because `channels_provider.dart` sits against the +/// repository-wide 1000-line file ceiling enforced by `just file-size-check`. +String? _observedUnreadRootId(NostrEvent event) => + _isBroadcastReply(event) ? null : event.threadReference.rootId; + +bool _isBroadcastReply(NostrEvent event) => event.tags.any( + (tag) => tag.length >= 2 && tag[0] == 'broadcast' && tag[1] == '1', +); + +Set _readRootIdSet(String? raw) { + if (raw == null || raw.isEmpty) return {}; + try { + final decoded = jsonDecode(raw); + if (decoded is! List) return {}; + return { + for (final value in decoded) + if (value is String) value, + }; + } catch (_) { + return {}; + } +} + +String _encodeRootIdSet(Set values) => jsonEncode(values.toList()); + +/// Records one observed unread event for a channel's badge state. +/// +/// An extension in this part file rather than a method on the notifier because +/// `channels_provider.dart` sits against the repository-wide 1000-line file +/// ceiling enforced by `just file-size-check`. Private members stay reachable: +/// a part shares its parent's library. +extension _ObservedUnreadRecording on ChannelsNotifier { + void _recordUnreadEvent(Channel channel, NostrEvent event, String myPk) { + final isThreadedReply = + event.threadReference.parentId != null && !_isBroadcastReply(event); + final isHighPriority = + channel.isDm || isHighPriorityEvent(event.tags, myPk); + recordObservedUnreadEvent( + _observedUnreadEventsByChannel, + channel.id, + makeObservedUnreadEvent( + id: event.id, + createdAt: event.createdAt, + rootId: _observedUnreadRootId(event), + highPriority: isHighPriority, + channelType: channel.channelType, + isThreadedReply: isThreadedReply, + ), + _unreadCatchUpLimit, + ); + + final current = _latestObservedByChannel[channel.id] ?? 0; + if (event.createdAt > current) { + _latestObservedByChannel[channel.id] = event.createdAt; + } + } +} diff --git a/mobile/lib/features/channels/channels_provider.dart b/mobile/lib/features/channels/channels_provider.dart index 9a883db55c9..59a8c99c1f5 100644 --- a/mobile/lib/features/channels/channels_provider.dart +++ b/mobile/lib/features/channels/channels_provider.dart @@ -354,7 +354,7 @@ class ChannelsNotifier extends AsyncNotifier> { // Subscriptions are shared relay state, so a retired refresh must not // install them even though its channel list is already built. fence?.ensureCurrent(); - await _fenced(fence, _subscribeLive(channels)); + await _fenced(fence, _subscribeLive(channels, fence)); } // Guard the provider-state write in `retryDirectory` and `build`: the // caller assigns whatever this returns, so the last check belongs here. @@ -534,7 +534,12 @@ class ChannelsNotifier extends AsyncNotifier> { /// Subscribe per-channel to live events (requires `#h` tag for relay /// channel-scoped fan-out). Also starts a 60s WS backstop poll to reconcile /// membership changes without repeatedly downloading the global directory. - Future _subscribeLive(List channels) { + /// The [fence] rides along so the detached unread catch-up started at the + /// end of the sync can still tell whether its scope is current. + Future _subscribeLive( + List channels, + _DirectoryRefreshFence? fence, + ) { final channelIds = { for (final channel in channels) if (channel.isMember && !channel.isArchived) channel.id, @@ -545,8 +550,12 @@ class ChannelsNotifier extends AsyncNotifier> { final subscriptionVersion = ++_subscriptionVersion; final sync = _liveSubscriptionQueue.then( - (_) => - _syncLiveSubscriptions(relayBaseUrl, subscriptionVersion, channels), + (_) => _syncLiveSubscriptions( + relayBaseUrl, + subscriptionVersion, + channels, + fence, + ), ); _liveSubscriptionQueue = sync.catchError((Object error, StackTrace stack) { debugPrint( @@ -560,6 +569,7 @@ class ChannelsNotifier extends AsyncNotifier> { String relayBaseUrl, int subscriptionVersion, List channels, + _DirectoryRefreshFence? fence, ) async { if (ref.read(relaySessionProvider).status != SessionStatus.connected) { return; @@ -570,6 +580,7 @@ class ChannelsNotifier extends AsyncNotifier> { ref.read(relayConfigProvider).baseUrl, _subscriptionVersion, _desiredLiveChannels, + fence, ); return; } @@ -643,7 +654,7 @@ class ChannelsNotifier extends AsyncNotifier> { return; } - unawaited(_catchUpUnreadEvents(channels)); + unawaited(_catchUpUnreadEvents(channels, fence)); _backstopTimer?.cancel(); _backstopTimer = Timer.periodic( @@ -652,7 +663,21 @@ class ChannelsNotifier extends AsyncNotifier> { ); } - Future _catchUpUnreadEvents(List channels) async { + /// Backfills unread badges for the channels this refresh just installed. + /// + /// Runs detached from the refresh that starts it, so [fence] is what keeps a + /// response that outlived its community or identity from writing unread + /// state into the scope the user switched to. A retired refresh returns + /// instead of throwing: nothing awaits this future, so a thrown + /// [_StaleDirectoryRequest] would only surface as an unhandled error. + /// + /// The relay round-trip is the only await here, so one re-check after it + /// covers every write below. Guards before it or after the writes were + /// measured unreachable: retirement cannot land in a synchronous gap. + Future _catchUpUnreadEvents( + List channels, + _DirectoryRefreshFence? fence, + ) async { final myPk = ref.read(myPubkeyProvider); if (myPk == null) return; @@ -694,6 +719,10 @@ class ChannelsNotifier extends AsyncNotifier> { filters, operation: 'unread catch-up', ); + // The relay round-trip above is the window Jed's probe parks in: a + // community or identity switch here means every write below belongs to a + // scope the user has left. + if (_isRetired(fence)) return; for (final event in events) { if (event.pubkey.toLowerCase() == myPk.toLowerCase()) { @@ -825,31 +854,6 @@ class ChannelsNotifier extends AsyncNotifier> { } } - void _recordUnreadEvent(Channel channel, NostrEvent event, String myPk) { - final isThreadedReply = - event.threadReference.parentId != null && !_isBroadcastReply(event); - final isHighPriority = - channel.isDm || isHighPriorityEvent(event.tags, myPk); - recordObservedUnreadEvent( - _observedUnreadEventsByChannel, - channel.id, - makeObservedUnreadEvent( - id: event.id, - createdAt: event.createdAt, - rootId: _observedUnreadRootId(event), - highPriority: isHighPriority, - channelType: channel.channelType, - isThreadedReply: isThreadedReply, - ), - _unreadCatchUpLimit, - ); - - final current = _latestObservedByChannel[channel.id] ?? 0; - if (event.createdAt > current) { - _latestObservedByChannel[channel.id] = event.createdAt; - } - } - void clearObservedUnreadForChannel(String channelId) { _latestObservedByChannel.remove(channelId); _observedUnreadEventsByChannel.remove(channelId); @@ -968,26 +972,3 @@ class ChannelsNotifier extends AsyncNotifier> { final channelsProvider = AsyncNotifierProvider>( ChannelsNotifier.new, ); - -String? _observedUnreadRootId(NostrEvent event) => - _isBroadcastReply(event) ? null : event.threadReference.rootId; - -bool _isBroadcastReply(NostrEvent event) => event.tags.any( - (tag) => tag.length >= 2 && tag[0] == 'broadcast' && tag[1] == '1', -); - -Set _readRootIdSet(String? raw) { - if (raw == null || raw.isEmpty) return {}; - try { - final decoded = jsonDecode(raw); - if (decoded is! List) return {}; - return { - for (final value in decoded) - if (value is String) value, - }; - } catch (_) { - return {}; - } -} - -String _encodeRootIdSet(Set values) => jsonEncode(values.toList()); diff --git a/mobile/test/features/channels/channels_provider_test.dart b/mobile/test/features/channels/channels_provider_test.dart index 9dd7b914b52..b9991bacd32 100644 --- a/mobile/test/features/channels/channels_provider_test.dart +++ b/mobile/test/features/channels/channels_provider_test.dart @@ -653,6 +653,283 @@ void main() { }, ); + test('community switch discards a parked unread catch-up', () async { + final session = _FakeRelaySession( + memberships: const [], + metadata: [_meta(id: _channelA, name: 'community-a-open')], + recentMessages: const [ + NostrEvent( + id: 'mention-in-a', + pubkey: 'alice', + createdAt: 50, + kind: 9, + tags: [ + ['h', _channelA], + ['p', myPk], + ], + content: 'direct mention in community A', + sig: 'sig', + ), + ], + ); + final container = _buildContainer(session: session); + addTearDown(container.dispose); + + expect(await container.read(channelsProvider.future), isEmpty); + + // The directory query succeeds and the refresh completes, but the unread + // catch-up it kicks off stays parked across the community switch. + session.memberships = [_membership(_channelA, myPk)]; + session.pauseNextUnreadCatchUpQuery(); + final staleRefresh = container + .read(channelsProvider.notifier) + .retryDirectory(); + await session.nextUnreadCatchUpQueryStarted; + + session.memberships = [_membership(_channelB, myPk)]; + session.metadata = [_meta(id: _channelB, name: 'community-b-general')]; + container + .read(relayConfigProvider.notifier) + .update(baseUrl: 'https://community-b.example'); + await container.read(channelsProvider.future); + + session.resumePausedUnreadCatchUpQuery(); + await staleRefresh; + await _settle(); + + final notifier = container.read(channelsProvider.notifier); + expect( + notifier.latestObservedByChannel, + isNot(contains(_channelA)), + reason: + 'community A unread state landed in community B: ' + '${notifier.latestObservedByChannel}', + ); + expect( + notifier.observedUnreadEventsByChannel, + isNot(contains(_channelA)), + reason: + 'community A unread events landed in community B: ' + '${notifier.observedUnreadEventsByChannel}', + ); + }); + + test('identity switch discards a parked unread catch-up', () async { + final session = _FakeRelaySession( + memberships: const [], + metadata: [_meta(id: _channelA, name: 'first-identity-open')], + recentMessages: const [ + NostrEvent( + id: 'mention-in-a', + pubkey: 'alice', + createdAt: 50, + kind: 9, + tags: [ + ['h', _channelA], + ['p', myPk], + ], + content: 'direct mention for the first identity', + sig: 'sig', + ), + ], + ); + final container = _buildContainer(session: session); + addTearDown(container.dispose); + + expect(await container.read(channelsProvider.future), isEmpty); + + session.memberships = [_membership(_channelA, myPk)]; + session.pauseNextUnreadCatchUpQuery(); + final staleRefresh = container + .read(channelsProvider.notifier) + .retryDirectory(); + await session.nextUnreadCatchUpQueryStarted; + + session.memberships = [_membership(_channelB, _otherPk)]; + session.metadata = [_meta(id: _channelB, name: 'second-identity-joined')]; + container.read(_testPubkeyProvider.notifier).set(_otherPk); + await container.read(channelsProvider.future); + + session.resumePausedUnreadCatchUpQuery(); + await staleRefresh; + await _settle(); + + final notifier = container.read(channelsProvider.notifier); + expect( + notifier.latestObservedByChannel, + isNot(contains(_channelA)), + reason: + 'first identity unread state landed on the second identity: ' + '${notifier.latestObservedByChannel}', + ); + expect( + notifier.observedUnreadEventsByChannel, + isNot(contains(_channelA)), + reason: + 'first identity unread events landed on the second identity: ' + '${notifier.observedUnreadEventsByChannel}', + ); + }); + + test('community switch discards a parked catch-up failure', () async { + final session = _FakeRelaySession( + memberships: const [], + metadata: [_meta(id: _channelA, name: 'community-a-open')], + ); + final container = _buildContainer(session: session); + addTearDown(container.dispose); + + expect(await container.read(channelsProvider.future), isEmpty); + + session.memberships = [_membership(_channelA, myPk)]; + session.pauseNextUnreadCatchUpQuery(); + final staleRefresh = container + .read(channelsProvider.notifier) + .retryDirectory(); + await session.nextUnreadCatchUpQueryStarted; + + session.memberships = [_membership(_channelB, myPk)]; + session.metadata = [_meta(id: _channelB, name: 'community-b-general')]; + container + .read(relayConfigProvider.notifier) + .update(baseUrl: 'https://community-b.example'); + await container.read(channelsProvider.future); + // Let community B's own refresh and its own catch-up settle first, so the + // emissions counted below can only come from the parked community A work. + await _settle(); + + // A retired catch-up that fails must publish nothing: the trailing `state` + // write belongs to whichever community is active now. + final staleEmissions = []; + final subscription = container.listen( + channelsProvider, + (_, next) => staleEmissions.add(next.requireValue.length), + fireImmediately: false, + ); + addTearDown(subscription.close); + + session.failClaimedUnreadCatchUpQuery = true; + session.resumePausedUnreadCatchUpQuery(); + await staleRefresh; + await _settle(); + + final current = container.read(channelsProvider); + expect(current.hasError, isFalse); + expect(current.requireValue.map((channel) => channel.id), [_channelB]); + expect( + staleEmissions, + isEmpty, + reason: + 'a retired catch-up failure republished community B provider state: ' + '$staleEmissions', + ); + }); + + test('identity switch discards a parked catch-up failure', () async { + final session = _FakeRelaySession( + memberships: const [], + metadata: [_meta(id: _channelA, name: 'first-identity-open')], + ); + final container = _buildContainer(session: session); + addTearDown(container.dispose); + + expect(await container.read(channelsProvider.future), isEmpty); + + session.memberships = [_membership(_channelA, myPk)]; + session.pauseNextUnreadCatchUpQuery(); + final staleRefresh = container + .read(channelsProvider.notifier) + .retryDirectory(); + await session.nextUnreadCatchUpQueryStarted; + + session.memberships = [_membership(_channelB, _otherPk)]; + session.metadata = [_meta(id: _channelB, name: 'second-identity-joined')]; + container.read(_testPubkeyProvider.notifier).set(_otherPk); + await container.read(channelsProvider.future); + await _settle(); + + final staleEmissions = []; + final subscription = container.listen( + channelsProvider, + (_, next) => staleEmissions.add(next.requireValue.length), + fireImmediately: false, + ); + addTearDown(subscription.close); + + session.failClaimedUnreadCatchUpQuery = true; + session.resumePausedUnreadCatchUpQuery(); + await staleRefresh; + await _settle(); + + final current = container.read(channelsProvider); + expect(current.hasError, isFalse); + expect(current.requireValue.map((channel) => channel.id), [_channelB]); + expect( + staleEmissions, + isEmpty, + reason: + 'a retired catch-up failure republished the second identity provider ' + 'state: $staleEmissions', + ); + }); + + test('community switch drops unread state recorded before it', () async { + final session = _FakeRelaySession( + memberships: [_membership(_channelA, myPk)], + metadata: [_meta(id: _channelA, name: 'community-a-general')], + ); + final container = _buildContainer(session: session); + addTearDown(container.dispose); + + await container.read(channelsProvider.future); + + // A live mention in community A records unread state the badges read. + session.emit( + const NostrEvent( + id: 'mention-in-a', + pubkey: 'alice', + createdAt: 50, + kind: 9, + tags: [ + ['h', _channelA], + ['p', myPk], + ], + content: 'direct mention in community A', + sig: 'sig', + ), + ); + await _settle(); + final notifier = container.read(channelsProvider.notifier); + expect( + notifier.latestObservedByChannel, + contains(_channelA), + reason: 'precondition: community A unread state was never recorded', + ); + + session.memberships = [_membership(_channelB, myPk)]; + session.metadata = [_meta(id: _channelB, name: 'community-b-general')]; + container + .read(relayConfigProvider.notifier) + .update(baseUrl: 'https://community-b.example'); + await container.read(channelsProvider.future); + await _settle(); + + expect( + notifier.latestObservedByChannel, + isNot(contains(_channelA)), + reason: + 'community A unread state survived into community B: ' + '${notifier.latestObservedByChannel}', + ); + expect( + notifier.observedUnreadEventsByChannel, + isNot(contains(_channelA)), + reason: + 'community A unread events survived into community B: ' + '${notifier.observedUnreadEventsByChannel}', + ); + }); + test('reconnect backstop does not refetch the channel directory', () async { final session = _FakeRelaySession( memberships: [_membership(_channelA, myPk)], @@ -1506,6 +1783,7 @@ class _FakeRelaySession extends RelaySessionNotifier { int membershipFailures; int directoryFailures = 0; bool failClaimedMemberCountQuery = false; + bool failClaimedUnreadCatchUpQuery = false; int membershipRequestCount = 0; int metadataPageRequestCount = 0; @@ -1526,6 +1804,9 @@ class _FakeRelaySession extends RelaySessionNotifier { Completer? _pausedMemberCount; Completer? _memberCountStarted; Completer? _claimedMemberCount; + Completer? _pausedUnreadCatchUp; + Completer? _unreadCatchUpStarted; + Completer? _claimedUnreadCatchUp; int unsubscribeCount = 0; int totalSubscribeCount = 0; @@ -1637,6 +1918,37 @@ class _FakeRelaySession extends RelaySessionNotifier { paused.complete(); } + /// Holds the next unread catch-up batch open, one shot only. + /// + /// The catch-up runs detached from the refresh that starts it, so this is the + /// window where a community or identity switch can land between the request + /// and the writes its response drives. + void pauseNextUnreadCatchUpQuery() { + if (_pausedUnreadCatchUp != null) { + throw StateError('An unread catch-up query is already paused'); + } + _pausedUnreadCatchUp = Completer(); + _unreadCatchUpStarted = Completer(); + } + + /// Completes once the parked unread catch-up batch has been requested. + Future get nextUnreadCatchUpQueryStarted async { + final started = _unreadCatchUpStarted; + if (started == null) { + throw StateError('No unread catch-up query is pending'); + } + await started.future; + } + + /// Releases the parked unread catch-up batch so its response lands. + void resumePausedUnreadCatchUpQuery() { + final paused = _claimedUnreadCatchUp ?? _pausedUnreadCatchUp; + if (paused == null) { + throw StateError('No unread catch-up query is paused'); + } + paused.complete(); + } + @override SessionState build() => const SessionState(status: SessionStatus.connected); @@ -1775,7 +2087,28 @@ class _FakeRelaySession extends RelaySessionNotifier { return filter.until == null ? directorySnapshot : const []; } queryBatches.add(filters); - return recentMessages.where((event) { + // The unread catch-up is the only batch that carries `since` on every + // filter; the latest-message batch leaves it null. Snapshot the messages at + // request time so a parked response reflects the scope that asked for it. + final isUnreadCatchUp = + filters.isNotEmpty && filters.every((filter) => filter.since != null); + final messageSnapshot = List.of(recentMessages); + if (isUnreadCatchUp) { + // Claim the parked slot so the refresh that follows the switch can run + // its own catch-up unblocked while this one stays parked. + final paused = _pausedUnreadCatchUp; + if (paused != null) { + _claimedUnreadCatchUp = paused; + _pausedUnreadCatchUp = null; + _unreadCatchUpStarted!.complete(); + _unreadCatchUpStarted = null; + await paused.future; + if (failClaimedUnreadCatchUpQuery) { + throw Exception('unread catch-up fetch failed'); + } + } + } + return messageSnapshot.where((event) { return filters.any((filter) { if (!filter.kinds.contains(event.kind)) return false; for (final entry in filter.tags.entries) { From 7b4c17d6f7db575cf13b6b9208423138fa17739d Mon Sep 17 00:00:00 2001 From: leader <71e9f2c44a6932b6772caaaccda1911d010463c3e2c6c40410b8329956046801@buzz.block.builderlab.xyz> Date: Fri, 21 Aug 2026 23:18:00 -0700 Subject: [PATCH 11/14] fix(mobile): fence every detached unread catch-up, not just directory ones Round 3 fenced the refreshes that fetch the directory, so it missed the one thing that outlives them. `_syncLiveSubscriptions` launches the unread catch-up fire-and-forget, and the initial load, the ordinary membership refresh a join performs, and the reconnect backstop all reach it with no fence at all. A catch-up parked on its relay query could therefore wake up after a newer refresh, a community switch or an identity switch and write another list's unread state into whatever the user is looking at now. The catch-up now carries its own lifecycle token instead of borrowing the directory fence. It captures `_subscriptionVersion` synchronously at entry, which is safe because the method runs synchronously up to its relay query, and returns without writing anything if that generation is no longer current. One monotonic counter covers both hazards: every channel-list refresh bumps it, and a community or identity switch rebuilds the notifier, whose disposal bumps it too. That redundancy is measured rather than assumed, and a disconnected-community-switch arm is checked in to keep it honest. The trailing republish moves inside the try and only fires when the batch actually recorded something. Before, a failed or empty batch still emitted a fresh list, repainting on behalf of a refresh that produced nothing. Six new arms cover the paths round 3 never reached: ordinary membership/join overlap, backstop overlap driven through a real reconnect, identity switch, community switch, the disconnected switch above, and the empty-batch repaint. All five leak arms were confirmed red against unmodified production code before any fix was written. Positive controls: neutering the generation or deleting the guard kills seven arms, and restoring the old unconditional emit kills the empty-batch arm. Co-authored-by: Tom Brow Signed-off-by: Tom Brow --- .../features/channels/channel_directory.dart | 31 +- .../features/channels/channels_provider.dart | 64 ++-- .../channels/channels_provider_test.dart | 310 +++++++++++++++++- 3 files changed, 365 insertions(+), 40 deletions(-) diff --git a/mobile/lib/features/channels/channel_directory.dart b/mobile/lib/features/channels/channel_directory.dart index 58fa9ab13f3..86efb2d2edc 100644 --- a/mobile/lib/features/channels/channel_directory.dart +++ b/mobile/lib/features/channels/channel_directory.dart @@ -132,13 +132,32 @@ class _DirectoryRefreshFence { } } -/// Whether [fence] has been retired, so its refresh must write nothing. +/// Whether a detached unread catch-up has been superseded, so it writes nothing. /// -/// Used on the detached unread catch-up path, where throwing -/// [_StaleDirectoryRequest] would only surface as an unhandled error: the -/// catch-up runs fire-and-forget, so a retired scope simply stops. -bool _isRetired(_DirectoryRefreshFence? fence) => - fence != null && !fence.isCurrent; +/// The directory fence above covers only refreshes that fetch the directory, so +/// it cannot describe the initial load, an ordinary membership refresh (the one +/// a join performs) or the reconnect backstop. Those refreshes also start a +/// detached catch-up that can outlive them, so every catch-up captures +/// [_subscriptionVersion] as its generation instead, and this rejects any +/// generation that is no longer the current one. +/// +/// One monotonic counter covers both hazards. Every channel-list refresh bumps +/// it in `_subscribeLive`, which retires an older catch-up on the same relay and +/// identity. A community or identity switch rebuilds the notifier, and the +/// disposal that rebuild runs bumps it too, so a switch retires the catch-up +/// without needing a second relay-and-identity comparison. That redundancy was +/// measured, not assumed: a disconnected-community-switch arm passes with this +/// check alone and fails when it is removed. +/// +/// A retired catch-up returns rather than throwing [_StaleDirectoryRequest]: +/// nothing awaits it, so a throw would only surface as an unhandled error. +/// +/// An extension in this part file rather than a method on the notifier because +/// `channels_provider.dart` sits against the repository-wide 1000-line file +/// ceiling enforced by `just file-size-check`. +extension _CatchUpFencing on ChannelsNotifier { + bool _isCatchUpRetired(int generation) => generation != _subscriptionVersion; +} /// Awaits [future], then rejects the result if the refresh was retired. /// diff --git a/mobile/lib/features/channels/channels_provider.dart b/mobile/lib/features/channels/channels_provider.dart index 59a8c99c1f5..c628aedcbe4 100644 --- a/mobile/lib/features/channels/channels_provider.dart +++ b/mobile/lib/features/channels/channels_provider.dart @@ -354,7 +354,7 @@ class ChannelsNotifier extends AsyncNotifier> { // Subscriptions are shared relay state, so a retired refresh must not // install them even though its channel list is already built. fence?.ensureCurrent(); - await _fenced(fence, _subscribeLive(channels, fence)); + await _fenced(fence, _subscribeLive(channels)); } // Guard the provider-state write in `retryDirectory` and `build`: the // caller assigns whatever this returns, so the last check belongs here. @@ -534,12 +534,7 @@ class ChannelsNotifier extends AsyncNotifier> { /// Subscribe per-channel to live events (requires `#h` tag for relay /// channel-scoped fan-out). Also starts a 60s WS backstop poll to reconcile /// membership changes without repeatedly downloading the global directory. - /// The [fence] rides along so the detached unread catch-up started at the - /// end of the sync can still tell whether its scope is current. - Future _subscribeLive( - List channels, - _DirectoryRefreshFence? fence, - ) { + Future _subscribeLive(List channels) { final channelIds = { for (final channel in channels) if (channel.isMember && !channel.isArchived) channel.id, @@ -550,12 +545,8 @@ class ChannelsNotifier extends AsyncNotifier> { final subscriptionVersion = ++_subscriptionVersion; final sync = _liveSubscriptionQueue.then( - (_) => _syncLiveSubscriptions( - relayBaseUrl, - subscriptionVersion, - channels, - fence, - ), + (_) => + _syncLiveSubscriptions(relayBaseUrl, subscriptionVersion, channels), ); _liveSubscriptionQueue = sync.catchError((Object error, StackTrace stack) { debugPrint( @@ -569,7 +560,6 @@ class ChannelsNotifier extends AsyncNotifier> { String relayBaseUrl, int subscriptionVersion, List channels, - _DirectoryRefreshFence? fence, ) async { if (ref.read(relaySessionProvider).status != SessionStatus.connected) { return; @@ -580,7 +570,6 @@ class ChannelsNotifier extends AsyncNotifier> { ref.read(relayConfigProvider).baseUrl, _subscriptionVersion, _desiredLiveChannels, - fence, ); return; } @@ -654,7 +643,7 @@ class ChannelsNotifier extends AsyncNotifier> { return; } - unawaited(_catchUpUnreadEvents(channels, fence)); + unawaited(_catchUpUnreadEvents(channels)); _backstopTimer?.cancel(); _backstopTimer = Timer.periodic( @@ -665,19 +654,20 @@ class ChannelsNotifier extends AsyncNotifier> { /// Backfills unread badges for the channels this refresh just installed. /// - /// Runs detached from the refresh that starts it, so [fence] is what keeps a - /// response that outlived its community or identity from writing unread - /// state into the scope the user switched to. A retired refresh returns - /// instead of throwing: nothing awaits this future, so a thrown + /// Runs detached from the refresh that starts it, so the lifecycle token + /// captured below is what keeps a response that outlived its refresh from + /// writing unread state into whatever the user is looking at now. Every + /// refresh path starts a catch-up, including the initial load, the ordinary + /// membership refresh a join performs and the reconnect backstop, so the + /// token is unconditional rather than tied to discovery. A retired refresh + /// returns instead of throwing: nothing awaits this future, so a thrown /// [_StaleDirectoryRequest] would only surface as an unhandled error. /// - /// The relay round-trip is the only await here, so one re-check after it - /// covers every write below. Guards before it or after the writes were - /// measured unreachable: retirement cannot land in a synchronous gap. - Future _catchUpUnreadEvents( - List channels, - _DirectoryRefreshFence? fence, - ) async { + /// The generation is captured here rather than passed in because this method + /// runs synchronously up to its relay query, so the value it reads is still + /// the starting refresh's own. + Future _catchUpUnreadEvents(List channels) async { + final generation = _subscriptionVersion; final myPk = ref.read(myPubkeyProvider); if (myPk == null) return; @@ -719,10 +709,10 @@ class ChannelsNotifier extends AsyncNotifier> { filters, operation: 'unread catch-up', ); - // The relay round-trip above is the window Jed's probe parks in: a - // community or identity switch here means every write below belongs to a - // scope the user has left. - if (_isRetired(fence)) return; + // The relay round-trip above is the window Jed's probes park in: a newer + // refresh, a community switch or an identity switch here means every + // write below belongs to a channel list the user has left. + if (_isCatchUpRetired(generation)) return; for (final event in events) { if (event.pubkey.toLowerCase() == myPk.toLowerCase()) { @@ -730,6 +720,7 @@ class ChannelsNotifier extends AsyncNotifier> { } } + var recorded = false; for (final event in events) { final channelId = event.channelId; if (channelId == null) continue; @@ -750,12 +741,19 @@ class ChannelsNotifier extends AsyncNotifier> { continue; } _recordUnreadEvent(channel, event, myPk); + recorded = true; + } + // Republish only when this catch-up actually changed unread state. A + // batch that recorded nothing has nothing to show, and a failed or + // superseded batch must not repaint another refresh's list: the retired + // check above already returned in that case, and no await separates it + // from here, so a second check would be dead code. + if (recorded) { + state = state.whenData((channels) => List.of(channels)); } } catch (error) { debugPrint('[ChannelsNotifier] unread catch-up failed: $error'); } - - state = state.whenData((channels) => List.of(channels)); } void _handleLiveEvent(NostrEvent event) { diff --git a/mobile/test/features/channels/channels_provider_test.dart b/mobile/test/features/channels/channels_provider_test.dart index b9991bacd32..31181b716dc 100644 --- a/mobile/test/features/channels/channels_provider_test.dart +++ b/mobile/test/features/channels/channels_provider_test.dart @@ -873,6 +873,314 @@ void main() { ); }); + test('an ordinary membership refresh retires an older catch-up', () async { + final session = _FakeRelaySession( + memberships: [_membership(_channelA, myPk)], + metadata: [_meta(id: _channelA, name: 'joined-a')], + ); + final container = _buildContainer(session: session); + addTearDown(container.dispose); + + await container.read(channelsProvider.future); + + // Same relay, same identity. Only the refresh generation separates the + // parked catch-up from the membership list the user is looking at, which + // is the window the post-join membership refresh opens. + session.recentMessages = const [ + NostrEvent( + id: 'mention-in-a', + pubkey: 'alice', + createdAt: 50, + kind: 9, + tags: [ + ['h', _channelA], + ['p', myPk], + ], + content: 'direct mention in channel A', + sig: 'sig', + ), + ]; + session.pauseNextUnreadCatchUpQuery(); + final staleRefresh = container.read(channelsProvider.notifier).refresh(); + await session.nextUnreadCatchUpQueryStarted; + + session.memberships = [_membership(_channelB, myPk)]; + session.metadata = [_meta(id: _channelB, name: 'joined-b')]; + await container.read(channelsProvider.notifier).refresh(); + await _settle(); + + session.resumePausedUnreadCatchUpQuery(); + await staleRefresh; + await _settle(); + + final notifier = container.read(channelsProvider.notifier); + expect( + notifier.latestObservedByChannel, + isNot(contains(_channelA)), + reason: + 'a superseded ordinary refresh wrote unread state for a channel the ' + 'user has left: ${notifier.latestObservedByChannel}', + ); + expect( + notifier.observedUnreadEventsByChannel, + isNot(contains(_channelA)), + reason: + 'a superseded ordinary refresh wrote unread events for a channel the ' + 'user has left: ${notifier.observedUnreadEventsByChannel}', + ); + }); + + test('a newer refresh retires a parked backstop catch-up', () async { + final session = _FakeRelaySession( + memberships: [_membership(_channelA, myPk)], + metadata: [_meta(id: _channelA, name: 'joined-a')], + ); + final container = _buildContainer(session: session); + addTearDown(container.dispose); + + await container.read(channelsProvider.future); + + session.recentMessages = const [ + NostrEvent( + id: 'mention-in-a', + pubkey: 'alice', + createdAt: 50, + kind: 9, + tags: [ + ['h', _channelA], + ['p', myPk], + ], + content: 'direct mention in channel A', + sig: 'sig', + ), + ]; + session.pauseNextUnreadCatchUpQuery(); + // Reconnecting runs the backstop refresh, which never fetches the + // directory, so this is the path that carried no lifecycle token at all. + session.setStatus(SessionStatus.reconnecting); + session.setStatus(SessionStatus.connected); + await session.nextUnreadCatchUpQueryStarted; + + session.memberships = [_membership(_channelB, myPk)]; + session.metadata = [_meta(id: _channelB, name: 'joined-b')]; + await container.read(channelsProvider.notifier).refresh(); + await _settle(); + + session.resumePausedUnreadCatchUpQuery(); + await _settle(); + + final notifier = container.read(channelsProvider.notifier); + expect( + notifier.latestObservedByChannel, + isNot(contains(_channelA)), + reason: + 'a superseded backstop refresh wrote unread state for a channel the ' + 'user has left: ${notifier.latestObservedByChannel}', + ); + expect( + notifier.observedUnreadEventsByChannel, + isNot(contains(_channelA)), + reason: + 'a superseded backstop refresh wrote unread events for a channel the ' + 'user has left: ${notifier.observedUnreadEventsByChannel}', + ); + }); + + test( + 'community switch discards a parked catch-up from an ordinary refresh', + () async { + final session = _FakeRelaySession( + memberships: [_membership(_channelA, myPk)], + metadata: [_meta(id: _channelA, name: 'community-a-general')], + ); + final container = _buildContainer(session: session); + addTearDown(container.dispose); + + await container.read(channelsProvider.future); + + session.recentMessages = const [ + NostrEvent( + id: 'mention-in-a', + pubkey: 'alice', + createdAt: 50, + kind: 9, + tags: [ + ['h', _channelA], + ['p', myPk], + ], + content: 'direct mention in community A', + sig: 'sig', + ), + ]; + session.pauseNextUnreadCatchUpQuery(); + final staleRefresh = container.read(channelsProvider.notifier).refresh(); + await session.nextUnreadCatchUpQueryStarted; + + session.memberships = [_membership(_channelB, myPk)]; + session.metadata = [_meta(id: _channelB, name: 'community-b-general')]; + container + .read(relayConfigProvider.notifier) + .update(baseUrl: 'https://community-b.example'); + await container.read(channelsProvider.future); + await _settle(); + + session.resumePausedUnreadCatchUpQuery(); + await staleRefresh; + await _settle(); + + final notifier = container.read(channelsProvider.notifier); + expect( + notifier.latestObservedByChannel, + isNot(contains(_channelA)), + reason: + 'community A unread state landed in community B: ' + '${notifier.latestObservedByChannel}', + ); + }, + ); + + test( + 'identity switch discards a parked catch-up from an ordinary refresh', + () async { + final session = _FakeRelaySession( + memberships: [_membership(_channelA, myPk)], + metadata: [_meta(id: _channelA, name: 'first-identity-general')], + ); + final container = _buildContainer(session: session); + addTearDown(container.dispose); + + await container.read(channelsProvider.future); + + session.recentMessages = const [ + NostrEvent( + id: 'mention-in-a', + pubkey: 'alice', + createdAt: 50, + kind: 9, + tags: [ + ['h', _channelA], + ['p', myPk], + ], + content: 'direct mention for the first identity', + sig: 'sig', + ), + ]; + session.pauseNextUnreadCatchUpQuery(); + final staleRefresh = container.read(channelsProvider.notifier).refresh(); + await session.nextUnreadCatchUpQueryStarted; + + session.memberships = [_membership(_channelB, _otherPk)]; + session.metadata = [_meta(id: _channelB, name: 'second-identity-joined')]; + container.read(_testPubkeyProvider.notifier).set(_otherPk); + await container.read(channelsProvider.future); + await _settle(); + + session.resumePausedUnreadCatchUpQuery(); + await staleRefresh; + await _settle(); + + final notifier = container.read(channelsProvider.notifier); + expect( + notifier.latestObservedByChannel, + isNot(contains(_channelA)), + reason: + 'first identity unread state landed on the second identity: ' + '${notifier.latestObservedByChannel}', + ); + }, + ); + + test('a disconnected community switch retires a parked catch-up', () async { + final session = _FakeRelaySession( + memberships: [_membership(_channelA, myPk)], + metadata: [_meta(id: _channelA, name: 'community-a-general')], + ); + final container = _buildContainer(session: session); + addTearDown(container.dispose); + + await container.read(channelsProvider.future); + + session.recentMessages = const [ + NostrEvent( + id: 'mention-in-a', + pubkey: 'alice', + createdAt: 50, + kind: 9, + tags: [ + ['h', _channelA], + ['p', myPk], + ], + content: 'direct mention in community A', + sig: 'sig', + ), + ]; + session.pauseNextUnreadCatchUpQuery(); + final started = session.nextUnreadCatchUpQueryStarted; + final staleRefresh = container.read(channelsProvider.notifier).refresh(); + await started; + + // Switch community while the session is down, so the new scope runs no + // refresh of its own. This is the arm that shows the single generation + // counter is sufficient: the rebuild's disposal bumps it even though no + // new refresh does. + session.setStatus(SessionStatus.disconnected); + container + .read(relayConfigProvider.notifier) + .update(baseUrl: 'https://community-b.example'); + await _settle(); + + session.resumePausedUnreadCatchUpQuery(); + await staleRefresh; + await _settle(); + + final notifier = container.read(channelsProvider.notifier); + expect( + notifier.latestObservedByChannel, + isNot(contains(_channelA)), + reason: + 'community A unread state survived a disconnected switch: ' + '${notifier.latestObservedByChannel}', + ); + }); + + test('a catch-up that records nothing does not repaint the list', () async { + final session = _FakeRelaySession( + memberships: [_membership(_channelA, myPk)], + metadata: [_meta(id: _channelA, name: 'joined-a')], + ); + final container = _buildContainer(session: session); + addTearDown(container.dispose); + + await container.read(channelsProvider.future); + + // No unread events to record, so the catch-up has nothing to publish. + // The catch-up is detached, so its query starts inside the refresh: hold + // the started future before awaiting the refresh or the one-shot slot is + // already claimed and reset by the time we ask for it. + session.pauseNextUnreadCatchUpQuery(); + final started = session.nextUnreadCatchUpQueryStarted; + await container.read(channelsProvider.notifier).refresh(); + await started; + await _settle(); + + final emissions = []; + final subscription = container.listen( + channelsProvider, + (_, next) => emissions.add(next.requireValue.length), + fireImmediately: false, + ); + addTearDown(subscription.close); + + session.resumePausedUnreadCatchUpQuery(); + await _settle(); + + expect( + emissions, + isEmpty, + reason: 'an empty unread catch-up republished provider state: $emissions', + ); + }); + test('community switch drops unread state recorded before it', () async { final session = _FakeRelaySession( memberships: [_membership(_channelA, myPk)], @@ -1779,7 +2087,7 @@ class _FakeRelaySession extends RelaySessionNotifier { final bool repeatLastMetadataPage; final int? maxMetadataPageRequests; final List hiddenDmEvents; - final List recentMessages; + List recentMessages; int membershipFailures; int directoryFailures = 0; bool failClaimedMemberCountQuery = false; From 7df8254e83d2efe340737b72facb40a69c5b8a27 Mon Sep 17 00:00:00 2001 From: Tom Brow Date: Sat, 22 Aug 2026 06:09:29 -0700 Subject: [PATCH 12/14] fix(mobile): order channel refreshes by request start Signed-off-by: Tom Brow --- .../features/channels/channel_directory.dart | 113 ++++++++++-------- .../features/channels/channels_provider.dart | 102 +++++++++------- .../channels/channels_provider_test.dart | 70 +++++++++++ 3 files changed, 192 insertions(+), 93 deletions(-) diff --git a/mobile/lib/features/channels/channel_directory.dart b/mobile/lib/features/channels/channel_directory.dart index 86efb2d2edc..dbbce80d7e0 100644 --- a/mobile/lib/features/channels/channel_directory.dart +++ b/mobile/lib/features/channels/channel_directory.dart @@ -44,12 +44,23 @@ class ChannelDirectoryLoadNotifier extends Notifier { @override ChannelDirectoryLoadState build() => const ChannelDirectoryLoadState.idle(); + /// Whether [scope] currently owns an in-flight directory request. + bool isLoading(String scope) => + state.scope == scope && + state.status == ChannelDirectoryLoadStatus.loading; + /// Marks the directory as loading. void markLoading(String scope) => state = ChannelDirectoryLoadState( scope: scope, status: ChannelDirectoryLoadStatus.loading, ); + /// Marks the directory as eligible for a fresh request. + void markIdle(String scope) => state = ChannelDirectoryLoadState( + scope: scope, + status: ChannelDirectoryLoadStatus.idle, + ); + /// Marks the directory as successfully loaded. void markLoaded(String scope) => state = ChannelDirectoryLoadState( scope: scope, @@ -89,74 +100,63 @@ Future> _fetchChannelDirectoryMetas( operation: 'Channel directory', ); -/// Thrown when a directory request is retired before its response lands. +/// Thrown when a channel-list request is retired before it settles. /// -/// Callers must treat this as "write nothing": the scope that issued the -/// request is no longer active, so both its data and its load status belong to -/// a community the user has left. -class _StaleDirectoryRequest implements Exception { - const _StaleDirectoryRequest(); +/// Callers must treat this as "write nothing": a newer request or scope now +/// owns the installed list and its related cache, subscription, and load state. +class _StaleChannelRefresh implements Exception { + const _StaleChannelRefresh(); @override String toString() => - 'Channel directory request retired by a community or identity switch'; + 'Channel refresh retired by a newer request or scope change'; } -/// Carries one directory-triggered refresh's scope across every later await. +/// Carries one channel-list refresh's request ownership across every await. /// -/// The loader's own fence only covers the directory query. The refresh that -/// query feeds keeps crossing awaits for DM profiles, hidden DMs, member -/// counts, latest messages and live subscription sync, and each one is another -/// chance for the user to switch community or identity. This token is captured -/// once at the start of the refresh and re-checked after every await, so a -/// response that outlived its scope cannot reach shared state. -class _DirectoryRefreshFence { +/// This token is captured once at the start of every ordinary, directory, and +/// reconnect refresh. It is re-checked after each relay await, so an older +/// request cannot regain ownership by reaching subscription setup last. +class _ChannelRefreshFence { /// Relay-and-identity scope that started the refresh. final String scope; - final _ChannelDirectoryLoader _loader; + final _ChannelRefreshCoordinator _coordinator; final int _generation; - _DirectoryRefreshFence(this._loader, this.scope, this._generation); + _ChannelRefreshFence(this._coordinator, this.scope, this._generation); /// Whether the refresh still owns the active scope and generation. bool get isCurrent => - _generation == _loader.generation && scope == _loader.currentScope(); + _generation == _coordinator.generation && + scope == _coordinator.currentScope(); - /// Throws [_StaleDirectoryRequest] once this refresh has been retired. + /// Throws [_StaleChannelRefresh] once this refresh has been retired. /// /// Call after every await and immediately before every write to metadata, /// cache, load status, subscriptions or provider state. void ensureCurrent() { - if (!isCurrent) throw const _StaleDirectoryRequest(); + if (!isCurrent) throw const _StaleChannelRefresh(); } } /// Whether a detached unread catch-up has been superseded, so it writes nothing. /// -/// The directory fence above covers only refreshes that fetch the directory, so -/// it cannot describe the initial load, an ordinary membership refresh (the one -/// a join performs) or the reconnect backstop. Those refreshes also start a -/// detached catch-up that can outlive them, so every catch-up captures -/// [_subscriptionVersion] as its generation instead, and this rejects any -/// generation that is no longer the current one. -/// -/// One monotonic counter covers both hazards. Every channel-list refresh bumps -/// it in `_subscribeLive`, which retires an older catch-up on the same relay and -/// identity. A community or identity switch rebuilds the notifier, and the -/// disposal that rebuild runs bumps it too, so a switch retires the catch-up -/// without needing a second relay-and-identity comparison. That redundancy was -/// measured, not assumed: a disconnected-community-switch arm passes with this -/// check alone and fails when it is removed. +/// The refresh fence is acquired before the first relay await, which makes this +/// request-ordered rather than subscription-completion-ordered. The subscription +/// generation remains a second lifecycle check for disconnect and disposal. /// -/// A retired catch-up returns rather than throwing [_StaleDirectoryRequest]: +/// A retired catch-up returns rather than throwing [_StaleChannelRefresh]: /// nothing awaits it, so a throw would only surface as an unhandled error. /// /// An extension in this part file rather than a method on the notifier because /// `channels_provider.dart` sits against the repository-wide 1000-line file /// ceiling enforced by `just file-size-check`. extension _CatchUpFencing on ChannelsNotifier { - bool _isCatchUpRetired(int generation) => generation != _subscriptionVersion; + bool _isCatchUpRetired( + _ChannelRefreshFence fence, + int subscriptionGeneration, + ) => !fence.isCurrent || subscriptionGeneration != _subscriptionVersion; } /// Awaits [future], then rejects the result if the refresh was retired. @@ -168,15 +168,15 @@ extension _CatchUpFencing on ChannelsNotifier { /// surface an ordinary exception, and `retryDirectory` would treat it as a /// failure of the current scope: it would mark the wrong scope's status and /// reinstall the channel list it captured before the switch. -Future _fenced(_DirectoryRefreshFence? fence, Future future) async { +Future _fenced(_ChannelRefreshFence fence, Future future) async { final T value; try { value = await future; } catch (_) { - if (fence != null && !fence.isCurrent) throw const _StaleDirectoryRequest(); + if (!fence.isCurrent) throw const _StaleChannelRefresh(); rethrow; } - fence?.ensureCurrent(); + fence.ensureCurrent(); return value; } @@ -191,7 +191,7 @@ Future _fenced(_DirectoryRefreshFence? fence, Future future) async { /// repository-wide 1000-line file ceiling enforced by `just file-size-check`. Future> _resolveDmDisplayNames( RelaySessionNotifier session, - _DirectoryRefreshFence? fence, + _ChannelRefreshFence fence, Iterable dedupedMetas, String myPk, ) async { @@ -255,7 +255,7 @@ Map _memberCountsByChannelId(Iterable memberEvents) { /// /// Lives in this part file because `channels_provider.dart` sits against the /// repository-wide 1000-line file ceiling enforced by `just file-size-check`. -class _ChannelDirectoryLoader { +class _ChannelRefreshCoordinator { /// Resolves the relay-and-identity scope that is active right now. final String Function() currentScope; @@ -267,7 +267,7 @@ class _ChannelDirectoryLoader { /// Generation of the most recently issued or retired request. int get generation => _generation; - _ChannelDirectoryLoader({ + _ChannelRefreshCoordinator({ required this.currentScope, required this.loadStatus, }); @@ -276,13 +276,14 @@ class _ChannelDirectoryLoader { /// /// Both closures read rather than watch: the fence asks what the scope is /// right now, and must not make the notifier depend on it. - factory _ChannelDirectoryLoader.forRef(Ref ref) => _ChannelDirectoryLoader( - currentScope: () => channelDirectoryScope( - ref.read(relayConfigProvider).baseUrl, - ref.read(myPubkeyProvider), - ), - loadStatus: () => ref.read(channelDirectoryLoadStatusProvider.notifier), - ); + factory _ChannelRefreshCoordinator.forRef(Ref ref) => + _ChannelRefreshCoordinator( + currentScope: () => channelDirectoryScope( + ref.read(relayConfigProvider).baseUrl, + ref.read(myPubkeyProvider), + ), + loadStatus: () => ref.read(channelDirectoryLoadStatusProvider.notifier), + ); /// Retires any in-flight request without starting a new one. /// @@ -295,10 +296,16 @@ class _ChannelDirectoryLoader { /// Used by callers that must carry the scope across later awaits even when /// they do not refresh discovery, so a membership-only refresh cannot install /// an old scope's list either. - _DirectoryRefreshFence beginRefresh() { + _ChannelRefreshFence beginRefresh({required bool fetchesDirectory}) { final scope = currentScope(); final generation = ++_generation; - return _DirectoryRefreshFence(this, scope, generation); + if (!fetchesDirectory) { + final status = loadStatus(); + if (status.isLoading(scope)) { + status.markIdle(scope); + } + } + return _ChannelRefreshFence(this, scope, generation); } /// Fetches the directory under [fence], or throws if the fence is retired. @@ -306,9 +313,9 @@ class _ChannelDirectoryLoader { /// Returns null when the request failed inside the current scope, which means /// "retain the cached discovery". The fence is re-checked after the await and /// before every write, on both the success and the failure path. - Future?> load( + Future?> loadDirectory( RelaySessionNotifier session, - _DirectoryRefreshFence fence, + _ChannelRefreshFence fence, ) async { loadStatus().markLoading(fence.scope); final List metas; diff --git a/mobile/lib/features/channels/channels_provider.dart b/mobile/lib/features/channels/channels_provider.dart index c628aedcbe4..d68f992fce9 100644 --- a/mobile/lib/features/channels/channels_provider.dart +++ b/mobile/lib/features/channels/channels_provider.dart @@ -40,7 +40,6 @@ class ChannelsNotifier extends AsyncNotifier> { final Map _unsubscribersByChannel = {}; Future _liveSubscriptionQueue = Future.value(); - List _desiredLiveChannels = const []; Set _desiredLiveChannelIds = const {}; int _subscriptionVersion = 0; String? _subscriptionRelayBaseUrl; @@ -58,8 +57,8 @@ class ChannelsNotifier extends AsyncNotifier> { List _directoryMetas = const []; /// Fences directory responses to the relay and identity that requested them. - late final _ChannelDirectoryLoader _directoryLoader = - _ChannelDirectoryLoader.forRef(ref); + late final _ChannelRefreshCoordinator _refreshCoordinator = + _ChannelRefreshCoordinator.forRef(ref); /// The member snapshot already returned while loading the channel list. /// @@ -90,7 +89,7 @@ class ChannelsNotifier extends AsyncNotifier> { _directoryMetas = const []; // Retire any in-flight directory request: its response describes the // previous relay or identity and must not reach this scope's state. - _directoryLoader.retireInFlight(); + _refreshCoordinator.retireInFlight(); } final connected = Completer(); final sessionState = ref.read(relaySessionProvider); @@ -157,12 +156,12 @@ class ChannelsNotifier extends AsyncNotifier> { final session = ref.read(relaySessionProvider.notifier); - // A directory-triggered refresh is fenced end to end. The fence is captured - // before the first await and re-checked after every later one, so a - // community or identity switch retires the whole refresh rather than only - // its directory query. Refreshes that do not touch discovery keep the - // previous unfenced behaviour. - final fence = fetchDirectory ? _directoryLoader.beginRefresh() : null; + // Acquire request ownership before the first relay await. Every channel-list + // path uses this fence so completion order cannot let an older ordinary, + // directory, or reconnect refresh replace a newer membership list. + final fence = _refreshCoordinator.beginRefresh( + fetchesDirectory: fetchDirectory, + ); // Step 1: find the channels I'm a member of via kind:39002. final memberships = await _fenced( @@ -190,8 +189,8 @@ class ChannelsNotifier extends AsyncNotifier> { // kind:39000 query by the caller's access, but the client still rejects // private channels and DMs below so discovery fails closed if that contract // ever regresses. The composite cursor preserves tied-timestamp rows. - if (fence != null) { - final metas = await _directoryLoader.load(session, fence); + if (fetchDirectory) { + final metas = await _refreshCoordinator.loadDirectory(session, fence); if (metas != null) _directoryMetas = metas; } @@ -338,7 +337,7 @@ class ChannelsNotifier extends AsyncNotifier> { // await above is fenced, but the switch can also land in the synchronous // gap, so the guard sits immediately before the write rather than only // after the await. - fence?.ensureCurrent(); + fence.ensureCurrent(); final prevById = { for (final c in state.value ?? const []) c.id: c, @@ -353,12 +352,12 @@ class ChannelsNotifier extends AsyncNotifier> { if (subscribeLive) { // Subscriptions are shared relay state, so a retired refresh must not // install them even though its channel list is already built. - fence?.ensureCurrent(); - await _fenced(fence, _subscribeLive(channels)); + fence.ensureCurrent(); + await _fenced(fence, _subscribeLive(channels, fence)); } // Guard the provider-state write in `retryDirectory` and `build`: the // caller assigns whatever this returns, so the last check belongs here. - fence?.ensureCurrent(); + fence.ensureCurrent(); return channels; } @@ -534,24 +533,32 @@ class ChannelsNotifier extends AsyncNotifier> { /// Subscribe per-channel to live events (requires `#h` tag for relay /// channel-scoped fan-out). Also starts a 60s WS backstop poll to reconcile /// membership changes without repeatedly downloading the global directory. - Future _subscribeLive(List channels) { + Future _subscribeLive( + List channels, + _ChannelRefreshFence fence, + ) { final channelIds = { for (final channel in channels) if (channel.isMember && !channel.isArchived) channel.id, }; final relayBaseUrl = ref.read(relayConfigProvider).baseUrl; - _desiredLiveChannels = channels; _desiredLiveChannelIds = channelIds; final subscriptionVersion = ++_subscriptionVersion; final sync = _liveSubscriptionQueue.then( - (_) => - _syncLiveSubscriptions(relayBaseUrl, subscriptionVersion, channels), + (_) => _syncLiveSubscriptions( + relayBaseUrl, + subscriptionVersion, + channels, + fence, + ), ); _liveSubscriptionQueue = sync.catchError((Object error, StackTrace stack) { - debugPrint( - '[ChannelsNotifier] live subscription sync failed: $error\n$stack', - ); + if (error is! _StaleChannelRefresh) { + debugPrint( + '[ChannelsNotifier] live subscription sync failed: $error\n$stack', + ); + } }); return sync; } @@ -560,17 +567,14 @@ class ChannelsNotifier extends AsyncNotifier> { String relayBaseUrl, int subscriptionVersion, List channels, + _ChannelRefreshFence fence, ) async { + fence.ensureCurrent(); if (ref.read(relaySessionProvider).status != SessionStatus.connected) { return; } if (subscriptionVersion != _subscriptionVersion) { - await _syncLiveSubscriptions( - ref.read(relayConfigProvider).baseUrl, - _subscriptionVersion, - _desiredLiveChannels, - ); return; } @@ -609,7 +613,12 @@ class ChannelsNotifier extends AsyncNotifier> { ), _handleLiveEvent, ); - if (ref.read(relaySessionProvider).status != SessionStatus.connected || + if (!fence.isCurrent) { + unsubscribe(); + throw const _StaleChannelRefresh(); + } + if (subscriptionVersion != _subscriptionVersion || + ref.read(relaySessionProvider).status != SessionStatus.connected || !_desiredLiveChannelIds.contains(channelId) || ref.read(relayConfigProvider).baseUrl != relayBaseUrl || _subscriptionRelayBaseUrl != relayBaseUrl) { @@ -622,6 +631,8 @@ class ChannelsNotifier extends AsyncNotifier> { continue; } _unsubscribersByChannel[channelId] = unsubscribe; + } on _StaleChannelRefresh { + rethrow; } catch (error) { debugPrint( '[ChannelsNotifier] live subscription failed for $channelId: $error', @@ -643,7 +654,8 @@ class ChannelsNotifier extends AsyncNotifier> { return; } - unawaited(_catchUpUnreadEvents(channels)); + fence.ensureCurrent(); + unawaited(_catchUpUnreadEvents(channels, fence, subscriptionVersion)); _backstopTimer?.cancel(); _backstopTimer = Timer.periodic( @@ -661,13 +673,15 @@ class ChannelsNotifier extends AsyncNotifier> { /// membership refresh a join performs and the reconnect backstop, so the /// token is unconditional rather than tied to discovery. A retired refresh /// returns instead of throwing: nothing awaits this future, so a thrown - /// [_StaleDirectoryRequest] would only surface as an unhandled error. + /// [_StaleChannelRefresh] would only surface as an unhandled error. /// - /// The generation is captured here rather than passed in because this method - /// runs synchronously up to its relay query, so the value it reads is still - /// the starting refresh's own. - Future _catchUpUnreadEvents(List channels) async { - final generation = _subscriptionVersion; + /// The request fence and subscription generation are passed from the refresh + /// that installed [channels], preserving request ownership after detachment. + Future _catchUpUnreadEvents( + List channels, + _ChannelRefreshFence fence, + int subscriptionGeneration, + ) async { final myPk = ref.read(myPubkeyProvider); if (myPk == null) return; @@ -712,7 +726,7 @@ class ChannelsNotifier extends AsyncNotifier> { // The relay round-trip above is the window Jed's probes park in: a newer // refresh, a community switch or an identity switch here means every // write below belongs to a channel list the user has left. - if (_isCatchUpRetired(generation)) return; + if (_isCatchUpRetired(fence, subscriptionGeneration)) return; for (final event in events) { if (event.pubkey.toLowerCase() == myPk.toLowerCase()) { @@ -886,6 +900,8 @@ class ChannelsNotifier extends AsyncNotifier> { } } state = AsyncData(channels); + } on _StaleChannelRefresh { + return; } catch (error) { debugPrint('[ChannelsNotifier] backstop refresh failed: $error'); } @@ -899,7 +915,14 @@ class ChannelsNotifier extends AsyncNotifier> { // cached channel list with [] or an error. Wait for `build()` to re-run // when the session transitions to connected. if (sessionState.status != SessionStatus.connected) return; - state = await AsyncValue.guard(() => _fetch(subscribeLive: true)); + try { + final channels = await _fetch(subscribeLive: true); + state = AsyncData(channels); + } on _StaleChannelRefresh { + return; + } catch (error, stackTrace) { + state = AsyncError(error, stackTrace); + } } /// Loads the directory when Browse channels opens after startup or an error. @@ -940,7 +963,7 @@ class ChannelsNotifier extends AsyncNotifier> { state = AsyncData( await _fetch(subscribeLive: true, fetchDirectory: true), ); - } on _StaleDirectoryRequest { + } on _StaleChannelRefresh { // A community or identity switch retired this request. Its response // describes a scope the user has left, so write neither the channel list // nor the load status; the new scope owns both now. @@ -955,7 +978,6 @@ class ChannelsNotifier extends AsyncNotifier> { void _clearLiveSubscriptions() { _subscriptionVersion++; - _desiredLiveChannels = const []; _desiredLiveChannelIds = const {}; for (final unsubscribe in _unsubscribersByChannel.values) { unsubscribe(); diff --git a/mobile/test/features/channels/channels_provider_test.dart b/mobile/test/features/channels/channels_provider_test.dart index 31181b716dc..5d93513e36e 100644 --- a/mobile/test/features/channels/channels_provider_test.dart +++ b/mobile/test/features/channels/channels_provider_test.dart @@ -653,6 +653,76 @@ void main() { }, ); + test('a newer ordinary refresh owns the installed membership list', () async { + final session = _FakeRelaySession( + memberships: [_membership(_channelA, myPk)], + metadata: [_meta(id: _channelA, name: 'joined-a')], + ); + final container = _buildContainer(session: session); + addTearDown(container.dispose); + + await container.read(channelsProvider.future); + + session.pauseNextHiddenDmQuery(); + final olderRefresh = container.read(channelsProvider.notifier).refresh(); + await session.nextHiddenDmQueryStarted; + + session.memberships = [_membership(_channelB, myPk)]; + session.metadata = [_meta(id: _channelB, name: 'joined-b')]; + await container.read(channelsProvider.notifier).refresh(); + + session.resumePausedHiddenDmQuery(); + await olderRefresh; + await _settle(); + + expect( + container + .read(channelsProvider) + .requireValue + .map((channel) => channel.id), + [_channelB], + reason: 'an older ordinary refresh overwrote the newer membership list', + ); + expect(session.activeChannels, {_channelB}); + }); + + test( + 'a newer refresh owns the list over an older reconnect backstop', + () async { + final session = _FakeRelaySession( + memberships: [_membership(_channelA, myPk)], + metadata: [_meta(id: _channelA, name: 'joined-a')], + ); + final container = _buildContainer(session: session); + addTearDown(container.dispose); + + await container.read(channelsProvider.future); + + session.pauseNextHiddenDmQuery(); + session.setStatus(SessionStatus.reconnecting); + session.setStatus(SessionStatus.connected); + await session.nextHiddenDmQueryStarted; + + session.memberships = [_membership(_channelB, myPk)]; + session.metadata = [_meta(id: _channelB, name: 'joined-b')]; + await container.read(channelsProvider.notifier).refresh(); + + session.resumePausedHiddenDmQuery(); + await _settle(); + + expect( + container + .read(channelsProvider) + .requireValue + .map((channel) => channel.id), + [_channelB], + reason: + 'an older reconnect backstop overwrote the newer membership list', + ); + expect(session.activeChannels, {_channelB}); + }, + ); + test('community switch discards a parked unread catch-up', () async { final session = _FakeRelaySession( memberships: const [], From 0d2f15edfb2bd077fb1e5ff8556ce441058443e2 Mon Sep 17 00:00:00 2001 From: leader <71e9f2c44a6932b6772caaaccda1911d010463c3e2c6c40410b8329956046801@buzz.block.builderlab.xyz> Date: Sat, 22 Aug 2026 06:49:39 -0700 Subject: [PATCH 13/14] Add regression arm for the stale-request Huddle leg fence The merge with main routed the kind:39002 membership fetch that feeds Huddle backing channels through the same refresh fence as the channel list, which closed a stale-write path into the cached member snapshots that main's unfenced early fetch left open. Nothing covered that leg. This arm parks an older refresh on its Huddle-start query, after both of its membership fetches have landed, lets a newer refresh settle on a disjoint membership set, then releases the older request and asserts it writes no member snapshot. Unfencing the Huddle leg back to main's shape reds it, so the arm tracks the fence rather than the surrounding refresh ordering. Test only, no production change. Co-authored-by: Tom Brow Signed-off-by: Tom Brow --- .../channels/channels_provider_test.dart | 109 ++++++++++++++++++ 1 file changed, 109 insertions(+) diff --git a/mobile/test/features/channels/channels_provider_test.dart b/mobile/test/features/channels/channels_provider_test.dart index a5c500878e1..7324d713dc9 100644 --- a/mobile/test/features/channels/channels_provider_test.dart +++ b/mobile/test/features/channels/channels_provider_test.dart @@ -723,6 +723,71 @@ void main() { }, ); + test('a stale request\'s Huddle leg writes no member snapshot', () async { + final now = DateTime.now().millisecondsSinceEpoch ~/ 1000; + final session = _FakeRelaySession( + memberships: [_membership(_channelA, myPk)], + metadata: [_meta(id: _channelA, name: 'joined-a')], + huddleStarts: [ + NostrEvent( + id: 'huddle-start', + pubkey: myPk, + createdAt: now, + kind: EventKind.huddleStarted, + tags: const [ + ['h', _channelA], + ], + content: '{"ephemeral_channel_id":"$_channelB"}', + sig: 'sig', + ), + ], + ); + final container = _buildContainer(session: session); + addTearDown(container.dispose); + + await container.read(channelsProvider.future); + final notifier = container.read(channelsProvider.notifier); + + // Park an older refresh on its Huddle-start query. Both of its membership + // fetches have already landed, so channel A is the list it is carrying. + session.pauseNextHuddleStartQuery(); + final olderRefresh = notifier.refresh(); + await session.nextHuddleStartQueryStarted; + + // A newer refresh completes on a disjoint membership set. + session.memberships = [ + _membership(_channelB, myPk, additionalPubkey: _otherPk), + ]; + session.metadata = [_meta(id: _channelB, name: 'joined-b')]; + await notifier.refresh(); + + // Release the older request. Its member-snapshot write sits AFTER the + // Huddle leg, so if that leg is unfenced the stale snapshot lands. + session.resumePausedHuddleStartQuery(); + await olderRefresh; + await _settle(); + + expect( + container + .read(channelsProvider) + .requireValue + .map((channel) => channel.id), + [_channelB], + reason: 'a stale request installed its own channel list', + ); + expect( + notifier.cachedMembersForChannel(_channelA), + isEmpty, + reason: + 'a stale request wrote a member snapshot past the Huddle leg fence', + ); + expect( + notifier.cachedMembersForChannel(_channelB).map((m) => m.pubkey), + containsAll([myPk, _otherPk]), + reason: 'the newer request\'s member snapshot was clobbered', + ); + }); + test('community switch discards a parked unread catch-up', () async { final session = _FakeRelaySession( memberships: const [], @@ -2309,6 +2374,9 @@ class _FakeRelaySession extends RelaySessionNotifier { Completer? _pausedMemberCount; Completer? _memberCountStarted; Completer? _claimedMemberCount; + Completer? _pausedHuddleStarts; + Completer? _huddleStartsStarted; + Completer? _claimedHuddleStarts; Completer? _pausedUnreadCatchUp; Completer? _unreadCatchUpStarted; Completer? _claimedUnreadCatchUp; @@ -2398,6 +2466,37 @@ class _FakeRelaySession extends RelaySessionNotifier { paused.complete(); } + /// Holds the next Huddle-start query open, one shot only. + /// + /// Parks the older refresh AFTER both membership fetches have landed, so the + /// only guard left between the park and the member-snapshot write is the + /// fence on this leg. + void pauseNextHuddleStartQuery() { + if (_pausedHuddleStarts != null) { + throw StateError('A Huddle-start query is already paused'); + } + _pausedHuddleStarts = Completer(); + _huddleStartsStarted = Completer(); + } + + /// Completes once the parked Huddle-start query has been requested. + Future get nextHuddleStartQueryStarted async { + final started = _huddleStartsStarted; + if (started == null) { + throw StateError('No Huddle-start query is pending'); + } + await started.future; + } + + /// Releases the parked Huddle-start query so its response lands. + void resumePausedHuddleStartQuery() { + final paused = _claimedHuddleStarts ?? _pausedHuddleStarts; + if (paused == null) throw StateError('No Huddle-start query is paused'); + _claimedHuddleStarts = null; + _pausedHuddleStarts = null; + paused.complete(); + } + /// Holds the next member-count query open, one shot only. void pauseNextMemberCountQuery() { if (_pausedMemberCount != null) { @@ -2508,6 +2607,16 @@ class _FakeRelaySession extends RelaySessionNotifier { return hiddenDmEvents; } if (filter.kinds.contains(EventKind.huddleStarted)) { + // Claim the parked slot so the newer refresh's own Huddle query runs + // unblocked: one shot, exactly like the hidden-DM and member-count hooks. + final paused = _pausedHuddleStarts; + if (paused != null) { + _claimedHuddleStarts = paused; + _pausedHuddleStarts = null; + _huddleStartsStarted!.complete(); + _huddleStartsStarted = null; + await paused.future; + } return huddleStarts; } if (filter.kinds.contains(39000)) { From aa63159c8a3dfb37feb0c2d5f6ca5ef5a28c9376 Mon Sep 17 00:00:00 2001 From: Tom Brow Date: Mon, 24 Aug 2026 08:53:48 -0700 Subject: [PATCH 14/14] fix(mobile): settle superseded directory loads Signed-off-by: Tom Brow Co-authored-by: Codex Ai-assisted: true --- .../features/channels/channel_directory.dart | 9 +- .../features/channels/channels_page_test.dart | 96 +++++++++++++++++++ .../channels/channels_provider_test.dart | 62 ++++++++++++ 3 files changed, 166 insertions(+), 1 deletion(-) diff --git a/mobile/lib/features/channels/channel_directory.dart b/mobile/lib/features/channels/channel_directory.dart index 2b7a6a07cd0..a99822ede5e 100644 --- a/mobile/lib/features/channels/channel_directory.dart +++ b/mobile/lib/features/channels/channel_directory.dart @@ -345,7 +345,14 @@ class _ChannelRefreshCoordinator { if (!fetchesDirectory) { final status = loadStatus(); if (status.isLoading(scope)) { - status.markIdle(scope); + // This refresh takes ownership of the shared generation, so the + // in-flight directory response will be discarded. The mounted Browse + // sheet cannot restart an idle request on its own: it only starts a + // load when it mounts and deliberately renders idle as its initial + // spinner. Settle the displaced load to a retryable terminal state so + // the sheet never waits forever for a response that may no longer + // write results. + status.markError(scope); } } return _ChannelRefreshFence(this, scope, generation); diff --git a/mobile/test/features/channels/channels_page_test.dart b/mobile/test/features/channels/channels_page_test.dart index 982dd968dce..68fa3152365 100644 --- a/mobile/test/features/channels/channels_page_test.dart +++ b/mobile/test/features/channels/channels_page_test.dart @@ -24,6 +24,7 @@ import 'package:buzz/shared/community/community_icon_provider.dart'; import 'package:buzz/shared/relay/relay.dart'; import 'package:buzz/shared/theme/theme.dart'; import 'package:buzz/shared/widgets/avatar_image.dart'; +import 'package:buzz/shared/widgets/buzz_loading_indicator.dart'; import 'package:buzz/shared/widgets/frosted_app_bar.dart'; import 'package:buzz/shared/widgets/masked_avatar_badge.dart'; import 'package:buzz/shared/widgets/skeleton.dart'; @@ -1508,6 +1509,61 @@ void main() { ); }); + testWidgets('browse action exposes retry when refresh supersedes loading', ( + tester, + ) async { + final joinable = Channel( + id: 'superseded-directory', + name: 'community-help', + channelType: 'stream', + visibility: 'open', + description: 'Help from the community', + createdBy: 'abc', + createdAt: DateTime(2025), + memberCount: 0, + ); + late _SupersededDirectoryNotifier notifier; + await tester.pumpWidget( + buildTestable( + disableAnimations: true, + overrides: [ + channelsProvider.overrideWith( + () => notifier = _SupersededDirectoryNotifier( + initialChannels: testChannels, + retriedChannels: [...testChannels, joinable], + ), + ), + ], + ), + ); + await tester.pumpAndSettle(); + + await tester.tap(find.byTooltip('Create or start conversation')); + await tester.pump(); + await tester.tap( + find.byKey(const Key('quick-action-browse-channels-card')), + ); + await tester.pumpAndSettle(); + + expect(find.byType(BuzzLoadingIndicator), findsOneWidget); + + notifier.supersedeLoadingDirectory(); + await tester.pumpAndSettle(); + + expect(find.byType(BuzzLoadingIndicator), findsNothing); + expect(find.text('Couldn’t load open channels.'), findsOneWidget); + expect(find.byKey(const Key('browse-channels-retry')), findsOneWidget); + + await tester.tap(find.byKey(const Key('browse-channels-retry'))); + await tester.pumpAndSettle(); + + expect(notifier.retryCount, 1); + expect( + find.byKey(const Key('browse-channel-superseded-directory')), + findsOneWidget, + ); + }); + testWidgets('browse action scrolls and joins an offscreen channel', ( tester, ) async { @@ -2319,6 +2375,46 @@ class _RetryingDirectoryNotifier extends ChannelsNotifier { } } +class _SupersededDirectoryNotifier extends ChannelsNotifier { + _SupersededDirectoryNotifier({ + required this.initialChannels, + required this.retriedChannels, + }); + + final List initialChannels; + final List retriedChannels; + final _directoryCompletion = Completer(); + int retryCount = 0; + + @override + Future> build() async => initialChannels; + + @override + Future ensureDirectoryLoaded() async { + ref + .read(channelDirectoryLoadStatusProvider.notifier) + .markLoading(_activeDirectoryScope(ref)); + await _directoryCompletion.future; + } + + /// Mirrors an ordinary refresh invalidating the active directory request. + void supersedeLoadingDirectory() { + ref + .read(channelDirectoryLoadStatusProvider.notifier) + .markError(_activeDirectoryScope(ref)); + _directoryCompletion.complete(); + } + + @override + Future retryDirectory() async { + retryCount++; + state = AsyncData(retriedChannels); + ref + .read(channelDirectoryLoadStatusProvider.notifier) + .markLoaded(_activeDirectoryScope(ref)); + } +} + String _activeDirectoryScope(Ref ref) => channelDirectoryScope( ref.read(relayConfigProvider).baseUrl, ref.read(myPubkeyProvider), diff --git a/mobile/test/features/channels/channels_provider_test.dart b/mobile/test/features/channels/channels_provider_test.dart index 7324d713dc9..ea6c38cdfd1 100644 --- a/mobile/test/features/channels/channels_provider_test.dart +++ b/mobile/test/features/channels/channels_provider_test.dart @@ -289,6 +289,68 @@ void main() { ); }); + test( + 'ordinary refresh settles a superseded directory load for retry', + () async { + final session = _FakeRelaySession( + memberships: [_membership(_channelA, myPk)], + metadata: [ + _meta(id: _channelA, name: 'general'), + _meta(id: _channelB, name: 'discoverable'), + ], + ); + final container = _buildContainer(session: session); + addTearDown(container.dispose); + + expect( + (await container.read( + channelsProvider.future, + )).map((channel) => channel.id), + [_channelA], + ); + + session.pauseNextDirectoryQuery(); + final directory = container + .read(channelsProvider.notifier) + .retryDirectory(); + await session.nextDirectoryQueryStarted; + + // A foreground, reconnect, pull-to-refresh, or membership update can + // start an ordinary refresh while Browse is still loading discovery. + await container.read(channelsProvider.notifier).refresh(); + + session.resumePausedDirectoryQuery(); + await directory; + await _settle(); + + expect( + container.read(channelDirectoryLoadStatusProvider).status, + ChannelDirectoryLoadStatus.error, + ); + expect( + container + .read(channelsProvider) + .requireValue + .map((channel) => channel.id), + [_channelA], + ); + + await container.read(channelsProvider.notifier).retryDirectory(); + + expect( + container.read(channelDirectoryLoadStatusProvider).status, + ChannelDirectoryLoadStatus.loaded, + ); + expect( + container + .read(channelsProvider) + .requireValue + .map((channel) => channel.id), + unorderedEquals([_channelA, _channelB]), + ); + }, + ); + test( 'community switch discards a stale directory success from the old relay', () async {