fix: surface InvalidOrderId to clients as CantDo(NotFound)

[AndreaDiazCorreia]

Summary

Closes #751.

Admin actions (AdminSettle, AdminCancel) — and any handler that called get_order — used to return MostroInternalErr(ServiceError::InvalidOrderId) when the message lacked an id or the order was not present in the database. That error was routed to manage_errors, which only calls warning_msg and never enqueues a DM back to the sender. The client therefore never learned the action failed and timed out.

This change promotes the user-facing path of InvalidOrderId to MostroCantDo(CantDoReason::NotFound), so the sender receives a CantDo DM. It matches the existing CantDo pattern already used in last_trade_index_action and orders_action, and is the option recommended in the issue.

Changes

• util.rs::get_order: return CantDo(NotFound) when the message lacks an id and when the order is not present in the database.
• app/dispute.rs::dispute_action: return CantDo(NotFound) when the message lacks an order_id.
• app/admin_take_dispute.rs::admin_take_dispute_action:
  • Preserve DbAccessError semantics on real database failures (previously silently mapped to InvalidOrderId).
  • Return CantDo(NotFound) when the dispute's referenced order is missing, so the admin gets feedback.
• Unit tests for the three get_order branches: missing id, order not found, order found.

Affected actions

AdminSettle, AdminCancel, Dispute, AdminTakeDispute, and any other handler that resolves an order through get_order (FiatSent, TakeBuy, TakeSell, AddInvoice, Cancel, Release, RateUser, TradePubkey).

Test plan

• cargo build --tests -p mostro — clean.
• cargo test -p mostro --bin mostrod — 334 passed, 0 failed (3 new tests added).
• cargo clippy -p mostro --bin mostrod --tests — no new warnings.
• Manual: send AdminSettle with a non-existent order_id and verify the admin receives a CantDo(NotFound) DM instead of a timeout.

Related

• CLI side: admsettle reports success even when the order id does not exist mostro-cli#170

[coderabbitai]

Warning

Review limit reached

@AndreaDiazCorreia, we couldn't start this review because you've reached your PR review rate limit.

More reviews will be available in 7 minutes and 53 seconds. Learn how PR review limits work.

Your organization has run out of usage credits. Purchase more in the billing tab.

⌛ How to resolve this issue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available.

Please see our Fair Usage Limits Policy for further information.

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: a2b4ec0a-9913-4e71-a191-a01ce6c7cbb3

📥 Commits

Reviewing files that changed from the base of the PR and between d3f5bfa and 18ffee9.

📒 Files selected for processing (3)

• src/app/admin_take_dispute.rs
• src/app/dispute.rs
• src/util.rs

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch fix/issue-751-invalid-order-id-cantdo

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

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

[ermeme]

Hermes Agent Review

Strict re-check on the current head: the error mapping is consistent with the issue, the new branches return CantDo(NotFound) where expected, and the added tests cover the key get_order cases. LGTM.

[codaMW]

Reviewed the three changed files and the test cases. The fix is correct and the approach is well-chosen.

The core change in get_order is exactly right, both the missing-id case and the not-found case are genuinely user-facing conditions, not internal server errors. Routing them through MostroCantDo(CantDoReason::NotFound) means the sender receives a CantDo DM instead of silently timing out. The distinction preserved in admin_take_dispute.rs is also important: real database failures correctly stay as DbAccessError (internal), while a missing order correctly becomes NotFound (user-facing). That separation shows good understanding of the error model.

The three unit tests cover the full decision tree of get_order missing id, id present but order absent, and the happy path. The use of setup_orders_pool() with in-memory SQLite and the actual migration SQL is the right pattern it tests against the real schema rather than a hand-rolled approximation.

One observation worth noting: the admin_take_dispute fix also silently upgraded what was previously a blanket InvalidOrderId mapping on the dispute's order lookup to a proper DbAccessError for real DB failures. That's a correctness improvement worth calling out explicitly in the PR description since it changes error semantics for infrastructure failures, not just the not-found case.

cACK: logic is sound, tests are solid, change is minimal and targeted.

---