Skip to content

Conversation

@BraCR10
Copy link
Collaborator

@BraCR10 BraCR10 commented Jan 13, 2026

Phase 3 & 4: Core Services Migration & Isolate Logging

This PR migrates 5 core services to use the logging system and enables background isolate logging.

What's Included

Phase 3 - Service Migrations (5 files):

  • lib/services/nostr_service.dart - Migrated to logger singleton (45+ log calls)
  • lib/services/mostro_service.dart - Migrated to logger singleton (~12 log calls)
  • lib/services/deep_link_service.dart - Migrated to logger singleton
  • lib/data/repositories/mostro_storage.dart - Migrated to logger singleton
  • lib/features/order/notifiers/abstract_mostro_notifier.dart + subclasses - Migrated to logger singleton

Phase 4 - Isolate Logging:

  • lib/main.dart - Added initIsolateLogReceiver() to capture background service logs
  • background\mobile_background_service.dart
  • desktop_background_service.dart

Expected Logs

With this PR, the logs screen will now display:

  • Nostr event publishing, relay connections, subscriptions
  • Mostro order operations and messaging
  • Deep link handling
  • Storage operations
  • Order state changes and notifications
  • Background service activity (via isolate logging)

Testing

Navigate to: Settings → Dev Tools → Logs Report

  1. Enable logging toggle
  2. Create/take orders, interact with Mostro
  3. Open deep links, manage relays
  4. Check logs for service activity
  5. Test filters by level and search

See docs/LOGGING_IMPLEMENTATION.md for complete details.

Summary by CodeRabbit

  • New Features

    • Added a method to fetch relay information from the Nostr service.
  • Chores

    • Centralized and standardized logging across mobile, desktop, background services, and app startup with isolate-aware plumbing.
    • Initialized isolate log receiver at startup.
    • Suppressed console log output in non-debug runs to reduce production noise.
  • Documentation

    • Updated version, phase status, and timestamps to reflect completed phases.

✏️ Tip: You can customize this high-level summary in your review settings.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Jan 13, 2026

Walkthrough

Centralized logging was integrated across the app: local Logger instances replaced with a shared logger_service; isolates now accept a SendPort and use IsolateLogOutput; main() initializes the isolate log receiver; documentation updated to Phase 4 Completed.

Changes

Cohort / File(s) Summary
Documentation & App Init
docs/LOGGING_IMPLEMENTATION.md, lib/main.dart
Docs updated to Phase 4 Completed; main() now calls initIsolateLogReceiver() to start centralized isolate log receiver.
Background Isolates
lib/background/background.dart, lib/background/desktop_background_service.dart, lib/background/mobile_background_service.dart
Background isolates accept a SendPort (loggerSendPort) and construct Logger with IsolateLogOutput targeting that port; isolate startup payloads include the logger port and logging is routed through centralized logger.
Logger Service
lib/services/logger_service.dart
logger_service now guards stdout printing with Config.isDebug (skips printing in non-debug), preserving public API; used as the centralized logger target for isolates and services.
Core Services
lib/services/mostro_service.dart, lib/services/nostr_service.dart, lib/services/deep_link_service.dart
Local _logger fields removed; all logging switched to shared logger. MostroService added restore-skip logic and payload validation. NostrService added getRelayInfo, relay validation, stronger error handling, and defensive checks for publishing/fetching.
Storage & Notifiers
lib/data/repositories/mostro_storage.dart, lib/features/order/notfiers/abstract_mostro_notifier.dart, lib/features/order/notfiers/add_order_notifier.dart, lib/features/order/notfiers/order_notifier.dart
Switched imports to logger_service and refactored logging calls to use centralized logger; no public API or control-flow changes.

Sequence Diagram(s)

sequenceDiagram
    participant Main as main()
    participant LogReceiver as IsolateLogReceiver
    participant Background as Background Isolate
    participant LoggerSvc as logger_service

    Main->>LogReceiver: initIsolateLogReceiver()
    LogReceiver-->>Main: provide loggerSendPort
    Main->>Background: start isolate (payload includes loggerSendPort)
    Background->>LoggerSvc: log via IsolateLogOutput -> sends to loggerSendPort
    LoggerSvc->>LogReceiver: receive log messages
    LogReceiver->>Main: forward/record logs (console/file/UI)
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Possibly related PRs

Suggested reviewers

  • Catrya
  • grunch

Poem

🐰
I hopped through threads and glowing logs tonight,
Sewed send‑ports, stitched isolates tight,
One shared lamp to guide each bite,
Carrots crunch as errors take flight,
Logs aligned — the code feels light! 🥕

🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The PR title accurately captures the main change: integrating logger service into background/isolate services, which is the core objective of Phase 3 (core services migration) and Phase 4 (isolate/background logging).
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.



📜 Recent review details

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 3ea02f5 and 30546b7.

📒 Files selected for processing (1)
  • lib/background/background.dart
🧰 Additional context used
📓 Path-based instructions (2)
**/*.{dart,flutter}

📄 CodeRabbit inference engine (CLAUDE.md)

**/*.{dart,flutter}: Run flutter analyze after any code change - Mandatory before commits to ensure zero linting issues
Run flutter test after any code change - Mandatory before commits to ensure all unit tests pass

Files:

  • lib/background/background.dart
**/*.dart

📄 CodeRabbit inference engine (CLAUDE.md)

**/*.dart: Use Riverpod for all state management - encapsulate business logic in Notifiers and access data only through repository classes
All code comments must be in English - use clear, concise English for variable names, function names, and comments
Always check mounted before using BuildContext after async operations to prevent errors on disposed widgets
Use const constructors where possible for better performance and immutability
Remove unused imports and dependencies to maintain code cleanliness and reduce build size

**/*.dart: Application code should be organized under lib/, grouped by domain with lib/features/<feature>/ structure, shared utilities in lib/shared/, dependency wiring in lib/core/, and services in lib/services/
Persistence, APIs, and background jobs should live in lib/data/ and lib/background/; generated localization output must be in lib/generated/ and must stay untouched
Apply flutter format . to enforce canonical Dart formatting (two-space indentation, trailing commas) before committing
Resolve every analyzer warning in Dart code
Name Riverpod providers using the <Feature>Provider or <Feature>Notifier convention
Localize all user-facing strings via ARB files and access them with S.of(context) rather than hard-coded literals

Files:

  • lib/background/background.dart
🧠 Learnings (13)
📚 Learning: 2025-11-27T12:10:26.407Z
Learnt from: CR
Repo: MostroP2P/mobile PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-27T12:10:26.407Z
Learning: Applies to **/*.dart : Persistence, APIs, and background jobs should live in `lib/data/` and `lib/background/`; generated localization output must be in `lib/generated/` and must stay untouched

Applied to files:

  • lib/background/background.dart
