feat: implement simplified mostro: deep linking scheme #145
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Replace nostr: nevent-based deep links with simplified mostro: format. New format: mostro:order-id?relays=wss://relay1,wss://relay2 - Add mostro URL validation and parsing in NostrUtils - Update DeepLinkService to fetch orders by ID using NIP-69 filters - Change DeepLinkHandler to handle mostro: scheme instead of nostr: - Add comprehensive test coverage for new URL format - Maintain backward compatibility with existing nevent methods
Contributor
WalkthroughThe changes introduce support for a new "mostro" deep link scheme throughout the application. Deep link handling, service logic, and utility methods are updated or added to validate, parse, and process "mostro:" URLs, focusing on order IDs and relay lists. Corresponding unit tests are included to ensure correct functionality. Additionally, a new deep link interceptor is added to capture custom scheme URLs before GoRouter processes them, and navigation error handling is enhanced. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant App
participant DeepLinkInterceptor
participant DeepLinkHandler
participant DeepLinkService
participant NostrService
participant NostrUtils
User->>App: Opens mostro: URL
App->>DeepLinkInterceptor: didPushRouteInformation(uri)
alt URI scheme is mostro
DeepLinkInterceptor->>App: Emit intercepted URL
App->>DeepLinkHandler: handleInitialDeepLink(uri, router)
DeepLinkHandler->>DeepLinkService: processMostroLink(url, nostrService)
DeepLinkService->>NostrUtils: isValidMostroUrl(url)
alt valid URL
DeepLinkService->>NostrUtils: parseMostroUrl(url)
DeepLinkService->>NostrService: fetch order event by order ID and relays
NostrService-->>DeepLinkService: Order event data
DeepLinkService-->>DeepLinkHandler: DeepLinkResult (order info)
DeepLinkHandler-->>App: Navigate to order (post-frame callback)
else invalid URL
DeepLinkService-->>DeepLinkHandler: Error result
DeepLinkHandler-->>App: Show error
end
else
DeepLinkInterceptor->>GoRouter: didPushRouteInformation(uri)
end
Possibly related PRs
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (3)
🚧 Files skipped from review as they are similar to previous changes (3)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 4
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
lib/core/deep_link_handler.dart(4 hunks)lib/services/deep_link_service.dart(2 hunks)lib/shared/utils/nostr_utils.dart(1 hunks)test/utils/nostr_utils_test.dart(1 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
lib/**/*.dart
Instructions used from:
Sources:
📄 CodeRabbit Inference Engine
- CLAUDE.md
**/*.dart
Instructions used from:
Sources:
📄 CodeRabbit Inference Engine
- CLAUDE.md
🧠 Learnings (5)
📓 Common learnings
Learnt from: chebizarro
PR: MostroP2P/mobile#74
File: lib/background/background.dart:74-77
Timestamp: 2025-05-08T16:29:52.154Z
Learning: In the Mostro Mobile background service architecture, events aren't stored by the background process. Instead, the background service only checks if events exist in the eventStore and sends notifications for new ones, while the foreground process is responsible for storing and processing events. This is an intentional design decision to separate concerns.
Learnt from: chebizarro
PR: MostroP2P/mobile#74
File: lib/services/mostro_service.dart:97-103
Timestamp: 2025-05-06T15:49:55.079Z
Learning: In the Mostro protocol, an order cannot be canceled unless it has a valid orderId, so null checks on orderId during cancel operations are unnecessary.
Learnt from: chebizarro
PR: MostroP2P/mobile#127
File: lib/features/order/notfiers/abstract_mostro_notifier.dart:45-54
Timestamp: 2025-06-26T15:03:23.529Z
Learning: In AbstractMostroNotifier, state updates occur for all messages regardless of timestamp to hydrate the OrderNotifier from MostroStorage during sync, while handleEvent is only called for recent messages (within 60 seconds) to prevent re-triggering side effects like notifications and navigation for previously handled messages. This design prevents displaying stale notifications when the app is reopened or brought to the foreground.
Learnt from: chebizarro
PR: MostroP2P/mobile#110
File: test/notifiers/take_order_notifier_test.dart:72-74
Timestamp: 2025-06-04T19:35:20.209Z
Learning: MostroService methods like takeBuyOrder() and takeSellOrder() return Future<void> and trigger side effects through other mechanisms rather than direct return values. When testing these methods, focus on verifying method calls and testing state changes through the provider system rather than mocking return values.
lib/core/deep_link_handler.dart (2)
Learnt from: chebizarro
PR: MostroP2P/mobile#74
File: lib/services/mostro_service.dart:70-76
Timestamp: 2025-05-06T15:49:26.443Z
Learning: In the Mostro Mobile codebase, `eventStorageProvider` is exported from `package:mostro_mobile/shared/providers/mostro_service_provider.dart` and not from a separate `event_storage_provider.dart` file.
Learnt from: chebizarro
PR: MostroP2P/mobile#127
File: lib/features/order/notfiers/abstract_mostro_notifier.dart:45-54
Timestamp: 2025-06-26T15:03:23.529Z
Learning: In AbstractMostroNotifier, state updates occur for all messages regardless of timestamp to hydrate the OrderNotifier from MostroStorage during sync, while handleEvent is only called for recent messages (within 60 seconds) to prevent re-triggering side effects like notifications and navigation for previously handled messages. This design prevents displaying stale notifications when the app is reopened or brought to the foreground.
test/utils/nostr_utils_test.dart (2)
Learnt from: CR
PR: MostroP2P/mobile#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-08T17:55:04.549Z
Learning: Applies to test/mocks.dart : Mocks are generated using Mockito in `test/mocks.dart`
Learnt from: chebizarro
PR: MostroP2P/mobile#110
File: test/notifiers/take_order_notifier_test.dart:72-74
Timestamp: 2025-06-04T19:35:20.209Z
Learning: MostroService methods like takeBuyOrder() and takeSellOrder() return Future<void> and trigger side effects through other mechanisms rather than direct return values. When testing these methods, focus on verifying method calls and testing state changes through the provider system rather than mocking return values.
lib/shared/utils/nostr_utils.dart (1)
Learnt from: chebizarro
PR: MostroP2P/mobile#110
File: test/notifiers/take_order_notifier_test.dart:72-74
Timestamp: 2025-06-04T19:35:20.209Z
Learning: MostroService methods like takeBuyOrder() and takeSellOrder() return Future<void> and trigger side effects through other mechanisms rather than direct return values. When testing these methods, focus on verifying method calls and testing state changes through the provider system rather than mocking return values.
lib/services/deep_link_service.dart (13)
Learnt from: chebizarro
PR: MostroP2P/mobile#110
File: test/notifiers/take_order_notifier_test.dart:72-74
Timestamp: 2025-06-04T19:35:20.209Z
Learning: MostroService methods like takeBuyOrder() and takeSellOrder() return Future<void> and trigger side effects through other mechanisms rather than direct return values. When testing these methods, focus on verifying method calls and testing state changes through the provider system rather than mocking return values.
Learnt from: chebizarro
PR: MostroP2P/mobile#74
File: lib/services/mostro_service.dart:70-76
Timestamp: 2025-05-06T15:49:26.443Z
Learning: In the Mostro Mobile codebase, `eventStorageProvider` is exported from `package:mostro_mobile/shared/providers/mostro_service_provider.dart` and not from a separate `event_storage_provider.dart` file.
Learnt from: chebizarro
PR: MostroP2P/mobile#127
File: lib/features/order/notfiers/abstract_mostro_notifier.dart:45-54
Timestamp: 2025-06-26T15:03:23.529Z
Learning: In AbstractMostroNotifier, state updates occur for all messages regardless of timestamp to hydrate the OrderNotifier from MostroStorage during sync, while handleEvent is only called for recent messages (within 60 seconds) to prevent re-triggering side effects like notifications and navigation for previously handled messages. This design prevents displaying stale notifications when the app is reopened or brought to the foreground.
Learnt from: CR
PR: MostroP2P/mobile#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-08T17:55:04.549Z
Learning: Use Riverpod for all state management
Learnt from: chebizarro
PR: MostroP2P/mobile#74
File: lib/services/mostro_service.dart:70-76
Timestamp: 2025-05-06T15:49:26.443Z
Learning: In the Mostro Mobile codebase, Riverpod code generation is used with `@Riverpod` annotations. Providers like `eventStorageProvider` are generated in `.g.dart` files from annotated functions in the main provider files. These providers are accessible by importing the main provider file (e.g., `mostro_service_provider.dart`), not by importing a separate provider file.
Learnt from: CR
PR: MostroP2P/mobile#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-08T17:55:04.549Z
Learning: Applies to lib/data/models/nostr_event.dart : Custom `timeAgoWithLocale()` method is implemented in NostrEvent extension
Learnt from: CR
PR: MostroP2P/mobile#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-08T17:55:04.549Z
Learning: Applies to **/*.dart : Remove unused imports and dependencies
Learnt from: chebizarro
PR: MostroP2P/mobile#74
File: lib/services/nostr_service.dart:3-4
Timestamp: 2025-05-06T15:46:08.942Z
Learning: The file in dart_nostr library is named "ease.dart" (not "eose.dart" as might be expected), as confirmed by the project maintainer.
Learnt from: CR
PR: MostroP2P/mobile#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-08T17:55:04.549Z
Learning: Applies to lib/main.dart : Locales for the timeago package are configured in `main.dart` with `timeago.setLocaleMessages()`
Learnt from: CR
PR: MostroP2P/mobile#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-08T17:55:04.549Z
Learning: Applies to lib/**/*.dart : Use `S.of(context).yourKey` for all user-facing strings
Learnt from: CR
PR: MostroP2P/mobile#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-08T17:55:04.549Z
Learning: Target zero `flutter analyze` issues
Learnt from: CR
PR: MostroP2P/mobile#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-08T17:55:04.549Z
Learning: Applies to test/mocks.dart : Mocks are generated using Mockito in `test/mocks.dart`
Learnt from: CR
PR: MostroP2P/mobile#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-08T17:55:04.549Z
Learning: Applies to **/*.dart : Use `const` constructors where possible
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: build
🔇 Additional comments (2)
test/utils/nostr_utils_test.dart (1)
36-87: Well-structured test coverage for mostro URL handling!The test suite comprehensively covers all scenarios including valid URLs, various invalid formats, edge cases with spaces, and proper null handling. Good attention to detail in testing protocol validation (line 50).
lib/core/deep_link_handler.dart (1)
42-108: Clean migration from nostr to mostro scheme!All references have been consistently updated throughout the deep link handling logic. The error handling and logging messages properly reflect the new scheme.
1. lib/shared/utils/nostr_utils.dart - Removed three old functions: - decodeNevent() - NIP-19 nevent decoding logic - isValidNostrUrl() - Validation for nostr: URLs - parseNostrUrl() - Parsing logic for nostr: URLs 2. test/utils/nostr_utils_test.dart - Removed the entire "NostrUtils nevent decoding tests" test group that was testing the old functions 3. ios/Runner/Info.plist - Updated the iOS deep link configuration: - Changed from nostr.scheme to mostro.scheme - Changed URL scheme from nostr to mostro - Updated comment to reference mostro: scheme 4. lib/services/nostr_service.dart - Updated comment to reference the new mostro: URL format instead of nevent The codebase now only contains the simplified mostro: scheme implementation with functions like isValidMostroUrl() and parseMostroUrl(), which handle the new format: mostro:order-id?relays=wss://relay1,wss://relay2 All tests for the new mostro: scheme functionality have been preserved and should continue to work correctly.
Root Cause Analysis The GoRouter assertion failure was occurring because the mostro: URL was being passed directly to GoRouter's internal URL parsing system at the platform level (GoRouteInformationProvider.didPushRouteInformation), which doesn't understand custom schemes and triggers the assertion 'uriPathToCompare.startsWith(newMatchedLocationToCompare)'. Solution Implemented 1. Deep Link Interceptor (lib/core/deep_link_interceptor.dart) - Created a WidgetsBindingObserver that intercepts deep links at the system level - Overrides didPushRouteInformation() to catch mostro: URLs before they reach GoRouter - Returns true for custom schemes to indicate the URL was handled, preventing GoRouter from seeing it - Streams intercepted URLs for processing by our custom handlers 2. App-Level Integration (lib/core/app.dart) - Integrated the interceptor during app initialization - Set up stream listeners to process intercepted mostro: URLs - Routes intercepted URLs through the existing deep link handler - Proper disposal of resources when the app shuts down 3. Enhanced Deep Link Processing - Maintained all existing deep link functionality - Added better error handling and logging - Preserved timing controls with post-frame callbacks - Maintained compatibility with existing mostro: URL processing Key Features of the Fix 1. Prevention: mostro: URLs are intercepted before reaching GoRouter's assertion checks 2. Transparency: Normal HTTP URLs pass through unchanged to GoRouter 3. Compatibility: All existing deep link functionality is preserved 4. Performance: Minimal overhead with efficient stream-based processing 5. Robustness: Comprehensive error handling and logging Technical Benefits - Eliminates the assertion failure completely by preventing custom schemes from reaching GoRouter - Maintains full functionality for mostro: deep links - Preserves existing behavior for all other URLs and navigation - Future-proof approach that will work with GoRouter updates - Clean architecture with proper separation of concerns Testing Results - All existing tests pass - Flutter analyze reports no issues - The solution is ready for mostro: deep link testing
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
lib/core/app.dart (1)
92-105: Consider making the router initialization delay more robust.The hardcoded 100ms delay might not be sufficient in all cases and could lead to race conditions.
Consider using a more robust approach to ensure router readiness:
Future<void> _handleInitialMostroLink(Uri uri) async { try { - // Wait for router to be ready - await Future.delayed(const Duration(milliseconds: 100)); + // Wait for router to be ready with retry logic + int retries = 0; + while (_router == null && retries < 10) { + await Future.delayed(const Duration(milliseconds: 50)); + retries++; + } if (_router != null) { final deepLinkHandler = ref.read(deepLinkHandlerProvider); await deepLinkHandler.handleInitialDeepLink(uri, _router!); + } else { + debugPrint('Router not ready after ${retries * 50}ms, skipping initial mostro link'); } } catch (e) { debugPrint('Error handling initial mostro link: $e'); } }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
lib/core/app.dart(3 hunks)lib/core/app_routes.dart(2 hunks)lib/core/deep_link_handler.dart(5 hunks)lib/core/deep_link_interceptor.dart(1 hunks)lib/services/deep_link_service.dart(5 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- lib/core/deep_link_handler.dart
🧰 Additional context used
📓 Path-based instructions (2)
lib/**/*.dart
Instructions used from:
Sources:
📄 CodeRabbit Inference Engine
- CLAUDE.md
**/*.dart
Instructions used from:
Sources:
📄 CodeRabbit Inference Engine
- CLAUDE.md
🧠 Learnings (4)
📓 Common learnings
Learnt from: chebizarro
PR: MostroP2P/mobile#74
File: lib/background/background.dart:74-77
Timestamp: 2025-05-08T16:29:52.154Z
Learning: In the Mostro Mobile background service architecture, events aren't stored by the background process. Instead, the background service only checks if events exist in the eventStore and sends notifications for new ones, while the foreground process is responsible for storing and processing events. This is an intentional design decision to separate concerns.
Learnt from: chebizarro
PR: MostroP2P/mobile#74
File: lib/services/mostro_service.dart:97-103
Timestamp: 2025-05-06T15:49:55.079Z
Learning: In the Mostro protocol, an order cannot be canceled unless it has a valid orderId, so null checks on orderId during cancel operations are unnecessary.
Learnt from: chebizarro
PR: MostroP2P/mobile#127
File: lib/features/order/notfiers/abstract_mostro_notifier.dart:45-54
Timestamp: 2025-06-26T15:03:23.529Z
Learning: In AbstractMostroNotifier, state updates occur for all messages regardless of timestamp to hydrate the OrderNotifier from MostroStorage during sync, while handleEvent is only called for recent messages (within 60 seconds) to prevent re-triggering side effects like notifications and navigation for previously handled messages. This design prevents displaying stale notifications when the app is reopened or brought to the foreground.
lib/core/app.dart (9)
Learnt from: CR
PR: MostroP2P/mobile#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-08T17:55:04.549Z
Learning: Use Riverpod for all state management
Learnt from: chebizarro
PR: MostroP2P/mobile#74
File: lib/services/mostro_service.dart:70-76
Timestamp: 2025-05-06T15:49:26.443Z
Learning: In the Mostro Mobile codebase, Riverpod code generation is used with `@Riverpod` annotations. Providers like `eventStorageProvider` are generated in `.g.dart` files from annotated functions in the main provider files. These providers are accessible by importing the main provider file (e.g., `mostro_service_provider.dart`), not by importing a separate provider file.
Learnt from: CR
PR: MostroP2P/mobile#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-08T17:55:04.549Z
Learning: Applies to lib/main.dart : Locales for the timeago package are configured in `main.dart` with `timeago.setLocaleMessages()`
Learnt from: CR
PR: MostroP2P/mobile#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-08T17:55:04.549Z
Learning: Applies to lib/**/*.dart : Use `S.of(context).yourKey` for all user-facing strings
Learnt from: CR
PR: MostroP2P/mobile#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-08T17:55:04.549Z
Learning: Applies to **/*.dart : Remove unused imports and dependencies
Learnt from: chebizarro
PR: MostroP2P/mobile#74
File: lib/services/mostro_service.dart:70-76
Timestamp: 2025-05-06T15:49:26.443Z
Learning: In the Mostro Mobile codebase, `eventStorageProvider` is exported from `package:mostro_mobile/shared/providers/mostro_service_provider.dart` and not from a separate `event_storage_provider.dart` file.
Learnt from: CR
PR: MostroP2P/mobile#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-08T17:55:04.549Z
Learning: Applies to test/mocks.dart : Mocks are generated using Mockito in `test/mocks.dart`
Learnt from: CR
PR: MostroP2P/mobile#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-08T17:55:04.549Z
Learning: Target zero `flutter analyze` issues
Learnt from: CR
PR: MostroP2P/mobile#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-08T17:55:04.549Z
Learning: Regenerate localization files after ARB modifications by running `dart run build_runner build -d`
lib/core/app_routes.dart (3)
Learnt from: chebizarro
PR: MostroP2P/mobile#74
File: lib/services/mostro_service.dart:70-76
Timestamp: 2025-05-06T15:49:26.443Z
Learning: In the Mostro Mobile codebase, Riverpod code generation is used with `@Riverpod` annotations. Providers like `eventStorageProvider` are generated in `.g.dart` files from annotated functions in the main provider files. These providers are accessible by importing the main provider file (e.g., `mostro_service_provider.dart`), not by importing a separate provider file.
Learnt from: chebizarro
PR: MostroP2P/mobile#74
File: lib/services/mostro_service.dart:70-76
Timestamp: 2025-05-06T15:49:26.443Z
Learning: In the Mostro Mobile codebase, `eventStorageProvider` is exported from `package:mostro_mobile/shared/providers/mostro_service_provider.dart` and not from a separate `event_storage_provider.dart` file.
Learnt from: CR
PR: MostroP2P/mobile#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-08T17:55:04.549Z
Learning: Use Riverpod for all state management
lib/services/deep_link_service.dart (15)
Learnt from: chebizarro
PR: MostroP2P/mobile#127
File: lib/features/order/notfiers/abstract_mostro_notifier.dart:45-54
Timestamp: 2025-06-26T15:03:23.529Z
Learning: In AbstractMostroNotifier, state updates occur for all messages regardless of timestamp to hydrate the OrderNotifier from MostroStorage during sync, while handleEvent is only called for recent messages (within 60 seconds) to prevent re-triggering side effects like notifications and navigation for previously handled messages. This design prevents displaying stale notifications when the app is reopened or brought to the foreground.
Learnt from: chebizarro
PR: MostroP2P/mobile#110
File: test/notifiers/take_order_notifier_test.dart:72-74
Timestamp: 2025-06-04T19:35:20.209Z
Learning: MostroService methods like takeBuyOrder() and takeSellOrder() return Future<void> and trigger side effects through other mechanisms rather than direct return values. When testing these methods, focus on verifying method calls and testing state changes through the provider system rather than mocking return values.
Learnt from: chebizarro
PR: MostroP2P/mobile#74
File: lib/services/mostro_service.dart:70-76
Timestamp: 2025-05-06T15:49:26.443Z
Learning: In the Mostro Mobile codebase, `eventStorageProvider` is exported from `package:mostro_mobile/shared/providers/mostro_service_provider.dart` and not from a separate `event_storage_provider.dart` file.
Learnt from: chebizarro
PR: MostroP2P/mobile#74
File: lib/background/background.dart:74-77
Timestamp: 2025-05-08T16:29:52.154Z
Learning: In the Mostro Mobile background service architecture, events aren't stored by the background process. Instead, the background service only checks if events exist in the eventStore and sends notifications for new ones, while the foreground process is responsible for storing and processing events. This is an intentional design decision to separate concerns.
Learnt from: CR
PR: MostroP2P/mobile#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-08T17:55:04.549Z
Learning: Applies to lib/data/models/nostr_event.dart : Custom `timeAgoWithLocale()` method is implemented in NostrEvent extension
Learnt from: chebizarro
PR: MostroP2P/mobile#74
File: lib/notifications/notification_service.dart:54-59
Timestamp: 2025-05-08T16:31:29.582Z
Learning: In the Nostr protocol, event.id will never be null in events returned by relay subscriptions, so null safety checks for this property are unnecessary.
Learnt from: CR
PR: MostroP2P/mobile#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-08T17:55:04.549Z
Learning: Use Riverpod for all state management
Learnt from: chebizarro
PR: MostroP2P/mobile#74
File: lib/services/mostro_service.dart:70-76
Timestamp: 2025-05-06T15:49:26.443Z
Learning: In the Mostro Mobile codebase, Riverpod code generation is used with `@Riverpod` annotations. Providers like `eventStorageProvider` are generated in `.g.dart` files from annotated functions in the main provider files. These providers are accessible by importing the main provider file (e.g., `mostro_service_provider.dart`), not by importing a separate provider file.
Learnt from: CR
PR: MostroP2P/mobile#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-08T17:55:04.549Z
Learning: Applies to **/*.dart : Remove unused imports and dependencies
Learnt from: CR
PR: MostroP2P/mobile#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-08T17:55:04.549Z
Learning: Applies to lib/main.dart : Locales for the timeago package are configured in `main.dart` with `timeago.setLocaleMessages()`
Learnt from: CR
PR: MostroP2P/mobile#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-08T17:55:04.549Z
Learning: Applies to lib/**/*.dart : Use `S.of(context).yourKey` for all user-facing strings
Learnt from: chebizarro
PR: MostroP2P/mobile#74
File: lib/services/nostr_service.dart:3-4
Timestamp: 2025-05-06T15:46:08.942Z
Learning: The file in dart_nostr library is named "ease.dart" (not "eose.dart" as might be expected), as confirmed by the project maintainer.
Learnt from: CR
PR: MostroP2P/mobile#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-08T17:55:04.549Z
Learning: Target zero `flutter analyze` issues
Learnt from: CR
PR: MostroP2P/mobile#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-08T17:55:04.549Z
Learning: Applies to test/mocks.dart : Mocks are generated using Mockito in `test/mocks.dart`
Learnt from: CR
PR: MostroP2P/mobile#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-08T17:55:04.549Z
Learning: Applies to **/*.g.dart : Do not manually edit files ending with `.g.dart`
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: build
🔇 Additional comments (1)
lib/core/deep_link_interceptor.dart (1)
1-57: Well-implemented deep link interception mechanism!The
DeepLinkInterceptoreffectively prevents custom URL schemes from reaching GoRouter, avoiding assertion failures. The implementation follows Flutter patterns correctly with proper lifecycle management and resource cleanup.
In the deep linking system have been replaced with localized strings. The only remaining issue
is a minor warning about a duplicate ignore in the mock file, which is not related to our
changes.
1. ✅ Added localization keys: Added deep link error message keys to all three language files
(English, Spanish, Italian)
2. ✅ Updated DeepLinkService: Modified processMostroLink to accept BuildContext and use
localized error messages
3. ✅ Updated DeepLinkHandler: Updated the call to pass the context parameter
4. ✅ Updated app_routes.dart: Replaced hardcoded error messages in the GoRouter errorBuilder
with localized strings
5. ✅ Regenerated localization files: Successfully generated new localization files with the
deep link keys
The implementation now supports the following localized error messages:
- deepLinkInvalidFormat - "Invalid mostro: URL format"
- deepLinkParseError - "Failed to parse mostro: URL"
- deepLinkInvalidOrderId - "Invalid order ID format"
- deepLinkNoRelays - "No relays specified in URL"
- deepLinkOrderNotFound - "Order not found or invalid"
- deepLinkNavigationError - "Navigation Error: {error}"
- deepLinkGoHome - "Go Home"
All of these strings are now properly localized and will display in the user's selected
language (English, Spanish, or Italian) when deep link errors occur.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
lib/services/deep_link_service.dart (1)
140-154: Relay parameter is not passed to fetchEvents calls.The parsed
relaysparameter is not being utilized in thefetchEventscalls, which means events are always fetched from default relays regardless of the relays specified in the deep link.The NostrService's
fetchEventsmethod signature needs to be verified and updated to accept a relays parameter:#!/bin/bash # Check if NostrService.fetchEvents supports relay parameter ast-grep --pattern 'fetchEvents($$$) { $$$ }' # Search for the method signature rg -A 10 "fetchEvents.*\(" lib/services/nostr_service.dartThen update both calls to pass the relays:
- final orderEvents = await nostrService.fetchEvents(filter); + final orderEvents = await nostrService.fetchEvents(filter, relays: relays); - final defaultEvents = await nostrService.fetchEvents(filter); + final defaultEvents = await nostrService.fetchEvents(filter, relays: relays);
🧹 Nitpick comments (1)
lib/services/deep_link_service.dart (1)
110-118: Good localization implementation with context safety.The error handling properly uses localized strings and includes a safety check for mounted context. However, the fallback error message on line 114 is still hardcoded.
- return DeepLinkResult.error('Order not found or invalid'); + return DeepLinkResult.error('Order not found or invalid'); // TODO: Add localized fallbackConsider extracting this to a constant or adding a localized fallback message.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
lib/core/app_routes.dart(2 hunks)lib/core/deep_link_handler.dart(5 hunks)lib/l10n/intl_en.arb(1 hunks)lib/l10n/intl_es.arb(1 hunks)lib/l10n/intl_it.arb(1 hunks)lib/services/deep_link_service.dart(5 hunks)test/mocks.mocks.dart(1 hunks)
✅ Files skipped from review due to trivial changes (3)
- lib/l10n/intl_en.arb
- lib/l10n/intl_es.arb
- lib/l10n/intl_it.arb
🚧 Files skipped from review as they are similar to previous changes (2)
- lib/core/app_routes.dart
- lib/core/deep_link_handler.dart
🧰 Additional context used
📓 Path-based instructions (2)
**/*.dart
Instructions used from:
Sources:
📄 CodeRabbit Inference Engine
- CLAUDE.md
lib/**/*.dart
Instructions used from:
Sources:
📄 CodeRabbit Inference Engine
- CLAUDE.md
🧠 Learnings (3)
📓 Common learnings
Learnt from: chebizarro
PR: MostroP2P/mobile#74
File: lib/background/background.dart:74-77
Timestamp: 2025-05-08T16:29:52.154Z
Learning: In the Mostro Mobile background service architecture, events aren't stored by the background process. Instead, the background service only checks if events exist in the eventStore and sends notifications for new ones, while the foreground process is responsible for storing and processing events. This is an intentional design decision to separate concerns.
Learnt from: chebizarro
PR: MostroP2P/mobile#74
File: lib/services/mostro_service.dart:97-103
Timestamp: 2025-05-06T15:49:55.079Z
Learning: In the Mostro protocol, an order cannot be canceled unless it has a valid orderId, so null checks on orderId during cancel operations are unnecessary.
test/mocks.mocks.dart (7)
Learnt from: CR
PR: MostroP2P/mobile#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-08T17:55:04.549Z
Learning: Applies to test/mocks.dart : Mocks are generated using Mockito in `test/mocks.dart`
Learnt from: CR
PR: MostroP2P/mobile#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-08T17:55:04.549Z
Learning: Applies to **/*.dart : Use `const` constructors where possible
Learnt from: CR
PR: MostroP2P/mobile#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-08T17:55:04.549Z
Learning: Target zero `flutter analyze` issues
Learnt from: CR
PR: MostroP2P/mobile#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-08T17:55:04.549Z
Learning: Applies to **/*.g.dart : Do not manually edit files ending with `.g.dart`
Learnt from: CR
PR: MostroP2P/mobile#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-08T17:55:04.549Z
Learning: Applies to **/*.dart : Remove unused imports and dependencies
Learnt from: chebizarro
PR: MostroP2P/mobile#127
File: lib/data/models/cant_do.dart:10-27
Timestamp: 2025-07-08T05:40:47.527Z
Learning: In the CantDo model parsing in lib/data/models/cant_do.dart, the inconsistent key naming between 'cant_do' (top-level) and 'cant-do' (nested) is required by the protocol specification and should not be changed as it would break message parsing compatibility.
Learnt from: CR
PR: MostroP2P/mobile#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-08T17:55:04.549Z
Learning: Always maintain zero Flutter analyze issues
lib/services/deep_link_service.dart (11)
Learnt from: chebizarro
PR: MostroP2P/mobile#110
File: test/notifiers/take_order_notifier_test.dart:72-74
Timestamp: 2025-06-04T19:35:20.209Z
Learning: MostroService methods like takeBuyOrder() and takeSellOrder() return Future<void> and trigger side effects through other mechanisms rather than direct return values. When testing these methods, focus on verifying method calls and testing state changes through the provider system rather than mocking return values.
Learnt from: chebizarro
PR: MostroP2P/mobile#127
File: lib/features/order/notfiers/abstract_mostro_notifier.dart:45-54
Timestamp: 2025-06-26T15:03:23.529Z
Learning: In AbstractMostroNotifier, state updates occur for all messages regardless of timestamp to hydrate the OrderNotifier from MostroStorage during sync, while handleEvent is only called for recent messages (within 60 seconds) to prevent re-triggering side effects like notifications and navigation for previously handled messages. This design prevents displaying stale notifications when the app is reopened or brought to the foreground.
Learnt from: chebizarro
PR: MostroP2P/mobile#74
File: lib/services/mostro_service.dart:70-76
Timestamp: 2025-05-06T15:49:26.443Z
Learning: In the Mostro Mobile codebase, `eventStorageProvider` is exported from `package:mostro_mobile/shared/providers/mostro_service_provider.dart` and not from a separate `event_storage_provider.dart` file.
Learnt from: chebizarro
PR: MostroP2P/mobile#74
File: lib/background/background.dart:74-77
Timestamp: 2025-05-08T16:29:52.154Z
Learning: In the Mostro Mobile background service architecture, events aren't stored by the background process. Instead, the background service only checks if events exist in the eventStore and sends notifications for new ones, while the foreground process is responsible for storing and processing events. This is an intentional design decision to separate concerns.
Learnt from: CR
PR: MostroP2P/mobile#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-08T17:55:04.549Z
Learning: Applies to lib/data/models/nostr_event.dart : Custom `timeAgoWithLocale()` method is implemented in NostrEvent extension
Learnt from: chebizarro
PR: MostroP2P/mobile#74
File: lib/notifications/notification_service.dart:54-59
Timestamp: 2025-05-08T16:31:29.582Z
Learning: In the Nostr protocol, event.id will never be null in events returned by relay subscriptions, so null safety checks for this property are unnecessary.
Learnt from: CR
PR: MostroP2P/mobile#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-08T17:55:04.549Z
Learning: Applies to lib/**/*.dart : Use `S.of(context).yourKey` for all user-facing strings
Learnt from: CR
PR: MostroP2P/mobile#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-08T17:55:04.549Z
Learning: Applies to lib/main.dart : Locales for the timeago package are configured in `main.dart` with `timeago.setLocaleMessages()`
Learnt from: chebizarro
PR: MostroP2P/mobile#127
File: lib/data/models/cant_do.dart:10-27
Timestamp: 2025-07-08T05:40:47.527Z
Learning: In the CantDo model parsing in lib/data/models/cant_do.dart, the inconsistent key naming between 'cant_do' (top-level) and 'cant-do' (nested) is required by the protocol specification and should not be changed as it would break message parsing compatibility.
Learnt from: chebizarro
PR: MostroP2P/mobile#74
File: lib/services/mostro_service.dart:70-76
Timestamp: 2025-05-06T15:49:26.443Z
Learning: In the Mostro Mobile codebase, Riverpod code generation is used with `@Riverpod` annotations. Providers like `eventStorageProvider` are generated in `.g.dart` files from annotated functions in the main provider files. These providers are accessible by importing the main provider file (e.g., `mostro_service_provider.dart`), not by importing a separate provider file.
Learnt from: chebizarro
PR: MostroP2P/mobile#74
File: lib/services/nostr_service.dart:3-4
Timestamp: 2025-05-06T15:46:08.942Z
Learning: The file in dart_nostr library is named "ease.dart" (not "eose.dart" as might be expected), as confirmed by the project maintainer.
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: build
🔇 Additional comments (7)
test/mocks.mocks.dart (1)
404-404: LGTM: Appropriate analyzer warning suppression.The
// ignore: must_be_immutabledirective is correctly placed to suppress the analyzer warning for the mock class, which is expected to be mutable by design for testing purposes.lib/services/deep_link_service.dart (6)
3-11: Import additions are appropriate for the new functionality.The added imports for
dart_nostr.dart,flutter/widgets.dart, and localization support the new mostro URL scheme handling and navigation improvements.
68-86: Good implementation of mostro URL validation and parsing.The method signature change to
processMostroLinkis appropriate, and the addition ofBuildContextparameter enables proper localization of error messages. The validation logic correctly uses the new mostro utilities.
88-99: Solid validation logic for order ID and relays.The validation ensures order IDs meet minimum length requirements and that relays are present. The localized error messages properly address the previous feedback about hardcoded strings.
125-138: Correct NIP-69 event filter implementation.The filter correctly targets kind 38383 events and uses the
#dtag to find events by order ID, which aligns with the NIP-69 specification.
156-183: Well-implemented event processing logic.The logic correctly extracts the order type from the 'k' tag and constructs the OrderInfo object. The null safety and error handling are appropriate.
199-223: Excellent navigation safety improvements.The post-frame callback approach prevents GoRouter assertion failures during app lifecycle transitions. The context validation and fallback mechanism with proper error logging make this robust.
✅ Changes Made
1. Updated NostrService.fetchEvents() method (lib/services/nostr_service.dart):
- Added optional List<String>? specificRelays parameter
- When specificRelays is provided, it uses the existing _fetchFromSpecificRelays() logic
- When specificRelays is null/empty, it uses the default behavior
- Maintains full backward compatibility
2. Updated DeepLinkService calls (lib/services/deep_link_service.dart):
- Line 145: Now passes specificRelays: relays to use the relays from the mostro: URL
- Line 152: Continues to call fetchEvents(filter) without relays parameter for fallback to
default relays
✅ Technical Implementation
The method signature change:
// Before
Future<List<NostrEvent>> fetchEvents(NostrFilter filter)
// After
Future<List<NostrEvent>> fetchEvents(NostrFilter filter, {List<String>? specificRelays})
✅ Benefits Achieved
1. Proper relay targeting: Deep links now first try the specific relays mentioned in the
mostro: URL
2. Efficient fallback: If specific relays don't have the order, it falls back to default
configured relays
3. Backward compatibility: All existing code continues to work unchanged
4. Performance improvement: Avoids connecting to unnecessary relays when specific ones are
known
✅ Quality Assurance
- ✅ All tests pass
- ✅ Flutter analyze shows no errors (only a minor unrelated warning)
- ✅ Mock files properly regenerated
- ✅ Backward compatibility verified
The implementation now properly supports the relay-specific fetching that was requested,
allowing deep links to efficiently target the correct relays first while maintaining a robust
fallback mechanism.
Removed the duplicate '// ignore: must_be_immutable' comment from the mock file since it was already covered by the file-level ignore directive.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Replace nostr: nevent-based deep links with simplified mostro: format.
New format: mostro:order-id?relays=wss://relay1,wss://relay2
Summary by CodeRabbit
New Features
Bug Fixes
Tests
Chores
Refactor