Actionable comments posted: 2

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@lib/features/order/notifiers/abstract_mostro_notifier.dart`:
- Around line 270-280: The session deletion logic has two issues: the
_bondCancelDeletionTimers.remove(orderId)?.cancel() call occurs before
validating the message history, which can strand timeout sessions if the history
read fails, and the bondSlashIsTimeout() check doesn't verify that dispute or
admin actions don't exist in the bondSlashMessages. Move the timer cancellation
to after the getAllMessagesForOrderId() call succeeds, and update the deletion
condition to ensure the session is only deleted when bondSlashIsTimeout()
returns true AND the bondSlashMessages contain no dispute or admin actions that
would indicate a terminal trade that should be preserved.

In `@lib/shared/utils/bond_slash_helpers.dart`:
- Around line 17-22: The _disputeActions set on line 17 is incomplete and
missing the base action variants Action.dispute, Action.adminSettle, and
Action.adminCancel, which should be included alongside the existing variant
actions. Additionally, the timeout detection logic around line 40-41 currently
classifies any Action.canceled as a timeout without checking if dispute markers
also exist, which can misclassify dispute slashes. Fix this by adding the
missing base action variants to the _disputeActions set, and modify the timeout
detection logic to ensure it only classifies an action as a timeout when no
dispute markers are present, preventing incorrect classification of
dispute-related actions as simple timeouts.

---

Nitpick comments:
In `@test/shared/utils/bond_slash_helpers_test.dart`:
- Around line 24-79: The bondSlashCause test group is missing regression
coverage for legacy action types and mixed history scenarios. Add new test cases
within the bondSlashCause group to cover legacy Action types such as
Action.dispute, Action.adminSettle, and Action.adminCancel, as well as mixed
history patterns combining canceled with dispute-related actions. This will
prevent silent drift in slash classification behavior for these edge cases.

I like the direction of this PR, but I do see one real correctness issue that I would block on before merge.

Blocking issue:

• In notification_data_extractor.dart, the slash-cause inference reads history with getAllMessagesForOrderId(event.id!).
• But in that same branch the canonical order identity is coming from the payload (order.id), and the extractor later persists order_id: order.id into the notification data.
• If event.id here is the incoming message/event identifier rather than the Mostro order id, the history lookup can miss the actual order history or read the wrong one, and the fallback path will silently persist slash_cause = timeout for a real dispute slash.

Why this matters:

• The inferred cause is persisted into the notification and later drives which copy the user sees.
• More importantly, this PR is explicitly about making timeout vs dispute handling cause-aware, so anchoring the inference on the wrong identifier would undermine the core contract of the feature.
• Because the fallback is timeout, this would fail in the misleading direction instead of failing loudly.

What I would want:

• Use the actual order id from the payload for the history lookup in this path, not event.id.
• In other words, the slash-cause inference should be keyed on the same durable order identity that the notification stores as order_id.

Aside from that, the helper extraction and the timeout-vs-dispute session handling direction look good to me.

I re-checked your comment and the underlying model, and you are right: the blocker I raised does not hold under the actual semantics here.

What changed my mind:

• In this path event is a MostroMessage, not the outer Nostr gift-wrap event.
• MostroMessage.id is the Mostro order id, and that is the same UUID carried in the payload order.id for this bond-slashed notice.
• So the history lookup is already anchored on the correct durable order identity, not on a separate transport-level event id.

That means my previous concern about silently reading the wrong history and defaulting a real dispute slash to timeout was based on the wrong identifier model.

I still agree with your readability point that order.id is more self-documenting in this branch, but that is not a correctness blocker on the current head.

I re-checked the rest of the PR and I do not see another blocking issue. The cause-aware notification copy, dispute-vs-timeout session handling, and dispute-only order-details notice all look coherent with the inferred-history model.

Approved on the current revision.

I re-reviewed the new head and I do not see a new blocking issue.

What I checked on the latest commit:

• The new UI move from the old trade-detail fallback into MostroMessageDetail does not appear to change the underlying cause inference model or the timeout-vs-dispute session behavior.
• The dispute-only rendering still depends on the same history-derived slash classification and slashed-amount extraction, so this looks like presentation cleanup rather than a semantic change.
• I do not see a regression that would justify reopening a blocker on the current revision.

Approved on the latest head.