feat: wire relay toggle, mostro node selector, log stream, and disput… - #90
Merged
Conversation
…e stubs to Rust bridge - Wire relay toggle persistence via addRelay/removeRelay with optimistic UI and safe rollback by URL lookup - Add runtime mostro pubkey override (config.rs + settings API) and update all order/reputation/dispute/relay code to use active_mostro_pubkey() - Wire MostroNodeSelector to call setMostroPubkey on confirm/reset - Add rust/src/api/logging.rs with install_log_bridge, forward_log, and on_log_entry stream; create logEntriesProvider and replace mock entries in LogReportScreen with live Rust log stream - Replace TODO stubs in DisputeChatScreen with Phase 12 context comments - Regenerate flutter_rust_bridge bindings
Contributor
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (3)
WalkthroughAdds a Rust→Flutter live logging bridge and FRB bindings, a runtime Mostro pubkey override with settings APIs, rewires Dart settings/widgets to call Rust bridge functions, replaces dispute chat TODOs with Phase‑12 documentation, and updates multiple Rust modules to use the runtime pubkey. Changes
Sequence Diagram(s)sequenceDiagram
participant Flutter as Flutter App (log_provider & UI)
participant FRB as FRB Bridge (frb_generated)
participant LogAPI as Rust Logging API (api/logging.rs)
participant Relay as Background Relay Thread (std→tokio broadcast)
Flutter->>FRB: call on_log_entry() -> LogEntryStream
FRB->>LogAPI: on_log_entry()
LogAPI->>Relay: subscribe (broadcast::Receiver)
LogAPI-->>FRB: return LogEntryStream wrapper
FRB-->>Flutter: stream handle
Note over Relay,LogAPI: install_log_bridge() spawns forwarder thread once
LogAPI->>Relay: forward_log(level,target,msg) via std channel
Relay->>LogAPI: broadcast LogEntry
LogAPI->>FRB: LogEntryStream.next() yields LogEntry
FRB-->>Flutter: LogEntry delivered
Flutter->>Flutter: accumulate entries (max 500, newest first)
sequenceDiagram
participant Flutter as Flutter Widget (mostro_node_selector)
participant FRB as FRB Bridge
participant SettingsAPI as Rust Settings API (api/settings.rs)
participant Config as Config State (config.rs)
Flutter->>FRB: settings_api.setMostroPubkey(pubkey)
FRB->>SettingsAPI: set_mostro_pubkey(Option<String>)
SettingsAPI->>SettingsAPI: validate hex (if Some)
SettingsAPI->>Config: set_active_mostro_pubkey(pubkey)
Config->>Config: update ACTIVE_MOSTRO_PUBKEY (RwLock)
SettingsAPI-->>FRB: return Result
FRB-->>Flutter: success / error (UI updates)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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. Comment |
![mostronatorcoder[bot]](https://avatars.githubusercontent.com/in/2922838?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Review
Good work across all four tasks. The architecture is correct throughout. Two issues to fix before merge.
🔴 Blocker — forward_log is dead code: nothing calls it
rust/src/api/logging.rs implements install_log_bridge() and forward_log() correctly, but install_log_bridge() is never called from the app's initialization path, and forward_log() is never invoked at any Rust log site.
The result: on_log_entry() is wired to Dart and the stream exists, but it will never emit a single entry — LogReportScreen will always show "No log entries" even when logging is enabled.
The fix has two parts:
Part A — Call install_log_bridge() during app startup. Find where the Rust runtime is initialized (likely rust/src/api/app.rs or rust/src/lib.rs) and add:
crate::api::logging::install_log_bridge();Part B — Actually forward log records. The simplest approach is to add explicit forward_log calls at the key sites that matter to users, for example in rust/src/api/orders.rs where order state changes are logged:
use crate::api::logging::forward_log;
// After publishing an order event:
forward_log(log::Level::Info, "orders", &format!("order created: {}", order_id));Alternatively — and more correctly — integrate forward_log into a custom log::Log impl so that every log::info!() / log::warn!() / log::error!() call in the codebase is automatically forwarded. Either approach works; the important thing is that something actually calls forward_log.
🟡 Major — logEntriesProvider never completes on widget disposal, stream leaks
lib/features/settings/providers/log_provider.dart uses:
final logEntriesProvider = StreamProvider.autoDispose<List<LogEntry>>((ref) async* {
final stream = await logging_api.onLogEntry();
while (true) {
final entry = await stream.next();
if (entry == null) break;
...
}
});The problem: stream.next() is a plain await with no cancellation path. When the widget is disposed, Riverpod cancels the autoDispose provider — but the await stream.next() is already in-flight and holds the async generator alive until the next log entry arrives. This is a resource leak; the Dart generator and the underlying Rust broadcast receiver are both kept alive indefinitely.
Fix: use ref.onDispose to signal cancellation:
final logEntriesProvider = StreamProvider.autoDispose<List<LogEntry>>((ref) async* {
var cancelled = false;
ref.onDispose(() => cancelled = true);
final stream = await logging_api.onLogEntry();
final entries = <LogEntry>[];
while (!cancelled) {
final entry = await stream.next();
if (entry == null || cancelled) break;
entries.insert(0, entry);
if (entries.length > 500) entries.removeLast();
yield List.unmodifiable(entries);
}
});✅ What's good
ACTIVE_MOSTRO_PUBKEY: RwLock<Option<String>>inconfig.rsis the right pattern for a runtime override — single source of truth, noArcneeded, read path is fast.- All six call sites (
orders.rs,reputation.rs,disputes.rs,relay_pool.rs) correctly migrated fromDEFAULT_MOSTRO_PUBKEYtoactive_mostro_pubkey(). Consistent. set_mostro_pubkeyvalidates the hex pubkey vianostr_sdk::PublicKey::from_hexbefore writing to the store. Correct — the Rust layer is the source of truth for validation._toggleRelayrollback usesindexWhereby URL instead of the captured index — safe against list mutations during the async call. Good catch.- l10n strings used for toggle snackbars (
relayAddFailed,relayRemoveFailed) instead of hardcoded strings. Correct. _sanitizeForShareinlog_report_screen.dartredacts hex keys, nsec/npub, Bearer tokens, and key=value secrets before sharing. Solid._formatTimestampshows only time for same-day entries, full date otherwise. Clean UX.- Dispute chat stubs replaced with
l10n.disputeMessagingComingSoonandl10n.disputeAttachmentsComingSoon— correct use of l10n, good UX. MostroNodeSelectorcallssettings_api.setMostroPubkey(pubkey: null)on reset andsettings_api.setMostroPubkey(pubkey: pubkey)on confirm — correctly wired to the Rust bridge.- Input normalized to lowercase before storing:
final pubkey = input.toLowerCase(). Correct. - FRB bindings regenerated. Good.
Install BridgeLogger as the global log::Log impl in init_app() so every log::info!/warn!/error! call in Rust is forwarded to the Flutter on_log_entry() stream automatically. Fix logEntriesProvider stream leak: add ref.onDispose cancellation flag so the async generator stops when the provider is disposed.
![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: 7
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
lib/features/settings/widgets/mostro_node_selector.dart (1)
62-68:⚠️ Potential issue | 🟡 MinorFire-and-forget async call lacks error handling.
settings_api.setMostroPubkey(pubkey: null)returns aFuturebut is not awaited and has no error handling. If the Rust bridge call fails, the UI will show the default pubkey while Rust retains the previous value, causing a state desync.Consider awaiting with error handling and rolling back the UI state on failure:
♻️ Suggested improvement
- void _useDefault() { + Future<void> _useDefault() async { + final previous = ref.read(mostroPubkeyProvider); ref.read(mostroPubkeyProvider.notifier).state = _defaultMostroPubkey; - settings_api.setMostroPubkey(pubkey: null); + try { + await settings_api.setMostroPubkey(pubkey: null); + } catch (e) { + ref.read(mostroPubkeyProvider.notifier).state = previous; + debugPrint('Failed to reset Mostro pubkey: $e'); + return; + } _controller.clear(); setState(() => _errorText = null); Navigator.of(context).pop(); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@lib/features/settings/widgets/mostro_node_selector.dart` around lines 62 - 68, In _useDefault(), the async call settings_api.setMostroPubkey(pubkey: null) is being fire-and-forgotten; change it to await the Future inside a try/catch so failures can be detected, capture the current pubkey before mutating ref.read(mostroPubkeyProvider.notifier).state so you can restore it on error, and on failure setState to show an error (e.g. set _errorText) and avoid closing the dialog (remove or defer Navigator.of(context).pop() until after success); ensure you still clear _controller and reset UI only after the Rust bridge call succeeds.
🧹 Nitpick comments (2)
rust/src/api/settings.rs (1)
183-201: Consider broadcasting pubkey changes for UI consistency.Unlike other settings setters (e.g.,
set_theme,set_language),set_mostro_pubkeydoes not notify theSettingsStreamsubscribers. While this may be intentional since the pubkey isn't part ofAppSettings, it creates an inconsistency — UI components watchingon_settings_changed()won't receive updates when the Mostro node changes.If subscribers should react to pubkey changes, consider either:
- Adding
mostro_pubkeytoAppSettingsand broadcasting, or- Creating a separate stream for pubkey changes.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@rust/src/api/settings.rs` around lines 183 - 201, set_mostro_pubkey currently validates and writes the value via crate::config::set_active_mostro_pubkey but does not notify SettingsStream subscribers (unlike set_theme/set_language), causing UI inconsistency; update set_mostro_pubkey to emit a change event after persisting the pubkey by either (A) adding mostro_pubkey to AppSettings and calling the existing SettingsStream/on_settings_changed notification path so subscribers see the update, or (B) creating a dedicated MostroPubkeyStream and broadcasting the new value from set_mostro_pubkey so components can subscribe to pubkey changes; reference set_mostro_pubkey, crate::config::set_active_mostro_pubkey, AppSettings, SettingsStream, and on_settings_changed when implementing the notification.rust/src/api/logging.rs (1)
25-26: Unnecessary#[allow(dead_code)]onCOUNTER.
COUNTERis actively used inforward_log()at line 60. The allow attribute is misleading.-#[allow(dead_code)] static COUNTER: AtomicU32 = AtomicU32::new(0);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@rust/src/api/logging.rs` around lines 25 - 26, The #[allow(dead_code)] attribute on the static COUNTER is misleading because COUNTER is used by forward_log(); remove the #[allow(dead_code)] annotation above the static COUNTER declaration so the code accurately reflects usage and lets the compiler warn if it ever becomes unused, leaving the static COUNTER: AtomicU32 = AtomicU32::new(0) definition intact and ensuring forward_log() continues to reference COUNTER.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@lib/features/settings/widgets/mostro_node_selector.dart`:
- Around line 80-83: The _confirm() handler currently sets mostroPubkeyProvider
and calls settings_api.setMostroPubkey without awaiting or handling errors;
change it to await settings_api.setMostroPubkey inside a try/catch, only update
ref.read(mostroPubkeyProvider.notifier).state and call
Navigator.of(context).pop() on success, and on failure revert any local state
change (or avoid mutating state until after the await) and surface the error
(e.g., show a SnackBar or dialog) in the catch block so the UI rolls back on
Rust call failure.
- Around line 12-16: mostroPubkeyProvider currently always initializes from
_defaultMostroPubkey so a user-selected pubkey is lost on restart; fix by
persisting/restoring it: either add a mostroPubkey field to AppSettingsState and
include it in fromPrefs()/SharedPreferences read/write (and ensure
set_mostro_pubkey() writes to prefs when changed), or hydrate the provider at
app startup by calling settings_api.getMostroPubkey() from main.dart and setting
mostroPubkeyProvider accordingly so the Dart state matches the Rust bridge
persisted value; reference symbols: mostroPubkeyProvider, _defaultMostroPubkey,
AppSettingsState.fromPrefs, set_mostro_pubkey(), getMostroPubkey(),
settings_api.getMostroPubkey(), main.dart.
In `@rust/src/api/logging.rs`:
- Around line 28-49: Call install_log_bridge() from the app initialization path
so the logging bridge and LOG_STD_TX are initialized before any forward_log()
calls; specifically, add a call to crate::api::logging::install_log_bridge()
inside the init_app() function (the flutter_rust_bridge frb init hook) during
startup, placing it before the log::info!("[init] Rust core initialized") line
so the background thread and std mpsc sender are set up and log forwarding will
not drop entries.
- Around line 51-78: forward_log is never registered so production logs never
reach Flutter; implement a logger that uses forward_log and register it with the
log crate (or call forward_log directly at key sites). Create a struct
implementing log::Log (e.g., FlutterLogger) whose log(&self, record:
&log::Record) converts record.level/target/message into a LogEntry and calls
forward_log (or sends via LOG_STD_TX), then call
log::set_boxed_logger(Box::new(FlutterLogger)) and set the max_level via
log::set_max_level during init; alternatively, add explicit forward_log(...)
calls at strategic production emit points using the existing forward_log,
LOG_STD_TX, COUNTER, and LogEntry symbols.
In `@rust/src/frb_generated.rs`:
- Around line 2462-2495: The generated FFI binding
wire__crate__api__logging__install_log_bridge_impl currently exposes
install_log_bridge but the function is never invoked from startup; call
crate::api::logging::install_log_bridge() during app initialization (preferably
inside your Rust init_app() function) or ensure Dart invokes the exposed
installLogBridge() from main.dart before any forward_log() calls so the
std::sync::Once guard runs and the logging bridge is actually installed; update
init_app() to call install_log_bridge() (or add a Dart-side startup call to
installLogBridge()) and verify the bridge is initialized before forwarding logs.
- Line 48: The Dart bindings for the updated Rust FRB API are missing: run the
flutter_rust_bridge_codegen generate command to regenerate the Dart files for
the Rust API (so that lib/src/rust/api/logging.dart,
lib/src/rust/api/settings.dart, etc. are created), add and commit those
generated files alongside the Rust changes (ensure the generated files
import/reflect the symbols used in
lib/features/settings/providers/log_provider.dart and
lib/features/settings/widgets/mostro_node_selector.dart such as
logging_api.onLogEntry and settings_api.setMostroPubkey), and verify the
rust/src/frb_generated.rs hash and Dart bindings are in sync before pushing.
---
Outside diff comments:
In `@lib/features/settings/widgets/mostro_node_selector.dart`:
- Around line 62-68: In _useDefault(), the async call
settings_api.setMostroPubkey(pubkey: null) is being fire-and-forgotten; change
it to await the Future inside a try/catch so failures can be detected, capture
the current pubkey before mutating ref.read(mostroPubkeyProvider.notifier).state
so you can restore it on error, and on failure setState to show an error (e.g.
set _errorText) and avoid closing the dialog (remove or defer
Navigator.of(context).pop() until after success); ensure you still clear
_controller and reset UI only after the Rust bridge call succeeds.
---
Nitpick comments:
In `@rust/src/api/logging.rs`:
- Around line 25-26: The #[allow(dead_code)] attribute on the static COUNTER is
misleading because COUNTER is used by forward_log(); remove the
#[allow(dead_code)] annotation above the static COUNTER declaration so the code
accurately reflects usage and lets the compiler warn if it ever becomes unused,
leaving the static COUNTER: AtomicU32 = AtomicU32::new(0) definition intact and
ensuring forward_log() continues to reference COUNTER.
In `@rust/src/api/settings.rs`:
- Around line 183-201: set_mostro_pubkey currently validates and writes the
value via crate::config::set_active_mostro_pubkey but does not notify
SettingsStream subscribers (unlike set_theme/set_language), causing UI
inconsistency; update set_mostro_pubkey to emit a change event after persisting
the pubkey by either (A) adding mostro_pubkey to AppSettings and calling the
existing SettingsStream/on_settings_changed notification path so subscribers see
the update, or (B) creating a dedicated MostroPubkeyStream and broadcasting the
new value from set_mostro_pubkey so components can subscribe to pubkey changes;
reference set_mostro_pubkey, crate::config::set_active_mostro_pubkey,
AppSettings, SettingsStream, and on_settings_changed when implementing the
notification.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 076fe717-219e-4ff0-aa70-cd781aff22be
📒 Files selected for processing (14)
lib/features/disputes/screens/dispute_chat_screen.dartlib/features/settings/providers/log_provider.dartlib/features/settings/screens/log_report_screen.dartlib/features/settings/widgets/mostro_node_selector.dartlib/features/settings/widgets/relay_management_card.dartrust/src/api/disputes.rsrust/src/api/logging.rsrust/src/api/mod.rsrust/src/api/orders.rsrust/src/api/reputation.rsrust/src/api/settings.rsrust/src/config.rsrust/src/frb_generated.rsrust/src/nostr/relay_pool.rs
Comment on lines
+12
to
16
| /// Active Mostro node pubkey — synced to the Rust bridge so outgoing events | ||
| /// are routed to the selected node. | ||
| final mostroPubkeyProvider = StateProvider<String>( | ||
| (ref) => _defaultMostroPubkey, | ||
| ); |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check if getMostroPubkey is called anywhere during app initialization
rg -n 'getMostroPubkey|get_mostro_pubkey' --type dart --type rustRepository: MostroP2P/app
Length of output: 550
🏁 Script executed:
# Find main.dart and related initialization code
fd -n "main\.dart" --type fRepository: MostroP2P/app
Length of output: 287
🏁 Script executed:
# Search for AppSettingsState definition
rg -n "class AppSettingsState|AppSettingsState\.fromPrefs" --type dart -A 10Repository: MostroP2P/app
Length of output: 2841
🏁 Script executed:
# Check if getMostroPubkey is called in Dart code (binding bridge)
rg -n "getMostroPubkey|settingsApi" --type dart -B 2 -A 2Repository: MostroP2P/app
Length of output: 39
🏁 Script executed:
# Examine the mostro_node_selector.dart file for the full provider context
cat -n lib/features/settings/widgets/mostro_node_selector.dartRepository: MostroP2P/app
Length of output: 9291
🏁 Script executed:
# Search for getMostroPubkey in ALL Dart files (case-insensitive)
rg -i "getmostroPubkey" --type dartRepository: MostroP2P/app
Length of output: 39
🏁 Script executed:
# Check main.dart initialization section more comprehensively
sed -n '45,80p' lib/main.dartRepository: MostroP2P/app
Length of output: 1557
🏁 Script executed:
# Search for any references to mostroPubkeyProvider initialization or hydration
rg "mostroPubkeyProvider" --type dart -B 2 -A 2Repository: MostroP2P/app
Length of output: 3948
🏁 Script executed:
# Check if settings_api.setMostroPubkey/getMostroPubkey are mentioned in Rust settings.rs
cat -n rust/src/api/settings.rs | grep -A 5 -B 5 "get_mostro_pubkey\|set_mostro_pubkey" | head -50Repository: MostroP2P/app
Length of output: 922
Custom Mostro node selection is ephemeral—will reset on app restart.
The mostroPubkeyProvider initializes with _defaultMostroPubkey on every app launch. Although the Rust bridge persists the value via set_mostro_pubkey(), Dart never restores it: main.dart does not call getMostroPubkey() on startup, and AppSettingsState.fromPrefs() does not include mostroPubkey.
To persist across restarts, either:
- Add
mostroPubkeytoAppSettingsStateandSharedPreferencespersistence, or - Call
settings_api.getMostroPubkey()during app initialization to hydrate the provider from Rust's persisted value.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@lib/features/settings/widgets/mostro_node_selector.dart` around lines 12 -
16, mostroPubkeyProvider currently always initializes from _defaultMostroPubkey
so a user-selected pubkey is lost on restart; fix by persisting/restoring it:
either add a mostroPubkey field to AppSettingsState and include it in
fromPrefs()/SharedPreferences read/write (and ensure set_mostro_pubkey() writes
to prefs when changed), or hydrate the provider at app startup by calling
settings_api.getMostroPubkey() from main.dart and setting mostroPubkeyProvider
accordingly so the Dart state matches the Rust bridge persisted value; reference
symbols: mostroPubkeyProvider, _defaultMostroPubkey, AppSettingsState.fromPrefs,
set_mostro_pubkey(), getMostroPubkey(), settings_api.getMostroPubkey(),
main.dart.
Comment on lines
+82
to
+89
| } catch (e) { | ||
| debugPrint('[RelayManagement] toggleRelay failed: $e'); | ||
| if (!mounted) return; | ||
| setState(() { | ||
| final currentIndex = _relays.indexWhere((r) => r.url == url); | ||
| if (currentIndex != -1) { | ||
| _relays[currentIndex].isActive = !value; | ||
| } |
Contributor
There was a problem hiding this comment.
Handle typed relay errors and stale rollback explicitly.
Lines 82-89 revert with !value for every exception. With non-idempotent relay ops (RelayAlreadyExists, LastRelay) and rapid re-toggles, an older failed request can overwrite a newer successful state and desync UI/backend state.
Proposed hardening
} catch (e) {
debugPrint('[RelayManagement] toggleRelay failed: $e');
if (!mounted) return;
+ final err = e.toString();
+ // Enabling an already-present relay is effectively desired state.
+ if (value && err.contains('RelayAlreadyExists')) {
+ return;
+ }
setState(() {
final currentIndex = _relays.indexWhere((r) => r.url == url);
- if (currentIndex != -1) {
+ // Avoid stale rollback if user already toggled again.
+ if (currentIndex != -1 && _relays[currentIndex].isActive == value) {
_relays[currentIndex].isActive = !value;
}
});
+ await _loadRelays(); // resync from source of truth after failure
final l10n = AppLocalizations.of(context);
ScaffoldMessenger.of(context).showSnackBar(
SnackBar(content: Text(value ? l10n.relayAddFailed : l10n.relayRemoveFailed)),
);
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| } catch (e) { | |
| debugPrint('[RelayManagement] toggleRelay failed: $e'); | |
| if (!mounted) return; | |
| setState(() { | |
| final currentIndex = _relays.indexWhere((r) => r.url == url); | |
| if (currentIndex != -1) { | |
| _relays[currentIndex].isActive = !value; | |
| } | |
| } catch (e) { | |
| debugPrint('[RelayManagement] toggleRelay failed: $e'); | |
| if (!mounted) return; | |
| final err = e.toString(); | |
| // Enabling an already-present relay is effectively desired state. | |
| if (value && err.contains('RelayAlreadyExists')) { | |
| return; | |
| } | |
| setState(() { | |
| final currentIndex = _relays.indexWhere((r) => r.url == url); | |
| // Avoid stale rollback if user already toggled again. | |
| if (currentIndex != -1 && _relays[currentIndex].isActive == value) { | |
| _relays[currentIndex].isActive = !value; | |
| } | |
| }); | |
| await _loadRelays(); // resync from source of truth after failure | |
| final l10n = AppLocalizations.of(context); | |
| ScaffoldMessenger.of(context).showSnackBar( | |
| SnackBar(content: Text(value ? l10n.relayAddFailed : l10n.relayRemoveFailed)), | |
| ); | |
| } |
| ); | ||
| pub(crate) const FLUTTER_RUST_BRIDGE_CODEGEN_VERSION: &str = "2.11.1"; | ||
| pub(crate) const FLUTTER_RUST_BRIDGE_CODEGEN_CONTENT_HASH: i32 = -1150097857; | ||
| pub(crate) const FLUTTER_RUST_BRIDGE_CODEGEN_CONTENT_HASH: i32 = -327029727; |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
fd -i '(frb_generated|bridge_generated).*\.(dart)$' -x sh -c '
echo "=== $1 ==="
rg -n "installLogBridge|onLogEntry|getMostroPubkey|setMostroPubkey|2\\.11\\.1|-327029727" "$1" || true
' sh {}Repository: MostroP2P/app
Length of output: 39
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# First, find all .dart files in the repository
echo "=== All .dart files in repository ==="
fd '\.dart$' | head -20
echo ""
echo "=== Total .dart file count ==="
fd '\.dart$' | wc -l
echo ""
echo "=== Check for Flutter/Dart directories ==="
fd -type d -i 'flutter|dart|lib' | head -20Repository: MostroP2P/app
Length of output: 1242
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Search for the symbols across all Dart files
echo "=== Searching for logging bridge symbols in .dart files ==="
rg "installLogBridge|onLogEntry|getMostroPubkey|setMostroPubkey" --type dart || echo "No matches found"
echo ""
echo "=== Searching for FRB version or hash markers in .dart files ==="
rg "2\.11\.1|-327029727|FLUTTER_RUST_BRIDGE|flutter_rust_bridge" --type dart || echo "No matches found"Repository: MostroP2P/app
Length of output: 3296
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Check if this is a Flutter project and where bindings might be
echo "=== Checking for pubspec.yaml or build configuration ==="
find . -maxdepth 3 -name "pubspec.yaml" -o -name "pubspec.lock" | head -5
echo ""
echo "=== Checking for codegen configuration ==="
rg "build_runner|frb|flutter_rust_bridge" --type yaml --type toml --type json 2>/dev/null | head -20Repository: MostroP2P/app
Length of output: 1305
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Find where logging_api and settings_api are imported from
echo "=== Imports of logging_api and settings_api ==="
rg "import.*logging_api|import.*settings_api" --no-filename
echo ""
echo "=== Search for any generated Dart files with different naming ==="
fd '.*\.dart$' | grep -E "(generated|api|bridge)" | head -30
echo ""
echo "=== Look for the actual bindings file ==="
find . -name "*_api*" -o -name "*generated*" | grep '\.dart$' | head -20Repository: MostroP2P/app
Length of output: 510
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Search for where the FRB symbols are defined
echo "=== Files containing installLogBridge definition or export ==="
rg "installLogBridge" -B 2 -A 2
echo ""
echo "=== Files containing getMostroPubkey definition or export ==="
rg "getMostroPubkey|get_mostro_pubkey" -B 1 -A 1Repository: MostroP2P/app
Length of output: 119
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Check the rust bindings generated file
echo "=== rust_bindings_generated.dart info ==="
wc -l ./rust_builder/lib/rust_bindings_generated.dart
echo ""
echo "=== Check for logging and settings functions in bindings ==="
rg "onLogEntry|getMostroPubkey|setMostroPubkey" ./rust_builder/lib/rust_bindings_generated.dart || echo "Not in bindings"
echo ""
echo "=== List all Dart files under src/rust/api ==="
find ./lib -path "*src/rust/api*" -name "*.dart" 2>/dev/null || echo "Not found under lib"
echo ""
echo "=== Check different src structure ==="
fd "api.*\.dart" | grep -i "logging\|settings" || echo "Not found"Repository: MostroP2P/app
Length of output: 324
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Find the actual API files
echo "=== Find all files with logging.dart or settings.dart ==="
fd -i "logging\.dart|settings\.dart"
echo ""
echo "=== Content of logging API file ==="
cat "lib/src/rust/api/logging.dart" 2>/dev/null | head -50 || echo "File not at lib/src/rust/api/logging.dart"
echo ""
echo "=== Content of settings API file (first 50 lines) ==="
cat "lib/src/rust/api/settings.dart" 2>/dev/null | head -50 || echo "File not at lib/src/rust/api/settings.dart"Repository: MostroP2P/app
Length of output: 345
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Search more broadly for where these symbols might be
echo "=== All files in src/rust ==="
find . -path "*/src/rust*" -name "*.dart" 2>/dev/null
echo ""
echo "=== Search pubspec.yaml to understand the package structure ==="
grep -A 5 -B 5 "name:" pubspec.yamlRepository: MostroP2P/app
Length of output: 322
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Read the generated bindings file
echo "=== Full content of rust_bindings_generated.dart ==="
cat ./rust_builder/lib/rust_bindings_generated.dart
echo ""
echo "=== Check flutter_rust_bridge.yaml config ==="
cat flutter_rust_bridge.yaml 2>/dev/null | head -50
echo ""
echo "=== Look for where src/rust/api files should be generated ==="
rg "output.*api|api.*output" --type yamlRepository: MostroP2P/app
Length of output: 2657
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Check what the actual package structure is for the main app
echo "=== Search lib directory structure ==="
ls -la lib/ | head -20
echo ""
echo "=== Look for any generated or FFI related files ==="
find lib -name "*generated*" -o -name "*ffi*" -o -name "*bridge*" 2>/dev/null | head -20
echo ""
echo "=== Check if there's a build or generated directory ==="
ls -la | grep -E "(build|generated|\.dart_tool)"Repository: MostroP2P/app
Length of output: 668
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Check if lib/src/rust exists
echo "=== Check lib/src directory ==="
ls -la lib/src/ 2>/dev/null || echo "lib/src does not exist"
echo ""
echo "=== Check if lib/src/rust exists ==="
ls -la lib/src/rust/ 2>/dev/null || echo "lib/src/rust does not exist"
echo ""
echo "=== Check if the expected generated files are missing ==="
ls -la lib/src/rust/api/ 2>/dev/null || echo "lib/src/rust/api directory does not exist"
echo ""
echo "=== Verify imports in log_provider.dart ==="
grep -n "import\|logging_api\|onLogEntry" lib/features/settings/providers/log_provider.dart | head -20
echo ""
echo "=== Verify imports in mostro_node_selector.dart ==="
grep -n "import\|settings_api\|setMostroPubkey" lib/features/settings/widgets/mostro_node_selector.dart | head -20Repository: MostroP2P/app
Length of output: 981
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Check the actual Rust generated file mentioned in the review
echo "=== Check if rust/src/frb_generated.rs exists ==="
ls -la rust/src/frb_generated.rs
echo ""
echo "=== Line 48 showing the hash ==="
sed -n '46,50p' rust/src/frb_generated.rs
echo ""
echo "=== Search for new dispatcher entries mentioned (5247-5445) ==="
sed -n '5247,5260p' rust/src/frb_generated.rs
echo ""
echo "=== Check if logging related handlers exist in frb_generated.rs ==="
rg "install_log_bridge|LogEntry|get_mostro_pubkey|set_mostro_pubkey" rust/src/frb_generated.rs | head -10Repository: MostroP2P/app
Length of output: 2148
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Check git status to see what was actually modified
echo "=== Recent changes to frb_generated.rs ==="
git log --oneline -10 -- rust/src/frb_generated.rs 2>/dev/null || echo "Not a git repo or file not tracked"
echo ""
echo "=== Check if Dart bindings are expected to be generated or committed ==="
git log --oneline -10 -- lib/src/rust/ 2>/dev/null || echo "lib/src/rust not tracked"
echo ""
echo "=== Check .gitignore for generated files ==="
cat .gitignore 2>/dev/null | grep -E "generated|lib/src" || echo "No matches in gitignore"Repository: MostroP2P/app
Length of output: 477
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Check the git status and what was actually changed in this PR
echo "=== Git diff stat for the commit ==="
git show --stat 9594227 2>/dev/null | head -50
echo ""
echo "=== Check if any Dart files in lib/src/rust were modified in this commit ==="
git show 9594227 -- lib/src/rust/ 2>/dev/null | head -50 || echo "No changes to lib/src/rust"
echo ""
echo "=== Full list of files changed in the commit ==="
git show --name-only 9594227 2>/dev/null | tail -30Repository: MostroP2P/app
Length of output: 3010
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Check what happens if we try to build or analyze
echo "=== Check if there are build scripts that generate these files ==="
find . -maxdepth 2 -type f \( -name "build.rs" -o -name "build.yaml" -o -name "Makefile" \) 2>/dev/null
echo ""
echo "=== Check flutter_rust_bridge.yaml for output config again ==="
grep -E "dart_output|rust_input" flutter_rust_bridge.yaml
echo ""
echo "=== Check if there's a README explaining the build process ==="
head -50 README.md 2>/dev/null | grep -i "generate\|build\|bridge" || echo "No matches in README"
echo ""
echo "=== Verify the actual imports are in use - check all imports from lib/src/rust/api ==="
rg "from 'package:mostro/src/rust/api" lib/ --type dartRepository: MostroP2P/app
Length of output: 429
🏁 Script executed:
#!/bin/bash
set -euo pipefixo
# Get the complete list of files changed in the commit
echo "=== All files changed in commit 9594227 ==="
git show 9594227 --name-status 2>/dev/null | head -100
echo ""
echo "=== Specifically check for lib/src/rust files ==="
git show 9594227 --name-status 2>/dev/null | grep "lib/src/rust"
echo ""
echo "=== Check if lib/src/rust/api files were generated but ignored ==="
git show 9594227 -- lib/src/rust/api/logging.dart 2>/dev/null | head -20 || echo "File not in commit"Repository: MostroP2P/app
Length of output: 110
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Since lib/src/rust is in .gitignore, check if the generated files exist in the working directory
echo "=== Actually list what's in lib/src/rust if it exists ==="
ls -laR lib/src/rust/ 2>/dev/null || echo "lib/src/rust directory doesn't exist in working directory"
echo ""
echo "=== Check if there's a build process that should generate it ==="
grep -r "flutter_rust_bridge_codegen generate" . --include="*.sh" --include="*.md" --include="*.yaml" --include="Makefile" 2>/dev/null | head -10
echo ""
echo "=== Check the build.rs file ==="
head -50 rust/build.rsRepository: MostroP2P/app
Length of output: 1982
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Check the commit more carefully
echo "=== Check what files were actually added/modified in this commit ==="
git diff 9594227^ 9594227 --name-status 2>/dev/null | grep -E "lib/src|rust/src/api" | head -20
echo ""
echo "=== Check if Dart files from api were REALLY not committed ==="
git show 9594227:lib/src/rust/api/logging.dart 2>/dev/null | head -20 || echo "File not in commit"
echo ""
echo "=== Verify README requirements ==="
grep -A 2 "Bridge changes" README.mdRepository: MostroP2P/app
Length of output: 125
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Check if there's a setup/install script that generates these
echo "=== Look for setup or install scripts ==="
fd -i "setup|install" --type f | grep -E "\.(sh|dart)$" | head -10
echo ""
echo "=== Check pubspec.yaml for build scripts ==="
grep -E "build_runner|build|codegen|generate" pubspec.yaml
echo ""
echo "=== Understand the project structure better - check if files should exist at build time ==="
grep -B 5 -A 5 "lib/src/rust" .gitignoreRepository: MostroP2P/app
Length of output: 98
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Check if git diff works differently
echo "=== Show the actual commit content ==="
git log -1 9594227 --stat 2>/dev/null | head -50
echo ""
echo "=== Check if api/logging.rs was added ==="
git show 9594227:rust/src/api/logging.rs 2>/dev/null | head -30 || echo "File not found in commit"
echo ""
echo "=== Verify the actual problem: search for where logging_api is coming from ==="
rg "pub.*onLogEntry|pub.*setMostroPubkey|pub.*getMostroPubkey" lib/ --type dart 2>/dev/null || echo "Not in lib"Repository: MostroP2P/app
Length of output: 3012
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# The key question: are lib/src/rust files supposed to be generated at build time or committed?
echo "=== Check pubspec.yaml for build_runner configuration ==="
cat pubspec.yaml | grep -A 10 "build_runner\|dev_dependencies"
echo ""
echo "=== Look for build.yaml or other codegen config in lib ==="
find lib -name "build.yaml" -o -name "pubspec_overrides.yaml" 2>/dev/null
echo ""
echo "=== Check if there's a .dart_tool directory (generated at runtime) ==="
ls -la .dart_tool/ 2>/dev/null | head -10 || echo "No .dart_tool directory"Repository: MostroP2P/app
Length of output: 556
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Final verification - see if the git log shows the full file list
echo "=== Full file list from commit (might be truncated earlier) ==="
git show 9594227 --name-only 2>/dev/null | tail -100
echo ""
echo "=== Count of files in the commit ==="
git show 9594227 --name-only 2>/dev/null | wc -l
echo ""
echo "=== Specific check: was rust/src/api/logging.rs added? ==="
git show 9594227:rust/src/api/logging.rs 2>/dev/null | wc -l || echo "File not in commit"Repository: MostroP2P/app
Length of output: 4814
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Verify if lib/src/rust was ignored in this commit
echo "=== Current .gitignore entry ==="
cat .gitignore | grep "lib/src/rust"
echo ""
echo "=== What the README says about committing generated bindings ==="
grep -B 2 -A 3 "Commit the generated files" README.md
echo ""
echo "=== Try to understand if the imports would even work ==="
echo "Can Dart resolve these imports at compile time if files don't exist?"
echo "Files needed:"
echo " - lib/src/rust/api/logging.dart (used in log_provider.dart and settings/widgets)"
echo " - lib/src/rust/api/settings.dart (used in mostro_node_selector.dart)"
echo " - lib/src/rust/api/types.dart (used in log_provider.dart)"
echo ""
echo "Actual files found:"
find lib/src -type f 2>/dev/null || echo "lib/src does not exist"Repository: MostroP2P/app
Length of output: 1333
Dart FRB generated bindings were not committed; this will cause Dart compilation failure.
The commit updated rust/src/api/logging.rs and regenerated rust/src/frb_generated.rs (line 48 hash now -327029727), but the matching Dart binding files (lib/src/rust/api/logging.dart, lib/src/rust/api/settings.dart, etc.) do not exist. Dart code in lib/features/settings/providers/log_provider.dart and lib/features/settings/widgets/mostro_node_selector.dart imports from these non-existent files:
import 'package:mostro/src/rust/api/logging.dart' as logging_api;
import 'package:mostro/src/rust/api/settings.dart' as settings_api;and uses them (logging_api.onLogEntry(), settings_api.setMostroPubkey()). Although lib/src/rust/ is in .gitignore, the README states: "Any modification to rust/src/api/ requires re-running flutter_rust_bridge_codegen generate. Commit the generated files together with the Rust changes."
Run flutter_rust_bridge_codegen generate locally and commit the generated files in lib/src/rust/.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@rust/src/frb_generated.rs` at line 48, The Dart bindings for the updated Rust
FRB API are missing: run the flutter_rust_bridge_codegen generate command to
regenerate the Dart files for the Rust API (so that
lib/src/rust/api/logging.dart, lib/src/rust/api/settings.dart, etc. are
created), add and commit those generated files alongside the Rust changes
(ensure the generated files import/reflect the symbols used in
lib/features/settings/providers/log_provider.dart and
lib/features/settings/widgets/mostro_node_selector.dart such as
logging_api.onLogEntry and settings_api.setMostroPubkey), and verify the
rust/src/frb_generated.rs hash and Dart bindings are in sync before pushing.
Make _confirm() and _useDefault() in MostroNodeSelector async, await the Rust bridge call, and rollback mostroPubkeyProvider on failure with user-visible error text. Remove stale #[allow(dead_code)] on COUNTER in logging.rs.
Contributor
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
rust/src/lib.rs (1)
21-30:⚠️ Potential issue | 🔴 CriticalLog bridge will silently fail to install on Android due to logger initialization order.
Rust's
log::set_logger()can only succeed once per process. On Android:
android_logger::init_once()registers its logger first (line 22-26).install_log_bridge()then callslog::set_logger(&BRIDGE_LOGGER)which fails—the error is discarded withlet _ =(context snippet 1, line 54).Result: On Android, logs are not forwarded to the Flutter
on_log_entry()stream, defeating the purpose of this change.Since
BridgeLoggeralready handles Android output viaeprintln!(context snippet 2, lines 79-86), you can remove theandroid_logger::init_once()call entirely, or reverse the order so the bridge is installed first.Proposed fix: Remove redundant android_logger initialization
#[flutter_rust_bridge::frb(init)] pub fn init_app() { - #[cfg(target_os = "android")] - android_logger::init_once( - android_logger::Config::default() - .with_max_level(log::LevelFilter::Debug) - .with_tag("mostro_rust"), - ); - // Install the log bridge so every log::info!/warn!/error! is forwarded // to the Flutter on_log_entry() stream. api::logging::install_log_bridge(); log::info!("[init] Rust core initialized"); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@rust/src/lib.rs` around lines 21 - 30, The android_logger::init_once() call is installed before api::logging::install_log_bridge(), causing log::set_logger() in install_log_bridge() (which sets BRIDGE_LOGGER/BridgeLogger) to fail silently on Android; remove the android_logger::init_once() invocation (or alternatively move it after api::logging::install_log_bridge()) so that install_log_bridge() can successfully call log::set_logger(&BRIDGE_LOGGER) and forward logs to the Flutter on_log_entry() stream.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@rust/src/lib.rs`:
- Around line 21-30: The android_logger::init_once() call is installed before
api::logging::install_log_bridge(), causing log::set_logger() in
install_log_bridge() (which sets BRIDGE_LOGGER/BridgeLogger) to fail silently on
Android; remove the android_logger::init_once() invocation (or alternatively
move it after api::logging::install_log_bridge()) so that install_log_bridge()
can successfully call log::set_logger(&BRIDGE_LOGGER) and forward logs to the
Flutter on_log_entry() stream.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: b689c6ba-0ecc-447e-99c1-0c5a5b794824
📒 Files selected for processing (3)
lib/features/settings/providers/log_provider.dartrust/src/api/logging.rsrust/src/lib.rs
✅ Files skipped from review due to trivial changes (1)
- lib/features/settings/providers/log_provider.dart
🚧 Files skipped from review as they are similar to previous changes (1)
- rust/src/api/logging.rs
android_logger::init_once() called log::set_logger() before install_log_bridge(), causing BridgeLogger registration to silently fail on Android. Remove android_logger — BridgeLogger prints to stderr which logcat captures, and forwards to the Flutter stream.
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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.
…e stubs to Rust bridge
Summary by CodeRabbit
New Features
Bug Fixes