📚 Learning: 2025-11-27T12:10:12.082Z
Learnt from: CR
Repo: MostroP2P/mobile PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-27T12:10:12.082Z
Learning: Applies to **/*.dart : Always check `mounted` before using BuildContext after async operations to prevent errors on disposed widgets

Applied to files:

  • lib/background/background.dart
📚 Learning: 2025-11-27T12:10:12.081Z
Learnt from: CR
Repo: MostroP2P/mobile PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-27T12:10:12.081Z
Learning: Run `flutter test integration_test/` only for significant changes affecting core services and main user flows

Applied to files:

  • lib/background/background.dart
📚 Learning: 2025-11-27T12:10:12.082Z
Learnt from: CR
Repo: MostroP2P/mobile PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-27T12:10:12.082Z
Learning: Applies to lib/services/nostr_service.dart : Manage all relay connections and Nostr messaging through NostrService - automatically reconnect when relay list updates

Applied to files:

  • lib/background/background.dart
📚 Learning: 2025-11-27T12:10:12.082Z
Learnt from: CR
Repo: MostroP2P/mobile PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-27T12:10:12.082Z
Learning: Applies to lib/services/**/*.dart : Access NostrService for all Nostr protocol interactions - NostrService manages relay connections and messaging

Applied to files:

  • lib/background/background.dart
📚 Learning: 2025-11-27T12:10:12.082Z
Learnt from: CR
Repo: MostroP2P/mobile PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-27T12:10:12.082Z
Learning: Applies to lib/features/subscriptions/subscription_manager.dart : Use SubscriptionManager with `fireImmediately: false` during SessionNotifier initialization to prevent premature execution

Applied to files:

  • lib/background/background.dart
📚 Learning: 2025-11-27T12:10:12.082Z
Learnt from: CR
Repo: MostroP2P/mobile PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-27T12:10:12.082Z
Learning: Applies to lib/features/**/notifiers/**/*.dart : Encapsulate business logic in Notifiers - Notifiers should expose state via providers and handle all complex state transitions

Applied to files:

  • lib/background/background.dart
📚 Learning: 2025-10-14T21:12:06.887Z
Learnt from: Catrya
Repo: MostroP2P/mobile PR: 327
File: lib/features/order/notfiers/abstract_mostro_notifier.dart:141-154
Timestamp: 2025-10-14T21:12:06.887Z
Learning: In the MostroP2P mobile codebase, the notification system uses a two-layer localization pattern: providers/notifiers (without BuildContext access) call `showCustomMessage()` with string keys (e.g., 'orderTimeoutMaker', 'orderCanceled'), and the UI layer's `NotificationListenerWidget` has a switch statement that maps these keys to localized strings using `S.of(context)`. This architectural pattern properly separates concerns while maintaining full localization support for all user-facing messages.

Applied to files:

  • lib/background/background.dart
📚 Learning: 2025-05-06T15:49:26.443Z
Learnt from: chebizarro
Repo: MostroP2P/mobile PR: 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.

Applied to files:

  • lib/background/background.dart
📚 Learning: 2025-11-27T12:10:12.082Z
Learnt from: CR
Repo: MostroP2P/mobile PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-27T12:10:12.082Z
Learning: Applies to lib/features/**/providers/**/*.dart : Use Notifier pattern instead of simple StateNotifier for complex state logic requiring business rule encapsulation

Applied to files:

  • lib/background/background.dart
📚 Learning: 2025-05-08T16:29:52.154Z
Learnt from: chebizarro
Repo: MostroP2P/mobile PR: 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.

Applied to files:

  • lib/background/background.dart
📚 Learning: 2025-11-27T12:10:12.082Z
Learnt from: CR
Repo: MostroP2P/mobile PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-27T12:10:12.082Z
Learning: Applies to lib/features/settings/settings.dart : Use null-aware operators (`??`) in Settings `copyWith()` method to preserve existing values for selectedLanguage and defaultLightningAddress when not explicitly overridden

Applied to files:

  • lib/background/background.dart
📚 Learning: 2025-07-20T23:33:17.689Z
Learnt from: Catrya
Repo: MostroP2P/mobile PR: 215
File: lib/features/order/notfiers/order_notifier.dart:0-0
Timestamp: 2025-07-20T23:33:17.689Z
Learning: In AbstractMostroNotifier, the handleEvent method is synchronous and returns void, not Future<void>. Do not suggest adding await to super.handleEvent() calls as this would cause compilation errors.

Applied to files:

  • lib/background/background.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 (4)
lib/background/background.dart (4)

2-2: LGTM!

The imports are correctly added: dart:isolate for SendPort and the aliased logger_service import to avoid naming conflicts with the logger package.

Also applies to: 12-12


20-21: LGTM!

Nullable types are appropriate here since the logger is initialized asynchronously when the 'start' event fires. The variables are correctly scoped to be captured by the event listener closures.


89-95: LGTM!

Good defensive pattern: capturing the logger reference to a local variable ensures null-safe access, and the fallback to print handles the edge case where errors occur before the 'start' event initializes the logger. The lint ignore comment is appropriate for this fallback scenario.


38-44: No changes needed. IsolateLogOutput already safely handles null SendPort with an explicit if (sendPort != null) guard clause at line 297 in lib/services/logger_service.dart. When sendPort is null, logs are silently dropped from being sent, while console output continues if debug mode is enabled.

Likely an incorrect or invalid review comment.

✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.


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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@BraCR10 BraCR10 changed the title Feat/logging phases 3&4 Feat: Logger service background integration Jan 13, 2026
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
lib/background/mobile_background_service.dart (1)

42-51: Remove duplicate 'start' invocation to prevent redundant settings transmission.

The 'start' command is invoked twice with identical payloads:

  1. In the on-start listener (lines 44-46) when service.startService() triggers the event
  2. In _startService() after confirming the service is running (lines 156-159)

Both send the same payload (settings and loggerSendPort). When _startService() calls service.startService(), it triggers the on-start listener, causing the first invocation. Then the method waits for the service to be running and invokes 'start' again. Remove one of these invocations to avoid redundant command execution.

🤖 Fix all issues with AI agents
In @docs/LOGGING_IMPLEMENTATION.md:
- Line 30: The documentation contains a typo: change "subclases" to "subclasses"
in the line referencing AbstractMostroNotifier and its implementations; open
docs/LOGGING_IMPLEMENTATION.md, find the line with "AbstractMostroNotifier +
subclases (AddOrderNotifier, OrderNotifier)" and correct the spelling to
"AbstractMostroNotifier + subclasses (AddOrderNotifier, OrderNotifier)" so the
class names (AbstractMostroNotifier, AddOrderNotifier, OrderNotifier) remain
unchanged.
🧹 Nitpick comments (3)
lib/services/mostro_service.dart (2)

151-153: Consider log level and content for production.

Logging the full decryptedEvent.content may expose sensitive trade details in log output. Consider using debug level (logger.d) for detailed payload logging, or truncating/redacting the content for info level.

♻️ Suggested change
-      logger.i(
-        'Received DM, Event ID: ${decryptedEvent.id ?? event.id} with payload: ${decryptedEvent.content}',
-      );
+      logger.d(
+        'Received DM, Event ID: ${decryptedEvent.id ?? event.id} with payload: ${decryptedEvent.content}',
+      );

355-356: Consider debug level for outbound payload logging.

Similar to the inbound DM logging, the outbound payload contains trade details. Using debug level would be more appropriate for detailed protocol logging.

♻️ Suggested change
-    logger
-        .i('Sending DM, Event ID: ${event.id} with payload: ${order.toJson()}');
+    logger
+        .d('Sending DM, Event ID: ${event.id} with payload: ${order.toJson()}');
lib/services/nostr_service.dart (1)

129-148: Consider error level for publish failures.

A failed event publish is typically a significant issue that could affect order processing. Using logger.e instead of logger.w on line 139 would better reflect the severity.

