Beautified lot of terminal output #146
Conversation
- removed openssl dep - prepared reception of order id to restore database
WalkthroughAdds a parser::common UI utilities module, introduces an OrdersInfo CLI command, refactors many CLI handlers to use Context and table/emoji helpers, changes DM printing/parsing (print_direct_messages signature), updates several function signatures, adjusts DM send/wait/parse flows, updates dependencies, and adds extensive tests/docs. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant CLI
participant Client as ctx.client
participant Relays
participant Parser
rect rgb(235,245,255)
User->>CLI: orders_info(order_ids)
CLI->>Client: send DM (Orders request w/ request_id)
Client->>Relays: broadcast DM
Relays-->>Client: deliver response DM(s)
CLI->>Parser: parse_dm_events(response DMs)
Parser->>Parser: match request_id
Parser-->>CLI: print_commands_results (orders)
CLI-->>User: rendered orders info table
end
sequenceDiagram
participant User
participant CLI
participant Client as ctx.client
participant Relays
participant Parser
User->>CLI: execute_restore(...)
CLI->>Client: send DM (RestoreSession request)
Client->>Relays: broadcast DM
Relays-->>Client: deliver response DM(s)
CLI->>Parser: parse_dm_events(response DMs)
alt action == RestoreSession
Parser-->>CLI: print_commands_results (restore)
CLI-->>User: restore summary
else
Parser-->>CLI: unexpected action
CLI-->>User: error
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
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 (2)
src/cli/get_dm_user.rs (2)
13-21: Empty-keys check is ineffective after pushing Mostro keyYou push ctx.mostro_pubkey before checking emptiness, so trade_keys_hex.is_empty() is practically unreachable. Decide the intended behavior:
- If you want to early-exit when there are no order-derived keys, move the emptiness check before adding the Mostro key.
- If you want to fall back to “admin inbox only,” keep fetching but change the message accordingly.
Suggested fix (keep fetching, but make the check meaningful and message explicit):
- // Include admin pubkey so we also fetch messages sent TO admin - let admin_pubkey_hex = ctx.mostro_pubkey.to_hex(); - if !trade_keys_hex.iter().any(|k| k == &admin_pubkey_hex) { - trade_keys_hex.push(admin_pubkey_hex); - } - // De-duplicate any repeated keys coming from DB/admin - trade_keys_hex.sort(); - trade_keys_hex.dedup(); - - // Check if the trade keys are empty - if trade_keys_hex.is_empty() { - println!("📭 No trade keys found in orders"); - return Ok(()); - } + let had_order_keys = !trade_keys_hex.is_empty(); + + // Include Mostro pubkey so we also fetch messages sent TO Mostro (admin inbox) + let mostro_hex = ctx.mostro_pubkey.to_hex(); + if !trade_keys_hex.iter().any(|k| k == &mostro_hex) { + trade_keys_hex.push(mostro_hex); + } + trade_keys_hex.sort(); + trade_keys_hex.dedup(); + + if !had_order_keys { + println!("📭 No trade keys found in orders — falling back to Mostro inbox only"); + }Based on learnings
Also applies to: 22-26
38-46: ListKind::DirectMessagesUser incorrectly queries GiftWrap instead of PrivateDirectMessageThe code review's request cannot be resolved as written because the current implementation contradicts it and prior design decisions. In
src/util/events.rslines 43–48,ListKind::DirectMessagesUsercreates a filter withKind::GiftWrap, notKind::PrivateDirectMessage.Per the previous decision (from get_dm.rs), non-admin flows should exclude GiftWrap messages. Correct this mapping: change
ListKind::DirectMessagesUserto filter onKind::PrivateDirectMessage(currently onlyPrivateDirectMessagesUserdoes this at lines 63–65).
🧹 Nitpick comments (18)
src/cli/rate_user.rs (1)
24-30: Nice beautification! Consider validation order for cleaner UX.The structured output with emojis aligns well with the PR's beautification goals. The formatting is clear and consistent.
Minor consideration: the output is displayed before rating validation (line 32). If a user passes an invalid rating (e.g., 6 or 0), they'll see this formatted output followed by an error message, which could be slightly confusing. Consider moving the validation check before the output for a cleaner failure path, though the current approach is acceptable.
src/cli/take_dispute.rs (5)
1-1: Preserve error context from serialization failuresmap_err(|_| …) hides the root cause; use anyhow::Context to keep it.
Apply:
-use anyhow::Result; +use anyhow::{Context, Result};And replace each map_err with context:
- .as_json() - .map_err(|_| anyhow::anyhow!("Failed to serialize message"))?; + .as_json() + .context("serializing AdminAddSolver DM payload")?; - .as_json() - .map_err(|_| anyhow::anyhow!("Failed to serialize message"))?; + .as_json() + .context("serializing AdminCancel DM payload")?; - .as_json() - .map_err(|_| anyhow::anyhow!("Failed to serialize message"))?; + .as_json() + .context("serializing AdminSettle DM payload")?; - .as_json() - .map_err(|_| anyhow::anyhow!("Failed to serialize message"))?; + .as_json() + .context("serializing AdminTakeDispute DM payload")?;Based on learnings.
Also applies to: 43-44, 82-83, 123-124, 167-168
2-3: Avoid glob import; make comfy_table imports explicitImproves readability and reduces namespace pollution.
Apply:
-use comfy_table::presets::UTF8_FULL; -use comfy_table::*; +use comfy_table::{ + presets::UTF8_FULL, Attribute, Cell, CellAlignment, ContentArrangement, Row, Table, +};
12-34: Deduplicate table rendering via a small helperFour near-identical blocks. Extract a helper to keep the file DRY and easier to evolve.
Example helper (add near top of file):
fn print_admin_header(title: &str, rows: &[(&str, String)]) { println!("{title}"); println!("═══════════════════════════════════════"); let mut table = Table::new(); table .load_preset(UTF8_FULL) .set_content_arrangement(ContentArrangement::Dynamic) .set_width(100) .set_header(vec![ Cell::new("Field").add_attribute(Attribute::Bold).set_alignment(CellAlignment::Center), Cell::new("Value").add_attribute(Attribute::Bold).set_alignment(CellAlignment::Center), ]); for (k, v) in rows { table.add_row(Row::from(vec![Cell::new(*k), Cell::new(v)])); } println!("{table}"); }Then call:
print_admin_header( "👑 Admin Cancel Dispute", &[("🆔 Dispute ID", dispute_id.to_string()), ("🎯 Mostro PubKey", ctx.mostro_pubkey.to_string())], ); println!("💡 Canceling dispute...\n");Also applies to: 56-78, 98-119, 136-158
16-16: Hard‑coded width may truncate/wrap on small terminalsEither omit set_width to let comfy_table adapt or detect terminal width dynamically.
Consider removing:
- .set_width(100)Or compute from terminal size (e.g., terminal_size crate) and pass that value.
Also applies to: 60-60, 101-101, 140-140
36-36: Rename take_dispute_message to msg for clarityVariable name doesn’t match context in 3 admin flows; keep it neutral.
Apply pattern:
- let take_dispute_message = /* ... */; + let msg = /* ... */; - admin_send_dm(ctx, take_dispute_message).await?; + admin_send_dm(ctx, msg).await?;Also applies to: 46-46, 80-80, 87-87, 121-121, 127-127, 160-160, 172-172
src/cli/list_disputes.rs (1)
8-12: LGTM: Consistent UI enhancement.The structured header output with emojis and separators significantly improves user experience. The pattern is consistent with the broader UI overhaul across the CLI.
Consider extracting the header formatting pattern into a reusable utility function to reduce duplication across CLI commands:
// In src/util/mod.rs or src/ui.rs pub fn print_command_header(title: &str, emoji: &str, details: Vec<(&str, String)>) { println!("{} {}", emoji, title); println!("═══════════════════════════════════════"); for (label, value) in details { println!("{} {}: {}", label, label.split_whitespace().last().unwrap_or(""), value); } println!(); }This would allow you to replace lines 8-12 with:
print_command_header( "List Disputes", "⚖️", vec![("🎯 Mostro PubKey", ctx.mostro_pubkey.to_string())], ); println!("💡 Fetching disputes from relays..."); println!();src/cli/add_invoice.rs (1)
26-51: Consider dynamic table width for better terminal compatibility.The hardcoded table width of 100 may cause display issues on narrower terminals or when users have smaller terminal windows.
Consider using terminal width detection:
+use std::io::{self, IsTerminal}; + +fn get_table_width() -> u16 { + if io::stdout().is_terminal() { + if let Some((w, _)) = term_size::dimensions() { + return w.min(120) as u16; + } + } + 100 +} let mut table = Table::new(); table .load_preset(UTF8_FULL) .set_content_arrangement(ContentArrangement::Dynamic) - .set_width(100) + .set_width(get_table_width())Alternatively, remove the
.set_width()call entirely and let comfy_table auto-detect terminal width, asContentArrangement::Dynamicshould handle this.src/cli/send_dm.rs (1)
14-20: Consider consistent formatting approach.The UI enhancements look good, but this file uses simple
println!statements while other files (likeadd_invoice.rs,take_order.rs,adm_send_dm.rs) use structured tables with comfy_table.For consistency across the CLI, consider using a table format similar to other commands:
let mut table = Table::new(); table .load_preset(UTF8_FULL) .set_content_arrangement(ContentArrangement::Dynamic) .set_header(vec![ Cell::new("Field").add_attribute(Attribute::Bold).set_alignment(CellAlignment::Center), Cell::new("Value").add_attribute(Attribute::Bold).set_alignment(CellAlignment::Center), ]); table.add_row(Row::from(vec![Cell::new("📋 Order ID"), Cell::new(order_id.to_string())])); table.add_row(Row::from(vec![Cell::new("🎯 Recipient"), Cell::new(receiver.to_string())])); table.add_row(Row::from(vec![Cell::new("💬 Message"), Cell::new(message)])); println!("{table}");Alternatively, if the simpler format is preferred for this command, that's also reasonable.
src/cli/take_order.rs (1)
69-103: Good conditional row handling; consider dynamic width.The conditional rendering of invoice and amount rows (lines 90-98) is well-implemented, showing only relevant information.
As with
add_invoice.rs, consider removing the hardcoded width of 100 or using dynamic terminal width detection. TheContentArrangement::Dynamicsetting should handle width automatically:table .load_preset(UTF8_FULL) .set_content_arrangement(ContentArrangement::Dynamic) - .set_width(100)This allows the table to adapt to different terminal sizes.
src/cli/adm_send_dm.rs (1)
9-34: Good admin DM visibility; consider dynamic width.The table clearly shows the admin context and message details before sending.
Same recommendation as
add_invoice.rsandtake_order.rs: consider removing the hardcoded width to allow terminal size adaptation:table .load_preset(UTF8_FULL) .set_content_arrangement(ContentArrangement::Dynamic) - .set_width(100)src/parser/orders.rs (1)
251-271: Duplicate status coloring logic should be extracted.This status coloring logic is nearly identical to the pattern in
src/parser/disputes.rs(lines 104-113). The only differences are the specific status values being checked. Consider extracting this into a shared helper function to reduce duplication and ensure consistency:Create a helper function in a common module:
// In src/util.rs or src/parser/common.rs pub fn apply_status_color(cell: Cell, status: &Status) -> Cell { match status { Status::Pending | Status::WaitingPayment | Status::WaitingBuyerInvoice => { cell.fg(Color::Yellow) } Status::Active | Status::Released | Status::Success => { cell.fg(Color::Green) } Status::FiatSent => { cell.fg(Color::Cyan) } Status::Dispute | Status::Cancel | Status::Canceled => { cell.fg(Color::Red) } _ => cell, } }Then use it in both files:
-{ - let status = single_order - .status - .unwrap_or(mostro_core::order::Status::Active) - .to_string(); - let s_lower = status.to_lowercase(); - let mut c = Cell::new(status).set_alignment(CellAlignment::Center); - if s_lower.contains("pending") || s_lower.contains("waiting") { - c = c.fg(Color::Yellow); - } else if s_lower.contains("active") - || s_lower.contains("released") - || s_lower.contains("settled") - { - c = c.fg(Color::Green); - } else if s_lower.contains("fiat") { - c = c.fg(Color::Cyan); - } else if s_lower.contains("dispute") || s_lower.contains("cancel") { - c = c.fg(Color::Red); - } - c -}, +{ + let status = single_order.status.unwrap_or(Status::Active); + let cell = Cell::new(status.to_string()).set_alignment(CellAlignment::Center); + apply_status_color(cell, &status) +},src/parser/dms.rs (1)
632-636: Consider making truncation length configurable.The 120-character truncation for details is hardcoded. While this works for the current table width of 160, consider making it configurable or calculating it dynamically based on table width for better maintainability.
+const MAX_DETAILS_LENGTH: usize = 120; + // Truncate long details for compact table row -let details = if details.len() > 120 { - format!("{}…", &details[..120]) +let details = if details.len() > MAX_DETAILS_LENGTH { + format!("{}…", &details[..MAX_DETAILS_LENGTH]) } else { details };src/cli/get_dm_user.rs (3)
28-36: Validate and clamp ‘since’ to avoid negative windowsA negative since will expand the window into the future. Clamp to >= 0 and show the effective value.
pub async fn execute_get_dm_user(since: &i64, ctx: &Context) -> Result<()> { + let since_clamped = if *since < 0 { 0 } else { *since }; @@ - println!("⏰ Since: {} minutes ago", since); + println!("⏰ Since: {} minutes ago", since_clamped); @@ - Some(since), + Some(&since_clamped),Also applies to: 34-36
48-60: Ensure deterministic ordering and handle duplicates from multiple relaysIf fetch_events_list doesn’t already sort/dedup, multiple relays can yield duplicates and non-deterministic order. Sort by created_at and uniq by (sender, created_at, action) as a light heuristic.
let mut dm_events: Vec<(Message, u64, PublicKey)> = Vec::new(); @@ for event in direct_messages { if let Event::MessageTuple(tuple) = event { dm_events.push(*tuple); } } + + // Stable chronological order + dm_events.sort_by(|a, b| a.1.cmp(&b.1)); + dm_events.dedup_by(|a, b| a.1 == b.1 && a.2 == b.2 && a.0.get_inner_message_kind().action == b.0.get_inner_message_kind().action);
51-54: Defer “no messages” messaging to print_direct_messages for consistencyYou already have centralized empty-state rendering inside print_direct_messages. Delegate to it and drop the local early-return to keep UX consistent.
- // Check if the direct messages are empty - if direct_messages.is_empty() { - println!("📭 You don't have any direct messages in your trade keys"); - return Ok(()); - }Also applies to: 62-63
src/cli/restore.rs (2)
65-76: Select the latest RestoreSession message, not just the firstIf multiple events arrive (e.g., duplicated via relays), prefer the newest RestoreSession.
- if let Some((message, _, _)) = messages.first() { - let message = message.get_inner_message_kind(); - if message.action == Action::RestoreSession { + if let Some((message, _, _)) = messages + .iter() + .rev() + .find(|(m, _, _)| m.get_inner_message_kind().action == Action::RestoreSession) + { + let message = message.get_inner_message_kind(); + if message.action == Action::RestoreSession { print_commands_results(message, ctx).await?; Ok(()) } else {
19-21: Preserve original error context when serializingInclude the underlying error for quicker troubleshooting.
- let message_json = restore_message - .as_json() - .map_err(|_| anyhow::anyhow!("Failed to serialize message"))?; + let message_json = restore_message + .as_json() + .map_err(|e| anyhow::anyhow!("Failed to serialize message: {}", e))?;
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (23)
Cargo.toml(1 hunks)src/cli.rs(4 hunks)src/cli/add_invoice.rs(2 hunks)src/cli/adm_send_dm.rs(1 hunks)src/cli/conversation_key.rs(2 hunks)src/cli/dm_to_user.rs(1 hunks)src/cli/get_dm.rs(2 hunks)src/cli/get_dm_user.rs(4 hunks)src/cli/last_trade_index.rs(2 hunks)src/cli/list_disputes.rs(1 hunks)src/cli/list_orders.rs(2 hunks)src/cli/new_order.rs(2 hunks)src/cli/orders_info.rs(1 hunks)src/cli/rate_user.rs(1 hunks)src/cli/restore.rs(1 hunks)src/cli/send_dm.rs(2 hunks)src/cli/send_msg.rs(2 hunks)src/cli/take_dispute.rs(3 hunks)src/cli/take_order.rs(3 hunks)src/parser/disputes.rs(3 hunks)src/parser/dms.rs(10 hunks)src/parser/orders.rs(6 hunks)src/util/misc.rs(1 hunks)
🧰 Additional context used
🧠 Learnings (3)
📚 Learning: 2025-09-13T10:31:42.313Z
Learnt from: arkanoider
PR: MostroP2P/mostro-cli#135
File: src/cli/get_dm.rs:24-29
Timestamp: 2025-09-13T10:31:42.313Z
Learning: In mostro-cli's get_dm.rs, the message filtering is intentional: non-admin users should only receive DirectMessage from users, while messages from Mostro are only fetched when the admin flag is true. Don't include GiftWrap messages for non-admin users.
Applied to files:
src/cli/get_dm.rssrc/cli/get_dm_user.rssrc/cli/adm_send_dm.rs
📚 Learning: 2025-09-13T20:48:31.581Z
Learnt from: arkanoider
PR: MostroP2P/mostro-cli#135
File: src/cli/send_msg.rs:89-110
Timestamp: 2025-09-13T20:48:31.581Z
Learning: In mostro-cli's send_msg.rs, tokio::spawn with async move is valid for sending DMs when PublicKey implements Copy, and arkanoider confirmed this approach works without memory issues or compilation errors.
Applied to files:
src/cli/take_order.rs
📚 Learning: 2025-09-13T20:40:51.992Z
Learnt from: arkanoider
PR: MostroP2P/mostro-cli#135
File: src/cli/get_dm_user.rs:38-40
Timestamp: 2025-09-13T20:40:51.992Z
Learning: The get_direct_messages_from_trade_keys function in src/util.rs already handles the `since` parameter through manual filtering after fetching events: it uses a hardcoded 2880-minute window in the Nostr filter, then manually filters results with `if event.created_at.as_u64() < since as u64 { continue; }`.
Applied to files:
src/cli/get_dm_user.rs
🧬 Code graph analysis (10)
src/cli/get_dm.rs (1)
src/parser/dms.rs (1)
print_direct_messages(541-662)
src/cli/orders_info.rs (2)
src/parser/dms.rs (2)
print_commands_results(21-447)parse_dm_events(449-539)src/util/messaging.rs (2)
send_dm(184-236)wait_for_dm(69-110)
src/cli/dm_to_user.rs (1)
src/util/messaging.rs (1)
send_gift_wrap_dm(23-30)
src/cli/restore.rs (2)
src/parser/dms.rs (2)
print_commands_results(21-447)parse_dm_events(449-539)src/util/messaging.rs (2)
send_dm(184-236)wait_for_dm(69-110)
src/cli.rs (2)
src/cli/orders_info.rs (1)
execute_orders_info(8-77)src/cli/restore.rs (1)
execute_restore(13-81)
src/cli/take_dispute.rs (1)
src/util/storage.rs (1)
admin_send_dm(46-58)
src/cli/get_dm_user.rs (1)
src/parser/dms.rs (1)
print_direct_messages(541-662)
src/parser/dms.rs (1)
src/db.rs (5)
get_by_id(519-535)delete_by_id(550-563)get(195-207)new(139-160)new(278-331)
src/cli/adm_send_dm.rs (1)
src/util/messaging.rs (1)
send_admin_gift_wrap_dm(14-21)
src/cli/last_trade_index.rs (1)
src/parser/dms.rs (1)
print_commands_results(21-447)
🔇 Additional comments (21)
src/cli/dm_to_user.rs (1)
36-37: LGTM! Clear success feedback.The success message provides clear confirmation to the user that the DM was sent. The formatting is consistent with the enhanced output style introduced earlier in the function.
</review_comment_end>
src/cli/send_msg.rs (1)
6-7: LGTM! Clean addition of table formatting dependencies.The comfy_table imports are appropriate for the enhanced CLI output functionality.
src/cli/take_dispute.rs (2)
85-85: Gate “Admin Keys” printing behind debug/verbose (and make consistent)Currently printed in 3 flows but not in add-solver. For consistency and cleaner UX, prefer logging (debug) or a --verbose flag instead of unconditional stdout.
Do you already have tracing/logging set up? If yes, switch to debug-level logging here. If not, I can wire a simple verbosity toggle.
Also applies to: 126-126, 170-170
36-44: Review comment is based on faulty assumption about operation semanticsAdminAddSolver is a system-level admin operation to add a new solver, not a dispute-specific action like AdminCancel/AdminSettle. The code correctly uses a random UUID because this operation doesn't reference an existing dispute—the payload is just the solver's npubkey. AdminCancel and AdminSettle use actual dispute_id because they operate on existing disputes. The different ID handling reflects the different operation types, not a bug.
Likely an incorrect or invalid review comment.
src/cli/add_invoice.rs (1)
4-5: LGTM: Appropriate dependency usage.The comfy_table imports are appropriate for the enhanced table-based output.
src/cli/last_trade_index.rs (1)
34-39: LGTM: Good placement of user-facing output.The header information is appropriately placed after the DM is sent but before waiting for a response, providing clear feedback to the user about what's happening.
src/cli/conversation_key.rs (1)
6-26: LGTM: Enhanced output for key generation.The structured output with headers and separators makes it clear when the key generation starts and successfully completes. The visual separation of the actual key with horizontal lines improves readability.
src/cli/take_order.rs (2)
2-3: LGTM: Appropriate imports for enhanced UI.
122-127: LGTM: Clear messaging flow indicators.The sending message section provides good visibility into the DM transmission process, showing the trade index and keys being used.
src/cli/adm_send_dm.rs (1)
38-41: LGTM: Clear success confirmation with recipient.Including the receiver in the success message provides good confirmation of the action taken.
src/cli/new_order.rs (1)
121-182: Excellent structured output for order creation.The new table-based presentation with emoji-prefixed labels significantly improves the user experience by clearly displaying order details before submission. The conditional rendering for fiat range vs. amount (lines 148-158) is well-implemented.
src/parser/orders.rs (1)
147-154: Nice UX enhancement with the order preview banner.The post-table informational banner provides clear guidance to users about what they're reviewing and what action is expected. This significantly improves the user confirmation flow.
src/parser/dms.rs (3)
174-216: Excellent structured error handling for CantDo action.The refactored error handling provides specific, user-friendly messages for each
CantDoReasonvariant. The emoji-prefixed headers and actionable guidance significantly improve the error reporting experience.
326-395: Well-implemented Orders action handler.The new handler properly displays multiple orders with rich details and handles the empty case gracefully. The structured output with emoji-prefixed labels makes it easy to scan order information.
541-662: Significant UX improvement with table-based DM rendering.The rewritten
print_direct_messagesfunction provides a much cleaner, more scannable view of direct messages. The table format with time, sender, action, and details columns is well-organized. The special "🧌 Mostro" marker for messages from the Mostro pubkey is a nice touch.A few observations:
- Details truncation at 120 characters (lines 632-636) is reasonable for table fit
- Action coloring based on keywords (lines 601-609) enhances visual scanning
- The signature change (adding
mostro_pubkey: Option<PublicKey>) is a breaking change but properly handled across all call sitessrc/cli/get_dm.rs (1)
17-24: Consistent UI pattern with informative header.The new header follows the same emoji-prefixed, structured format as other CLI commands in this PR, providing clear context about the operation being performed.
src/cli.rs (1)
11-11: Clean integration of new orders_info command.The new
OrdersInfocommand is well-integrated following the established patterns in the codebase. The command definition, import, and routing are all properly structured.Also applies to: 29-29, 300-305, 525-525
src/cli/orders_info.rs (1)
8-76: Well-implemented orders_info command handler.The function follows the established pattern for DM-based request/response flows and includes appropriate error handling. The structured output with emoji-prefixed labels is consistent with the broader PR theme.
One minor concern: The error message at lines 67-70 says "Expected: Orders" but doesn't validate that
message_kind.actionis actuallyAction::Orders. It only checksrequest_idmatching. Consider verifying the action type as well:if message_kind.request_id == Some(request_id) && message_kind.action == Action::Orders { print_commands_results(message_kind, ctx).await?; } else { return Err(anyhow::anyhow!( "Received response with unexpected message. Expected: Orders (request_id: {}), Got: {:?} (request_id: {:?})", request_id, message_kind.action, message_kind.request_id )); }src/cli/list_orders.rs (1)
30-58: Consistent UI improvements for list_orders command.The emoji-prefixed headers and structured filter messages align perfectly with the broader PR theme, improving readability and user experience.
src/cli/restore.rs (2)
24-33: Verify the key pairing used for send/waitYou pass identity_keys as both the sender (trade_keys param) and the subscription key. If the protocol expects using ctx.trade_keys for transport and identity_keys only for signature (Signed Gift Wrap), this might break round‑trip.
Action:
- Confirm Mostro restore expects messages sent from identity or trade keys.
- Align send_dm(trade_keys) and wait_for_dm(subscription key) to the same expected key.
Potential adjustment:
- let sent_message = send_dm(&ctx.client, Some(identity_keys), identity_keys, &mostro_key, message_json, None, false); + let sent_message = send_dm(&ctx.client, Some(identity_keys), &ctx.trade_keys, &mostro_key, message_json, None, false); @@ - let recv_event = wait_for_dm(ctx, Some(identity_keys), sent_message).await?; + let recv_event = wait_for_dm(ctx, Some(&ctx.trade_keys), sent_message).await?;Also applies to: 61-66
34-60: UI looks good and consistent with table patterns elsewhereHeader, fields, and progress logs align with recent CLI beautification. No issues.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
src/parser/dms.rs (1)
540-660: Excellent table-based rendering with one consideration.The new implementation transforms direct message display into a well-structured, emoji-enhanced table with intelligent color coding and sender identification. The logic is sound and the UX is significantly improved.
Consider making table width configurable:
The hardcoded width of 160 (line 555) may cause issues on smaller terminals. Consider detecting terminal width or making it configurable:
let term_width = terminal_size::terminal_size() .map(|(w, _)| w.0 as usize) .unwrap_or(160) .min(160); table.set_width(term_width)This would require adding
terminal_sizecrate, but improves adaptability.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
src/cli/get_dm.rs(2 hunks)src/cli/get_dm_user.rs(4 hunks)src/parser/dms.rs(10 hunks)tests/parser_dms.rs(1 hunks)
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2025-09-13T20:40:51.992Z
Learnt from: arkanoider
PR: MostroP2P/mostro-cli#135
File: src/cli/get_dm_user.rs:38-40
Timestamp: 2025-09-13T20:40:51.992Z
Learning: The get_direct_messages_from_trade_keys function in src/util.rs already handles the `since` parameter through manual filtering after fetching events: it uses a hardcoded 2880-minute window in the Nostr filter, then manually filters results with `if event.created_at.as_u64() < since as u64 { continue; }`.
Applied to files:
src/cli/get_dm_user.rs
📚 Learning: 2025-09-13T10:31:42.313Z
Learnt from: arkanoider
PR: MostroP2P/mostro-cli#135
File: src/cli/get_dm.rs:24-29
Timestamp: 2025-09-13T10:31:42.313Z
Learning: In mostro-cli's get_dm.rs, the message filtering is intentional: non-admin users should only receive DirectMessage from users, while messages from Mostro are only fetched when the admin flag is true. Don't include GiftWrap messages for non-admin users.
Applied to files:
src/cli/get_dm.rs
🧬 Code graph analysis (4)
tests/parser_dms.rs (1)
src/parser/dms.rs (1)
print_direct_messages(540-660)
src/cli/get_dm_user.rs (1)
src/parser/dms.rs (1)
print_direct_messages(540-660)
src/parser/dms.rs (1)
src/db.rs (5)
get_by_id(519-535)delete_by_id(550-563)get(195-207)new(139-160)new(278-331)
src/cli/get_dm.rs (1)
src/parser/dms.rs (1)
print_direct_messages(540-660)
🔇 Additional comments (9)
src/cli/get_dm_user.rs (2)
3-3: LGTM! Enhanced user-facing output.The import and emoji-enhanced console output improve the user experience with clear visual hierarchy and informative messages.
Also applies to: 24-36
62-62: LGTM! Correctly updated to new signature.The call properly passes
Some(ctx.mostro_pubkey)to enable Mostro-sender highlighting in the output table.src/parser/dms.rs (4)
7-8: LGTM! Required imports for table rendering.The comfy_table imports support the new table-based direct message display.
23-172: LGTM! Consistent UI enhancements across action handlers.The emoji-enhanced output with clear separators improves readability while preserving all existing logic for NewOrder, WaitingSellerToPay, AddInvoice, and PayInvoice actions.
173-215: LGTM! Excellent error handling improvements.The CantDo handler now provides specific, actionable error messages for each failure reason, significantly improving the user experience when operations cannot be completed.
325-443: LGTM! Well-structured new action handlers.The Orders and RestoreSession handlers provide comprehensive, user-friendly output for listing orders and restoring session state. The structured format with clear separators makes the information easy to scan.
tests/parser_dms.rs (1)
16-16: LGTM! Test correctly updated to new signature.The test properly reflects the signature change by passing
Nonefor the optionalmostro_pubkeyparameter instead of constructing an unused pool.src/cli/get_dm.rs (2)
17-24: LGTM! Consistent UI enhancements.The emoji-enhanced console output follows the same formatting pattern used in other DM commands, providing users with clear context about the fetch operation.
45-45: LGTM! Correctly updated to new signature.The call properly passes
Some(ctx.mostro_pubkey)to enable Mostro-sender identification in the message table.
|
Note Unit test generation is an Early Access feature. Expect some limitations and changes as we gather feedback and continue to improve it. Generating unit tests... This may take up to 20 minutes. |
…test documentation
|
✅ UTG Post-Process Complete No new issues were detected in the generated code and all check runs have completed. The unit test generation process has completed successfully. |
|
Creating a PR to put the unit tests in... The changes have been created in this pull request: View PR |
CodeRabbit Generated Unit Tests: Add 78 unit tests and comprehensive test documentation
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
tests/util_misc.rs (1)
57-88: Consider test isolation for filesystem side effects.The
get_mcli_pathfunction creates a directory in the user's home directory as a side effect. These tests don't clean up after themselves, which could lead to test pollution and issues in CI/CD environments. Consider using temporary directories or mocking the filesystem for better test isolation.Here's an approach using a temporary directory:
+use std::env; +use tempfile::TempDir; + #[test] fn test_get_mcli_path_returns_valid_path() { + let temp_dir = TempDir::new().unwrap(); + env::set_var("HOME", temp_dir.path()); + let path = get_mcli_path(); - - // Should return a non-empty string assert!(!path.is_empty()); - - // Should contain the mcli directory name assert!(path.contains(".mcli")); + + // Cleanup is automatic when temp_dir goes out of scope }Note: This would require adding
tempfileas a dev dependency.tests/cli_functions.rs (1)
5-19: Consider more explicit validation for rating bounds.The current tests check if ratings are in range but don't actually exercise the rating user functionality. Consider adding tests that attempt to create
RatingUserpayloads with invalid ratings and verify proper error handling or validation.Example:
#[test] fn test_rating_user_rejects_invalid_ratings() { for invalid_rating in [0u8, 6u8, 10u8, 255u8] { // If there's validation logic, test it here // For example: assert!(RatingUser::new(invalid_rating).is_err()); } }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
tests/README.md(1 hunks)tests/TESTS_COMPLETED.md(1 hunks)tests/TEST_SUMMARY.md(1 hunks)tests/cli_functions.rs(1 hunks)tests/parser_disputes.rs(1 hunks)tests/parser_dms.rs(1 hunks)tests/parser_orders.rs(1 hunks)tests/util_misc.rs(1 hunks)
✅ Files skipped from review due to trivial changes (2)
- tests/README.md
- tests/TEST_SUMMARY.md
🧰 Additional context used
🧬 Code graph analysis (4)
tests/util_misc.rs (1)
src/util/misc.rs (2)
get_mcli_path(11-19)uppercase_first(3-9)
tests/parser_orders.rs (1)
src/parser/orders.rs (2)
parse_orders_events(15-74)print_orders_table(158-306)
tests/parser_disputes.rs (1)
src/parser/disputes.rs (2)
parse_dispute_events(13-43)print_disputes_table(45-131)
tests/parser_dms.rs (1)
src/parser/dms.rs (2)
print_direct_messages(540-660)parse_dm_events(448-538)
⏰ 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 (5)
tests/parser_orders.rs (1)
83-302: Excellent test coverage for order parsing and filtering.The new tests comprehensively cover:
- Kind, status, and currency filtering
- Edge cases (no filters, empty results, market price)
- Multiple orders with various amounts and currencies
- Table rendering with different data sets
The tests are well-structured, use consistent helper functions, and are properly isolated.
tests/cli_functions.rs (1)
22-304: Comprehensive message construction test coverage.The test suite thoroughly validates:
- Message creation for all major action types
- Payload construction and extraction
- Serialization to JSON
- Message structure (action, id, request_id, trade_index, payload)
Well-organized and covers the critical paths for CLI functionality.
tests/parser_disputes.rs (1)
52-166: Well-structured dispute parsing tests.The new tests provide solid coverage:
- Multiple dispute statuses (Initiated, InProgress, Settled, SellerRefunded)
- Empty list handling
- Table rendering with multiple disputes
- Unique ID validation
- Status-specific parsing
Tests follow consistent patterns and are properly isolated.
tests/parser_dms.rs (2)
14-18: Correctly updated for new signature.The test now properly passes
Nonefor the optionalmostro_pubkeyparameter, aligning with the updatedprint_direct_messagessignature.
20-310: Excellent comprehensive DM testing.The test suite thoroughly covers:
- Empty messages and with/without mostro_pubkey
- Various payload types (Text, PaymentRequest, Dispute, Orders, RestoreSession, Rating)
- Multiple messages with different actions
- Edge cases (invalid timestamp, long details truncation)
- Time-based filtering
- Mostro identification logic
All tests are async-aware and properly structured.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
src/parser/orders.rs (1)
147-155: Good UX improvement with the banner block.The bordered informational block after the table provides clear context and prompts for the user.
Consider extracting the banner text as a constant at the module level for easier maintenance:
+const ORDER_PREVIEW_BANNER: &str = "═══════════════════════════════════════ +📋 Order Preview - Please review carefully +💡 This order will be submitted to Mostro +✅ All details look correct? (Y/n) +═══════════════════════════════════════ +"; + pub fn print_order_preview(ord: Payload) -> Result<String, String> { // ... existing code ... let mut result = table.to_string(); result.push('\n'); - result.push_str("═══════════════════════════════════════\n"); - result.push_str("📋 Order Preview - Please review carefully\n"); - result.push_str("💡 This order will be submitted to Mostro\n"); - result.push_str("✅ All details look correct? (Y/n)\n"); - result.push_str("═══════════════════════════════════════\n"); + result.push_str(ORDER_PREVIEW_BANNER); Ok(result) }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/parser/orders.rs(7 hunks)
🔇 Additional comments (4)
src/parser/orders.rs (4)
89-106: Nice visual enhancement with emoji headers.The emoji-prefixed headers improve the visual appeal of the table. Just be aware that some older terminals or CI environments may not render emojis correctly, potentially causing alignment issues.
Consider testing in various terminal environments (e.g., Windows CMD, older Linux terminals) to verify emoji rendering works as expected.
188-188: Good typographic improvement.Using the typographic ellipsis character "…" instead of three dots is a nice polish detail.
201-224: Consistent emoji headers across functions.The emoji-prefixed headers match the style in
print_order_preview, providing a consistent visual experience.Same caveat as the preview function: verify emoji rendering in various terminal environments.
124-124: Consistent lowercase "market" text.The change to lowercase "market" provides visual consistency across the table output.
Also applies to: 273-273
Fix race condition in get_mcli_path() at src/util/misc.rs:15. When tests ran in parallel, multiple threads could attempt to create the .mcli directory simultaneously, causing one to fail with "File exists". The fix: - Replaced expect() with proper error handling using pattern matching - Gracefully ignores ErrorKind::AlreadyExists (safe when another thread created it) - Preserves panic behavior for genuine errors (permissions, etc.) - Only prints "Directory created" when actually successful This makes the function thread-safe and eliminates the flaky test failure in GitHub Actions.
There was a problem hiding this comment.
Actionable comments posted: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
src/cli/send_dm.rs (1)
56-64: Bug: sending DM with to_user=false uses SignedGiftWrap instead of standard PrivateDirectMessage.The
determine_message_type()function shows thatto_user=falseproducesMessageType::SignedGiftWrap(kind 13), requiring NIP-59 rumor extraction. For user DMs,to_user=trueproducesMessageType::PrivateDirectMessage(kind 4), which is the standard pattern. Setto_user=true:send_dm( &ctx.client, None, &trade_keys, &receiver, message, None, - false, + true, )src/cli/get_dm_user.rs (1)
16-20: Remove unconditional Mostro pubkey addition from user DM fetch.Adding
ctx.mostro_pubkeytotrade_keys_hexinget_dm_user.rs(lines 16-20) violates the filtering principle for non-admin users. Per learnings, non-admin users should only receive DirectMessages from other users, not Mostro traffic. This addition causesDirectMessagesUserto fetch GiftWrap messages TO Mostro, mixing admin-related traffic into the user DM results.Remove lines 16-20, or make this addition conditional on an admin flag if the intent is to support that use case through a different flow.
src/parser/dms.rs (1)
801-805: Avoid potential panic onchecked_sub_signed(...).unwrap().Large
sincecan underflow; handleNonesafely.- let since_time = chrono::Utc::now() - .checked_sub_signed(chrono::Duration::minutes(*since_time)) - .unwrap() - .timestamp() as u64; + let since_time = chrono::Utc::now() + .checked_sub_signed(chrono::Duration::minutes(*since_time)) + .map(|t| t.timestamp() as u64) + .unwrap_or(0);
♻️ Duplicate comments (6)
src/util/misc.rs (1)
13-13: Critical: Inconsistency detected - previously resolved issue still present.Past review comments indicate the
.mcliUserBpath change was identified as a critical breaking change that would cause data loss, and you confirmed it was fixed in commits ede4da5 to 77641c9. However, the code here still shows.mcliUserBon line 13.Please verify:
- Is this code outdated (showing pre-fix state)?
- Or was the revert incomplete?
If the path is still
.mcliUserBin the current branch, it needs to be reverted to.mclito prevent user data loss.Apply this diff if the issue persists:
- let mcli_path = format!("{}/.mcliUserB", home_dir.display()); + let mcli_path = format!("{}/.mcli", home_dir.display());src/parser/disputes.rs (1)
103-107: Make status coloring type-safe (avoid substring false-positives).You switched to apply_status_color, but it still does substring checks on a lowercased string, which can miscolor unexpected statuses. Prefer matching on the Status enum directly and avoid allocating status_str.
Apply at call site:
- let status_str = single_dispute.status.to_string(); - let status_cell = apply_status_color( - Cell::new(&status_str).set_alignment(CellAlignment::Center), - &status_str, - ); + let status_cell = apply_status_color_status( + Cell::new(single_dispute.status.to_string()).set_alignment(CellAlignment::Center), + &single_dispute.status, + );And add this helper in src/parser/common.rs:
+pub fn apply_status_color_status(mut cell: Cell, status: &mostro_core::prelude::Status) -> Cell { + use mostro_core::prelude::Status::*; + cell = match status { + Init | Pending | WaitingPayment | WaitingBuyerInvoice | WaitingSellerToPay => cell.fg(Color::Yellow), + Taken | Active | Released | Settled | Success => cell.fg(Color::Green), + Dispute | Cancel | Canceled => cell.fg(Color::Red), + _ => cell, + }; + cell +}Optionally re-export the new helper from parser/mod.rs for consistency.
Also applies to: 111-111
src/cli/take_dispute.rs (3)
38-38: Don’t declare success before server ack (duplicate of earlier review).Print a neutral “sent” message instead; await Mostro confirmation.
- println!("✅ Solver added successfully!"); + println!("📤 Command sent. Await Mostro confirmation.");
70-70: Don’t declare success before server ack (duplicate of earlier review).Change to neutral “sent” message.
- println!("✅ Dispute canceled successfully!"); + println!("📤 Command sent. Await Mostro confirmation.");
101-101: Don’t declare success before server ack (duplicate of earlier review).Change to neutral “sent” message.
- println!("✅ Dispute settled successfully!"); + println!("📤 Command sent. Await Mostro confirmation.");src/parser/orders.rs (1)
242-251: Status coloring still risk-prone (substring matching).You now call
apply_status_color, but it still uses.contains()which can miscolor composite/unknown statuses. Prefer enum-based mapping.Example change (once you have
Statusin scope):- let status = single_order.status.unwrap_or(Status::Active).to_string(); - apply_status_color(Cell::new(&status).set_alignment(CellAlignment::Center), &status) + let status = single_order.status.unwrap_or(Status::Active); + let cell = Cell::new(status.to_string()).set_alignment(CellAlignment::Center); + apply_status_color(cell, &status.to_string()) // or refactor helper to accept &StatusConsider refactoring helper to accept
&Statusdirectly to eliminate substring logic.
🧹 Nitpick comments (9)
src/cli/rate_user.rs (1)
25-40: Suggest removing redundant validation.Lines 26-32 validate the rating and return early if invalid, which is good fail-fast design. However, line 40 calls
get_user_rate(rating)which performs the sameRATING_BOUNDARIESvalidation again. Since the early validation guarantees the rating is valid at this point, the error branch inget_user_rateis unreachable.Consider simplifying by either:
Option 1: Skip validation in get_user_rate (preferred if it's only called here):
fn get_user_rate(rating: &u8) -> Result<Payload> { - if let Some(rating) = RATING_BOUNDARIES.iter().find(|r| r == &rating) { - Ok(Payload::RatingUser(*rating)) - } else { - Err(anyhow::anyhow!("Rating must be in the range 1 - 5")) - } + Ok(Payload::RatingUser(*rating)) }Option 2: Remove the upfront check and rely solely on get_user_rate:
- // Validate rating before proceeding - if !RATING_BOUNDARIES.contains(rating) { - print_section_header("❌ Invalid Rating"); - print_key_value("⭐", "Rating", &rating.to_string()); - print_info_line("💡", "Rating must be between 1 and 5"); - print_info_line("📊", "Valid ratings: 1, 2, 3, 4, 5"); - return Err(anyhow::anyhow!("Rating must be in the range 1 - 5")); - } print_section_header("⭐ Rate User"); print_key_value("📋", "Order ID", &order_id.to_string()); print_key_value("⭐", "Rating", &format!("{}/5", rating)); print_info_line("💡", "Sending user rating..."); println!(); // Check boundaries - let rating_content = get_user_rate(rating)?; + let rating_content = get_user_rate(rating).map_err(|_| { + print_section_header("❌ Invalid Rating"); + // ... enhanced error output + anyhow::anyhow!("Rating must be in the range 1 - 5") + })?;Option 1 is cleaner if
get_user_rateis only called from this validated context.src/parser/disputes.rs (2)
112-116: Use format_timestamp helper for consistency and clearer UTC output.Unify date formatting with the shared helper and avoid inline unwrap/else.
- Cell::new( - date.map(|d| d.to_string()) - .unwrap_or_else(|| "Invalid date".to_string()), - ) - .set_alignment(CellAlignment::Center), + Cell::new(crate::parser::common::format_timestamp(single_dispute.created_at)) + .set_alignment(CellAlignment::Center),
88-97: Minor: reduce header boilerplate with create_centered_cell.Keeps headers uniform and concise.
- .set_header(vec![ - Cell::new("🆔 Dispute Id") - .add_attribute(Attribute::Bold) - .set_alignment(CellAlignment::Center), - Cell::new("📊 Status") - .add_attribute(Attribute::Bold) - .set_alignment(CellAlignment::Center), - Cell::new("📅 Created") - .add_attribute(Attribute::Bold) - .set_alignment(CellAlignment::Center), - ]); + .set_header(vec![ + crate::parser::common::create_centered_cell("🆔 Dispute Id", true), + crate::parser::common::create_centered_cell("📊 Status", true), + crate::parser::common::create_centered_cell("📅 Created", true), + ]);src/cli/orders_info.rs (1)
25-25: Minor: make request_id derivation explicit (avoid implicit truncation).Current cast from u128 to u64 truncates silently. Use explicit byte conversion for clarity.
- let request_id = Uuid::new_v4().as_u128() as u64; + let uuid = Uuid::new_v4(); + let mut bytes8 = [0u8; 8]; + bytes8.copy_from_slice(&uuid.as_bytes()[..8]); + let request_id = u64::from_le_bytes(bytes8);src/cli/send_dm.rs (1)
17-34: Optional: use shared printing helpers for consistency.Swap the manual header/lines for print_section_header and keep success via print_success_message.
- println!("💬 Send Direct Message"); - println!("═══════════════════════════════════════"); + crate::parser::common::print_section_header("💬 Send Direct Message"); ... - println!("💡 Sending direct message...\n"); + crate::parser::common::print_info_line("💡", "Sending direct message..."); + println!(); ... - println!("✅ Direct message sent successfully!"); + crate::parser::common::print_success_message("Direct message sent successfully!");src/parser/orders.rs (1)
274-277: Use shared timestamp formatter for consistency.Reuse
parser::common::format_timestampinstead of ad-hoc formatting.- Cell::new( - date.map(|d| d.format("%Y-%m-%d %H:%M").to_string()) - .unwrap_or_else(|| "Invalid date".to_string()), - ) + Cell::new( + date.map(|d| d.timestamp()) + .map(|ts| format_timestamp(ts)) + .unwrap_or_else(|| "Invalid date".to_string()), + )src/parser/dms.rs (2)
831-835: Reuse common timestamp formatter.Use
format_timestampfor consistent output.- let date = match DateTime::from_timestamp(*created_at as i64, 0) { - Some(dt) => dt.format("%Y-%m-%d %H:%M:%S").to_string(), - None => "Invalid timestamp".to_string(), - }; + let date = format_timestamp(*created_at as i64);
85-88: Optional: gate full invoice printing behind verbosity.Invoices are generally safe, but consider a
MOSTRO_VERBOSE=1check to avoid noisy logs by default.- format!("⚡ Lightning Invoice:\n{}", inv) + if std::env::var("MOSTRO_VERBOSE").ok().as_deref() == Some("1") { + format!("⚡ Lightning Invoice:\n{}", inv) + } else { + "⚡ Lightning Invoice (hidden; set MOSTRO_VERBOSE=1 to show)".to_string() + }Based on learnings.
src/parser/common.rs (1)
5-31: Make status-coloring deterministic (avoid substring heuristics).
.contains()can misclassify composite/unknown statuses. Add an enum-based variant and migrate callers gradually.pub fn apply_status_color(cell: Cell, status: &str) -> Cell { /* keep for compat */ } +pub fn apply_status_color_status(cell: Cell, status: &mostro_core::order::Status) -> Cell { + use mostro_core::order::Status::*; + let color = match status { + Pending | WaitingBuyerInvoice | WaitingPayment => Color::Yellow, + Active | Success | SettledByAdmin | CompletedByAdmin | Released => Color::Green, + FiatSent => Color::Cyan, + Dispute | Canceled | CanceledByAdmin | CooperativelyCanceled | Expired => Color::Red, + InProgress => Color::Yellow, + }; + cell.fg(color) +}Then update call sites to pass
&Statuswhere available.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (24)
src/cli.rs(4 hunks)src/cli/add_invoice.rs(2 hunks)src/cli/adm_send_dm.rs(1 hunks)src/cli/conversation_key.rs(2 hunks)src/cli/dm_to_user.rs(2 hunks)src/cli/get_dm.rs(3 hunks)src/cli/get_dm_user.rs(4 hunks)src/cli/last_trade_index.rs(3 hunks)src/cli/list_disputes.rs(1 hunks)src/cli/list_orders.rs(3 hunks)src/cli/new_order.rs(2 hunks)src/cli/orders_info.rs(1 hunks)src/cli/rate_user.rs(2 hunks)src/cli/restore.rs(1 hunks)src/cli/send_dm.rs(3 hunks)src/cli/send_msg.rs(2 hunks)src/cli/take_dispute.rs(3 hunks)src/cli/take_order.rs(3 hunks)src/parser/common.rs(1 hunks)src/parser/disputes.rs(4 hunks)src/parser/dms.rs(10 hunks)src/parser/mod.rs(1 hunks)src/parser/orders.rs(8 hunks)src/util/misc.rs(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (7)
- src/cli/list_disputes.rs
- src/cli/conversation_key.rs
- src/cli/restore.rs
- src/cli/list_orders.rs
- src/cli/adm_send_dm.rs
- src/cli.rs
- src/cli/last_trade_index.rs
🧰 Additional context used
🧠 Learnings (4)
📚 Learning: 2025-10-18T11:48:11.935Z
Learnt from: arkanoider
PR: MostroP2P/mostro-cli#146
File: src/cli/dm_to_user.rs:25-32
Timestamp: 2025-10-18T11:48:11.935Z
Learning: In mostro-cli's DM functionality (dm_to_user.rs and similar), message content consists of ephemeral public keys that change every time and contains no sensitive personal data. Logging message content to stdout is acceptable and not a privacy concern.
Applied to files:
src/cli/get_dm_user.rssrc/cli/dm_to_user.rssrc/parser/dms.rs
📚 Learning: 2025-09-13T10:31:42.313Z
Learnt from: arkanoider
PR: MostroP2P/mostro-cli#135
File: src/cli/get_dm.rs:24-29
Timestamp: 2025-09-13T10:31:42.313Z
Learning: In mostro-cli's get_dm.rs, the message filtering is intentional: non-admin users should only receive DirectMessage from users, while messages from Mostro are only fetched when the admin flag is true. Don't include GiftWrap messages for non-admin users.
Applied to files:
src/cli/get_dm_user.rssrc/cli/get_dm.rssrc/cli/dm_to_user.rs
📚 Learning: 2025-09-13T20:48:31.581Z
Learnt from: arkanoider
PR: MostroP2P/mostro-cli#135
File: src/cli/send_msg.rs:89-110
Timestamp: 2025-09-13T20:48:31.581Z
Learning: In mostro-cli's send_msg.rs, tokio::spawn with async move is valid for sending DMs when PublicKey implements Copy, and arkanoider confirmed this approach works without memory issues or compilation errors.
Applied to files:
src/cli/take_order.rssrc/cli/dm_to_user.rssrc/cli/send_dm.rs
📚 Learning: 2025-09-13T20:48:31.581Z
Learnt from: arkanoider
PR: MostroP2P/mostro-cli#135
File: src/cli/send_msg.rs:89-110
Timestamp: 2025-09-13T20:48:31.581Z
Learning: In mostro-cli's send_msg.rs, arkanoider confirmed that tokio::spawn with async move is correct and compiles without errors when using PublicKey (which implements Copy). The spawning approach is acceptable for DM sending in this context.
Applied to files:
src/cli/dm_to_user.rs
🧬 Code graph analysis (15)
src/cli/get_dm_user.rs (2)
src/parser/common.rs (4)
print_info_line(128-130)print_key_value(123-125)print_no_data_message(118-120)print_section_header(102-105)src/parser/dms.rs (1)
print_direct_messages(817-888)
src/parser/mod.rs (1)
src/parser/common.rs (24)
apply_kind_color(34-39)apply_status_color(6-31)create_centered_cell(70-76)create_emoji_field_row(94-99)create_error_cell(42-46)create_field_value_header(58-67)create_field_value_row(79-84)create_standard_table(49-55)format_timestamp(87-91)print_amount_info(153-155)print_fiat_code(163-165)print_info_line(128-130)print_info_message(113-115)print_key_value(123-125)print_no_data_message(118-120)print_order_count(183-185)print_order_info(133-145)print_order_status(148-150)print_payment_method(173-175)print_premium(168-170)print_required_amount(158-160)print_section_header(102-105)print_success_message(108-110)print_trade_index(178-180)
src/cli/get_dm.rs (2)
src/parser/common.rs (2)
print_key_value(123-125)print_section_header(102-105)src/parser/dms.rs (1)
print_direct_messages(817-888)
src/cli/take_order.rs (1)
src/parser/common.rs (3)
create_emoji_field_row(94-99)create_field_value_header(58-67)create_standard_table(49-55)
src/cli/rate_user.rs (1)
src/parser/common.rs (3)
print_info_line(128-130)print_key_value(123-125)print_section_header(102-105)
src/cli/dm_to_user.rs (2)
src/parser/common.rs (4)
print_info_line(128-130)print_key_value(123-125)print_section_header(102-105)print_success_message(108-110)src/util/messaging.rs (1)
send_gift_wrap_dm(23-30)
src/cli/orders_info.rs (3)
src/parser/common.rs (2)
print_key_value(123-125)print_section_header(102-105)src/parser/dms.rs (2)
print_commands_results(380-723)parse_dm_events(725-815)src/util/messaging.rs (2)
send_dm(184-236)wait_for_dm(69-110)
src/cli/add_invoice.rs (1)
src/parser/common.rs (3)
create_emoji_field_row(94-99)create_field_value_header(58-67)create_standard_table(49-55)
src/parser/orders.rs (1)
src/parser/common.rs (3)
apply_kind_color(34-39)apply_status_color(6-31)create_error_cell(42-46)
src/parser/disputes.rs (1)
src/parser/common.rs (2)
apply_status_color(6-31)create_error_cell(42-46)
src/cli/take_dispute.rs (4)
src/parser/common.rs (3)
create_emoji_field_row(94-99)create_field_value_header(58-67)create_standard_table(49-55)src/parser/dms.rs (2)
print_commands_results(380-723)parse_dm_events(725-815)src/util/storage.rs (1)
admin_send_dm(46-58)src/util/messaging.rs (3)
send_dm(184-236)wait_for_dm(69-110)None(51-51)
src/parser/dms.rs (3)
src/parser/common.rs (10)
format_timestamp(87-91)print_amount_info(153-155)print_fiat_code(163-165)print_order_count(183-185)print_payment_method(173-175)print_premium(168-170)print_required_amount(158-160)print_section_header(102-105)print_success_message(108-110)print_trade_index(178-180)src/util/storage.rs (1)
save_order(11-36)src/db.rs (5)
new(139-160)new(278-331)get_by_id(519-535)delete_by_id(550-563)get(195-207)
src/cli/send_msg.rs (1)
src/parser/common.rs (3)
create_emoji_field_row(94-99)create_field_value_header(58-67)create_standard_table(49-55)
src/cli/new_order.rs (1)
src/parser/common.rs (3)
create_emoji_field_row(94-99)create_field_value_header(58-67)create_standard_table(49-55)
src/cli/send_dm.rs (1)
src/parser/common.rs (3)
create_emoji_field_row(94-99)create_field_value_header(58-67)create_standard_table(49-55)
⏰ 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 (11)
src/util/misc.rs (1)
15-21: LGTM! Robust error handling.The enhanced directory creation logic properly handles race conditions with the
AlreadyExistscheck and provides clear error messages for other failure cases.src/cli/add_invoice.rs (1)
1-3: LGTM! Excellent UI enhancement.The structured table output provides clear visibility of the invoice submission details. The implementation correctly uses the centralized table helpers from
parser::commonand maintains consistency with the broader UI modernization across CLI commands.Also applies to: 24-45
src/cli/rate_user.rs (1)
33-37: LGTM! Clear user feedback.The structured header and key-value output provides excellent visibility into the rating operation before sending.
src/cli/get_dm.rs (1)
7-7: LGTM! Enhanced DM fetching experience.The structured output at the start (lines 18-23) provides clear visibility into the fetch parameters. Passing
Some(ctx.mostro_pubkey)toprint_direct_messages(line 45) enables context-aware rendering that can distinguish Mostro messages, improving UX.Based on learnings: The message filtering logic is intentional and correctly handles admin vs. non-admin message types.
Also applies to: 18-23, 45-45
src/cli/take_order.rs (1)
9-11: LGTM! Comprehensive order details display.The table-based output (lines 68-94) provides excellent visibility into all take order parameters, with proper conditional rendering for optional fields. The detailed sending message output (lines 112-117) gives users clear feedback during the DM transmission process.
Also applies to: 68-94, 112-117
src/cli/dm_to_user.rs (1)
1-3: LGTM! Clear DM sending experience.The structured output provides excellent visibility into the gift wrap message details before sending, and the success confirmation gives clear feedback. The implementation aligns well with the broader UI modernization.
Based on learnings: The message content logging is safe as it contains only ephemeral public keys with no sensitive personal data.
Also applies to: 28-38
src/cli/send_msg.rs (1)
3-5: LGTM! Safe and clear command display.The table-based output provides excellent visibility into the message command details. Line 46 correctly handles the optional
order_idusingmap_or_elseto display "N/A" when None, avoiding the panic that was present in earlier versions. The implementation is clean and consistent with the UI modernization pattern.Also applies to: 34-54
src/cli/new_order.rs (1)
2-4: LGTM! Comprehensive new order display.The table-based output provides excellent visibility into all new order parameters before submission. The conditional rendering (lines 136-148) properly handles both range and single fiat amount cases. The implementation is thorough, user-friendly, and consistent with the UI modernization across CLI commands.
Also applies to: 122-177
src/parser/disputes.rs (1)
69-71: LGTM: empty-state table looks clean and consistent.Header and error row use the shared helpers appropriately.
Also applies to: 76-78
src/parser/mod.rs (1)
6-13: Re-exports look good; consistent UI surface.Publicly surfacing common helpers aligns with broader CLI/parser styling. Keep the list updated as helpers evolve.
src/cli/get_dm_user.rs (1)
25-28: LGTM: clear empty states and consolidated printing.Use of print_no_data_message and print_direct_messages improves UX and consistency.
Also applies to: 55-56, 65-66
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/cli/new_order.rs (1)
93-110: Missing user prompt before reading input.The code reads user input to confirm the order, but no prompt message is displayed. After line 92 prints the order preview, users won't know the program is waiting for their Y/n confirmation. This creates a confusing experience where the program appears to hang.
Add a prompt message before line 97:
let ord_preview = print_order_preview(order_content.clone()) .map_err(|e| anyhow::anyhow!("Failed to generate order preview: {}", e))?; println!("{ord_preview}"); + println!("\nCheck your order! Is it correct? (Y/n)"); let mut user_input = String::new(); let _input = stdin(); stdout().flush()?;
♻️ Duplicate comments (1)
src/parser/dms.rs (1)
341-346: Don't print LN preimages to stdout.Preimages are sensitive proof-of-payment material and should be redacted or hidden behind a verbose flag.
This issue was already flagged in a previous review. Please address the security concern by either removing the preimage from the output entirely or masking it appropriately when a verbose flag is enabled.
🧹 Nitpick comments (1)
src/parser/dms.rs (1)
379-744: Consider extracting action handlers into separate functions.The
print_commands_resultsfunction is 365 lines long and handles many different action types in a single match statement. While functional, extracting individual action handlers into dedicated functions would improve maintainability and testability.For example, you could extract handlers like:
handle_waiting_seller_to_payhandle_cant_do_actionhandle_restore_sessionThis would make the main function a simpler dispatcher and make each handler easier to test and modify independently.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
src/cli/new_order.rs(2 hunks)src/cli/rate_user.rs(1 hunks)src/parser/dms.rs(12 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- src/cli/rate_user.rs
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2025-10-18T11:48:11.935Z
Learnt from: arkanoider
PR: MostroP2P/mostro-cli#146
File: src/cli/dm_to_user.rs:25-32
Timestamp: 2025-10-18T11:48:11.935Z
Learning: In mostro-cli's DM functionality (dm_to_user.rs and similar), message content consists of ephemeral public keys that change every time and contains no sensitive personal data. Logging message content to stdout is acceptable and not a privacy concern.
Applied to files:
src/parser/dms.rs
📚 Learning: 2025-09-13T10:31:42.313Z
Learnt from: arkanoider
PR: MostroP2P/mostro-cli#135
File: src/cli/get_dm.rs:24-29
Timestamp: 2025-09-13T10:31:42.313Z
Learning: In mostro-cli's get_dm.rs, the message filtering is intentional: non-admin users should only receive DirectMessage from users, while messages from Mostro are only fetched when the admin flag is true. Don't include GiftWrap messages for non-admin users.
Applied to files:
src/parser/dms.rs
🧬 Code graph analysis (2)
src/parser/dms.rs (3)
src/parser/common.rs (10)
format_timestamp(87-91)print_amount_info(153-155)print_fiat_code(163-165)print_order_count(183-185)print_payment_method(173-175)print_premium(168-170)print_required_amount(158-160)print_section_header(102-105)print_success_message(108-110)print_trade_index(178-180)src/util/storage.rs (1)
save_order(11-36)src/db.rs (5)
new(139-160)new(278-331)get_by_id(519-535)delete_by_id(550-563)get(195-207)
src/cli/new_order.rs (1)
src/parser/common.rs (3)
create_emoji_field_row(94-99)create_field_value_header(58-67)create_standard_table(49-55)
🪛 GitHub Actions: CI Mostro-cli
src/parser/dms.rs
[error] 884-884: Clippy: to_string is applied to a type that already implements Display in format! arguments. Remove to_string() in format!("🧌 {}", sender_pubkey) to avoid unnecessary allocation.
🔇 Additional comments (1)
src/cli/new_order.rs (1)
2-4: LGTM!The imports are appropriate for the table-based UI rendering implemented below.
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
src/parser/dms.rs (1)
341-346: Don't print LN preimages to stdout.Preimages are sensitive proof-of-payment material that should not be logged. This issue was flagged in a previous review but remains unaddressed. Consider redacting or masking the preimage, or only showing it behind a verbose flag.
Apply this diff to redact the preimage:
if let Some(preimage) = &dispute_info.preimage { + let redacted = if std::env::var("MOSTRO_VERBOSE").ok().as_deref() == Some("1") { + preimage.clone() + } else { + format!("{}...{}", &preimage[..8], &preimage[preimage.len().saturating_sub(8)..]) + }; rows.push(Row::from(vec![ Cell::new("🔑 Preimage"), - Cell::new(preimage.clone()), + Cell::new(redacted), ])); }Based on learnings.
🧹 Nitpick comments (2)
src/cli/rate_user.rs (1)
25-28: Consider displaying the order ID in the invalid rating case.For consistency with the valid rating path and to provide complete context to the user, consider displaying the order ID here as well. This would help users understand which order they attempted to rate with an invalid value.
Apply this diff to add the order ID:
print_section_header("❌ Invalid Rating"); +print_key_value("📋", "Order ID", &order_id.to_string()); print_key_value("⭐", "Rating", &rating.to_string()); print_info_line("💡", "Rating must be between 1 and 5"); print_info_line("📊", "Valid ratings: 1, 2, 3, 4, 5");src/parser/dms.rs (1)
23-23: Consider importing specific serde_json items.While
use serde_json;works, the idiomatic approach is to import specific items (e.g.,use serde_json::to_string_pretty;) or use an alias. This is purely a style preference.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/cli/rate_user.rs(1 hunks)src/parser/dms.rs(12 hunks)
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2025-10-18T11:48:11.935Z
Learnt from: arkanoider
PR: MostroP2P/mostro-cli#146
File: src/cli/dm_to_user.rs:25-32
Timestamp: 2025-10-18T11:48:11.935Z
Learning: In mostro-cli's DM functionality (dm_to_user.rs and similar), message content consists of ephemeral public keys that change every time and contains no sensitive personal data. Logging message content to stdout is acceptable and not a privacy concern.
Applied to files:
src/parser/dms.rs
📚 Learning: 2025-09-13T10:31:42.313Z
Learnt from: arkanoider
PR: MostroP2P/mostro-cli#135
File: src/cli/get_dm.rs:24-29
Timestamp: 2025-09-13T10:31:42.313Z
Learning: In mostro-cli's get_dm.rs, the message filtering is intentional: non-admin users should only receive DirectMessage from users, while messages from Mostro are only fetched when the admin flag is true. Don't include GiftWrap messages for non-admin users.
Applied to files:
src/parser/dms.rs
🧬 Code graph analysis (2)
src/cli/rate_user.rs (1)
src/parser/common.rs (3)
print_info_line(128-130)print_key_value(123-125)print_section_header(102-105)
src/parser/dms.rs (3)
src/parser/common.rs (9)
format_timestamp(87-91)print_amount_info(153-155)print_fiat_code(163-165)print_payment_method(173-175)print_premium(168-170)print_required_amount(158-160)print_section_header(102-105)print_success_message(108-110)print_trade_index(178-180)src/util/storage.rs (1)
save_order(11-36)src/db.rs (5)
new(139-160)new(278-331)get_by_id(519-535)delete_by_id(550-563)get(195-207)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: build
🔇 Additional comments (7)
src/cli/rate_user.rs (2)
18-22: LGTM! Clear and informative user feedback.The use of the new print utilities provides structured, emoji-enhanced output that clearly communicates the rating details to the user, including the order ID and rating value.
35-35: LGTM! Function call correctly updated.The call to
get_user_ratehas been properly updated to include theorder_idparameter, matching the new function signature.src/parser/dms.rs (5)
85-88: Full invoice display is appropriate for CLI.The comment notes showing the full invoice without truncation. This is correct for a CLI tool where users need to copy the complete invoice string. No changes needed.
693-741: RestoreSession handler looks comprehensive.The new RestoreSession action handler provides clear, structured output for both restored orders and disputes. The implementation correctly handles empty cases and provides useful information to users.
760-821: Sender propagation looks correct.The code now correctly extracts and returns the sender PublicKey for both GiftWrap (using
unwrapped_gift.sender) and PrivateDirectMessage (usingdm.pubkey) flows. This properly supports the downstream display requirements.
843-914: Well-structured message display implementation.The refactored
print_direct_messagesfunction provides an excellent user experience with:
- Clear message headers with numbering
- Formatted timestamps and sender labels
- Action-specific icons for visual scanning
- Properly formatted payload details via
format_payload_detailsThe signature change (removing
pool, addingmostro_pubkey) is appropriate for this display-only function.
172-178: Verify generic JSON payload serialization doesn't leak sensitive data.The fallback
serde_json::to_string_pretty(payload)for unhandled payload types might expose sensitive fields if new payload variants are added to the protocol. Review the mostro_core Payload enum to confirm all variants with sensitive data are explicitly handled above.
…rder is completed, no more errors on next flow. Still a bit trickier the buy range order thing
There was a problem hiding this comment.
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 (2)
src/util/messaging.rs (2)
90-107: Fix timeout error mapping (incorrect message on inner errors, no mapping on actual timeout).Current chaining returns Tokio Elapsed directly (so your custom timeout message never shows) and maps inner errors to “Timeout…”, which is misleading. Handle the outer timeout first, then propagate the inner Result as-is.
Apply:
- let event = tokio::time::timeout(super::events::FETCH_EVENTS_TIMEOUT, async move { + let result = tokio::time::timeout(super::events::FETCH_EVENTS_TIMEOUT, async move { loop { match notifications.recv().await { Ok(notification) => match notification { RelayPoolNotification::Event { event, .. } => { - return Ok(*event); + return Ok::<Event, anyhow::Error>(*event); } _ => continue, }, Err(e) => { return Err(anyhow::anyhow!("Error receiving notification: {:?}", e)); } } } }) - .await? - .map_err(|_| anyhow::anyhow!("Timeout waiting for DM or gift wrap event"))?; + .await; + + let event = match result { + Ok(inner) => inner?, // propagate inner error (recv failure) verbatim + Err(_) => return Err(anyhow::anyhow!("Timeout waiting for DM or gift wrap event")), + };
159-169: Sign payload with identity keys for signed gift wraps.For
signed == true, you validateidentity_keysbut still sign the message payload withtrade_keys. Use the identity keys to keep the signature consistent with the signer of the outer gift wrap.- let content = if signed { - let _identity_keys = identity_keys + let content = if signed { + let id_keys = identity_keys .ok_or_else(|| Error::msg("identity_keys required for signed messages"))?; - let sig = Message::sign(payload, trade_keys); + let sig = Message::sign(payload, id_keys); serde_json::to_string(&(message, sig)) .map_err(|e| anyhow::anyhow!("Failed to serialize message: {e}"))? } else {
♻️ Duplicate comments (2)
src/parser/dms.rs (1)
341-346: Don’t print LN preimages to stdout; redact by default.Preimages are sensitive. Show a redacted preview unless an explicit verbose flag is set.
Based on learnings.
- if let Some(preimage) = &dispute_info.preimage { - rows.push(Row::from(vec![ - Cell::new("🔑 Preimage"), - Cell::new(preimage.clone()), - ])); - } + if let Some(preimage) = &dispute_info.preimage { + let verbose = std::env::var("MOSTRO_VERBOSE").ok().as_deref() == Some("1"); + let value = if verbose && preimage.len() > 16 { + format!("{}…{}", &preimage[..8], &preimage[preimage.len().saturating_sub(8)..]) + } else if verbose { + preimage.clone() + } else { + "🔒 redacted".to_string() + }; + rows.push(Row::from(vec![Cell::new("🔑 Preimage"), Cell::new(value)])); + }src/cli/send_msg.rs (1)
36-58: Validateorder_idearly for these commands to avoid no-op.All mapped actions here operate on an order. If
order_idisNone, we currently print the table and return without sending. Fail fast with a clear error.// Printout command information println!("📤 Send Message Command"); println!("═══════════════════════════════════════"); + // Require order_id for the supported actions in this command + let requires_order_id = matches!( + requested_action, + Action::FiatSent + | Action::Release + | Action::Cancel + | Action::Dispute + | Action::AdminCancel + | Action::AdminSettle + | Action::AdminAddSolver + ); + if requires_order_id && order_id.is_none() { + return Err(anyhow::anyhow!("Order ID is required for this command")); + }
🧹 Nitpick comments (3)
src/util/messaging.rs (1)
81-85: Verify use of.limit(0); prefer removing or setting a positive limit.Some relays/SDKs treat zero as “no events” rather than “no limit.” To avoid missing the response, drop the limit or set
.limit(1).If appropriate:
- .limit(0); + .limit(1);Or remove
.limit(..)entirely.src/parser/dms.rs (1)
856-859: Use shared timestamp formatter for consistency.Prefer
format_timestampover manual DateTime formatting to keep output uniform.- let date = match DateTime::from_timestamp(*created_at as i64, 0) { - Some(dt) => dt.format("%Y-%m-%d %H:%M:%S").to_string(), - None => "Invalid timestamp".to_string(), - }; + let date = format_timestamp(*created_at as i64);src/cli/send_msg.rs (1)
81-84: Avoid cloning large payloads unnecessarily.
Actionis small;payload: Option<Payload>can be large. Instead of cloning, move references into the checks below.- let requested_action_clone = requested_action.clone(); - let payload_clone = payload.clone(); + let requested_action_clone = requested_action; // Copy/Clone cheap + let payload_ref = payload.as_ref();Adjust later uses accordingly (e.g.,
if requested_action_clone == Action::Releaseandif let Some(Payload::NextTrade(_, index)) = payload_ref { … }).
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
src/cli/new_order.rs(2 hunks)src/cli/send_msg.rs(4 hunks)src/db.rs(0 hunks)src/parser/dms.rs(12 hunks)src/util/messaging.rs(2 hunks)
💤 Files with no reviewable changes (1)
- src/db.rs
🚧 Files skipped from review as they are similar to previous changes (1)
- src/cli/new_order.rs
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2025-10-18T11:48:11.935Z
Learnt from: arkanoider
PR: MostroP2P/mostro-cli#146
File: src/cli/dm_to_user.rs:25-32
Timestamp: 2025-10-18T11:48:11.935Z
Learning: In mostro-cli's DM functionality (dm_to_user.rs and similar), message content consists of ephemeral public keys that change every time and contains no sensitive personal data. Logging message content to stdout is acceptable and not a privacy concern.
Applied to files:
src/parser/dms.rs
📚 Learning: 2025-09-13T10:31:42.313Z
Learnt from: arkanoider
PR: MostroP2P/mostro-cli#135
File: src/cli/get_dm.rs:24-29
Timestamp: 2025-09-13T10:31:42.313Z
Learning: In mostro-cli's get_dm.rs, the message filtering is intentional: non-admin users should only receive DirectMessage from users, while messages from Mostro are only fetched when the admin flag is true. Don't include GiftWrap messages for non-admin users.
Applied to files:
src/parser/dms.rs
🧬 Code graph analysis (2)
src/parser/dms.rs (3)
src/parser/common.rs (9)
format_timestamp(87-91)print_amount_info(153-155)print_fiat_code(163-165)print_payment_method(173-175)print_premium(168-170)print_required_amount(158-160)print_section_header(102-105)print_success_message(108-110)print_trade_index(178-180)src/util/storage.rs (1)
save_order(11-36)src/db.rs (5)
new(139-160)new(273-326)get_by_id(514-530)delete_by_id(545-558)get(190-202)
src/cli/send_msg.rs (5)
src/parser/common.rs (3)
create_emoji_field_row(94-99)create_field_value_header(58-67)create_standard_table(49-55)src/parser/dms.rs (2)
print_commands_results(380-742)parse_dm_events(744-839)src/util/events.rs (1)
create_filter(35-68)src/util/messaging.rs (4)
print_dm_events(240-272)send_dm(186-238)wait_for_dm(69-112)None(51-51)src/db.rs (1)
get_trade_keys(233-248)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: build
🔇 Additional comments (2)
src/util/messaging.rs (1)
256-258: LGTM: handle NewOrder without request_id.Allowing NewOrder with
Nonerequest_id unblocks range-order follow-ups. Matches the Release flow additions.Consider adding a quick test ensuring NewOrder without request_id routes to print_commands_results.
src/cli/send_msg.rs (1)
116-145: Nice follow-up flow for Release + range orders.Waiting briefly for a potential NewOrder on the next trade key improves UX. Good use of
parse_dm_eventswith a short since-window.Optional: add a unit/integration test that stubs a NewOrder on the derived key and asserts we print its results and return early.
Summary by CodeRabbit
New Features
Improvements
Documentation
Tests