BugFix: users can cancel an order cooperatively at the same time

[Matobi98]

Description

Fix race condition when processing simultaneous cooperative cancellations: if buyer and seller send /cancel order-id at nearly the same time both flags (buyer_cooperativecancel, seller_cooperativecancel) were set but the order remained in Active status.

Now the operation uses an atomic update/lock so when both confirm the order transitions to cancelled and bot replies remain consistent.

Summary by CodeRabbit

• Refactor
  • Internal optimization of the order cancellation flow to improve code maintainability and consistency.

[coderabbitai]

Walkthrough

Introduces a helper function setCooperativeCancelFlag to encapsulate cooperative cancel flag updates. Refactors cancelOrder to use this helper and work with an updateOrder object instead of directly manipulating the original order, ensuring consistent state management throughout the cancellation flow.

Changes

Cohort / File(s)	Summary
Cooperative Cancel Refactoring bot/commands.ts	Adds setCooperativeCancelFlag helper function and refactors cancelOrder to use it instead of direct property manipulation. Switches flow to work with updateOrder object, updates all messaging and state operations to reference the updated object, and removes redundant order.save() call in favor of updateOrder.save().

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

• PR #657: Both modify the cancelOrder implementation in bot/commands.ts—one refactors cooperative cancel flag handling while the other tightens WAITING_PAYMENT cancellation checks.

Suggested reviewers

• grunch

Poem

🐰 A helper hops in, clean and bright,
Flags set straight with all its might,
Order updates flow just right,
One save persists the cooperative light! ✨

🚥 Pre-merge checks | ✅ 3

✅ Passed checks (3 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title directly addresses the main issue: fixing a race condition where users can initiate cooperative cancellations simultaneously, which is the core problem solved by the refactored cooperative cancel flow.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

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

✨ Finishing touches

• 📝 Generate docstrings

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment

---

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.}

[coderabbitai]

Actionable comments posted: 0

Caution

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

⚠️ Outside diff range comments (1)

bot/commands.ts (1)

734-758: ⚠️ Potential issue | 🟠 Major

Emit orderUpdated with the freshly saved order to avoid stale event payloads.

updateOrder is persisted with status = 'CANCELED', but the event is emitted with the pre-update order, so listeners may see ACTIVE and miss the cancellation. Emit updateOrder instead (or re-fetch) to keep downstream state consistent.

🛠️ Proposed fix

-      logger.info('cancelOrder => OrderEvents.orderUpdated(order);');
-      OrderEvents.orderUpdated(order);
+      logger.info('cancelOrder => OrderEvents.orderUpdated(updateOrder);');
+      OrderEvents.orderUpdated(updateOrder);

[mostronator]

LGTM ✅ Good fix for the race condition in cooperative cancellation (#583).

What was wrong: Both parties could read the order, see the other hadn't cancelled yet, then both set their own flag and save — causing a race where the mutual-cancel branch was never reached.

What this fixes: Uses findOneAndUpdate with { new: true } to atomically set the cooperative cancel flag, then checks the returned document. This eliminates the TOCTOU race.

Minor note: On line OrderEvents.orderUpdated(order) (after the cancel branch), you're passing the original order object instead of updateOrder. This is likely fine since the event just needs the order ID, but for consistency it should ideally be updateOrder. Not a blocker.

Good work, resolves #583.