From dc6344e8b7d8fda453e398e55f444938c47a861c Mon Sep 17 00:00:00 2001 From: Logan Johnson Date: Mon, 2 Nov 2026 09:16:32 -0500 Subject: [PATCH] fix(mobile): skip an archived starter at desktop's id Desktop and mobile derive the same starter channel id, so an archived starter left at that id makes the relay answer "duplicate: channel already exists". The directory omits archived channels, so the reload found no holder and the invite stayed stuck in setup. When a create is refused as a duplicate and the reload has no match, read the channel at that id directly (kind:39000 via fetchOpenChannel) and skip the starter if it is archived. Every other case still rethrows so setup stays recoverable. Also make the scope-fence test able to fail: it now flips the scope on the reload after the last starter's refusal, so deleting the post-reload fence fails it. relayErrorMessage also strips the relay's error:, rate-limited: and auth-required: prefixes. Co-authored-by: Larry <627498bd4bd1f281a16431e3c6cce3b5c25b6692798c78672298aefbf2f8f8b5@buzz.block.builderlab.xyz> Co-Authored-By: Claude Opus 5.5 Signed-off-by: Logan Johnson --- mobile/lib/app.dart | 35 ++++-- .../lib/shared/relay/relay_closed_policy.dart | 4 +- .../invites/invite_join_provider_test.dart | 101 ++++++++++++++++-- .../relay/relay_closed_policy_test.dart | 3 + 4 files changed, 125 insertions(+), 18 deletions(-) diff --git a/mobile/lib/app.dart b/mobile/lib/app.dart index 7195544f0c..2e5d8a7111 100644 --- a/mobile/lib/app.dart +++ b/mobile/lib/app.dart @@ -84,6 +84,7 @@ final _inviteRelayConnectedProvider = FutureProvider.family(( /// App-level bridge from invite joining to the channels feature. class MobileInviteJoinRecovery implements InviteJoinRecovery { final Future> Function() _loadChannels; + final Future Function(String channelId) _fetchChannel; final Future Function({ required String channelId, required String name, @@ -97,9 +98,11 @@ class MobileInviteJoinRecovery implements InviteJoinRecovery { final String _relayHttpOrigin; final bool Function() _isScopeCurrent; - /// Creates recovery from channel-loading, creation, and join operations. + /// Creates recovery from channel-loading, single-channel lookup, creation, + /// and join operations. MobileInviteJoinRecovery({ required Future> Function() loadChannels, + required Future Function(String channelId) fetchChannel, required Future Function({ required String channelId, required String name, @@ -113,6 +116,7 @@ class MobileInviteJoinRecovery implements InviteJoinRecovery { required String relayHttpOrigin, bool Function()? isScopeCurrent, }) : _loadChannels = loadChannels, + _fetchChannel = fetchChannel, _createChannel = createChannel, _joinChannel = joinChannel, _relayHttpOrigin = relayHttpOrigin, @@ -124,8 +128,9 @@ class MobileInviteJoinRecovery implements InviteJoinRecovery { /// A partial setup remains recoverable, so a failed operation deliberately /// propagates to the invite flow instead of being reported as success. The /// one exception is a starter whose name or id the relay reports as taken - /// by a channel the directory shows cannot serve as the starter: that is - /// permanent, so the starter is skipped instead of blocking the join. + /// by a channel the directory shows cannot serve as the starter, or an + /// archived channel at the starter's own id that the directory omits: both + /// are permanent, so the starter is skipped instead of blocking the join. @override Future ensureStarterChannels() async { _ensureScopeCurrent(); @@ -162,9 +167,19 @@ class MobileInviteJoinRecovery implements InviteJoinRecovery { .where((candidate) => candidate.id == channelId) .firstOrNull ?? (nameTaken ? _findListedNamesake(channels, starter.slug) : null); - // A holder the directory does not show yet may still be usable, so - // keep the setup recoverable. - if (holder == null) rethrow; + if (holder == null) { + // The directory omits archived channels, yet the relay still + // keeps the id (desktop uses the same one), so a duplicate with + // no match may be an archived starter. Read that id directly. + if (!nameTaken && _isDuplicateChannelError(error)) { + final atId = await _fetchChannel(channelId); + _ensureScopeCurrent(); + if (atId != null && atId.isArchived) continue; + } + // A holder not visible yet may still be usable, so keep the setup + // recoverable. + rethrow; + } // The relay keeps a channel's id and a listed channel's name, archived // ones included, so an unusable holder never frees the starter. Skip // it rather than strand the invite in permanent recovery. @@ -274,6 +289,14 @@ InviteJoinRecovery buildMobileInviteJoinRecovery( ensureScopeCurrent(); return channels; }, + fetchChannel: (channelId) async { + ensureScopeCurrent(); + final channel = await ref + .read(channelsProvider.notifier) + .fetchOpenChannel(channelId); + ensureScopeCurrent(); + return channel; + }, createChannel: ({ required channelId, diff --git a/mobile/lib/shared/relay/relay_closed_policy.dart b/mobile/lib/shared/relay/relay_closed_policy.dart index fd4d608142..9f0b0dfa28 100644 --- a/mobile/lib/shared/relay/relay_closed_policy.dart +++ b/mobile/lib/shared/relay/relay_closed_policy.dart @@ -81,14 +81,14 @@ RelayClosedClass classifyRelayClosed(String message) { } final _relayErrorPrefixPattern = RegExp( - r'^(?:exception:\s*)?(?:(?:invalid|blocked|restricted|duplicate|pow|unsupported):\s*)?', + r'^(?:exception:\s*)?(?:(?:invalid|blocked|restricted|duplicate|pow|unsupported|error|rate-limited|auth-required):\s*)?', caseSensitive: false, ); /// User-facing text for a failed relay publish or request. /// /// Drops the Dart `Exception: ` wrapper and a leading NIP-01 machine prefix -/// (`invalid: `, `blocked: `, ...) so a relay refusal such as +/// (`invalid: `, `blocked: `, `error: `, ...) so a relay refusal such as /// `invalid: an open channel named "Launch" already exists` reads as /// `An open channel named "Launch" already exists`. String relayErrorMessage(Object error) { diff --git a/mobile/test/features/invites/invite_join_provider_test.dart b/mobile/test/features/invites/invite_join_provider_test.dart index f7de2f2a67..94008e0230 100644 --- a/mobile/test/features/invites/invite_join_provider_test.dart +++ b/mobile/test/features/invites/invite_join_provider_test.dart @@ -258,6 +258,7 @@ void main() { visibility: 'private', ), ], + fetchChannel: (_) async => fail('no direct lookup expected'), createChannel: ({ required channelId, @@ -284,6 +285,7 @@ void main() { final created = >{}; final recovery = MobileInviteJoinRecovery( loadChannels: () async => const [], + fetchChannel: (_) async => fail('no direct lookup expected'), createChannel: ({ required channelId, @@ -342,6 +344,7 @@ void main() { _channel(id: 'relay-welcome', name: 'welcome-everyone'), ]; }, + fetchChannel: (_) async => fail('no direct lookup expected'), createChannel: ({ required channelId, @@ -382,19 +385,29 @@ void main() { ({ List created, List joined, + List lookups, MobileInviteJoinRecovery recovery, int Function() loads, }) starterSetup({ required List Function(int load) channelsOnLoad, - required String generalError, + required String refusalError, + Channel? atId, + Set refused = const {'general'}, bool Function()? isScopeCurrent, + void Function()? onLookup, }) { final created = []; final joined = []; + final lookups = []; var loadCount = 0; final recovery = MobileInviteJoinRecovery( loadChannels: () async => channelsOnLoad(++loadCount), + fetchChannel: (channelId) async { + lookups.add(channelId); + onLookup?.call(); + return atId; + }, createChannel: ({ required channelId, @@ -405,7 +418,9 @@ void main() { ttlSeconds, }) async { created.add(name); - if (name == 'general') throw Exception(generalError); + if (refused.contains(name)) { + throw Exception(refusalError); + } return _channel(id: channelId, name: name, isMember: true); }, joinChannel: (channelId) async => joined.add(channelId), @@ -415,6 +430,7 @@ void main() { return ( created: created, joined: joined, + lookups: lookups, recovery: recovery, loads: () => loadCount, ); @@ -447,7 +463,7 @@ void main() { for (final (holder, error) in unusableHolders) { final setup = starterSetup( channelsOnLoad: (_) => [holder], - generalError: error, + refusalError: error, ); final focusChannelId = await setup.recovery.ensureStarterChannels(); @@ -461,11 +477,69 @@ void main() { expect(focusChannelId, welcomeId, reason: holder.id); } + // An archived channel at the starter's own id, which the directory does + // not return, is found by a direct lookup and skipped, not rethrown. + final archivedAtId = starterSetup( + channelsOnLoad: (_) => const [], + refusalError: duplicateId, + atId: _channel( + id: generalId, + name: 'general', + archivedAt: DateTime.utc(2025), + ), + ); + final focusAfterArchived = await archivedAtId.recovery + .ensureStarterChannels(); + expect(archivedAtId.created, ['general', 'welcome-everyone']); + expect(archivedAtId.lookups, [generalId]); + expect(archivedAtId.joined, isEmpty); + expect(archivedAtId.loads(), 2); + expect(focusAfterArchived, welcomeId); + + // The scope fence also holds across the direct lookup, checked on the + // last starter so the next starter's own fence cannot mask it. + var lookedUp = false; + final fencedLookup = starterSetup( + channelsOnLoad: (_) => const [], + refusalError: duplicateId, + refused: const {'welcome-everyone'}, + atId: _channel( + id: welcomeId, + name: 'welcome-everyone', + archivedAt: DateTime.utc(2025), + ), + isScopeCurrent: () => !lookedUp, + onLookup: () => lookedUp = true, + ); + await expectLater( + fencedLookup.recovery.ensureStarterChannels(), + throwsStateError, + ); + expect(fencedLookup.lookups, [welcomeId]); + + // Any other duplicate stays recoverable: nothing at the id, or a + // channel there that is not archived, may be a starter still settling. + for (final atId in [null, _channel(id: generalId, name: 'general')]) { + final settling = starterSetup( + channelsOnLoad: (_) => const [], + refusalError: duplicateId, + atId: atId, + ); + await expectLater( + settling.recovery.ensureStarterChannels(), + throwsException, + reason: '$atId', + ); + expect(settling.created, ['general']); + expect(settling.lookups, [generalId]); + expect(settling.joined, isEmpty); + } + // A taken name whose holder the directory does not show yet stays // recoverable: it may be a usable starter another client just created. final unseen = starterSetup( channelsOnLoad: (_) => const [], - generalError: refusal, + refusalError: refusal, ); await expectLater( unseen.recovery.ensureStarterChannels(), @@ -474,33 +548,38 @@ void main() { expect(unseen.created, ['general']); expect(unseen.joined, isEmpty); expect(unseen.loads(), 2); + expect(unseen.lookups, isEmpty); // A usable open channel that appeared after the first load (lost race) // is joined rather than skipped. final raced = starterSetup( channelsOnLoad: (load) => load == 1 ? const [] : [_channel(id: 'raced', name: 'general')], - generalError: refusal, + refusalError: refusal, ); await raced.recovery.ensureStarterChannels(); expect(raced.joined, ['raced']); // The scope fence still holds after a refusal and its reload. + // The reload after the last starter's refusal must be the one that + // observes the change, so only the post-reload fence can catch it. var current = true; + var fencedLoads = 0; final fenced = starterSetup( - channelsOnLoad: (load) { - if (load == 2) current = false; + channelsOnLoad: (_) { + if (++fencedLoads > 2) current = false; return [unusableHolders.first.$1]; }, - generalError: refusal, + refusalError: refusal, + refused: const {'general', 'welcome-everyone'}, isScopeCurrent: () => current, ); await expectLater( fenced.recovery.ensureStarterChannels(), throwsStateError, ); - expect(fenced.created, ['general']); - expect(fenced.loads(), 2); + expect(fenced.created, ['general', 'welcome-everyone']); + expect(fenced.loads(), 3); expect(fenced.joined, isEmpty); }, ); @@ -511,6 +590,7 @@ void main() { loadChannels: () async => [ _channel(id: 'welcome-id', name: 'welcome-everyone'), ], + fetchChannel: (_) async => fail('no direct lookup expected'), createChannel: ({ required channelId, @@ -637,6 +717,7 @@ void main() { _channel(id: 'general-id', name: 'general'), _channel(id: 'welcome-id', name: 'welcome-everyone'), ], + fetchChannel: (_) async => fail('no direct lookup expected'), createChannel: ({ required channelId, diff --git a/mobile/test/shared/relay/relay_closed_policy_test.dart b/mobile/test/shared/relay/relay_closed_policy_test.dart index 21cfcc36e9..b9f098e429 100644 --- a/mobile/test/shared/relay/relay_closed_policy_test.dart +++ b/mobile/test/shared/relay/relay_closed_policy_test.dart @@ -50,6 +50,9 @@ void main() { 'invalid: an open channel named "Launch" already exists': 'An open channel named "Launch" already exists', 'blocked: community policy': 'Community policy', + 'error: internal failure': 'Internal failure', + 'rate-limited: slow down, retry in 3s': 'Slow down, retry in 3s', + 'auth-required: sign in first': 'Sign in first', 'relay unavailable': 'Relay unavailable', 'Event rejected': 'Event rejected', };