♻️ Suggested change
     } catch (e) {
-      logger.w('Failed to publish event ${event.id}: $e');
+      logger.e('Failed to publish event ${event.id}: $e');
📜 Review details

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 39bab8f and d36ff88.

📒 Files selected for processing (11)
  • docs/LOGGING_IMPLEMENTATION.md
  • lib/background/desktop_background_service.dart
  • lib/background/mobile_background_service.dart
  • lib/data/repositories/mostro_storage.dart
  • lib/features/order/notfiers/abstract_mostro_notifier.dart
  • lib/features/order/notfiers/add_order_notifier.dart
  • lib/features/order/notfiers/order_notifier.dart
  • lib/main.dart
  • lib/services/deep_link_service.dart
  • lib/services/mostro_service.dart
  • lib/services/nostr_service.dart
🧰 Additional context used
📓 Path-based instructions (6)
**/*.{dart,flutter}

📄 CodeRabbit inference engine (CLAUDE.md)

**/*.{dart,flutter}: Run flutter analyze after any code change - Mandatory before commits to ensure zero linting issues
Run flutter test after any code change - Mandatory before commits to ensure all unit tests pass

Files:

  • lib/main.dart
  • lib/features/order/notfiers/add_order_notifier.dart
  • lib/background/desktop_background_service.dart
  • lib/data/repositories/mostro_storage.dart
  • lib/features/order/notfiers/abstract_mostro_notifier.dart
  • lib/background/mobile_background_service.dart
  • lib/services/deep_link_service.dart
  • lib/features/order/notfiers/order_notifier.dart
  • lib/services/mostro_service.dart
  • lib/services/nostr_service.dart
**/*.dart

📄 CodeRabbit inference engine (CLAUDE.md)

**/*.dart: Use Riverpod for all state management - encapsulate business logic in Notifiers and access data only through repository classes
All code comments must be in English - use clear, concise English for variable names, function names, and comments
Always check mounted before using BuildContext after async operations to prevent errors on disposed widgets
Use const constructors where possible for better performance and immutability
Remove unused imports and dependencies to maintain code cleanliness and reduce build size

**/*.dart: Application code should be organized under lib/, grouped by domain with lib/features/<feature>/ structure, shared utilities in lib/shared/, dependency wiring in lib/core/, and services in lib/services/
Persistence, APIs, and background jobs should live in lib/data/ and lib/background/; generated localization output must be in lib/generated/ and must stay untouched
Apply flutter format . to enforce canonical Dart formatting (two-space indentation, trailing commas) before committing
Resolve every analyzer warning in Dart code
Name Riverpod providers using the <Feature>Provider or <Feature>Notifier convention
Localize all user-facing strings via ARB files and access them with S.of(context) rather than hard-coded literals

Files:

  • lib/main.dart
  • lib/features/order/notfiers/add_order_notifier.dart
  • lib/background/desktop_background_service.dart
  • lib/data/repositories/mostro_storage.dart
  • lib/features/order/notfiers/abstract_mostro_notifier.dart
  • lib/background/mobile_background_service.dart
  • lib/services/deep_link_service.dart
  • lib/features/order/notfiers/order_notifier.dart
  • lib/services/mostro_service.dart
  • lib/services/nostr_service.dart
lib/main.dart

📄 CodeRabbit inference engine (CLAUDE.md)

Configure timeago package locales in app initialization for proper relative time formatting (e.g., 'hace X horas' vs 'hours ago')

Files:

  • lib/main.dart
lib/data/repositories/**/*.dart

📄 CodeRabbit inference engine (CLAUDE.md)

Implement Repository pattern for all data access - all data operations must go through repository classes

Files:

  • lib/data/repositories/mostro_storage.dart
lib/services/**/*.dart

📄 CodeRabbit inference engine (CLAUDE.md)

Access NostrService for all Nostr protocol interactions - NostrService manages relay connections and messaging

Files:

  • lib/services/deep_link_service.dart
  • lib/services/mostro_service.dart
  • lib/services/nostr_service.dart
lib/services/nostr_service.dart

📄 CodeRabbit inference engine (CLAUDE.md)

Manage all relay connections and Nostr messaging through NostrService - automatically reconnect when relay list updates

Files:

  • lib/services/nostr_service.dart
🧠 Learnings (39)
📓 Common learnings
Learnt from: chebizarro
Repo: MostroP2P/mobile PR: 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
Repo: MostroP2P/mobile PR: 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.
📚 Learning: 2025-05-06T15:49:26.443Z
Learnt from: chebizarro
Repo: MostroP2P/mobile PR: 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.

Applied to files:

  • lib/main.dart
  • lib/data/repositories/mostro_storage.dart
  • lib/services/deep_link_service.dart
  • lib/features/order/notfiers/order_notifier.dart
  • lib/services/mostro_service.dart
  • lib/services/nostr_service.dart
📚 Learning: 2025-11-27T12:10:12.081Z
Learnt from: CR
Repo: MostroP2P/mobile PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-27T12:10:12.081Z
Learning: Run `flutter test integration_test/` only for significant changes affecting core services and main user flows

Applied to files:

  • lib/main.dart
📚 Learning: 2025-11-27T12:10:12.082Z
Learnt from: CR
Repo: MostroP2P/mobile PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-27T12:10:12.082Z
Learning: Applies to lib/features/subscriptions/subscription_manager.dart : Use SubscriptionManager with `fireImmediately: false` during SessionNotifier initialization to prevent premature execution

Applied to files:

  • lib/main.dart
  • lib/features/order/notfiers/abstract_mostro_notifier.dart
  • lib/background/mobile_background_service.dart
  • lib/services/mostro_service.dart
📚 Learning: 2025-11-27T12:10:12.082Z
Learnt from: CR
Repo: MostroP2P/mobile PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-27T12:10:12.082Z
Learning: Applies to lib/features/**/notifiers/**/*.dart : Encapsulate business logic in Notifiers - Notifiers should expose state via providers and handle all complex state transitions

Applied to files:

  • lib/features/order/notfiers/add_order_notifier.dart
  • lib/features/order/notfiers/abstract_mostro_notifier.dart
  • lib/services/deep_link_service.dart
  • lib/features/order/notfiers/order_notifier.dart
  • lib/services/mostro_service.dart
  • lib/services/nostr_service.dart
📚 Learning: 2025-11-27T12:10:12.082Z
Learnt from: CR
Repo: MostroP2P/mobile PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-27T12:10:12.082Z
Learning: Applies to lib/features/**/providers/**/*.dart : Use Notifier pattern instead of simple StateNotifier for complex state logic requiring business rule encapsulation

Applied to files:

  • lib/features/order/notfiers/add_order_notifier.dart
  • lib/features/order/notfiers/abstract_mostro_notifier.dart
  • lib/services/deep_link_service.dart
  • lib/features/order/notfiers/order_notifier.dart
  • lib/services/mostro_service.dart
  • lib/services/nostr_service.dart
📚 Learning: 2025-11-27T12:10:12.082Z
Learnt from: CR
Repo: MostroP2P/mobile PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-27T12:10:12.082Z
Learning: Applies to lib/features/**/providers/**/*.dart : Organize Riverpod providers by feature in `features/{feature}/providers/` using Notifier pattern for complex state logic

Applied to files:

  • lib/features/order/notfiers/add_order_notifier.dart
  • lib/features/order/notfiers/abstract_mostro_notifier.dart
  • lib/services/deep_link_service.dart
  • lib/features/order/notfiers/order_notifier.dart
  • lib/services/mostro_service.dart
📚 Learning: 2025-11-27T12:10:12.082Z
Learnt from: CR
Repo: MostroP2P/mobile PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-27T12:10:12.082Z
Learning: Applies to lib/features/orders/notifiers/add_order_notifier.dart : Start session timeout cleanup in `submitOrder()` method to prevent orphan sessions when Mostro doesn't respond within 10 seconds

Applied to files:

  • lib/features/order/notfiers/add_order_notifier.dart
  • lib/features/order/notfiers/abstract_mostro_notifier.dart
  • lib/features/order/notfiers/order_notifier.dart
  • lib/services/mostro_service.dart
📚 Learning: 2025-11-27T12:10:12.082Z
Learnt from: CR
Repo: MostroP2P/mobile PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-27T12:10:12.082Z
Learning: Applies to lib/features/auth/notifiers/abstract_mostro_notifier.dart : Start 10-second cleanup timer automatically when taking orders via `startSessionTimeoutCleanup()` to prevent orphan sessions

Applied to files:

  • lib/features/order/notfiers/add_order_notifier.dart
  • lib/features/order/notfiers/abstract_mostro_notifier.dart
  • lib/features/order/notfiers/order_notifier.dart
  • lib/services/mostro_service.dart
📚 Learning: 2025-11-27T12:10:12.082Z
Learnt from: CR
Repo: MostroP2P/mobile PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-27T12:10:12.082Z
Learning: Applies to lib/shared/**/*.dart : Follow existing feature patterns when adding new shared utilities - refer to order, chat, and auth features as implementation examples

Applied to files:

  • lib/features/order/notfiers/add_order_notifier.dart
  • lib/features/order/notfiers/order_notifier.dart
  • lib/services/nostr_service.dart
📚 Learning: 2025-11-27T12:10:12.082Z
Learnt from: CR
Repo: MostroP2P/mobile PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-27T12:10:12.082Z
Learning: Applies to lib/features/auth/notifiers/abstract_mostro_notifier.dart : Use `startSessionTimeoutCleanupForRequestId()` for order creation timeout protection and cancel timer automatically when any Mostro response received

Applied to files:

  • lib/features/order/notfiers/add_order_notifier.dart
  • lib/data/repositories/mostro_storage.dart
  • lib/features/order/notfiers/abstract_mostro_notifier.dart
  • lib/features/order/notfiers/order_notifier.dart
  • lib/services/mostro_service.dart
📚 Learning: 2025-10-21T21:47:03.451Z
Learnt from: Catrya
Repo: MostroP2P/mobile PR: 327
File: lib/features/order/notfiers/abstract_mostro_notifier.dart:157-182
Timestamp: 2025-10-21T21:47:03.451Z
Learning: In MostroP2P/mobile, for Action.canceled handling in abstract_mostro_notifier.dart (Riverpod StateNotifier), do not add mounted checks after async sessionNotifier.deleteSession(orderId) as they break order state synchronization during app restart. The Action.canceled flow contains critical business logic that must complete fully; Riverpod handles provider disposal automatically. Mounted checks should only protect UI operations, not business logic in StateNotifiers.

Applied to files:

  • lib/features/order/notfiers/add_order_notifier.dart
  • lib/features/order/notfiers/abstract_mostro_notifier.dart
  • lib/services/deep_link_service.dart
  • lib/features/order/notfiers/order_notifier.dart
  • lib/services/mostro_service.dart
📚 Learning: 2025-10-14T21:12:06.887Z
Learnt from: Catrya
Repo: MostroP2P/mobile PR: 327
File: lib/features/order/notfiers/abstract_mostro_notifier.dart:141-154
Timestamp: 2025-10-14T21:12:06.887Z
Learning: In the MostroP2P mobile codebase, the notification system uses a two-layer localization pattern: providers/notifiers (without BuildContext access) call `showCustomMessage()` with string keys (e.g., 'orderTimeoutMaker', 'orderCanceled'), and the UI layer's `NotificationListenerWidget` has a switch statement that maps these keys to localized strings using `S.of(context)`. This architectural pattern properly separates concerns while maintaining full localization support for all user-facing messages.

Applied to files:

  • lib/features/order/notfiers/add_order_notifier.dart
  • lib/features/order/notfiers/abstract_mostro_notifier.dart
  • lib/features/order/notfiers/order_notifier.dart
📚 Learning: 2025-06-26T15:03:23.529Z
Learnt from: chebizarro
Repo: MostroP2P/mobile PR: 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.

Applied to files:

  • lib/features/order/notfiers/add_order_notifier.dart
  • lib/features/order/notfiers/abstract_mostro_notifier.dart
  • lib/features/order/notfiers/order_notifier.dart
  • lib/services/mostro_service.dart
📚 Learning: 2025-11-27T12:10:12.082Z
Learnt from: CR
Repo: MostroP2P/mobile PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-27T12:10:12.082Z
Learning: Applies to lib/core/mostro_fsm.dart : Use MostroFSM for managing order state transitions - all state changes must go through FSM state methods

Applied to files:

  • lib/features/order/notfiers/add_order_notifier.dart
  • lib/services/deep_link_service.dart
  • lib/services/mostro_service.dart
  • lib/services/nostr_service.dart
📚 Learning: 2025-11-27T12:10:12.082Z
Learnt from: CR
Repo: MostroP2P/mobile PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-27T12:10:12.082Z
Learning: Applies to lib/services/nostr_service.dart : Manage all relay connections and Nostr messaging through NostrService - automatically reconnect when relay list updates

Applied to files:

  • lib/background/desktop_background_service.dart
  • lib/background/mobile_background_service.dart
  • lib/services/deep_link_service.dart
  • lib/services/mostro_service.dart
  • lib/services/nostr_service.dart
📚 Learning: 2025-11-27T12:10:12.082Z
Learnt from: CR
Repo: MostroP2P/mobile PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-27T12:10:12.082Z
Learning: Applies to lib/services/**/*.dart : Access NostrService for all Nostr protocol interactions - NostrService manages relay connections and messaging

Applied to files:

  • lib/background/desktop_background_service.dart
  • lib/background/mobile_background_service.dart
  • lib/services/mostro_service.dart
  • lib/services/nostr_service.dart
📚 Learning: 2025-11-27T12:10:12.082Z
Learnt from: CR
Repo: MostroP2P/mobile PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-27T12:10:12.082Z
Learning: Applies to lib/features/relays/**/*.dart : Use dual storage strategy: store Mostro/default relays in `settings.relays` and user relays in `settings.userRelays` with full JSON metadata via `toJson()`/`fromJson()`

Applied to files:

  • lib/data/repositories/mostro_storage.dart
  • lib/services/mostro_service.dart
  • lib/services/nostr_service.dart
📚 Learning: 2025-11-27T12:10:12.082Z
Learnt from: CR
Repo: MostroP2P/mobile PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-27T12:10:12.082Z
Learning: Applies to lib/data/repositories/**/*.dart : Implement Repository pattern for all data access - all data operations must go through repository classes

Applied to files:

  • lib/data/repositories/mostro_storage.dart
📚 Learning: 2025-05-06T15:49:26.443Z
Learnt from: chebizarro
Repo: MostroP2P/mobile PR: 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.

Applied to files:

  • lib/data/repositories/mostro_storage.dart
  • lib/services/deep_link_service.dart
  • lib/services/mostro_service.dart
📚 Learning: 2025-07-20T23:33:17.689Z
Learnt from: Catrya
Repo: MostroP2P/mobile PR: 215
File: lib/features/order/notfiers/order_notifier.dart:0-0
Timestamp: 2025-07-20T23:33:17.689Z
Learning: In AbstractMostroNotifier, the handleEvent method is synchronous and returns void, not Future<void>. Do not suggest adding await to super.handleEvent() calls as this would cause compilation errors.

Applied to files:

  • lib/features/order/notfiers/abstract_mostro_notifier.dart
📚 Learning: 2025-05-06T15:49:55.079Z
Learnt from: chebizarro
Repo: MostroP2P/mobile PR: 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.

Applied to files:

  • lib/features/order/notfiers/abstract_mostro_notifier.dart
  • lib/services/nostr_service.dart
📚 Learning: 2025-05-08T16:29:52.154Z
Learnt from: chebizarro
Repo: MostroP2P/mobile PR: 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.

Applied to files:

  • lib/background/mobile_background_service.dart
  • docs/LOGGING_IMPLEMENTATION.md
📚 Learning: 2025-11-27T12:10:12.082Z
Learnt from: CR
Repo: MostroP2P/mobile PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-27T12:10:12.082Z
Learning: Applies to lib/core/app_routes.dart : Use GoRouter for all navigation configuration instead of Navigator API - maintain all routes in app_routes.dart

Applied to files:

  • lib/services/deep_link_service.dart
📚 Learning: 2025-11-27T12:10:12.082Z
Learnt from: CR
Repo: MostroP2P/mobile PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-27T12:10:12.082Z
Learning: Applies to **/*.dart : Use Riverpod for all state management - encapsulate business logic in Notifiers and access data only through repository classes

Applied to files:

  • lib/services/deep_link_service.dart
  • lib/services/mostro_service.dart
  • lib/services/nostr_service.dart
📚 Learning: 2025-11-27T12:10:26.407Z
Learnt from: CR
Repo: MostroP2P/mobile PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-27T12:10:26.407Z
Learning: Applies to **/*.dart : Name Riverpod providers using the `<Feature>Provider` or `<Feature>Notifier` convention

Applied to files:

  • lib/services/deep_link_service.dart
  • lib/features/order/notfiers/order_notifier.dart
  • lib/services/mostro_service.dart
📚 Learning: 2025-05-08T16:31:29.582Z
Learnt from: chebizarro
Repo: MostroP2P/mobile PR: 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.

Applied to files:

  • lib/services/deep_link_service.dart
  • lib/services/nostr_service.dart
📚 Learning: 2025-11-27T12:10:12.082Z
Learnt from: CR
Repo: MostroP2P/mobile PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-27T12:10:12.082Z
Learning: Applies to lib/core/models/relay_list_event.dart : Parse NIP-65 (kind 10002) events with robust handling of different timestamp formats and null-safe parsing for malformed events

Applied to files:

  • lib/services/deep_link_service.dart
  • lib/services/nostr_service.dart
📚 Learning: 2025-11-27T12:10:12.082Z
Learnt from: CR
Repo: MostroP2P/mobile PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-27T12:10:12.082Z
Learning: Applies to **/*.dart : Remove unused imports and dependencies to maintain code cleanliness and reduce build size

Applied to files:

  • lib/features/order/notfiers/order_notifier.dart
📚 Learning: 2025-11-27T12:10:12.082Z
Learnt from: CR
Repo: MostroP2P/mobile PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-27T12:10:12.082Z
Learning: Applies to lib/features/settings/settings.dart : Use null-aware operators (`??`) in Settings `copyWith()` method to preserve existing values for selectedLanguage and defaultLightningAddress when not explicitly overridden

Applied to files:

  • lib/services/mostro_service.dart
📚 Learning: 2025-06-04T19:35:20.209Z
Learnt from: chebizarro
Repo: MostroP2P/mobile PR: 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.

Applied to files:

  • lib/services/mostro_service.dart
📚 Learning: 2025-06-08T23:54:09.987Z
Learnt from: chebizarro
Repo: MostroP2P/mobile PR: 110
File: lib/features/trades/widgets/mostro_message_detail_widget.dart:134-141
Timestamp: 2025-06-08T23:54:09.987Z
Learning: In the OrderState refactor, public keys should be accessed via `tradeState.peer?.publicKey` from the OrderState instance rather than through `session?.peer?.publicKey`, as the OrderState encapsulates peer information directly.

Applied to files:

  • lib/services/mostro_service.dart
📚 Learning: 2025-11-27T12:10:12.082Z
Learnt from: CR
Repo: MostroP2P/mobile PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-27T12:10:12.082Z
Learning: Applies to lib/features/relays/**/*.dart : Implement two-tier relay validation: primary Nostr protocol test (REQ/EVENT/EOSE) with WebSocket fallback for connectivity testing

Applied to files:

  • lib/services/nostr_service.dart
📚 Learning: 2025-11-27T12:10:12.082Z
Learnt from: CR
Repo: MostroP2P/mobile PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-27T12:10:12.082Z
Learning: Applies to lib/features/relays/**/*.dart : Track relay sources (user, mostro, default) using RelaySource enum for appropriate handling and storage strategy

Applied to files:

  • lib/services/nostr_service.dart
📚 Learning: 2025-11-27T12:10:12.082Z
Learnt from: CR
Repo: MostroP2P/mobile PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-27T12:10:12.082Z
Learning: Applies to lib/features/relays/relays_notifier.dart : Use `removeRelayWithBlacklist()` for Mostro/default relays (adds to blacklist) and `removeRelay()` for permanent user relay deletion

Applied to files:

  • lib/services/nostr_service.dart
📚 Learning: 2025-05-06T15:46:08.942Z
Learnt from: chebizarro
Repo: MostroP2P/mobile PR: 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.

Applied to files:

  • lib/services/nostr_service.dart
📚 Learning: 2025-08-19T17:54:15.016Z
Learnt from: Catrya
Repo: MostroP2P/mobile PR: 272
File: lib/features/relays/relays_notifier.dart:41-49
Timestamp: 2025-08-19T17:54:15.016Z
Learning: In the Mostro mobile app relay synchronization system, the hardcoded 'wss://relay.mostro.network' relay in RelaysNotifier._loadRelays() is intentional for bootstrapping. Config.nostrRelays only contains this single active relay anyway (other entries are commented-out dev relays), so hardcoding is functionally equivalent and more explicit about the bootstrap requirement for fetching kind 10002 relay list events.

Applied to files:

  • lib/services/nostr_service.dart
📚 Learning: 2026-01-09T22:09:06.887Z
Learnt from: BraCR10
Repo: MostroP2P/mobile PR: 403
File: lib/features/settings/settings_notifier.dart:153-156
Timestamp: 2026-01-09T22:09:06.887Z
Learning: In lib/features/settings/settings_notifier.dart, the updateLoggingEnabled method intentionally does NOT call _saveToPrefs() because the logging toggle should not persist across app restarts. The isLoggingEnabled field is deliberately excluded from Settings.toJson() and always returns false in fromJson() for security and performance reasons, requiring users to explicitly enable logging each session.

Applied to files:

  • lib/services/nostr_service.dart
📚 Learning: 2025-08-21T14:45:43.974Z
Learnt from: Catrya
Repo: MostroP2P/mobile PR: 272
File: lib/features/relays/widgets/relay_selector.dart:13-15
Timestamp: 2025-08-21T14:45:43.974Z
Learning: In the Mostro mobile app's RelaySelector widget (lib/features/relays/widgets/relay_selector.dart), watching relaysProvider.notifier correctly triggers rebuilds because the relaysProvider itself depends on settingsProvider (line 8 in relays_provider.dart). When blacklist changes via toggleMostroRelayBlacklist(), the settingsProvider updates, causing relaysProvider to rebuild, which then notifies widgets watching the notifier. The UI correctly reflects active/inactive states in real-time through this dependency chain.

Applied to files:

  • lib/services/nostr_service.dart
🪛 LanguageTool
docs/LOGGING_IMPLEMENTATION.md

[grammar] ~30-~30: Ensure spelling is correct
Context: ...tro_storage - AbstractMostroNotifier + subclases (AddOrderNotifier, OrderNotifier) ### ...

(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)

⏰ 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 (24)
lib/data/repositories/mostro_storage.dart (1)

1-8: LGTM! Clean migration to centralized logger.

The logging migration correctly removes the private _logger field and imports the shared logger_service. All subsequent logging calls properly use the global logger singleton with appropriate log levels (logger.i for info, logger.e for errors with error and stackTrace parameters).

lib/services/deep_link_service.dart (2)

7-7: LGTM! Logging migration follows established patterns.

The migration correctly replaces the local Logger instance with the centralized logger singleton. The service appropriately uses different log levels (logger.i, logger.e, logger.w) throughout the deep link processing flow.

Also applies to: 25-25


111-116: Good defensive coding with context.mounted check.

The mounted check before using BuildContext after the async _fetchOrderInfoById call correctly prevents errors on disposed widgets, as per coding guidelines.

lib/background/mobile_background_service.dart (1)

7-7: LGTM! Isolate logging properly wired.

The aliased import and passing of isolateLogSenderPort enables log forwarding from the background isolate to the main isolate's memory buffer. This correctly implements the isolate logging pattern documented in Phase 4.

Also applies to: 46-50

docs/LOGGING_IMPLEMENTATION.md (1)

25-35: Documentation accurately reflects Phase 3 & 4 completion.

The updates correctly document the migration of core services (mostro_storage, AbstractMostroNotifier and its subclasses) and the isolate logging integration for both mobile and desktop background services. Version and status metadata are appropriately updated.

Also applies to: 209-211

lib/background/desktop_background_service.dart (2)

22-36: Isolate logger correctly configured with dedicated output.

The isolate properly creates its own Logger instance with IsolateLogOutput to forward logs to the main isolate via SendPort. This follows the documented pattern in LOGGING_IMPLEMENTATION.md for background isolate integration.


26-26: IsolateLogOutput correctly handles null SendPort.

The implementation (lib/services/logger_service.dart:283-311) always prints logs to console and conditionally sends to the SendPort only when it's not null (line 295: if (sendPort != null)). When args.length <= 2 and loggerSendPort is null, logs will still be output to console—no runtime errors possible.

lib/services/mostro_service.dart (3)

7-7: Import migration to centralized logger service.

The import change aligns with the project-wide logging standardization.


30-41: Logging in subscription and dispose is well-structured.

Error logging correctly includes both error and stackTrace parameters for proper debugging. The dispose log provides useful lifecycle visibility.


45-95: Defensive restore payload detection is well-implemented.

The _isRestorePayload method includes thorough type validation at each step, preventing runtime type errors. The comments clearly explain the purpose of each check.

lib/services/nostr_service.dart (5)

6-6: Import migration to centralized logger service.


29-73: Comprehensive logging in initialization flow.

Excellent use of log levels:

  • Debug for protocol-level details (OK, EOSE, Count responses)
  • Info for operational events (Notice, connection success)
  • Warning for recoverable issues (connection errors)
  • Error for failures

This provides good observability without overwhelming logs.


75-111: Robust relay update with fallback and logging.

The update logic properly handles failures by attempting to restore previous configuration. Logging at each step provides a clear audit trail for debugging relay connectivity issues.


264-320: Thorough logging in event fetch operations.

The fetchEventById method has comprehensive logging covering the success path, validation failures, and error handling. Log levels are appropriate for each scenario.


410-457: Good error recovery in relay-specific fetch.

The _fetchFromSpecificRelays method properly restores original relay settings even on error, with logging at each step for debugging.

lib/features/order/notfiers/add_order_notifier.dart (2)

9-9: Import added for centralized logger service.

The import enables usage of the shared logger instance throughout the notifier, consistent with other migrated files.


39-43: Logger usage in subscription handler is appropriate.

The info log for received actions provides useful visibility into the order creation flow.

lib/features/order/notfiers/order_notifier.dart (4)

9-9: Import added for centralized logger service.

Consistent with the project-wide logging migration.


24-30: Event handling logging provides good observability.

Logging received events with action and order ID helps trace the order lifecycle.


32-72: Sync method has comprehensive logging.

The sync method appropriately logs:

  • Warning when no messages found (helps identify data issues)
  • Info on successful sync with status
  • Error with full stack trace on failures

This provides good debugging capability for order state synchronization.


157-192: Automatic expiration handling is well-logged.

The public events subscription logs automatic order expiration clearly, which is valuable for understanding order lifecycle transitions. Error handling includes full stack traces for debugging.

lib/main.dart (1)

19-27: Logger service integration is correctly implemented.

The initIsolateLogReceiver() function is synchronous (returns void), so the call at line 26 is appropriately without await. Placement early in main() ensures background isolate logs are captured from startup. Timeago localization is properly configured at line 42 as required by guidelines.

lib/features/order/notfiers/abstract_mostro_notifier.dart (2)

14-14: LGTM!

Import correctly migrated to the centralized logger service, aligning with the Phase 3 logging migration objectives.


499-575: LGTM!

The static methods correctly use the global logger singleton from the centralized logger service. The logging pattern is consistent throughout:

  • logger.i() for informational messages about timer lifecycle
  • logger.e() with the error: named parameter for structured error capture

This is the appropriate approach for static methods where instance-level dependencies aren't available.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 0

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (4)
lib/services/logger_service.dart (3)

109-113: Potential runtime exception if timestamp is shorter than 19 characters.

The format() method uses substring(0, 19) without checking string length. While DateTime.toString() typically produces strings longer than 19 characters, this is an implicit assumption.

🐛 Safer substring handling
 String format() {
-  final time = timestamp.toString().substring(0, 19);
+  final timeStr = timestamp.toString();
+  final time = timeStr.length >= 19 ? timeStr.substring(0, 19) : timeStr;
   final levelStr = level.toString().split('.').last.toUpperCase();
   return '[$levelStr]($service:$line) $time - $message';
 }

186-187: Same substring safety concern in SimplePrinter.

Line 187 uses substring(0, 19) on the timestamp string without length validation, same issue as in LogEntry.format().

🐛 Safer substring handling
 List<String> log(LogEvent event) {
   final level = _formatLevel(event.level);
   final message = event.message.toString();
-  final timestamp = event.time.toString().substring(0, 19);
+  final timeStr = event.time.toString();
+  final timestamp = timeStr.length >= 19 ? timeStr.substring(0, 19) : timeStr;

155-164: Fix inverted condition in buffer deletion logic — the inner check is logically impossible.

The condition _buffer.length < Config.logBatchDeleteSize inside a block where _buffer.length > Config.logMaxEntries can never be true given the constants (logMaxEntries = 1000, logBatchDeleteSize = 100). This means deleteCount will always be 100, and the first branch is dead code.

The intent appears to be: delete only the overflow amount if it's small, otherwise delete a batch. The correct condition should check if the overflow exceeds the batch size: (_buffer.length - Config.logMaxEntries) < Config.logBatchDeleteSize.

lib/background/background.dart (1)

66-92: Store the StreamSubscription returned by .listen() to enable proper cleanup.

The subscription.listen() call on line 82 creates a StreamSubscription that isn't stored. Only the Stream object is kept in activeSubscriptions, while the listener subscription itself is lost. When cancel-subscription fires, nostrService.unsubscribe(id) is called, but there's no local handle to explicitly cancel the listener. This is inconsistent with the pattern used elsewhere in the codebase (e.g., SubscriptionManager, OpenOrdersRepository, DisputeChatNotifier) where StreamSubscription objects are explicitly stored and managed.

Store the StreamSubscription in activeSubscriptions so it can be properly cancelled when the subscription is removed. Update line 82-91:

final streamSubscription = subscription.listen((event) async {
  try {
    if (await eventStore.hasItem(event.id!)) {
      return;
    }
    await notification_service.retryNotification(event);
  } catch (e) {
    logger?.e('Error processing event', error: e);
  }
});

activeSubscriptions[request.subscriptionId!] = {
  'filters': filters,
  'subscription': subscription,
  'streamSubscription': streamSubscription,
};

Then update the cancel handler (line 98-100) to also cancel the listener:

if (id != null && activeSubscriptions.containsKey(id)) {
  final sub = activeSubscriptions[id];
  (sub?['streamSubscription'] as StreamSubscription?)?.cancel();
  activeSubscriptions.remove(id);
  nostrService.unsubscribe(id);
}
🧹 Nitpick comments (3)
lib/services/logger_service.dart (1)

33-50: Comprehensive log sanitization, but consider pre-compiling regex patterns.

The cleanMessage function properly sanitizes logs by:

  • Removing ANSI escape codes
  • Stripping box-drawing characters and emojis
  • Redacting sensitive data (private keys, mnemonics)

However, these regex patterns are recompiled on every call. For a logging service that may process many messages, pre-compiling these patterns would improve performance.

♻️ Consider pre-compiling regex patterns
// At module level, before cleanMessage function:
final _ansiEscapeRegex = RegExp(r'\x1B\[[0-9;]*[a-zA-Z]');
final _digitBracketRegex = RegExp(r'\[\d+m');
final _ansi256Regex = RegExp(r'\[38;5;\d+m');
final _ansi39Regex = RegExp(r'\[39m');
final _ansi2Regex = RegExp(r'\[2m');
final _ansi22Regex = RegExp(r'\[22m');
final _boxCharsRegex = RegExp(r'[┌┐└┘├┤─│┬┴┼╭╮╰╯╔╗╚╝╠╣═║╦╩╬━┃┄├]');
final _emojiRegex = RegExp(r'[\u{1F300}-\u{1F9FF}]', unicode: true);
final _nsecRegex = RegExp(r'nsec[0-9a-z]+');
final _privateKeyRegex = RegExp(r'"privateKey"\s*:\s*"[^"]*"');
final _mnemonicRegex = RegExp(r'"mnemonic"\s*:\s*"[^"]*"');
final _nonAlnumRegex = RegExp(r'[^A-Za-z0-9\s.:,!?\-_/\[\]]');
final _whitespaceRegex = RegExp(r'\s+');

String cleanMessage(String message) {
  return message
      .replaceAll(_ansiEscapeRegex, '')
      // ... use pre-compiled patterns
      .trim();
}
lib/background/background.dart (2)

20-21: Logger initialization placement may cause missed logs during startup.

The logger is only initialized inside the 'start' listener after receiving the loggerSendPort. Any logs emitted before the 'start' event is received (e.g., during NostrService instantiation on line 24 or database opening on line 25) will not be captured.

This may be intentional given the isolate architecture constraints, but consider whether early startup diagnostics are needed.

Additionally, note that the logger is set to Level.debug unconditionally on line 43. In production builds, this might generate more verbose logs than desired through the isolate channel. Consider aligning with Config.isDebug:

♻️ Consider conditional log level
 logger = Logger(
   printer: logger_service.SimplePrinter(),
   output: logger_service.IsolateLogOutput(loggerSendPort),
-  level: Level.debug,
+  level: Config.isDebug ? Level.debug : Level.warning,
 );

This would require importing Config if not already available.

Also applies to: 38-44


89-89: Null-safe logging is correct but errors may be silently lost.

Using logger?.e(...) safely handles the case where logger hasn't been initialized. However, if an error occurs before the 'start' event (when logger is still null), the error will be silently swallowed.

Consider adding a fallback for critical errors:

♻️ Fallback for pre-initialization errors
       } catch (e) {
-        logger?.e('Error processing event', error: e);
+        if (logger != null) {
+          logger!.e('Error processing event', error: e);
+        } else {
+          // ignore: avoid_print
+          print('Error processing event before logger init: $e');
+        }
       }
📜 Review details

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 8382b55 and 3ea02f5.

📒 Files selected for processing (2)
  • lib/background/background.dart
  • lib/services/logger_service.dart
🧰 Additional context used
📓 Path-based instructions (3)
**/*.{dart,flutter}

📄 CodeRabbit inference engine (CLAUDE.md)

**/*.{dart,flutter}: Run flutter analyze after any code change - Mandatory before commits to ensure zero linting issues
Run flutter test after any code change - Mandatory before commits to ensure all unit tests pass

Files:

  • lib/services/logger_service.dart
  • lib/background/background.dart
**/*.dart

📄 CodeRabbit inference engine (CLAUDE.md)

**/*.dart: Use Riverpod for all state management - encapsulate business logic in Notifiers and access data only through repository classes
All code comments must be in English - use clear, concise English for variable names, function names, and comments
Always check mounted before using BuildContext after async operations to prevent errors on disposed widgets
Use const constructors where possible for better performance and immutability
Remove unused imports and dependencies to maintain code cleanliness and reduce build size

**/*.dart: Application code should be organized under lib/, grouped by domain with lib/features/<feature>/ structure, shared utilities in lib/shared/, dependency wiring in lib/core/, and services in lib/services/
Persistence, APIs, and background jobs should live in lib/data/ and lib/background/; generated localization output must be in lib/generated/ and must stay untouched
Apply flutter format . to enforce canonical Dart formatting (two-space indentation, trailing commas) before committing
Resolve every analyzer warning in Dart code
Name Riverpod providers using the <Feature>Provider or <Feature>Notifier convention
Localize all user-facing strings via ARB files and access them with S.of(context) rather than hard-coded literals

Files:

  • lib/services/logger_service.dart
  • lib/background/background.dart
lib/services/**/*.dart

📄 CodeRabbit inference engine (CLAUDE.md)

Access NostrService for all Nostr protocol interactions - NostrService manages relay connections and messaging

Files:

  • lib/services/logger_service.dart
🧠 Learnings (13)
📚 Learning: 2025-11-27T12:10:26.407Z
Learnt from: CR
Repo: MostroP2P/mobile PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-27T12:10:26.407Z
Learning: Applies to **/*.dart : Persistence, APIs, and background jobs should live in `lib/data/` and `lib/background/`; generated localization output must be in `lib/generated/` and must stay untouched

Applied to files:

  • lib/background/background.dart
📚 Learning: 2025-11-27T12:10:12.082Z
Learnt from: CR
Repo: MostroP2P/mobile PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-27T12:10:12.082Z
Learning: Applies to lib/features/**/notifiers/**/*.dart : Encapsulate business logic in Notifiers - Notifiers should expose state via providers and handle all complex state transitions

Applied to files:

  • lib/background/background.dart
📚 Learning: 2025-11-27T12:10:12.082Z
Learnt from: CR
Repo: MostroP2P/mobile PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-27T12:10:12.082Z
Learning: Applies to **/*.dart : Always check `mounted` before using BuildContext after async operations to prevent errors on disposed widgets

Applied to files:

  • lib/background/background.dart
📚 Learning: 2025-11-27T12:10:12.081Z
Learnt from: CR
Repo: MostroP2P/mobile PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-27T12:10:12.081Z
Learning: Run `flutter test integration_test/` only for significant changes affecting core services and main user flows

Applied to files:

  • lib/background/background.dart
📚 Learning: 2025-11-27T12:10:12.082Z
Learnt from: CR
Repo: MostroP2P/mobile PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-27T12:10:12.082Z
Learning: Applies to lib/services/nostr_service.dart : Manage all relay connections and Nostr messaging through NostrService - automatically reconnect when relay list updates

Applied to files:

  • lib/background/background.dart
📚 Learning: 2025-11-27T12:10:12.082Z
Learnt from: CR
Repo: MostroP2P/mobile PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-27T12:10:12.082Z
Learning: Applies to lib/services/**/*.dart : Access NostrService for all Nostr protocol interactions - NostrService manages relay connections and messaging

Applied to files:

  • lib/background/background.dart
📚 Learning: 2025-11-27T12:10:12.082Z
Learnt from: CR
Repo: MostroP2P/mobile PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-27T12:10:12.082Z
Learning: Applies to lib/features/subscriptions/subscription_manager.dart : Use SubscriptionManager with `fireImmediately: false` during SessionNotifier initialization to prevent premature execution

Applied to files:

  • lib/background/background.dart
📚 Learning: 2025-10-14T21:12:06.887Z
Learnt from: Catrya
Repo: MostroP2P/mobile PR: 327
File: lib/features/order/notfiers/abstract_mostro_notifier.dart:141-154
Timestamp: 2025-10-14T21:12:06.887Z
Learning: In the MostroP2P mobile codebase, the notification system uses a two-layer localization pattern: providers/notifiers (without BuildContext access) call `showCustomMessage()` with string keys (e.g., 'orderTimeoutMaker', 'orderCanceled'), and the UI layer's `NotificationListenerWidget` has a switch statement that maps these keys to localized strings using `S.of(context)`. This architectural pattern properly separates concerns while maintaining full localization support for all user-facing messages.

Applied to files:

  • lib/background/background.dart
📚 Learning: 2025-05-06T15:49:26.443Z
Learnt from: chebizarro
Repo: MostroP2P/mobile PR: 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.

Applied to files:

  • lib/background/background.dart
📚 Learning: 2025-11-27T12:10:12.082Z
Learnt from: CR
Repo: MostroP2P/mobile PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-27T12:10:12.082Z
Learning: Applies to lib/features/**/providers/**/*.dart : Use Notifier pattern instead of simple StateNotifier for complex state logic requiring business rule encapsulation

Applied to files:

  • lib/background/background.dart
📚 Learning: 2025-05-08T16:29:52.154Z
Learnt from: chebizarro
Repo: MostroP2P/mobile PR: 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.

Applied to files:

  • lib/background/background.dart
📚 Learning: 2025-11-27T12:10:12.082Z
Learnt from: CR
Repo: MostroP2P/mobile PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-27T12:10:12.082Z
Learning: Applies to lib/features/settings/settings.dart : Use null-aware operators (`??`) in Settings `copyWith()` method to preserve existing values for selectedLanguage and defaultLightningAddress when not explicitly overridden

Applied to files:

  • lib/background/background.dart
📚 Learning: 2025-07-20T23:33:17.689Z
Learnt from: Catrya
Repo: MostroP2P/mobile PR: 215
File: lib/features/order/notfiers/order_notifier.dart:0-0
Timestamp: 2025-07-20T23:33:17.689Z
Learning: In AbstractMostroNotifier, the handleEvent method is synchronous and returns void, not Future<void>. Do not suggest adding await to super.handleEvent() calls as this would cause compilation errors.

Applied to files:

  • lib/background/background.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 (4)
lib/services/logger_service.dart (3)

11-29: LGTM! Proper isolate receiver initialization with idempotency guard.

The initialization correctly prevents duplicate receivers with the early return check on line 12. The listener handles message processing with appropriate error catching. One minor observation: the error handling uses print with ignore comments, which is acceptable for this edge case where the logger itself has failed.


52-81: Timestamp parsing is robust with fallback handling.

The function correctly handles null and invalid timestamps by falling back to DateTime.now(). The message sanitization via cleanMessage ensures sensitive data is redacted before storage.


289-312: Changed code segment: Debug-only console printing is correctly gated.

The changes on lines 290-295 ensure console output only occurs in debug mode via Config.isDebug, which aligns with the PR objective to "print background isolate logs to console only in debug mode." The log message is properly sanitized before being sent via the SendPort.

The implementation correctly:

  1. Prints to console only in debug builds
  2. Sends sanitized log data through the isolate channel regardless of debug mode (when sendPort is available)
lib/background/background.dart (1)

2-2: Imports correctly added for isolate and logger service integration.

The dart:isolate import is needed for SendPort, and the aliased import of logger_service provides access to SimplePrinter and IsolateLogOutput.

Also applies to: 12-12

Copy link
Member

@AndreaDiazCorreia AndreaDiazCorreia left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was testing it, and it seems to me that everything works fine. tACK

Image

Copy link
Member

@Catrya Catrya left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@BraCR10 Hola, porfa revisa los Nitpick comments que puso coderabbit, sobre todo el 2 y 3 me parecen importantes, qué crees de esos 2?

@BraCR10
Copy link
Collaborator Author

BraCR10 commented Jan 15, 2026

20-21: Logger initialization placement may cause missed logs during startup.
The logger is only initialized inside the 'start' listener after receiving the loggerSendPort. Any logs emitted before the 'start' event is received (e.g., during NostrService instantiation on line 24 or database opening on line 25) will not be captured.

This may be intentional given the isolate architecture constraints, but consider whether early startup diagnostics are needed.

Additionally, note that the logger is set to Level.debug unconditionally on line 43. In production builds, this might generate more verbose logs than desired through the isolate channel. Consider aligning with Config.isDebug

Hola @Catrya, respecto a este Nitpick revisé y lo que advierte es que se podría perder algún log antes de la inicialización. Sin embargo, los servicios instanciados antes (NostrService, EventStorage) tienen constructores sin logging, y todas las operaciones reales ocurren después de que el logger está listo.

Mover la inicialización de estos servicios al listener 'start' complicaría el código porque requeriría usar variables late, agregar verificaciones de null y aumentaría el riesgo de errores en tiempo de ejecución.

Además, prefiero mantener el nivel debug para mostrar los logs completos en la UI.

Copy link
Member

@Catrya Catrya left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

tACK

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants